On 12/4/2020 7:57 PM, Andreas Rheinhardt wrote:
A reference to an AV1RawFrameHeader and consequently the
AV1RawFrameHeader itself and everything it has a reference to leak
if the hardware has no AV1 decoding capabilities.
It looks like it can happen if get_buffer() fails even with hardware
support.
It happens e.g.
in the cbs-av1-av1-1-b8-02-allintra FATE-test; it has just been masked
because the return value of ffmpeg (which indicates failure when using
Valgrind or ASAN) is ignored when doing tests of type md5.
Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com>
---
I switched the av_buffer_ref() and update_context_with_frame_header()
because the latter does not need any cleanup on failure.
Also notice that actual decoding with this patch applied is completely
untested.
libavcodec/av1dec.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
index 1589b8f0c0..d7b2ac9d46 100644
--- a/libavcodec/av1dec.c
+++ b/libavcodec/av1dec.c
@@ -674,20 +674,20 @@ static int av1_frame_alloc(AVCodecContext *avctx,
AV1Frame *f)
AVFrame *frame;
int ret;
- f->header_ref = av_buffer_ref(s->header_ref);
- if (!f->header_ref)
- return AVERROR(ENOMEM);
-
- f->raw_frame_header = s->raw_frame_header;
-
ret = update_context_with_frame_header(avctx, header);
if (ret < 0) {
av_log(avctx, AV_LOG_ERROR, "Failed to update context with frame
header\n");
return ret;
}
+ f->header_ref = av_buffer_ref(s->header_ref);
+ if (!f->header_ref)
+ return AVERROR(ENOMEM);
+
+ f->raw_frame_header = s->raw_frame_header;
+
if ((ret = ff_thread_get_buffer(avctx, &f->tf, AV_GET_BUFFER_FLAG_REF)) <
0)
- return ret;
+ goto fail;
frame = f->tf.f;
frame->key_frame = header->frame_type == AV1_FRAME_KEY;
@@ -710,8 +710,10 @@ static int av1_frame_alloc(AVCodecContext *avctx, AV1Frame
*f)
if (hwaccel->frame_priv_data_size) {
f->hwaccel_priv_buf =
av_buffer_allocz(hwaccel->frame_priv_data_size);
- if (!f->hwaccel_priv_buf)
+ if (!f->hwaccel_priv_buf) {
+ ret = AVERROR(ENOMEM);
goto fail;
+ }
f->hwaccel_picture_private = f->hwaccel_priv_buf->data;
}
}
@@ -719,7 +721,7 @@ static int av1_frame_alloc(AVCodecContext *avctx, AV1Frame
*f)
fail:
Just to be safe, add a ret = 0 above this line.
av1_frame_unref(avctx, f);
- return AVERROR(ENOMEM);
+ return ret;
}
static int set_output_frame(AVCodecContext *avctx, AVFrame *frame,
LGTM, and while unrelated to this fix, this also reveals that in some
scenarios, decoding without hardware support reaches this point, when
it's meant to abort when parsing the sequence header and being unable to
set up a hardware pixel format in avctx.
Looks like when parsing a second sequence header (Like in the second
keyframe) where s->pix_fmt is already set to a software format,
get_pixel_format() is not called, and so decoding proceeds to deal with
frames.
_______________________________________________
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".
