[FFmpeg-devel] [PATCH v16 01/16] global: Prepare AVFrame for subtitle handling

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Anton
> Khirnov
> Sent: Saturday, November 27, 2021 11:33 AM
> To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v16 01/16] global: Prepare AVFrame for
> subtitle handling
> 
> Quoting Andreas Rheinhardt (2021-11-27 10:42:31)
> > Anton Khirnov:
> > > Quoting Andreas Rheinhardt (2021-11-27 10:06:35)
> > >> Anton Khirnov:
> > >>>
> > >>> Not sure whether this was asked already - why do we need this new
> > >>> function? Seems to me you can accomplish the same thing by just adding
> > >>> the type field to AVFrame. Then
> > >>> - if type is AVMEDIA_TYPE_SUBTITLE -> allocate a subtitle
> > >>> - if type is AVMEDIA_TYPE_{VIDEO,AUDIO} -> allocate video/audio
> > >>> - otherwise detect video/audio as we do now
> > >>>
> > >>
> > >> https://ffmpeg.org/pipermail/ffmpeg-devel/2021-September/285185.html
> > >
> > > So IIUC the only concern is that a user might "manually" unref the frame
> > > without calling av_frame_unref(). I would say that this is already
> > > illegal, because we can (and did) add new allocated objects to AVFrame
> > > that would break things if you just kept them from one frame to the
> > > other (especially of a different type), e.g. hw_frames_ctx.
> > >
> >
> > And how is a user then supposed to know which modifications of an
> > AVFrame are legal or not? I think the onus is on us not to break user
> > apps and not on the user to behave in such a way that it is compatible
> > with all possible future additions to AVFrames (which would be
> > impossible anyway).
> 
> We explicitly allow adding new fields to AVFrame, so valid user code has
> to consider the possibility. So any "substracting" operations on frames
> it does not fully control (e.g. those it received from decoders or
> filters) are potentially broken.
> 
> I believe we have to assume this, otherwise we cannot extend AVFrame or
> other structs like we've been doing so far.

Back to the subtitles patch: 

I have added the AVMediaType field to the start of the struct. Once, because 
it seems too elementary to come at position sixty-something and also because
I was assuming it would cause a break anway.

Leaving the AVSubtitle and AVSubtitleRect structs inside libavcodec has definitely
reduced the list of breakage candidates.

Next candidate to consider regarding breakage would probably be the promotion 
of those ass functions from avcodec to avutil..

sw