[FFmpeg-devel] [PATCH] Exposing forced flag for DVD and PGS subtitles

[Erik Miranda]

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
>>> AVSubtitleRect
>>> 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
>>> AVFrame
>>> 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?
> yes
>
>
>> 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.