Also avoid a potential race with frame threading, where a frame's
metadata could be modified concurrently with that frame being passed to
another thread.
Fixes #8972
Found-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com>
---
libavcodec/pngdec.c | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
index a5a71ef161..00fabec34c 100644
--- a/libavcodec/pngdec.c
+++ b/libavcodec/pngdec.c
@@ -57,6 +57,8 @@ typedef struct PNGDecContext {
ThreadFrame last_picture;
ThreadFrame picture;
+ AVDictionary *frame_metadata;
+
enum PNGHeaderState hdr_state;
enum PNGImageState pic_state;
int width, height;
@@ -509,8 +511,7 @@ static uint8_t *iso88591_to_utf8(const uint8_t *in, size_t
size_in)
return out;
}
-static int decode_text_chunk(PNGDecContext *s, uint32_t length, int compressed,
- AVDictionary **dict)
+static int decode_text_chunk(PNGDecContext *s, uint32_t length, int compressed)
{
int ret, method;
const uint8_t *data = s->gb.buffer;
@@ -552,7 +553,7 @@ static int decode_text_chunk(PNGDecContext *s, uint32_t
length, int compressed,
return AVERROR(ENOMEM);
}
- av_dict_set(dict, kw_utf8, txt_utf8,
+ av_dict_set(&s->frame_metadata, kw_utf8, txt_utf8,
AV_DICT_DONT_STRDUP_KEY | AV_DICT_DONT_STRDUP_VAL);
return 0;
}
@@ -710,6 +711,8 @@ static int decode_idat_chunk(AVCodecContext *avctx,
PNGDecContext *s,
s->bpp += byte_depth;
}
+ av_dict_free(&s->frame_metadata);
+
ff_thread_release_buffer(avctx, &s->picture);
if ((ret = ff_thread_get_buffer(avctx, &s->picture,
AV_GET_BUFFER_FLAG_REF)) < 0)
return ret;
@@ -1182,7 +1185,6 @@ static int decode_frame_common(AVCodecContext *avctx,
PNGDecContext *s,
AVFrame *p, const AVPacket *avpkt)
{
const AVCRC *crc_tab = av_crc_get_table(AV_CRC_32_IEEE_LE);
- AVDictionary **metadatap = NULL;
uint32_t tag, length;
int decode_next_dat = 0;
int i, ret;
@@ -1250,7 +1252,6 @@ static int decode_frame_common(AVCodecContext *avctx,
PNGDecContext *s,
}
}
- metadatap = &p->metadata;
switch (tag) {
case MKTAG('I', 'H', 'D', 'R'):
if ((ret = decode_ihdr_chunk(avctx, s, length)) < 0)
@@ -1292,12 +1293,12 @@ static int decode_frame_common(AVCodecContext *avctx,
PNGDecContext *s,
goto skip_tag;
break;
case MKTAG('t', 'E', 'X', 't'):
- if (decode_text_chunk(s, length, 0, metadatap) < 0)
+ if (decode_text_chunk(s, length, 0) < 0)
av_log(avctx, AV_LOG_WARNING, "Broken tEXt chunk\n");
bytestream2_skip(&s->gb, length + 4);
break;
case MKTAG('z', 'T', 'X', 't'):
- if (decode_text_chunk(s, length, 1, metadatap) < 0)
+ if (decode_text_chunk(s, length, 1) < 0)
av_log(avctx, AV_LOG_WARNING, "Broken zTXt chunk\n");
bytestream2_skip(&s->gb, length + 4);
break;
@@ -1355,7 +1356,7 @@ static int decode_frame_common(AVCodecContext *avctx,
PNGDecContext *s,
if (ret < 0)
return ret;
- av_dict_set(&p->metadata, "gamma", gamma_str,
AV_DICT_DONT_STRDUP_VAL);
+ av_dict_set(&s->frame_metadata, "gamma", gamma_str,
AV_DICT_DONT_STRDUP_VAL);
bytestream2_skip(&s->gb, 4); /* crc */
break;
@@ -1466,6 +1467,7 @@ static int decode_frame_png(AVCodecContext *avctx,
PNGDecContext *const s = avctx->priv_data;
const uint8_t *buf = avpkt->data;
int buf_size = avpkt->size;
+ AVFrame *dst_frame = data;
AVFrame *p = s->picture.f;
int64_t sig;
int ret;
@@ -1503,9 +1505,11 @@ static int decode_frame_png(AVCodecContext *avctx,
goto the_end;
}
- if ((ret = av_frame_ref(data, s->picture.f)) < 0)
+ if ((ret = av_frame_ref(dst_frame, s->picture.f)) < 0)
goto the_end;
+ FFSWAP(AVDictionary*, dst_frame->metadata, s->frame_metadata);
+
if (!(avctx->active_thread_type & FF_THREAD_FRAME)) {
ff_thread_release_buffer(avctx, &s->last_picture);
FFSWAP(ThreadFrame, s->picture, s->last_picture);
@@ -1527,6 +1531,7 @@ static int decode_frame_apng(AVCodecContext *avctx,
AVPacket *avpkt)
{
PNGDecContext *const s = avctx->priv_data;
+ AVFrame *dst_frame = data;
int ret;
AVFrame *p = s->picture.f;
@@ -1564,6 +1569,8 @@ static int decode_frame_apng(AVCodecContext *avctx,
if ((ret = av_frame_ref(data, s->picture.f)) < 0)
goto end;
+ FFSWAP(AVDictionary*, dst_frame->metadata, s->frame_metadata);
+
if (!(avctx->active_thread_type & FF_THREAD_FRAME)) {
if (s->dispose_op == APNG_DISPOSE_OP_PREVIOUS) {
ff_thread_release_buffer(avctx, &s->picture);
@@ -1665,6 +1672,8 @@ static av_cold int png_dec_end(AVCodecContext *avctx)
av_freep(&s->tmp_row);
s->tmp_row_size = 0;
+ av_dict_free(&s->frame_metadata);
+
return 0;
}
--
2.30.1
_______________________________________________
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".
