[FFmpeg-devel] [PATCH v5 00/12] Subtitle Filtering

Soft Works softworkz at hotmail.com
Sun Sep 12 23:07:37 EEST 2021



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Hendrik Leppkes
> Sent: Sunday, 12 September 2021 07:30
> To: FFmpeg development discussions and patches <ffmpeg-
> devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v5 00/12] Subtitle Filtering
> 
> On Sun, Sep 12, 2021 at 6:21 AM Soft Works <softworkz at hotmail.com>
> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> > > Soft Works
> > > Sent: Sunday, 12 September 2021 05:21
> > > To: ffmpeg-devel at ffmpeg.org
> > > Subject: [FFmpeg-devel] [PATCH v5 00/12] Subtitle Filtering
> > >
> > > v5 Update:
> > >
> > > - Merge AVSubtitle into AVFrame
> > > - Move FATE test adjustments to corresponding commit
> > > - Move documentation updates to corresponding filter commits
> > > - Remove mediatype parameter from av_frame_get_buffer2
> >
> > This patchset provides proof that merging AVSubtitle into AVFrame
> > is doable and not far away to achieve. It obsoletes the need for
> > a "subtitle api", as that purpose will be served by the AVFrame
> > API. It might still make sense to add a number of functions for
> > dealing with AVSubtitleRect (e.g. av_subtitlerect_alloc, _free)
> > as these should probably be ref-counted to avoid copying.
> > I have added a minimal internal ref counting which serves the
> current
> > use cases, but I'm open to suggestions..
> >
> 
> The key that this patchset does not do however is preserve
> compatibility.
> As mentioned in another thread, we don't just change APIs, certainly
> not overnight, and basically not ever. We introduce replacements, and
> deprecate the old ones (and remove after a grace period)
> 
> That means if my application does not use deprecated functionality,
> it
> has to continue working after this patchset - and that is absolutely
> not the case.
> 
> So what needs to happen is that existing subtitle functions need to
> continue to be functional and unchanged, including all structures and
> whatnot. They can be deprecated and removed in ~2 years
> This fact alone makes it also easier to split such changes into
> smaller pieces, because you only add, you don't remove or replace.
> 
> So back to small atomic commits, which is easy if you don't update
> everything in one go, because you are not allowed to anyway. Old
> functions have to be preserved.
> 
> Internally, there should be a compatibility layer. I would recommend
> at first not changing encoders and decoders internally, and instead
> build the compatibility so that it uses the old internal API to feed
> the new external API.
> This can be changed and reversed in a follow-up change later.

Thanks Hendrik, but then we are just back at my previous patchset
which provides exactly this: compatibility.

If you say APIs should not (or even "never" be changed), then we
need to have AVSubtitle around for quite a long time.

In that case though I see absolutely no benefit in merging AVSubtitle
into AVFrame at all. That "glue/compatibility code" would make things
just unnecessarily complicated. It would be even impossible at some 
point because we can't copy the AVSubtitleRect data because we don't
know the size of the allocated memory, so we can just free it at some
point.

All-in-all that doesn't seem to be worth all the hazzle. Again, 
that brings me back to my original approach:

Simply attach AVSubtitle to AVFrame (ref-counted of course).
That's nice and clean and compatible.

What I agree is that it's not nice to store it in buf[0]. I'll 
better add two separate properties for it: An AVBufferRef and
a direct AVSubtitle pointer for convenience.

Regards,
softworkz












More information about the ffmpeg-devel mailing list