[FFmpeg-devel] [PATCH v3 00/18] Subtitle Filtering

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Hendrik Leppkes
> Sent: Saturday, 11 September 2021 11:55
> To: FFmpeg development discussions and patches <ffmpeg-
> devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v3 00/18] Subtitle Filtering
> 
> On Sat, Sep 11, 2021 at 11:28 AM Soft Works <softworkz at hotmail.com>
> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> > > Hendrik Leppkes
> > > Sent: Saturday, 11 September 2021 10:40
> > > To: FFmpeg development discussions and patches <ffmpeg-
> > > devel at ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [PATCH v3 00/18] Subtitle Filtering
> > >
> > > On Sat, Sep 11, 2021 at 8:02 AM Soft Works
> <softworkz at hotmail.com>
> > > wrote:
> > > >
> > > > At a future point in time, it might make sense merge AVSubtitle
> > > into
> > > > AVFrame. I haven't done that yet to avoid cluttering the
> patchset
> > > with
> > > > too much distraction and keep focus on the relevant changes.
> > > >
> > >
> > > Actually I would want to see subtitles to fully move into
> AVFrames
> > > before any filtering, if at all, and not half-assed. That means
> fully
> > > convert avcodec, deprecate AVSubtitle entirely (and not just
> shove it
> > > into AVFrame->data), and make them a first-class citizen inside
> > > AVFrame.
> > > Doing it half-assed just for filtering now binds us into that
> chosen
> > > API for years, and thats something we should try to avoid when we
> can
> > > do it properly right from the start.
> > >
> > > This can (and should) be done independent of filtering (in other
> > > words, avfilter doesn't necessarily need to accept subtitle
> frames
> > > yet), but it needs to be done *before* filtering, because you
> would
> > > be
> > > fundamentally re-defining the subtitle storage for years to come.
> > > Some developers have started work on this before, but due to time
> > > constraints it mostly got stuck in WIP phases.
> >
> > Yes, and same goes for all those "should be done first" stories.
> > They rarely become reality.
> >
> > It can be done:
> >
> > - before
> > - simultaneously
> > - afterwards
> 
> 
> It cannot be done afterwards, because you are already defining a new
> API how subtitles are being transported, and we can't just change the
> API every day.
> 
> >
> > The only option I don't like is the "before" option because it
> would
> > case another delay and another dependency on something that is not
> > really a complicated thing to do and has been unnecessarily stuck
> > for years.
> >
> 
> "Before" helps you as much as it helps us. If we haven't even agreed
> how patch 1 is supposed to look, it can be rather wasteful to review
> anything else, and similarly for you quite an extra burden to keep
> updating everything before even the most basic infrastructure is
> created and agree'ed upon.

I understand this, but when you take a close look you will realize that
this patch and the AVSubtitle=>AVFrame merging are semantically
more or less independent, which transfers to the code level as
a refactoring not a re-development.

 
> "How to store subtitles in AVFrame" is not something we will ever
> accept in a quick drive-by patch, this is fundamental FFmpeg
> architecture, and needs to be done right and thoroughly. I don't care
> about anything coming after that at this point, I care about the
> first
> step being done right. We can talk and think about next steps after
> this most crucial one was done correctly. This is how it has to be,
> and no other way.

I'm totally fine to do that before as long as not somebody starts to put up 
unrelated constraints onto that work.

My approach for the merging would be rather simple: 

Move all AVSubtitle properties to AVFrame (except the duplicate ones),
keep AVSubtitleRect and everything else as-is.

What doesn't make sense to me is trying to fiddle subtitle data into
AVFrame's data[] and buf[]. That just doesn't fit. Also, keeping AVSubtitleRect
(as array) makes sense to keep as it as is, as it matches the requirement;
finally it minimizes the required amount of changes.

Regards,
softworkz