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

[Erik Miranda]

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? 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()?