From patchwork Fri Mar 31 13:49:52 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Ronald S. Bultje" X-Patchwork-Id: 3219 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.44.195 with SMTP id s186csp2052282vss; Fri, 31 Mar 2017 06:50:05 -0700 (PDT) X-Received: by 10.223.176.175 with SMTP id i44mr3237742wra.96.1490968205589; Fri, 31 Mar 2017 06:50:05 -0700 (PDT) Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id p136si3555422wme.71.2017.03.31.06.50.05; Fri, 31 Mar 2017 06:50:05 -0700 (PDT) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@gmail.com; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=gmail.com Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 712F96882DD; Fri, 31 Mar 2017 16:50:02 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-qk0-f196.google.com (mail-qk0-f196.google.com [209.85.220.196]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 58C21688266 for ; Fri, 31 Mar 2017 16:49:56 +0300 (EEST) Received: by mail-qk0-f196.google.com with SMTP id 10so10489446qkh.1 for ; Fri, 31 Mar 2017 06:49:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=tD8XJqCKZoXmmAM/xmVSw3bg6BkazJ/GFTuGvITZmqM=; b=NGAMkHviQF5kM5PiEVP2byEnoXoebPiYP4oPr+uvS8fdZskiqUuO9ykmGWsMxTODI+ Y0v+t6YTIKQFXYwZ3n56gVQaT9/3Veuhx7ocqRzD654mYzRJqTzUrZM0Lvof5L8/AUSE Mx/EZIaVuB5BxEXNUCQv+K/KTu/4ONK5VmMPbas7MnbyCK/NFWuWA0bwft+NXF72HLnf TW2llWedkt6arElUP/ZLnE00nXdP15xdcfQrbEOz8jl4UhazrxCk0MTihKx5jD53l1ji EWtiedTTXPjTWDElRjJ1otMTjCBigvDiaqp5nEp5/aJ7i3vvQFxV16fjHMok7wdAtR3L fTGw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=tD8XJqCKZoXmmAM/xmVSw3bg6BkazJ/GFTuGvITZmqM=; b=RttFfgxFk7FMpTDMDx5oCovi9PC9F3FSGqD7QZgWUhK3l9c4XVdwJ27z2gtoN8S3jw 9Cpe8W+t9BGiVBsRrA1xgEkSkRBCQ1KLxSY9ZGAhSaLrXx7IxcvSk/RTktcbANyvq7f7 gYSAu6uyc7opNqwNx+6i21vN2y0b71ncZepsibHoo2bk1aP1RuaocNeDPzl9qEFgiVTh 3g46tVxZYDbLt1ysln5De4AvBf7zLeiT2Jh754KHMdZByjTNQimCLfG6619wilg8+zeL MTnHMXekjeN7JjcBrUVufkj/vyDsH+Etawv9/NSYagKeOCRi/rrZUo1eO+13kvYgfm4W MKUQ== X-Gm-Message-State: AFeK/H0reGcP1WzNbGQuAyZhhECteyQ+2GR9zyDVLcydikqws0U6XcsklofJ0WtafxnihA== X-Received: by 10.55.26.24 with SMTP id a24mr2921172qka.139.1490968195416; Fri, 31 Mar 2017 06:49:55 -0700 (PDT) Received: from localhost.localdomain ([65.206.95.146]) by smtp.gmail.com with ESMTPSA id g66sm3530914qkb.55.2017.03.31.06.49.54 (version=TLS1 cipher=AES128-SHA bits=128/128); Fri, 31 Mar 2017 06:49:54 -0700 (PDT) From: "Ronald S. Bultje" To: ffmpeg-devel@ffmpeg.org Date: Fri, 31 Mar 2017 09:49:52 -0400 Message-Id: <1490968192-55497-1-git-send-email-rsbultje@gmail.com> X-Mailer: git-send-email 2.8.1 In-Reply-To: <20170329215325.GB4714@nb4> References: <20170329215325.GB4714@nb4> Subject: [FFmpeg-devel] [PATCH] png: split header state and data state in two separate variables. X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: "Ronald S. Bultje" MIME-Version: 1.0 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Fixes a reported (but false) race condition in tsan for fate-apng. --- libavcodec/png.h | 5 ---- libavcodec/pngdec.c | 68 +++++++++++++++++++++++++++++++---------------------- 2 files changed, 40 insertions(+), 33 deletions(-) diff --git a/libavcodec/png.h b/libavcodec/png.h index 948c2f7..e967fcf 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 c08665b..80645f2 100644 --- a/libavcodec/pngdec.c +++ b/libavcodec/pngdec.c @@ -36,6 +36,17 @@ #include +enum PNGHeaderState { + PNG_IHDR = 1 << 0, + PNG_PLTE = 1 << 1, +}; +enum PNGImageState { + PNG_IMG_NONE, + PNG_IMG_ALL, + PNG_IMG_IDAT, +}; + + typedef struct PNGDecContext { PNGDSPContext dsp; AVCodecContext *avctx; @@ -45,7 +56,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; @@ -334,7 +346,7 @@ static void png_handle_row(PNGDecContext *s) } s->y++; if (s->y == s->cur_h) { - s->state |= PNG_ALLIMAGE; + s->pic_state = PNG_IMG_ALL; if (s->filter_type == PNG_FILTER_TYPE_LOCO) { if (s->bit_depth == 16) { deloco_rgb16((uint16_t *)ptr, s->row_size / 2, @@ -369,7 +381,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_IMG_ALL; goto the_end; } else { s->pass++; @@ -404,7 +416,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_IMG_ALL) { png_handle_row(s); } s->zstream.avail_out = s->crow_size; @@ -541,12 +553,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_IMG_NONE) { 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; } @@ -569,7 +581,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", @@ -585,7 +597,7 @@ error: static int decode_phys_chunk(AVCodecContext *avctx, PNGDecContext *s) { - if (s->state & PNG_IDAT) { + if (s->pic_state != PNG_IMG_NONE) { av_log(avctx, AV_LOG_ERROR, "pHYs after IDAT\n"); return AVERROR_INVALIDDATA; } @@ -605,11 +617,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_IMG_NONE) { /* init image info */ avctx->width = s->width; avctx->height = s->height; @@ -690,11 +702,10 @@ static int decode_idat_chunk(AVCodecContext *avctx, PNGDecContext *s, if ((ret = ff_thread_get_buffer(avctx, &s->previous_picture, AV_GET_BUFFER_FLAG_REF)) < 0) return ret; } - ff_thread_finish_setup(avctx); - p->pict_type = AV_PICTURE_TYPE_I; p->key_frame = 1; p->interlaced_frame = !!s->interlace_type; + ff_thread_finish_setup(avctx); /* compute the compressed row size */ if (!s->interlace_type) { @@ -734,7 +745,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_IMG_IDAT; /* set image to non-transparent bpp while decompressing */ if (s->has_trns && s->color_type != PNG_COLOR_TYPE_PALETTE) @@ -770,7 +781,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; @@ -781,18 +792,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_IMG_NONE) { 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++) { @@ -906,7 +917,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; } @@ -1122,13 +1133,13 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s, } if (CONFIG_APNG_DECODER && avctx->codec_id == AV_CODEC_ID_APNG && length == 0) { - if (!(s->state & PNG_IDAT)) + if (s->pic_state == PNG_IMG_NONE) 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_IMG_ALL && avctx->strict_std_compliance <= FF_COMPLIANCE_NORMAL) goto exit_loop; ret = AVERROR_INVALIDDATA; @@ -1228,9 +1239,9 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s, break; } case MKTAG('I', 'E', 'N', 'D'): - if (!(s->state & PNG_ALLIMAGE)) + if (s->pic_state != PNG_IMG_ALL) av_log(avctx, AV_LOG_ERROR, "IEND without all image\n"); - if (!(s->state & (PNG_ALLIMAGE|PNG_IDAT))) { + if (s->pic_state == PNG_IMG_NONE) { ret = AVERROR_INVALIDDATA; goto fail; } @@ -1330,7 +1341,8 @@ static int decode_frame_png(AVCodecContext *avctx, return AVERROR_INVALIDDATA; } - s->y = s->state = s->has_trns = 0; + s->y = s->hdr_state = s->has_trns = 0; + s->pic_state = PNG_IMG_NONE; /* init the zlib */ s->zstream.zalloc = ff_png_zalloc; @@ -1377,7 +1389,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; @@ -1397,14 +1409,14 @@ static int decode_frame_apng(AVCodecContext *avctx, goto end; } s->y = 0; - s->state &= ~(PNG_IDAT | PNG_ALLIMAGE); + s->pic_state = PNG_IMG_NONE; 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_IMG_ALL) 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_IMG_NONE) { ret = AVERROR_INVALIDDATA; goto end; } @@ -1453,7 +1465,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] &&