[FFmpeg-devel] [PATCH] lavc: deprecate AV_CODEC_FLAG_DROPCHANGED

[Anton Khirnov]

Quoting Gyan Doshi (2023-07-13 21:20:32)
> 
> 
> On 2023-07-12 07:14 pm, Anton Khirnov wrote:
> > Quoting Gyan Doshi (2023-07-12 15:31:57)
> >>
> >> On 2023-07-12 06:51 pm, James Almer wrote:
> >>> On 7/12/2023 10:14 AM, Gyan Doshi wrote:
> >>>>
> >>>> On 2023-07-12 06:12 pm, James Almer wrote:
> >>>>> On 7/9/2023 9:57 AM, Anton Khirnov wrote:
> >>>>>> This decoding flag makes decoders drop all frames after a parameter
> >>>>>> change, but what exactly constitutes a parameter change is not well
> >>>>>> defined and will typically depend on the exact use case.
> >>>>>> This functionality then does not belong in libavcodec, but rather in
> >>>>>> user code
> >>>> NAK.
> >>>>
> >>>> The applicable parameters are well defined as set by the implementation.
> >>> The implementation defined its own interpretation of what constitutes
> >>> a parameter change, yes, but callers may have their own
> >>> interpretations too. The result is they either don't set this flag, or
> >>> set it but still manually check every frame for other parameters.
> >>>
> >>>> They are whatever leads to a change in the size of the decoded frame
> >>>> payload or the layout/semantics of the elemental unit in a decoded
> >>>> frame, so width, height, pixel format, sample format/size,
> >>>> interleaving..etc
> >>> You give pixel format as an example, which would include a change from
> >>> yuv to a yuvj pseudo format. What about decoders that export yuv +
> >>> color_range mpeg and then yuv + color_range jpeg? This implementation
> >>> will not consider that a parameter change, despite being practically
> >>> the same scenario.
> >>> Once the yuvj formats are removed, this will be true for all decoders.
> >>> And suddenly starting to check for color_range would be a change in
> >>> implementation here.
> >>>
> >>> This is definitely something that should be left to the caller. They
> >>> get a frame out of the decoder, they decide if they want to drop it.
> >> They can still do that. Don't set the flag (default) and check
> >> manually.  Keeping the flag in lavc doesn't prevent callers from
> >> inserting their own gate.
> > The point is that what this code considers a parameter change is
> > entirely arbitrary.
> >
> > Why does it not consider any of the colorspace parameters? Or
> > interlacing, cropping, display transform matrix, or any of the other
> > AVFrame fields or side data we might add in the future that someone
> > migth consider a parameter change in their use case.
> 
> This flag considers the changes to AVframe raster data that can lead to 
> out of bounds read/writes or mangling of that data.
> All the side data add-ons are irrelevant.

That is inconsistent with its handling of
* audio sample rate (should be ignored)
* audio channel layouts (only channel count should be considered)
* YUVJ formats as James mentioned, possibly also some others

And however that may be, my main point still stands: I see no good
reason why this should be handled in libavcodec rather than in the
caller.

> When is major bump to 61 expected?

Most likely in 7.0 in early 2024, but the 61 check in the patch is a
placeholder - typically deprecated APIs are removed after they've been
deprecated for a longer time. So expect it to actually happen in major
62, probably in early 2025.

-- 
Anton Khirnov