From patchwork Sat Apr 24 11:14:34 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 27261 Delivered-To: andriy.gelman@gmail.com Received: by 2002:a25:49c5:0:0:0:0:0 with SMTP id w188csp2034399yba; Sat, 24 Apr 2021 04:15:20 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzkrHCByfeKt94hRgYLLpuOw95OsoILfK54h+zf0JTaSwR/B1Ub6B8Od5zC8OOf6eK4/fZT X-Received: by 2002:a17:906:2b05:: with SMTP id a5mr8800073ejg.446.1619262920023; Sat, 24 Apr 2021 04:15:20 -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 w2si9117023edi.199.2021.04.24.04.15.19; Sat, 24 Apr 2021 04:15:20 -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=@outlook.com header.s=selector1 header.b=XWPDU5Si; 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 1908168987B; Sat, 24 Apr 2021 14:15:18 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from EUR04-HE1-obe.outbound.protection.outlook.com (mail-oln040092073038.outbound.protection.outlook.com [40.92.73.38]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 045BC680260 for ; Sat, 24 Apr 2021 14:15:10 +0300 (EEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=XAoHJXhYyi9EOt5LkcrN9xmB5NRhWtrPZsoDT78niYzoH05gqSkMm2tkc81XvODHB3V/HwWL+PPf8xlIW/W1NsI373wWOSflb5Wr8ZQdbrLB7LSBMA2oKokPx4gD4nxqb6omx5Ghm2AZkUOFmNh3BvRoZIvdjfuWu5JE283rU80CkrvyMdOwj9E6jzS6b/Kfdul2r2TgGZTNrcv6RS7or6Kz8CrfYNX2Q8zznJpwWBkGC7Z9NBXpAZMqWWZtM+nRc5ZzgP59bbKkJR+zg7OuPX/vU2SBOFvD+w/Wwf/BSk1ECsgEmhv2k4rH/uA91ekUW6EZGFJtVh1HB3tFLiQ4hw== 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-SenderADCheck; bh=B1gS9zvHXM1geFrSEyBuEaKoF7M6pNCR13CErZFsYX4=; b=g2M+94kZGtlQtF8St7Dw/NUd6S7gmw0fiDdilFl48RdIIbPdCllLgdEP+wSuhs7oLQiwr3dV0QKoAOvLTNPV3IJgJcEq/8d1tK6E0fEoVccdBaBco+r6ayFVb0UG+y1zDzek+3fVaxF7WpnzkEyblFNydvQU3n83WKrWBPoctw4cQFhmG5seFlBu3Q9sAO1WI8gEIJA0vx/AeBqW7alKFw8tTUApT4pOqxibjhCtkQ9LnIvUJQdn+umcby64Ktv21eqXAKIXMsNynxt8cgue2nxRMzgBYvntV0HVs92j6DXwVFpsnyX4dOZ4LmB1fo+kPs0aQsELHTMWnkFYgyi70g== 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=B1gS9zvHXM1geFrSEyBuEaKoF7M6pNCR13CErZFsYX4=; b=XWPDU5SiBxCj4lVR950/HHgzfSzMsEiyTAedFISBStOhMw1hb3+AauEaqCUi0Xxe3aMH5kve1+aFIun7qQ6OCbSvulZgO0dAbqKnklbPXSTIKfJKCVtpI4T8hGf5RHpPbva0ou79Zy/lX8BLwSzlNlfkDm81MJ4hiNwAQcLam5QdgLbhT4r8B+Fw+knt4rBREzpHRi6GP+NyAEJbj4ixCgcLTMD/fIPkVh1XImXKHF2YK0bMXRTnmabaOHyfhmA+BWRu5p4Jd9ZhFxJFfyHOE7j4Upw7G2vhEC+Tz3VekdEuoQ9BZQRopM6bCHLIC0Z+7PiqJod/yN77HB0xLsORfA== Received: from HE1EUR04FT042.eop-eur04.prod.protection.outlook.com (2a01:111:e400:7e0d::40) by HE1EUR04HT079.eop-eur04.prod.protection.outlook.com (2a01:111:e400:7e0d::193) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4065.21; Sat, 24 Apr 2021 11:15:09 +0000 Received: from HE1PR0301MB2154.eurprd03.prod.outlook.com (2a01:111:e400:7e0d::46) by HE1EUR04FT042.mail.protection.outlook.com (2a01:111:e400:7e0d::159) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4065.21 via Frontend Transport; Sat, 24 Apr 2021 11:15:09 +0000 X-IncomingTopHeaderMarker: OriginalChecksum:2DB19B7F3223853F510C49BD5956B6D28584677F06A67E542F9B1D08E70A01CA; UpperCasedChecksum:994FC82A9A0831588C417218DBCF51E5DBAF676660E43269399C848B51DBC59A; SizeAsReceived:7588; Count:48 Received: from HE1PR0301MB2154.eurprd03.prod.outlook.com ([fe80::45bb:c44f:2b75:23b7]) by HE1PR0301MB2154.eurprd03.prod.outlook.com ([fe80::45bb:c44f:2b75:23b7%5]) with mapi id 15.20.4065.025; Sat, 24 Apr 2021 11:14:57 +0000 From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Sat, 24 Apr 2021 13:14:34 +0200 Message-ID: X-Mailer: git-send-email 2.27.0 In-Reply-To: References: X-TMN: [bsQYRLk1vQ97HKyttV7v6y65MyCknDKO] X-ClientProxiedBy: ZR0P278CA0160.CHEP278.PROD.OUTLOOK.COM (2603:10a6:910:41::18) To HE1PR0301MB2154.eurprd03.prod.outlook.com (2603:10a6:3:2a::22) X-Microsoft-Original-Message-ID: <20210424111446.30338-1-andreas.rheinhardt@outlook.com> MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from sblaptop.fritz.box (188.193.248.86) by ZR0P278CA0160.CHEP278.PROD.OUTLOOK.COM (2603:10a6:910:41::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4065.21 via Frontend Transport; Sat, 24 Apr 2021 11:14:56 +0000 X-MS-PublicTrafficType: Email X-IncomingHeaderCount: 48 X-EOPAttributedMessage: 0 X-MS-Office365-Filtering-Correlation-Id: b2958f18-16cd-4a3a-1e64-08d907123168 X-MS-Exchange-SLBlob-MailProps: +LiGfBxqLEuQ75gPcT42JOUsIEYyvf9YjmoxZbA1YbxeVKaxwGPngjqY8QEAPD5P2Jm98BZv96s6RR0Vbnm5YQrONCbF1N5thd6teGEPxkl0ZA606UiJaniO71+6vU4PLpS4B1nmFEok8rGkHQKRfO8ryWTVy8OyP1uN9NLVuPXhAFiMXSAFyKd2HlkUYL3b6i76Ry2I8y6UNHAXI5C2W1bWvw+zsSB6DqzqU/PKE/JdRWKXzWCb0unJ3ii1YwILVpvaqxrC/FLoiKiXytP2DkG/aczzRaFdqYa8PBeqXDs6vZRTPNwHQD/Ojk5coGBjwN9gfl5aC8l/4Q6bOwxMHhnAraVeQdM3ChFE9IKk1SlW0LI4qHTpeIzAnzHIEM2BL3lFjJFIzq38zKMDnXyhh+sbwqZxoYXSCjXGczmEv4TI1gjUm43Kpw26vI+uevnhNQS4tFSa828gtFODbgwM3MNDCefviosCMwVCS8aQfvEfvglS4lfrQs/p8ocXqLPfRKud2GF/j+GrAtXCiupgFIghInvqdO6DsSTOiKslKCQm3CXo11xlTvmoP6eS4wznEiW7ZIq1J/W9C07GpHuS3JHQ7PIIs//zOuvob/s8IOCCY57B5yQymnZCE6ZkLuTUap47ythrwlsXEAvvY2PLxosw44oyhGtFa2yNTm/HojEYRAdXppfFDYsP+ykMYaAGuNRLRdq1rzFgqTaRwKa0TzbsratCDJotqJ8i5db+pfaoCnGNyy2jJtZS5rlFhcsMs6wsVM2NNoPC2Li+yG7+z3dYYuYUdNC6 X-MS-TrafficTypeDiagnostic: HE1EUR04HT079: X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: Dk/ERvASt/6qWLgagQUyuP/aUU7HxpCbtwQQG6FESKh2O4bZYiZ8oipaZ5uRVXOdR5/utpj81EZpz/4R8+gCpX0kZQL1/YUA+8zfgCIynmHQ/aGD8tMskkoul/O+PJ2HbQM91OZ0j5gPZBm/j+weNMiF67M7ZHXT8WcYE0uRfC0b+Sekgbzeof2dqr8YcFUFTbLNbGWKHyV0GX97Hylw/VCgIWL8VzpdzQ6A5aBgIAE/u2VXTDZJNuogqnLWud+t0oSiTs/PMrEdMxyZc+yaNa1W8U8hVYnikM1bK1NqbLGlbsnARd65X+CBKCaYnUjWvruJM+r7gr4zJxFnUHj8D3FphAsVOuVhW7o+o1FJ4KQszt39CFZZHv88Emp0PR7DBjCl8o+m69pq/g7uwwjVow== X-MS-Exchange-AntiSpam-MessageData: KwPw/PFTmNQPUVawLSwtNBL7ToDWHG4iRKJ1oWZ8Iwas2LSeg4tFxTfKcJ1dwL+dAU7Rl34JpMZHyqfH4LWaneLW046f9cODTSinlk2XRxJCt4nk8C4rwTfnNdScA6snqZvwKSGPN2l0u4yCDC4big== X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: b2958f18-16cd-4a3a-1e64-08d907123168 X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Apr 2021 11:14:57.1587 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-CrossTenant-AuthSource: HE1EUR04FT042.eop-eur04.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: Internet X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1EUR04HT079 Subject: [FFmpeg-devel] [PATCH 02/14] 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: FqkNXdAQfgYr Content-Length: 6008 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. 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 --- The spec [1] does not rule out that these values differ on a per-slice basis; I don't know whether this is even intended (is there a use-case for this?). That the slice dimensions needn't be compatible with subsampling is very weird. I think it is possible for two adjacent slices to set different values for the same chroma pixel. libavcodec/ffv1.h | 2 ++ libavcodec/ffv1dec.c | 62 ++++++++++++++++++++++++++++---------------- 2 files changed, 42 insertions(+), 22 deletions(-) diff --git a/libavcodec/ffv1.h b/libavcodec/ffv1.h index 147fe7ae16..81cbe8757d 100644 --- a/libavcodec/ffv1.h +++ b/libavcodec/ffv1.h @@ -96,6 +96,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 diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c index 24dfc68ca4..b3481df922 100644 --- a/libavcodec/ffv1dec.c +++ b/libavcodec/ffv1dec.c @@ -165,7 +165,7 @@ static int decode_slice_header(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); @@ -203,26 +203,9 @@ static int decode_slice_header(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) { - 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->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); if (fs->version > 3) { fs->slice_reset_contexts = get_rac(c, state); @@ -967,9 +950,44 @@ static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame, AVPac if (f->last_picture.f) ff_thread_release_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 picture_structure; + + for (i = f->slice_count - 1; i > 0; i--) + if (f->slice_context[i]->sample_aspect_ratio.num != sar.num || + f->slice_context[i]->sample_aspect_ratio.den != sar.den) + break; + if (i > 0) { + av_log(avctx, AV_LOG_WARNING, "ignoring inconsistent SAR\n"); + p->sample_aspect_ratio = (AVRational){ 0, 1 }; + } else if (av_image_check_sar(f->width, f->height, sar) < 0) { + av_log(avctx, AV_LOG_WARNING, "ignoring invalid SAR: %u/%u\n", + sar.num, sar.den); + p->sample_aspect_ratio = (AVRational){ 0, 1 }; + } else + p->sample_aspect_ratio = sar; + + picture_structure = f->slice_context[0]->picture_structure; + for (i = f->slice_count - 1; i > 0; i--) + if (f->slice_context[i]->picture_structure != picture_structure) + break; + if (!i) { + 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;