On Wed, 13 Dec 2023, Anton Khirnov wrote:

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?

I thought about using that, but there are plenty of error concealment code which only checks if avctx->error_concealment is nonzero or zero, and not specific EC flags. So unless that is fixed (which might break existing behaviour) one cannot introduce a new EC flag and disable error concealment at the same time...

Regards,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to