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

[Michael Niedermayer]

On Tue, Dec 12, 2023 at 12:23:38PM +0100, 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.

For security, there may be other less vissible things that should be cleared too
for example B frames in some codecs can use motion vectors from surrounding frames.

Also the correct thing is to apply error concealment to replace all parts which
have not been filled in. Not to leave them uninitialized.
Not only does that preserve privacy it also produces much better looking frames
We have error concealment code, it should be used.

Not arguing against this feature here. Just saying for security/privacy it has
a price as it needs to be done before we know if a frame is damaged, while error
concealment is done only on actually damaged frames
Of course you may want to do both to be really "sure" ...

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Nations do behave wisely once they have exhausted all other alternatives. 
-- Abba Eban
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20231212/b4dd8b9f/attachment.sig>