From patchwork Fri Mar 4 03:40:54 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 34597 Delivered-To: ffmpegpatchwork2@gmail.com Received: by 2002:a05:6838:d078:0:0:0:0 with SMTP id x24csp1213500nkx; Thu, 3 Mar 2022 19:41:21 -0800 (PST) X-Google-Smtp-Source: ABdhPJzP923w4mqSmTaSGiXgBa/kyv/kE+diDdRjnY8BJhkRg4Cqa2DS8mbvdfGVPmjZ2Dwwc2WU X-Received: by 2002:a17:907:a40c:b0:6da:b4bd:c38a with SMTP id sg12-20020a170907a40c00b006dab4bdc38amr1395760ejc.385.1646365281546; Thu, 03 Mar 2022 19:41:21 -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 s15-20020a056402520f00b004138d050eb6si2778388edd.173.2022.03.03.19.41.20; Thu, 03 Mar 2022 19:41:21 -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=ca8wwf1C; 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 8BAB968B0BE; Fri, 4 Mar 2022 05:41:16 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from EUR02-HE1-obe.outbound.protection.outlook.com (mail-oln040092068025.outbound.protection.outlook.com [40.92.68.25]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id A548968AB18 for ; Fri, 4 Mar 2022 05:41:09 +0200 (EET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ZRtQ8raTSldrIcwbBGmh9TB6mzKZ+HkEwkToroPDAhKgG/nkFWinVNBVWmGbLXw1uYirOkGju5HHm4iEzDD3+YhFWphz9Q7R4BThYiW/ToMLK6F65Z1VuM1JGmRLg/smNnYx+7anTC6wm0+RmpIjlEppZa1tQgxol2DJTxpQZiMxSiYbTeyDHu9jb24go6kLAqrNbg5D3DdWw90I7habUkEer5oHMtHQL59Xw9gdER6EA7uSgEMHio4O+K5lSAbP1tVf0GVoVABdNasPzDlfGbn1V+m5Im1XSja5TX/muEfCT7F+taUpxNgXbnlGE7z4BF+dXY9hscIT2D9oHkOmrg== 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=RG9Ph05ECjIBEXBqwFQfEyFTETk+8xErzgwFM3lfKEE=; b=B75pq6kPfdltPznQHAG+9V8cmPj7iljzEtfdVrDxfRq5MOI99rvxsgw44kylwm/ZwFURZkdDVgVZ0A50raUNBIM69rbMHr1c+NQ1KVEf60kk0Cs0U1WmpH8DRjpy8F76nGRQfoKOQkjHAz21IlX325FDHWoMeS4grJO2pvQnUSQuPPBGVwFgVJxTXAYsFG43YQP6GxGk9rtIMiARWtbmcV+PzHuhM7KTplGfyMGePTqnuZ8LV3IEi4MavgwBwkqkDfrRjEfUDJ5MHBDkZj01XAbIqTisu+CbuIL+0qh6I03agdI+/XvDF+ucBbCPGQ261AMcxa3QJVoPVA/2Y8A/6Q== 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=RG9Ph05ECjIBEXBqwFQfEyFTETk+8xErzgwFM3lfKEE=; b=ca8wwf1CW2Ye8u5o2Y/HEX3NNwTeOFyeIHuZywT6maREbZTz2zmJtnXwRORVmzcYAm4P9Ro1TM0L/dFUOMXvz6D1hp1/MnQhMVUiEQ8my3ZoS8lnAl7dmEhFPyI7kH0crbzFJ/2QVpM1dkkuRKE9x5NC4NhppQZfrT6T9ODchENVc3bAUuP6oquUo8u0uJmy9L/kQKE6B0N/z9tmybHD4YklfTbDM4MbRP6LLovmhSQMV3StMAB6dYZPNXzca9FSfMsD3qryYA0fFtPZKX6HrNUUf3XL39afzrJbWY5+YB0EexDvUBxX/Oyr9h5xk3qccQLTZAe/TAeQAfERwCimlg== Received: from AM7PR03MB6660.eurprd03.prod.outlook.com (2603:10a6:20b:1c1::22) by VI1PR03MB4333.eurprd03.prod.outlook.com (2603:10a6:803:60::25) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5017.25; Fri, 4 Mar 2022 03:41:06 +0000 Received: from AM7PR03MB6660.eurprd03.prod.outlook.com ([fe80::b8d3:effd:9a3c:4090]) by AM7PR03MB6660.eurprd03.prod.outlook.com ([fe80::b8d3:effd:9a3c:4090%5]) with mapi id 15.20.5038.016; Fri, 4 Mar 2022 03:41:06 +0000 From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Fri, 4 Mar 2022 04:40:54 +0100 Message-ID: X-Mailer: git-send-email 2.32.0 X-TMN: [X9IX/7DnUJdsFJ4gaLwfr6T6omuWwYta] X-ClientProxiedBy: AM5PR0202CA0011.eurprd02.prod.outlook.com (2603:10a6:203:69::21) To AM7PR03MB6660.eurprd03.prod.outlook.com (2603:10a6:20b:1c1::22) X-Microsoft-Original-Message-ID: <20220304034054.29825-1-andreas.rheinhardt@outlook.com> MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: dbbcc2ab-c737-4b22-aa11-08d9fd90d058 X-MS-Exchange-SLBlob-MailProps: +LiGfBxqLEtfxYSlGQowr1hS2Iop5Tj6u0DHpJGST3/YTGTFoyuOT2NIoBmU192OVGubJJMTb4wgU+RYjZSoWUXC5uteB6amLY6G2d0sAls3+7X75icqV6psc37ZwbknT68HxjTp/lW7a2lc9u/nZcX9o3eZXVVYvLaVgjP6XryvcpDOZwLBmt1S2AX1DJsgbpwhF8kWNW4kGvmpLhYGjvfoOa+PpJA7NScn5npvq0UK/krBuiN+huhqFA2iD/NDATXdUj62PZ53h/dqg4FP26QxBlpYI+Z+RTA8Pv0yy9g1hpxEppQlISvz1PBe8/plstj51Z3RH0N5GH7PcLNNTCdVnPJGQlPcdmrX8mgsPPm38PFtRjHo1tnuCHS5Rqv6yYZl60ZRbb4XHuCttgiCGqLyGY7w33roPG4uiTqOC27NB4ous0yLh4XkLXQRgy9CWDfYNvHXiG/xZ0iRJai6qHEI67Hf8wDP24vaS9PocP6cBjtS+Pjr7e2N7eSmZ5eqggVrkgeCtA5GWmTBt50s559OkHmMwOE5cAMe4v8hmrq1g1ugppLkKSJtF565gzWEid6WIWBJBOHG/OxBmdPTKvtjymHVMuTbLdMl8KKX9kqZr3UnodQ60zwCiiPeUgt1mguk33w7GrT7akPx6TPU7DCC7SEuJMGI6wbw/1Xx5B3zYJ3Q3JiQBDgQHF69PEVNyzMzyl7YF8rstWA1AQJx9ZsKHP4uRLFtzpw5HTmkplwfImldSDDh5S50YAVX7QVo5qwhRZI4DDxeStaZ/WznxFY+rCvyEYPQ X-MS-TrafficTypeDiagnostic: VI1PR03MB4333:EE_ X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 76iNtTBJSOMASsu67g50CVrHK9kjyMDUHVvNDLl59xCePocQ2BuPNrH64qo6RsdmEsjMrQBchTDUeoWIEPQmkwZJqKzn03PFmmJ0mEz4DEZWE8isLo452+HTGEX4QOXwefTs8WL6FUh0L+6xFFvyak6peDs0xlQ0lK4r/twLc3EcvQN8IHbZldqSayD3nNoZDeHjEiDXA8D/Lt3U5qj+9tyK97SiNrOKXO/QCFzcwTmh5ZB2zS6IXLNnd7uTvoJgmFhGig9xx0ruFieNpYkyxtomdZG/93cxBSjgpnJJCOUz/ba19K82PTtNfkRm2MdWPtL9eJEcJM10JYpVCCntxj04PFFeaMLmf7Fa8/z6oN/wt4H/PxwjbrTsO4v1ciuB/UFBAijBOvVPpYJCOyVkJovM4fMKTVGL70ZNhZNObkIs+2HuOk/owwCtmbaB9j5S0GxqGfKeJrsfZTmhUcTGamPBmbFS0QB01KuXGynVr3gl4HpQ1/HaHY+Q3ZcmloSczUpZq+916BewibcDZlS2G9xsMn6OH+XKB63rmCvAtaFgQ2mOlC79dFz+Fxq33cuGzilYlNaEWVdE99WiyQHW7g== X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: 5ZvNykm9OA1cqmQdQ19K3mtEe37/ufR7heepM1HGGOFMBdr1mKW0vR12OiMGstP3OIy4vi9AxCnLQIY7YQgE3tqw9F3kAD0b4KtcAn92iDIU//JQNhTf0GQof0c0qMVV/om/uazdt2Yszz0vSIOLXNG5AkWFcm/v63YRCMho80hKuYWNuzUDcCd5J/8WCqwfjIxqHrvnm9EVbTDjt/vQXFeaYdkw3BVGn0PyAciLfMJc3espfIHrGNpffwZiPdhVefJIgclNM2RtPTuh4UC7mR/A+3nNNqCjMb4UOKNDL6p8xwis8CONr1vrAR6stUPRkX0PdWggLXc8Lm7rbQ8liKGWTbLu9juGFwDdxsCMvd2OY1uZGLjecXMNXvDP4Fd7lqdTwpg1hHzRxKIRVoBVCiND3/3GaNFgxnZZphti6YvLzPtpZjm4ouhzXIk3cgCR3nyGWAeuIeiR6RPf3cKciqSAwGlImD+HirNMohUldxhRcPrrOZCiCVWG8KWJw6eLab/JeJEfdCPnBHsStaPJ0eam/PAvsZ8jlMmiiiiYxUOsBN0GBf266NLCqgu0AKUJolOR0yFz7Ym7TZXWX7tS/XevG5aw2Yx1D2Cpq4oFFfkew3cbLhXPxaLi78wkRUmyq0y03Bn1MiOhvtIDaptvjF8yIPIGD594qvGJgQT7AXPqgHcMQaalTeG/06xQw/c21Yxr4Lyc2+IrN3yU7sc8KwzZjUyuWPx4y9zibub36NFjeNifJfqI8N5BMX4+iAuEtLlkLcrhbuZueEovJrl7LCu9KSRHnh4PuTNlyxODJNWsIA74OEO9tHvyqKeBlCkdt/I/JbKdvgkcsRY7ChLWiHYxD04r/aQSGeXvctK0ieIHglp7wqmWn9BiQ11YJe/bSH8z9E5LHRC+ZX2FXGMJBkXBiY79HMS92L1092LW0rUKiIB+hBeyJvEujenhqtmSoEeG/htz0939obR4E6A6Uw== X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: dbbcc2ab-c737-4b22-aa11-08d9fd90d058 X-MS-Exchange-CrossTenant-AuthSource: AM7PR03MB6660.eurprd03.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Mar 2022 03:41:06.8486 (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: VI1PR03MB4333 Subject: [FFmpeg-devel] [PATCH v3] avcodec/ffv1dec: Don't set ThreadFrame properties, fix 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: xgggK6kI0Y9/ Each FFV1 slice has its own SAR and picture structure encoded; when a slice header was parsed, the relevant fields of a ThreadFrame's AVFrame were directly set based upon the parsed values. This is a data race in case slice threading is in use because of the concurrent writes. In case of frame threading, it is also a data race because the writes happen after ff_thread_finish_setup(), so that the reads performed by ff_thread_ref_frame() are unsynchronized with the writes performed when parsing the header. This commit fixes these issues by not writing to the ThreadFrame at all; instead the raw data is read into the each SliceContext first; after decoding the current frame and creating the actual output frame these values are compared to each other. If they are valid and coincide, the derived value is written directly to the output frame, not to the ThreadFrame, thereby avoiding data races; in case they are not valid or inconsistent the most commonly used valid value is used instead. This fixes most FFV1 FATE-tests completely when using slice threading; the exceptions are fate-vsynth3-ffv1, vsynth3-ffv1-v3-yuv420p and vsynth3-ffv1-v3-yuv422p10. (In these tests the partitioning into slices does not honour chroma subsampling; as a result, chroma pixels at slice borders get set by more than one thread without any synchronization.) Signed-off-by: Andreas Rheinhardt --- libavcodec/ffv1.h | 4 ++ libavcodec/ffv1dec.c | 130 ++++++++++++++++++++++++++++++++++++------- 2 files changed, 114 insertions(+), 20 deletions(-) diff --git a/libavcodec/ffv1.h b/libavcodec/ffv1.h index ac80fa85ce..f640d5a710 100644 --- a/libavcodec/ffv1.h +++ b/libavcodec/ffv1.h @@ -91,6 +91,8 @@ typedef struct FFV1Context { struct FFV1Context *fsrc; AVFrame *cur; + int picture_structure; + AVRational sample_aspect_ratio; int plane_count; int ac; ///< 1=range coder <-> 0=golomb rice int ac_byte_count; ///< number of bytes used for AC coding @@ -132,6 +134,8 @@ typedef struct FFV1Context { int slice_coding_mode; int slice_rct_by_coef; int slice_rct_ry_coef; + + AVRational slice_sample_aspect_ratios[MAX_SLICES]; } FFV1Context; int ff_ffv1_common_init(AVCodecContext *avctx); diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c index 201630167d..8a8ab90b2b 100644 --- a/libavcodec/ffv1dec.c +++ b/libavcodec/ffv1dec.c @@ -167,7 +167,7 @@ static int decode_slice_header(const FFV1Context *f, FFV1Context *fs) { RangeCoder *c = &fs->c; uint8_t state[CONTEXT_SIZE]; - unsigned ps, i, context_count; + unsigned i, context_count; memset(state, 128, sizeof(state)); av_assert0(f->version > 2); @@ -205,25 +205,17 @@ static int decode_slice_header(const FFV1Context *f, FFV1Context *fs) p->context_count = context_count; } - ps = get_symbol(c, state, 0); - if (ps == 1) { - f->cur->interlaced_frame = 1; - f->cur->top_field_first = 1; - } else if (ps == 2) { - f->cur->interlaced_frame = 1; - f->cur->top_field_first = 0; - } else if (ps == 3) { - f->cur->interlaced_frame = 0; - } - f->cur->sample_aspect_ratio.num = get_symbol(c, state, 0); - f->cur->sample_aspect_ratio.den = get_symbol(c, state, 0); - - if (av_image_check_sar(f->width, f->height, - f->cur->sample_aspect_ratio) < 0) { + fs->picture_structure = get_symbol(c, state, 0); + fs->sample_aspect_ratio.num = get_symbol(c, state, 0); + fs->sample_aspect_ratio.den = get_symbol(c, state, 0); + /* Num or den being zero means unknown for FFV1; our unknown is 0 / 1. */ + if (fs->sample_aspect_ratio.num == 0 || fs->sample_aspect_ratio.den == 0) { + fs->sample_aspect_ratio = (AVRational) { 0, 1 }; + } else if (av_image_check_sar(f->width, f->height, + fs->sample_aspect_ratio) < 0) { av_log(f->avctx, AV_LOG_WARNING, "ignoring invalid SAR: %u/%u\n", - f->cur->sample_aspect_ratio.num, - f->cur->sample_aspect_ratio.den); - f->cur->sample_aspect_ratio = (AVRational){ 0, 1 }; + fs->sample_aspect_ratio.num, fs->sample_aspect_ratio.den); + fs->sample_aspect_ratio = (AVRational) { 0, 0 }; } if (fs->version > 3) { @@ -251,6 +243,9 @@ static int decode_slice(AVCodecContext *c, void *arg) AVFrame * const p = f->cur; int i, si; + fs->picture_structure = 0; + fs->sample_aspect_ratio = (AVRational){ 0, 0 }; + for( si=0; fs != f->slice_context[si]; si ++) ; @@ -831,6 +826,28 @@ static av_cold int decode_init(AVCodecContext *avctx) return 0; } +/* Macro to simplify comparisons of the rational values we deal with here. + * get_symbol() ensures that these fit into 32bits, so that one can just + * compare them in 64bits; they are also actually unsigned, so cast to that. + * Notice that av_image_check_sar() checks for these values to be + * positive integers. */ +#define CMP_Q(x, y) (FFDIFFSIGN((unsigned)(x).num * (uint64_t)(unsigned)(y).den,\ + (unsigned)(y).num * (uint64_t)(unsigned)(x).den)) +static int compare_sar(const void *a, const void *b) +{ + const AVRational x = *(const AVRational*)a; + const AVRational y = *(const AVRational*)b; + int cmp; + + /* 0/0 means invalid and 0/1 means unknown. + * Order the fractions so that the former are at the end. */ + if (x.den == 0 || y.den == 0) + return y.den - x.den; + cmp = CMP_Q(x, y); + /* For definiteness, order equivalent fractions by "lower terms last". */ + return cmp ? cmp : y.num - x.num; +} + static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame, AVPacket *avpkt) { uint8_t *buf = avpkt->data; @@ -969,9 +986,82 @@ static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame, AVPac if (f->last_picture.f) ff_thread_release_ext_buffer(avctx, &f->last_picture); - if ((ret = av_frame_ref(data, f->picture.f)) < 0) + p = data; + if ((ret = av_frame_ref(p, f->picture.f)) < 0) return ret; + if (f->version > 2) { + AVRational sar = f->slice_context[0]->sample_aspect_ratio; + int pic_structures[4] = { 0 }, picture_structure; + int inconsistent_sar = 0, has_sar = 0; + + for (int i = 0; i < f->slice_count; i++) { + const FFV1Context *const fs = f->slice_context[i]; + int slice_picture_structure = (unsigned)fs->picture_structure > 3 ? + 0 : fs->picture_structure; + + pic_structures[slice_picture_structure]++; + + if (fs->sample_aspect_ratio.num != sar.num || + fs->sample_aspect_ratio.den != sar.den) + inconsistent_sar = 1; + sar = fs->sample_aspect_ratio; + if (sar.den == 0) + inconsistent_sar = 1; + else + has_sar = 1; + f->slice_sample_aspect_ratios[i] = sar; + } + if (has_sar) { + if (inconsistent_sar) { + AVRational last_sar = (AVRational){ 0, 0 }; + qsort(f->slice_sample_aspect_ratios, f->slice_count, + sizeof(f->slice_sample_aspect_ratios[0]), compare_sar); + + for (int i = 0, current_run = 0, longest_run = 0; + i < f->slice_count; i++) { + AVRational cur_sar = f->slice_sample_aspect_ratios[i]; + + if (cur_sar.den == 0) + break; + if (CMP_Q(cur_sar, last_sar)) + current_run = 1; + else + current_run++; + /* Using >= here has the effect of preferring + * bigger ratios if the runs are identical; + * this ensures that a known SAR is preferred over + * an unknown SAR in case there are equally many. */ + if (current_run >= longest_run) { + longest_run = current_run; + sar = cur_sar; + } + last_sar = cur_sar; + } + av_log(avctx, AV_LOG_WARNING, "SAR inconsistent, using %u/%u\n", + sar.num, sar.den); + } + p->sample_aspect_ratio = sar; + } + + for (int i = 0, count = 0; i < FF_ARRAY_ELEMS(pic_structures); i++) { + if (pic_structures[i] > count) { + picture_structure = i; + count = pic_structures[i]; + } + } + /* Set picture structure based upon the most commonly encountered + * picture structure. */ + if (picture_structure == 1) { + p->interlaced_frame = 1; + p->top_field_first = 1; + } else if (picture_structure == 2) { + p->interlaced_frame = 1; + p->top_field_first = 0; + } else if (picture_structure == 3) + p->interlaced_frame = 0; + } + *got_frame = 1; return buf_size;