ffmpeg | branch: release/2.8 | Ronald S. Bultje <rsbul...@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 <mich...@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] && _______________________________________________ ffmpeg-cvslog mailing list ffmpeg-cvslog@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-cvslog To unsubscribe, visit link above, or email ffmpeg-cvslog-requ...@ffmpeg.org with subject "unsubscribe".