[FFmpeg-cvslog] png: split header state and data state in two separate variables.
Ronald S. Bultje
git at videolan.org
Sun Jul 5 20:51:41 EEST 2020
ffmpeg | branch: release/2.8 | Ronald S. Bultje <rsbultje at gmail.com> | Mon Apr 3 10:08:29 2017 -0400| [453519af0d4a50bbc62999b312de9692e429bf22] | committer: Michael Niedermayer
png: split header state and data state in two separate variables.
Fixes a reported (but false) race condition in tsan for fate-apng:
WARNING: ThreadSanitizer: data race (pid=6274)
Read of size 4 at 0x7d680001ec78 by main thread (mutexes: write M1338):
#0 update_thread_context src/libavcodec/pngdec.c:1456 (ffmpeg+0x000000dacf0c)
[..]
Previous write of size 4 at 0x7d680001ec78 by thread T1 (mutexes: write M1335):
#0 decode_idat_chunk src/libavcodec/pngdec.c:737 (ffmpeg+0x000000dae951)
(cherry picked from commit 478f1c3d5e5463a284ea7efecfc62d47ba3be11a)
Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=453519af0d4a50bbc62999b312de9692e429bf22
---
libavcodec/png.h | 5 -----
libavcodec/pngdec.c | 65 ++++++++++++++++++++++++++++++++---------------------
2 files changed, 39 insertions(+), 31 deletions(-)
diff --git a/libavcodec/png.h b/libavcodec/png.h
index 948c2f714f..e967fcf38f 100644
--- a/libavcodec/png.h
+++ b/libavcodec/png.h
@@ -42,11 +42,6 @@
#define PNG_FILTER_VALUE_PAETH 4
#define PNG_FILTER_VALUE_MIXED 5
-#define PNG_IHDR 0x0001
-#define PNG_IDAT 0x0002
-#define PNG_ALLIMAGE 0x0004
-#define PNG_PLTE 0x0008
-
#define NB_PASSES 7
#define PNGSIG 0x89504e470d0a1a0a
diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
index 3fa4e4e4c0..bfc7c9da0b 100644
--- a/libavcodec/pngdec.c
+++ b/libavcodec/pngdec.c
@@ -34,6 +34,16 @@
#include <zlib.h>
+enum PNGHeaderState {
+ PNG_IHDR = 1 << 0,
+ PNG_PLTE = 1 << 1,
+};
+
+enum PNGImageState {
+ PNG_IDAT = 1 << 0,
+ PNG_ALLIMAGE = 1 << 1,
+};
+
typedef struct PNGDecContext {
PNGDSPContext dsp;
AVCodecContext *avctx;
@@ -43,7 +53,8 @@ typedef struct PNGDecContext {
ThreadFrame last_picture;
ThreadFrame picture;
- int state;
+ enum PNGHeaderState hdr_state;
+ enum PNGImageState pic_state;
int width, height;
int cur_w, cur_h;
int last_w, last_h;
@@ -332,7 +343,7 @@ static void png_handle_row(PNGDecContext *s)
}
s->y++;
if (s->y == s->cur_h) {
- s->state |= PNG_ALLIMAGE;
+ s->pic_state |= PNG_ALLIMAGE;
if (s->filter_type == PNG_FILTER_TYPE_LOCO) {
if (s->bit_depth == 16) {
deloco_rgb16((uint16_t *)ptr, s->row_size / 2,
@@ -367,7 +378,7 @@ static void png_handle_row(PNGDecContext *s)
memset(s->last_row, 0, s->row_size);
for (;;) {
if (s->pass == NB_PASSES - 1) {
- s->state |= PNG_ALLIMAGE;
+ s->pic_state |= PNG_ALLIMAGE;
goto the_end;
} else {
s->pass++;
@@ -402,7 +413,7 @@ static int png_decode_idat(PNGDecContext *s, int length)
return AVERROR_EXTERNAL;
}
if (s->zstream.avail_out == 0) {
- if (!(s->state & PNG_ALLIMAGE)) {
+ if (!(s->pic_state & PNG_ALLIMAGE)) {
png_handle_row(s);
}
s->zstream.avail_out = s->crow_size;
@@ -539,12 +550,12 @@ static int decode_ihdr_chunk(AVCodecContext *avctx, PNGDecContext *s,
if (length != 13)
return AVERROR_INVALIDDATA;
- if (s->state & PNG_IDAT) {
+ if (s->pic_state & PNG_IDAT) {
av_log(avctx, AV_LOG_ERROR, "IHDR after IDAT\n");
return AVERROR_INVALIDDATA;
}
- if (s->state & PNG_IHDR) {
+ if (s->hdr_state & PNG_IHDR) {
av_log(avctx, AV_LOG_ERROR, "Multiple IHDR\n");
return AVERROR_INVALIDDATA;
}
@@ -571,7 +582,7 @@ static int decode_ihdr_chunk(AVCodecContext *avctx, PNGDecContext *s,
s->filter_type = bytestream2_get_byte(&s->gb);
s->interlace_type = bytestream2_get_byte(&s->gb);
bytestream2_skip(&s->gb, 4); /* crc */
- s->state |= PNG_IHDR;
+ s->hdr_state |= PNG_IHDR;
if (avctx->debug & FF_DEBUG_PICT_INFO)
av_log(avctx, AV_LOG_DEBUG, "width=%d height=%d depth=%d color_type=%d "
"compression_type=%d filter_type=%d interlace_type=%d\n",
@@ -587,7 +598,7 @@ error:
static int decode_phys_chunk(AVCodecContext *avctx, PNGDecContext *s)
{
- if (s->state & PNG_IDAT) {
+ if (s->pic_state & PNG_IDAT) {
av_log(avctx, AV_LOG_ERROR, "pHYs after IDAT\n");
return AVERROR_INVALIDDATA;
}
@@ -607,11 +618,11 @@ static int decode_idat_chunk(AVCodecContext *avctx, PNGDecContext *s,
int ret;
size_t byte_depth = s->bit_depth > 8 ? 2 : 1;
- if (!(s->state & PNG_IHDR)) {
+ if (!(s->hdr_state & PNG_IHDR)) {
av_log(avctx, AV_LOG_ERROR, "IDAT without IHDR\n");
return AVERROR_INVALIDDATA;
}
- if (!(s->state & PNG_IDAT)) {
+ if (!(s->pic_state & PNG_IDAT)) {
/* init image info */
ret = ff_set_dimensions(avctx, s->width, s->height);
if (ret < 0)
@@ -737,7 +748,7 @@ static int decode_idat_chunk(AVCodecContext *avctx, PNGDecContext *s,
s->zstream.next_out = s->crow_buf;
}
- s->state |= PNG_IDAT;
+ s->pic_state |= PNG_IDAT;
/* set image to non-transparent bpp while decompressing */
if (s->has_trns && s->color_type != PNG_COLOR_TYPE_PALETTE)
@@ -773,7 +784,7 @@ static int decode_plte_chunk(AVCodecContext *avctx, PNGDecContext *s,
}
for (; i < 256; i++)
s->palette[i] = (0xFFU << 24);
- s->state |= PNG_PLTE;
+ s->hdr_state |= PNG_PLTE;
bytestream2_skip(&s->gb, 4); /* crc */
return 0;
@@ -784,18 +795,18 @@ static int decode_trns_chunk(AVCodecContext *avctx, PNGDecContext *s,
{
int v, i;
- if (!(s->state & PNG_IHDR)) {
+ if (!(s->hdr_state & PNG_IHDR)) {
av_log(avctx, AV_LOG_ERROR, "trns before IHDR\n");
return AVERROR_INVALIDDATA;
}
- if (s->state & PNG_IDAT) {
+ if (s->pic_state & PNG_IDAT) {
av_log(avctx, AV_LOG_ERROR, "trns after IDAT\n");
return AVERROR_INVALIDDATA;
}
if (s->color_type == PNG_COLOR_TYPE_PALETTE) {
- if (length > 256 || !(s->state & PNG_PLTE))
+ if (length > 256 || !(s->hdr_state & PNG_PLTE))
return AVERROR_INVALIDDATA;
for (i = 0; i < length; i++) {
@@ -909,7 +920,7 @@ static int decode_fctl_chunk(AVCodecContext *avctx, PNGDecContext *s,
if (length != 26)
return AVERROR_INVALIDDATA;
- if (!(s->state & PNG_IHDR)) {
+ if (!(s->hdr_state & PNG_IHDR)) {
av_log(avctx, AV_LOG_ERROR, "fctl before IHDR\n");
return AVERROR_INVALIDDATA;
}
@@ -1116,13 +1127,13 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s,
length = bytestream2_get_bytes_left(&s->gb);
if (length <= 0) {
if (CONFIG_APNG_DECODER && avctx->codec_id == AV_CODEC_ID_APNG && length == 0) {
- if (!(s->state & PNG_IDAT))
+ if (!(s->pic_state & PNG_IDAT))
return 0;
else
goto exit_loop;
}
av_log(avctx, AV_LOG_ERROR, "%d bytes left\n", length);
- if ( s->state & PNG_ALLIMAGE
+ if ( s->pic_state & PNG_ALLIMAGE
&& avctx->strict_std_compliance <= FF_COMPLIANCE_NORMAL)
goto exit_loop;
ret = AVERROR_INVALIDDATA;
@@ -1193,9 +1204,9 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s,
bytestream2_skip(&s->gb, length + 4);
break;
case MKTAG('I', 'E', 'N', 'D'):
- if (!(s->state & PNG_ALLIMAGE))
+ if (!(s->pic_state & PNG_ALLIMAGE))
av_log(avctx, AV_LOG_ERROR, "IEND without all image\n");
- if (!(s->state & (PNG_ALLIMAGE|PNG_IDAT))) {
+ if (!(s->pic_state & (PNG_ALLIMAGE|PNG_IDAT))) {
ret = AVERROR_INVALIDDATA;
goto fail;
}
@@ -1294,7 +1305,9 @@ static int decode_frame_png(AVCodecContext *avctx,
return AVERROR_INVALIDDATA;
}
- s->y = s->state = s->has_trns = 0;
+ s->y = s->has_trns = 0;
+ s->hdr_state = 0;
+ s->pic_state = 0;
/* init the zlib */
s->zstream.zalloc = ff_png_zalloc;
@@ -1335,7 +1348,7 @@ static int decode_frame_apng(AVCodecContext *avctx,
FFSWAP(ThreadFrame, s->picture, s->last_picture);
p = s->picture.f;
- if (!(s->state & PNG_IHDR)) {
+ if (!(s->hdr_state & PNG_IHDR)) {
if (!avctx->extradata_size)
return AVERROR_INVALIDDATA;
@@ -1355,14 +1368,14 @@ static int decode_frame_apng(AVCodecContext *avctx,
goto end;
}
s->y = 0;
- s->state &= ~(PNG_IDAT | PNG_ALLIMAGE);
+ s->pic_state = 0;
bytestream2_init(&s->gb, avpkt->data, avpkt->size);
if ((ret = decode_frame_common(avctx, s, p, avpkt)) < 0)
goto end;
- if (!(s->state & PNG_ALLIMAGE))
+ if (!(s->pic_state & PNG_ALLIMAGE))
av_log(avctx, AV_LOG_WARNING, "Frame did not contain a complete image\n");
- if (!(s->state & (PNG_ALLIMAGE|PNG_IDAT))) {
+ if (!(s->pic_state & (PNG_ALLIMAGE|PNG_IDAT))) {
ret = AVERROR_INVALIDDATA;
goto end;
}
@@ -1410,7 +1423,7 @@ static int update_thread_context(AVCodecContext *dst, const AVCodecContext *src)
memcpy(pdst->palette, psrc->palette, sizeof(pdst->palette));
- pdst->state |= psrc->state & (PNG_IHDR | PNG_PLTE);
+ pdst->hdr_state |= psrc->hdr_state;
ff_thread_release_buffer(dst, &pdst->last_picture);
if (psrc->last_picture.f->data[0] &&
More information about the ffmpeg-cvslog
mailing list