[FFmpeg-devel] [PATCH v2 7/7] avcodec: add AV_CODEC_FLAG_CLEAR

[Marton Balint]

On Thu, 7 Dec 2023, Anton Khirnov wrote:

> Quoting Marton Balint (2023-12-06 09:22:20)
>> Signed-off-by: Marton Balint <cus at passwd.hu>
>> ---
>>  doc/APIchanges             |  3 +++
>>  doc/codecs.texi            | 14 ++++++++++++++
>>  libavcodec/avcodec.h       |  4 ++++
>>  libavcodec/decode.c        |  6 ++++++
>>  libavcodec/options_table.h |  1 +
>>  libavcodec/version.h       |  2 +-
>>  6 files changed, 29 insertions(+), 1 deletion(-)
>>
>> diff --git a/doc/APIchanges b/doc/APIchanges
>> index 416e2bec5e..f839504a64 100644
>> --- a/doc/APIchanges
>> +++ b/doc/APIchanges
>> @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2023-02-09
>>
>>  API changes, most recent first:
>>
>> +2023-12-xx - xxxxxxxxxxx - lavc 60.36.100 - avcodec.h
>> +  Add AV_CODEC_FLAG_CLEAR.
>
> I'm not a huge fan of calling it just 'clear'. How about something more
> descriptive like 'wipe_frames'.

Wipe reminds me of the wipe effect. How about 'predecode_clear'?

>> +
>>  2023-12-xx - xxxxxxxxxxx - lavu 58.33.100 - imgutils.h
>>    Add av_image_fill_color()
>>
>> diff --git a/doc/codecs.texi b/doc/codecs.texi
>> index 5b950b4560..0504a535f2 100644
>> --- a/doc/codecs.texi
>> +++ b/doc/codecs.texi
>> @@ -76,6 +76,20 @@ Apply interlaced motion estimation.
>>  Use closed gop.
>>  @item output_corrupt
>>  Output even potentially corrupted frames.
>> + at item clear
>> +Clear the contents of the video buffer before decoding the next picture to it.
>> +
>> +Usually if only a part of a picture is affected by a decode error then the
>> +decoder (if it implements error concealment) tries to hide it by interpolating
>> +pixels from neighbouring areas or in some cases from the previous frame. Even
>> +without error concealment it is quite likely that the affected area will
>> +contain pixels from an earlier frame, due to frame pooling.
>> +
>> +For quality checking this might not be desirable, because it makes the errors
>> +less noticable. By using this flag, and combining it with disabled error
>> +concealment (@code{-ec 0}) it is possible to ensure that no leftover data from
>> +an earlier frame is presented in areas affected by decode errors.
>> +
>>  @end table
>>
>>  @item time_base @var{rational number}
>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>> index 7fb44e28f4..97848e942f 100644
>> --- a/libavcodec/avcodec.h
>> +++ b/libavcodec/avcodec.h
>> @@ -312,6 +312,10 @@ typedef struct RcOverride{
>>   * loop filter.
>>   */
>>  #define AV_CODEC_FLAG_LOOP_FILTER     (1 << 11)
>> +/**
>> + * Clear frame buffer contents before decoding.
>
> Clear the contents of each frame buffer after it is allocated with
> AVCodecContext.get_buffer2() and before anything is decoded into it.
>
> Note that this may have a significant performance cost.

ok.

>
>> + */
>> +#define AV_CODEC_FLAG_CLEAR           (1 << 12)
>>  /**
>>   * Only decode/encode grayscale.
>>   */
>> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
>> index 2cfb3fcf97..f9b18a2c35 100644
>> --- a/libavcodec/decode.c
>> +++ b/libavcodec/decode.c
>> @@ -1675,6 +1675,12 @@ FF_ENABLE_DEPRECATION_WARNINGS
>>
>>      validate_avframe_allocation(avctx, frame);
>>
>> +    if (avctx->flags & AV_CODEC_FLAG_CLEAR && avctx->codec_type == AVMEDIA_TYPE_VIDEO) {
>> +        uint32_t color[4] = {0};
>> +        ptrdiff_t linesize[4] = {frame->linesize[0], frame->linesize[1], frame->linesize[2], frame->linesize[3]};
>> +        av_image_fill_color(frame->data, linesize, frame->format, color, frame->width, frame->height);
>
> Should this check for errors?

Lack of error checking is intentional. av_image_fill_color might not 
support all pixel formats, definitely not support hwaccel formats. It 
might make sense to warn the user once, but I don't think propagating the 
error back is needed here.

I primarily thought of this as a QC feature (even thought about making the 
color fill green by default to make it more noticeable (YUV green happens 
to be 0,0,0), but for that I'd need similar checks for colorspaces to 
what I have for fill_black())...

Regards,
Marton