[FFmpeg-devel] [PATCH 1/3][RFC] avutil/frame: add AV_FRAME_FLAG_DISPOSABLE

[James Almer]

On 8/27/2019 10:03 AM, Michael Niedermayer wrote:
> On Tue, Aug 27, 2019 at 10:58:13AM +0200, Marton Balint wrote:
>>
>>
>> On Tue, 27 Aug 2019, Michael Niedermayer wrote:
>>
>>> On Tue, Aug 27, 2019 at 09:12:53AM +0200, Marton Balint wrote:
>>>>
>>>>
>>>> On Mon, 26 Aug 2019, James Almer wrote:
>>>>
>>>>> Used to signal frames that can be safely discarded without losing
>>>>> any picture data, side data, or metadata other than timing info.
>>>>>
>>>>> Signed-off-by: James Almer <jamrial at gmail.com>
>>>>> ---
>>>>> This implements the "disposable frame" solution to allow library
>>>>> users to drop duplicate frames before further processing if desired,
>>>>> instead of forcing decoders to output vfr content when cfr is coded
>>>>> in the bitstream.
>>>>>
>>>>> doc/APIchanges      | 3 +++
>>>>> libavutil/frame.h   | 5 +++++
>>>>> libavutil/version.h | 2 +-
>>>>> 3 files changed, 9 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/doc/APIchanges b/doc/APIchanges
>>>>> index 682b67aa25..b28d702bae 100644
>>>>> --- a/doc/APIchanges
>>>>> +++ b/doc/APIchanges
>>>>> @@ -15,6 +15,9 @@ libavutil:     2017-10-21
>>>>>
>>>>> API changes, most recent first:
>>>>>
>>>>> +2019-08-xx - xxxxxxxxxx - lavu 58.34.100 - avframe.h
>>>>> +  Add AV_FRAME_FLAG_DISPOSABLE
>>>>> +
>>>>> 2019-08-xx - xxxxxxxxxx - lavf 58.31.101 - avio.h
>>>>> 4K limit removed from avio_printf.
>>>>>
>>>>> diff --git a/libavutil/frame.h b/libavutil/frame.h
>>>>> index 5d3231e7bb..e1bf8795d2 100644
>>>>> --- a/libavutil/frame.h
>>>>> +++ b/libavutil/frame.h
>>>>> @@ -522,6 +522,11 @@ typedef struct AVFrame {
>>>>> * A flag to mark the frames which need to be decoded, but shouldn't be output.
>>>>> */
>>>>> #define AV_FRAME_FLAG_DISCARD   (1 << 2)
>>>>> +/**
>>>>> + * A flag to indicate frames that can be discarded by the encoder. I.e. frames
>>>>> + * that are an exact duplicate of the previous one.
>>>>> + */
>>>>> +#define AV_FRAME_FLAG_DISPOSABLE    (1 << 3)
>>>>
>>>> Such a flag has the problem that it is not really an attribute of a frame in
>>>> itself, but of a frame of the frame chain. That can cause various issues: If
>>>> the first frame is dropped, the next frame no longer becomes disposable.
>>>>
>>>
>>>> For the issue we are trying to solve I feel that it is better if we create a
>>>> video filter which checks the buffers (and side data and crop parameters) if
>>>> they are the same and drops the frame. It also allows more granular control
>>>> (or setting parameters, like making sure you get a frame every 1 second or
>>>> so, which is common requirement even for VFR), plus it can work for existing
>>>> cases without this flag implemented.
>>>
>>> One disadvantage of this with the current ref counting system is that in codecs
>>> which do in place changes in a buffer.
>>> Keeping a reference to a frame means the decoder must copy the frame to get a
>>> new writable one to do changes in. Not keeping a ref avoids this.
>>> A method which detects duplicates without keeping a +1 reference on the
>>> previous frame would be better in this respect ...
>>>
>>
>> That is a fair point. I am OK with the frame flags then.
> 
> a possible alternative would be some sort of unique id which
> is different for each decoder instance and only changes when
> any ref changes.
> This would avoid the problem of droping the first frame
> another one would be some sort of lightweight references
> which can be lost if no other reference is left. But this
> would probably requires more core changes
> 
> iam fine with the flag, just dumping my thoughts here
> 
> thx

I don't mind waiting a bit to further discuss how to signal these
frames, but in the meantime i want to push patches 2 and 4 from this
set, which are independent and fix ticket #7880 while reducing data copy
as much as possible.

I'll also send a patch to do the same as patch 4 from this set but for
the nuv decoder.