From patchwork Wed Jan 26 21:34:51 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 33875 Delivered-To: ffmpegpatchwork2@gmail.com Received: by 2002:a05:6602:2c4e:0:0:0:0 with SMTP id x14csp1883285iov; Wed, 26 Jan 2022 13:37:00 -0800 (PST) X-Google-Smtp-Source: ABdhPJzBKqszgiRxuSWcSJ1x6lXGfq0aulRXZgmVHnAxRDL7Clwnzt2CjQH4Fsc9PTxXQ1lTcgd0 X-Received: by 2002:a17:907:3d8f:: with SMTP id he15mr467196ejc.623.1643233020487; Wed, 26 Jan 2022 13:37:00 -0800 (PST) Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id n15si236509edy.584.2022.01.26.13.37.00; Wed, 26 Jan 2022 13:37:00 -0800 (PST) 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=@outlook.com header.s=selector1 header.b=DTp0+4k6; arc=fail (body hash mismatch); 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=QUARANTINE dis=NONE) header.from=outlook.com Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 8032E68B1B3; Wed, 26 Jan 2022 23:35:28 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from EUR03-VE1-obe.outbound.protection.outlook.com (mail-oln040092072020.outbound.protection.outlook.com [40.92.72.20]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 55BC168B185 for ; Wed, 26 Jan 2022 23:35:23 +0200 (EET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=BU1ToUPkzxmNuqX28qCXnaxNG9Av0SKpIVz2PxRiv2ZZu4cbuoyA2arlz/2jCV7Br+Umb+ywYv40FBhiEA2GXgi+RkBjLJ5L47VsbIjokfYetPx80Fz6H5+71nRrQcBl3YNoE5xyhpwUE/4E9sZcTIGsFnUjQWU1JxDgKgYOK1wRmkWsDLe1oADifwlO7p84HdBl2J5MVeYWZSiuTmHsXrRaWfJRFjQuKGB1tiy31aEHzRAahlOkrQJrz1v6qMhcJJP7XreC4U18y5jVyOCo1Z0htmvZkwHoo0isjqIijwF+0gOR7DN9g0cJCcy6znt3xhUjbEN2XjKoP5r9Lgf0JA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=/FvBrohaP0DckpDZx3q2FDNmng3it5duE8mBWygvQoA=; b=PUnG5tZO8EgDsNfEfc0cj7lnQmdDYRP11Oeswsp0lQf2L3GTc3Qj8ssnQh74borSVT6RmUSnFBeOGSo4QBrvLVtfjV0mlm8y1z+0MgdZePo2VzU16DrG6qUXXh9SstS3eP+FKTbGeOb729Beifz3kPRE3TRt/1VSRqH5ZTHvOEmPDPgrMy2xhFoK2ciH46RW9WCBJcDMTgvkzrH739VNbxdMC0w5yBvVU7Rgg7sCoMQ4NDcBUqf2XQeLubzlj8pKBMqwdhImq7N6Ci1dm0eXrha6rdTb4ELHgl611vTxl/jAUxjuDSsiuCb4L0uUib78GvcdOyW0sHx3L0glyWyg+w== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=outlook.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=/FvBrohaP0DckpDZx3q2FDNmng3it5duE8mBWygvQoA=; b=DTp0+4k6eTxHjhJdw1heh15q/FXsJEEiO3RtqZMPgNN2K3Pe4ygrsYftnddfvSTaBi1DANSiWFf+5znhuDbSQiS+EjKx/pK8czGUcbtA0tx74JYvzOBcl1aNrUip5Hrjh8LYVRWmpDuJOerFPOvybVSNcqSQxvUBzNkjTLsjwa9wLARopEwy7jK5XdWiN3dtFDnk95vvKnRacZWxc7vFTx+znwnBFPUrAiglvy9QXi3b99njL/MS5tm1OZSJxfCq+S7wcBnnOVBPOzx0RMocUtCugrr3IWVBwbs+uFkEvFO47IIynozc+WdaYsbm4UaNY61yTN6RHTmURg21c3mkLw== Received: from AM7PR03MB6660.eurprd03.prod.outlook.com (2603:10a6:20b:1c1::22) by AM9PR03MB7852.eurprd03.prod.outlook.com (2603:10a6:20b:433::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4909.17; Wed, 26 Jan 2022 21:35:19 +0000 Received: from AM7PR03MB6660.eurprd03.prod.outlook.com ([fe80::ac56:2ff4:d304:ab22]) by AM7PR03MB6660.eurprd03.prod.outlook.com ([fe80::ac56:2ff4:d304:ab22%4]) with mapi id 15.20.4930.017; Wed, 26 Jan 2022 21:35:19 +0000 From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Wed, 26 Jan 2022 22:34:51 +0100 Message-ID: X-Mailer: git-send-email 2.32.0 In-Reply-To: References: X-TMN: [UxPbrhi+91JrLWFui3vUCa8D9qnF36eG] X-ClientProxiedBy: AM3PR03CA0066.eurprd03.prod.outlook.com (2603:10a6:207:5::24) To AM7PR03MB6660.eurprd03.prod.outlook.com (2603:10a6:20b:1c1::22) X-Microsoft-Original-Message-ID: <20220126213451.1887291-12-andreas.rheinhardt@outlook.com> MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 3a1a0444-98c2-48a2-92f1-08d9e113bfee X-MS-TrafficTypeDiagnostic: AM9PR03MB7852:EE_ X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: /WOBhLQYlUvI+da08Dx2cWplll+w8gX7Rpfm3D3joG1gdvGUmh/UOOtRSviFQBPc5OyRmJcj7ldkwHk7HcpYOv7d5R4x0IgC+DPPmCODFgj7kvnVm54A5UKTf78jUuuusl2YbFgwy5FiNceMxZqhwQz9E+ICWY2ySVe8fc5X5TjzMqJAob0BDeZPMKQA2j3t/DGLLciJGwnPieqj6xdZHThZIihvnU501S2XOeeFD+ufD9Zty5mcBtPLNb7BvMymRvaHkyk7sna8tdDr78t7fZRL2oaxUOr0dbMgqjcK59RPJU86zk41x2gu8mDPxRYOlcBtvrpNphecmPNGWKRMEkp9mN7fZoQK8I0e5j9cWnGlGzNJZszR/8W4RqL/NaheIVK8SthP06oMJi2OG+BqIERisL4boSznvbNdcDHmoxlx3F8pUw3/jcYuudiJB91vuvbRORSXDxQHJpvoXb9ctQcJath22LlZNk8wCdwHfKDhQlpl0JscQZng5uD3qOzsS8se01WgvbnPaDVwuNinAH38P0BSHQABXD+1w5ZqWzdWNc31Qf0e22AjIbbfFCBi X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: Ss5FLPfwmmjv7KcQ4cjZNN5OYy49EsZRazOkl59Ui+dyJx+TEljocXDMl32IJgQlYmc9k4+TxDMwklol9Df7JGxb/fcESbO0DMSy+eh2timfbrf81Om1EvdGBhdZz7ha/NMQs/5nzXxkZKXiRxqOxgJIJi7/6/2x5O1u7aKi91JBE8iVf8epqFa2acNvxkR3XkYU+2iLCesDQrVQRpuE6OSCZN3uCkL0lk9nHX4L+FXCzlQTk/iVkaq/Pd82EA3SsIVJtWBtXbl48iXpbgO9bkAIJ0KELsISibePLZCrLcnYe5Rzsx3i09azgowoFFoMa3+ibX5nhbRo7bY9xfycQNDvwAvAML60IQ/8UiwALj0fhjFOpqDEvZpcpNWEhlF+vWlx1IomYYg2xF2K4DiHzTQgIswu2Wo6Bq7UXINohPkcWjtrCS7IlTxTL3HRRY6jO3frX7kEyJZ+bVXMARpBt6un0qKjyaGz20gZjepuPbVV62HP34CNrj0GlPeEK/n9afOTha1pS/rqZoIJqK/oRuu7W6Dfu3m7iRq+nPKH4csoqDFIaR30i9GU1DjLG9MVODj8PYx/Op5liCWuo03GvKi8A3oRyr2isHdqNEOCgjieA4KgPUFLIQNXqo6AXOIi7WPafQCtGAw/7HqW/ooG1WoryOl4xWla4Xd9ImrEU8SWaP46arPwpLh/DCygoOBeKSpcjRW5HXqmrJwqM3s+LRT4IfEHKxdTVUZ0NYRkVTnfEADSdOJaMvddTN5bcfjcVUhOFN6D1KAAjd3Tlwmqj6/3ZI4LAfGCidX0dcfZZxFsXqWWkAeIiRw9+PgKvv4YsufUxpsV5pqJtOx1w0rzZH2I0b0Mqp+rKIyVmj6cJuNisU+kxU8f+xv/lwwRBtTpkXtnUrvn1SLsyB6gZgXqLT6boOEsgOvdJuA4d1/xIwjRiBjliFrLvz3QJYDtlAn5gGFeXEFZmSfpx5jdxzVmHg== X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 3a1a0444-98c2-48a2-92f1-08d9e113bfee X-MS-Exchange-CrossTenant-AuthSource: AM7PR03MB6660.eurprd03.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Jan 2022 21:35:19.1129 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM9PR03MB7852 Subject: [FFmpeg-devel] [PATCH 33/33] avcodec/mpeg4videodec: Move use_intra_dc_vlc to stack, fix data race X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 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: Andreas Rheinhardt Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" X-TUID: +yvOzZXO4u7/ use_intra_dc_vlc is currently kept in sync between frame threads in mpeg4_update_thread_context(), yet it is set when decoding blocks, i.e. after ff_thread_finish_setup(). This is a data race and therefore undefined behaviour. This race can be fixed easily by moving the variable from the context to the stack: use_intra_dc_vlc is only read in mpeg4_decode_block() and only if one is decoding an intra block. There are three callsites for this function: One in mpeg4_decode_partitioned_mb() which always sets use_intra_dc_vlc before the call and two in mpeg4_decode_mb(). One of these callsites is for intra blocks and use_intra_dc_vlc is set before it; the last callsite is for non-intra blocks, where use_intra_dc_vlc is ignored. So if it is used, it always uses a new value and can therefore be moved to the stack. The above also explains why this data race did not lead to FATE-test failures. Signed-off-by: Andreas Rheinhardt --- libavcodec/mpeg4video.h | 1 - libavcodec/mpeg4videodec.c | 24 ++++++++++++++---------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/libavcodec/mpeg4video.h b/libavcodec/mpeg4video.h index 9fc79b1a22..14fc5e1396 100644 --- a/libavcodec/mpeg4video.h +++ b/libavcodec/mpeg4video.h @@ -94,7 +94,6 @@ typedef struct Mpeg4DecContext { int new_pred; int enhancement_type; int scalability; - int use_intra_dc_vlc; /// QP above which the ac VLC should be used for intra dc int intra_dc_threshold; diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c index b8118ff2d2..2aea845580 100644 --- a/libavcodec/mpeg4videodec.c +++ b/libavcodec/mpeg4videodec.c @@ -1105,7 +1105,8 @@ int ff_mpeg4_decode_partitions(Mpeg4DecContext *ctx) * @return <0 if an error occurred */ static inline int mpeg4_decode_block(Mpeg4DecContext *ctx, int16_t *block, - int n, int coded, int intra, int rvlc) + int n, int coded, int intra, + int use_intra_dc_vlc, int rvlc) { MpegEncContext *s = &ctx->m; int level, i, last, run, qmul, qadd; @@ -1117,7 +1118,7 @@ static inline int mpeg4_decode_block(Mpeg4DecContext *ctx, int16_t *block, // Note intra & rvlc should be optimized away if this is inlined if (intra) { - if (ctx->use_intra_dc_vlc) { + if (use_intra_dc_vlc) { /* DC coef */ if (s->partitioned_frame) { level = s->dc_val[0][s->block_index[n]]; @@ -1357,7 +1358,7 @@ static inline int mpeg4_decode_block(Mpeg4DecContext *ctx, int16_t *block, not_coded: if (intra) { - if (!ctx->use_intra_dc_vlc) { + if (!use_intra_dc_vlc) { block[0] = ff_mpeg4_pred_dc(s, n, block[0], &dc_pred_dir, 0); i -= i >> 31; // if (i == -1) i = 0; @@ -1378,7 +1379,7 @@ not_coded: static int mpeg4_decode_partitioned_mb(MpegEncContext *s, int16_t block[6][64]) { Mpeg4DecContext *ctx = s->avctx->priv_data; - int cbp, mb_type; + int cbp, mb_type, use_intra_dc_vlc; const int xy = s->mb_x + s->mb_y * s->mb_stride; av_assert2(s == (void*)ctx); @@ -1386,7 +1387,7 @@ static int mpeg4_decode_partitioned_mb(MpegEncContext *s, int16_t block[6][64]) mb_type = s->current_picture.mb_type[xy]; cbp = s->cbp_table[xy]; - ctx->use_intra_dc_vlc = s->qscale < ctx->intra_dc_threshold; + use_intra_dc_vlc = s->qscale < ctx->intra_dc_threshold; if (s->current_picture.qscale_table[xy] != s->qscale) ff_set_qscale(s, s->current_picture.qscale_table[xy]); @@ -1436,7 +1437,8 @@ static int mpeg4_decode_partitioned_mb(MpegEncContext *s, int16_t block[6][64]) s->bdsp.clear_blocks(s->block[0]); /* decode each block */ for (i = 0; i < 6; i++) { - if (mpeg4_decode_block(ctx, block[i], i, cbp & 32, s->mb_intra, ctx->rvlc) < 0) { + if (mpeg4_decode_block(ctx, block[i], i, cbp & 32, s->mb_intra, + use_intra_dc_vlc, ctx->rvlc) < 0) { av_log(s->avctx, AV_LOG_ERROR, "texture corrupted at %d %d %d\n", s->mb_x, s->mb_y, s->mb_intra); @@ -1763,6 +1765,8 @@ static int mpeg4_decode_mb(MpegEncContext *s, int16_t block[6][64]) } s->current_picture.mb_type[xy] = mb_type; } else { /* I-Frame */ + int use_intra_dc_vlc; + do { cbpc = get_vlc2(&s->gb, ff_h263_intra_MCBPC_vlc.table, INTRA_MCBPC_VLC_BITS, 2); if (cbpc < 0) { @@ -1790,7 +1794,7 @@ intra: } cbp = (cbpc & 3) | (cbpy << 2); - ctx->use_intra_dc_vlc = s->qscale < ctx->intra_dc_threshold; + use_intra_dc_vlc = s->qscale < ctx->intra_dc_threshold; if (dquant) ff_set_qscale(s, s->qscale + quant_tab[get_bits(&s->gb, 2)]); @@ -1801,7 +1805,8 @@ intra: s->bdsp.clear_blocks(s->block[0]); /* decode each block */ for (i = 0; i < 6; i++) { - if (mpeg4_decode_block(ctx, block[i], i, cbp & 32, 1, 0) < 0) + if (mpeg4_decode_block(ctx, block[i], i, cbp & 32, + 1, use_intra_dc_vlc, 0) < 0) return AVERROR_INVALIDDATA; cbp += cbp; } @@ -1810,7 +1815,7 @@ intra: /* decode each block */ for (i = 0; i < 6; i++) { - if (mpeg4_decode_block(ctx, block[i], i, cbp & 32, 0, 0) < 0) + if (mpeg4_decode_block(ctx, block[i], i, cbp & 32, 0, 0, 0) < 0) return AVERROR_INVALIDDATA; cbp += cbp; } @@ -3529,7 +3534,6 @@ static int mpeg4_update_thread_context(AVCodecContext *dst, s->new_pred = s1->new_pred; s->enhancement_type = s1->enhancement_type; s->scalability = s1->scalability; - s->use_intra_dc_vlc = s1->use_intra_dc_vlc; s->intra_dc_threshold = s1->intra_dc_threshold; s->divx_version = s1->divx_version; s->divx_build = s1->divx_build;