[FFmpeg-devel] [PATCH] Exposing forced flag for DVD and PGS subtitles
erikmiranda at gmail.com
Wed Apr 18 02:18:45 CEST 2012
On 17/04/2012 7:15 PM, Michael Niedermayer wrote:
> On Tue, Apr 17, 2012 at 06:52:00PM -0400, Erik Miranda wrote:
>> On 17/04/2012 10:50 AM, Michael Niedermayer wrote:
>>> On Mon, Apr 16, 2012 at 09:44:48PM +0200, Reimar Döffinger wrote:
>>>> On 16 Apr 2012, at 12:45, Joakim Plate<elupus at ecce.se> wrote:
>>>>> Erik Miranda<erikmiranda<at> gmail.com> writes:
>>>>>> This patch adds a "forced" member to the AVSubtitleRect structure and
>>>>>> implements setting this flag in the DVD and PGS subtitle decoders, since
>>>>>> these are the formats I'm aware of that include such a function. Since
>>>>>> this patch changes AVSubtitleRect, it leads to a public API change. As
>>>>>> far as I can tell, this is the best method to allow access to this
>>>>>> information. Please advise me if otherwise, or if further changes are
>>>>>> required as part of changing AVSubtitleRect. I think the libavcodec
>>>>>> version would need a bump, correct?
>>>>> There is already a content dispostion type for forced. That is stream
>>>>> global thou. So one option would be to duplicate forced subs into a
>>>>> secondary subtitle stream.
>>>> What would be the point? What would someone do with that information except drop all non-forced ones?
>>>> I guess it might be relevant if you want to preserve the info when reencoding.
>>>> It should be possible to avoid the API/ABI breakage by adding an extra array to AVSubtitle instead of putting it into the AVSubtitleRect.
>>>> Though we might consider changing the whole API so extending the rect is possible without changing the API in the future.
>>> Is there really a ABI/API issue with adding fields to AVSubtitleRect ?
>>> rect is a array of pointers
>>> assuming theres no inherent problem with adding fields to
>>> to be able to maintain compatibility with forks of ffmpeg (which we
>>> try to currently)
>>> a bit of extra red tape is needed see fef411ef for how it was done for
>>> Also just grep for avcodec_get_frame_class() to see code using this
>>> Code inside the lib declaring the AVSubtitleRect struct of course can
>>> do direct accesses
>>> its just applications which want to use shared libs that should avoid
>>> direct access
>>> This way shared libs can be exchanged between various forks in a binary
>>> compatibile way.
>> Just for clarification, I took a look at fef411ef, and I presume the
>> idea is to AVOption enable AVSubtitleRect. When I read the
>> documentation on AVOption, normally the struct would begin with a
>> "class" field, but this would break compatibility which is why I
>> understand the class field was not added when doing the same for
>> AVFrame in fef411ef, and instead a "get_frame_class" function was
>> added instead, correct?
>> Also, since this would be (from my
>> understanding) a "fake" object, would I need to make any changes to
>> follow the procedure of av_opt_set_defaults() and av_opt_free()?
> do you need these 2 ?
No, I don't. I was mearly double-checking that it would be okay to write
the code without using these two since we are deveating from the
conventions specified in the AVOption documentation.
More information about the ffmpeg-devel