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

[Anton Khirnov]

Quoting Marton Balint (2023-12-12 19:37:57)
> 
> 
> On Tue, 12 Dec 2023, Anton Khirnov wrote:
> 
> > Quoting Marton Balint (2023-12-08 00:11:21)
> >> Wipe reminds me of the wipe effect. How about 'predecode_clear'?
> >
> > Fine with me I guess.
> >
> >>>
> >>>> + */
> >>>> +#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())...
> >
> > As Mark said, I expect people to want to use it as a security feature.
> > So either it should work reliably, or it should be made very clear that
> > it's for debugging only.
> >
> > For non-hwaccel pixel formats, you can fall back on memsetting the
> > buffer to 0.
> 
> IMHO for security you don't need to clear every frame, only new ones in 
> the buffer pool. Considering that it only affects the first few 
> frames, we might want to do that unconditionally, and not based on a 
> flag. And it is better to do this on the buffer level (because you might 
> not overwrite some bytes because of alignment with a color fill).
> 
> So for this flag, I'd rather make it clear it is not security-related, and 
> also that it has performance impact.

So then maybe make a FF_EC flag?

-- 
Anton Khirnov