From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 0801543C49 for ; Sat, 23 Jul 2022 21:42:36 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 4B60C68B758; Sun, 24 Jul 2022 00:42:33 +0300 (EEST) Received: from EUR04-DB3-obe.outbound.protection.outlook.com (mail-oln040092074028.outbound.protection.outlook.com [40.92.74.28]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 114BB68B274 for ; Sun, 24 Jul 2022 00:42:27 +0300 (EEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=a4idzD14rQRZ6twhG5EjGaY/jvzPrh6MIkPyuqRjJ71rky59SLlhnPhzo5SeoqNvDDK7egNBdErQysO2v9wE1B0yE23bMU2sl/Vk7T96OJ3KnBLG0/mvM863QobfRmwr4Snh2fdZ/SjIZG+SAT0Ms4KvENkkeWz3uSjvcIYFOIOpN7C0vu0TZtxdkQQUG5AM4umN0GitEp/g2e6suMt6YBnjNoqJa2fvKjjY5khFShmQoXX6nMDjshEwCQe9Jo/dVJqIs/Kf3I9vVYMuaVKo+49X0bdxuAukXonqIn2OFFJ5dakPGjJW7B4ge8iU+uxr95svGOojpSIdKjlIHu3ZQw== 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=J7gFywLUXAYgCNjE81khswCLjDboPxGGpKNVuv0RjlM=; b=ZJbPbntTzIeig7jekyk8jz63w7jXjjUhEI4Ye4B+CDlkZldc0I+N5B3qogpZ+sYs2nE3fooBN1aBlOeXC1wtyW0D48vlExEij6xYilC7R/VZJBmL6bV/1nEoDM3vLgLaHDJC+5Yv49M1Qk8nTlmH+dOylV9OtCD8K1LGy0S7cTzAT27HtxiSOEDmgpDBVVaDMfmXFDp2ZWbS+5Wcj/klvwaAZY9jPtB0eKiFkytPJiqMUF9DMSzHA/ozRvnBmKyCdeGNYZqVpaajxWiOUChACdnUQOuCfxbWhl9yITL1KXIC1Y8u3V0sDca15kjnWC1J1deQN/MzToCikq+Be+Jn1A== 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=J7gFywLUXAYgCNjE81khswCLjDboPxGGpKNVuv0RjlM=; b=rWbOapNt3vwUdDmRhuqyih5qYcD7+aLncZCuQCRjZ/P2oVDkQ1ijTUS77aPGJmRrI54vkBuFKWf3dQaMRqyHNzkFVyU5tIprCNW4wyuW47uOPIPttitA2B9+F1UWnPXauUaUUWV+DG8XbTpUmPRc9L5vmEEUK000dWpJSLShhHz7eZSP3RPisiA8BDD1l1hakPib9zmgEjX45Z0UIgSf/kPY3gX74X9f5ys5rIe1MSAfqQyzlJbBhS5Ia2MsV7CPIUFP14Z5JBL/0WP2pFLEBNR2s/PBVY77DYaQCSCnLE6PPrqaYq4Hc4sITdGA9crOUZ3BX+3rKR+rYhddmubAfg== Received: from DB6PR0101MB2214.eurprd01.prod.exchangelabs.com (2603:10a6:4:42::27) by VI1PR01MB4784.eurprd01.prod.exchangelabs.com (2603:10a6:803:8d::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5458.18; Sat, 23 Jul 2022 21:42:25 +0000 Received: from DB6PR0101MB2214.eurprd01.prod.exchangelabs.com ([fe80::210e:b627:bcc9:8c46]) by DB6PR0101MB2214.eurprd01.prod.exchangelabs.com ([fe80::210e:b627:bcc9:8c46%11]) with mapi id 15.20.5458.020; Sat, 23 Jul 2022 21:42:24 +0000 Message-ID: Date: Sat, 23 Jul 2022 23:42:23 +0200 Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: <20220701212511.GY396728@pb2> <20220705222411.GM396728@pb2> <20220723143802.GR2088045@pb2> From: Andreas Rheinhardt In-Reply-To: <20220723143802.GR2088045@pb2> X-TMN: [xhr4UWi9TPTeQWtvPsaEUOc+1BXojdZ9] X-ClientProxiedBy: FR3P281CA0170.DEUP281.PROD.OUTLOOK.COM (2603:10a6:d10:a0::18) To DB6PR0101MB2214.eurprd01.prod.exchangelabs.com (2603:10a6:4:42::27) X-Microsoft-Original-Message-ID: MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 824cb507-c405-412d-6b5b-08da6cf43ae7 X-MS-TrafficTypeDiagnostic: VI1PR01MB4784:EE_ X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: rF8x81U5O1O0U66akUppEXJa7Gy5i6l7smszl0/tLx/firBVcoWE+QKWlhfROEwx1pGlTt6xZtbE4E3CLtUlBocAfNdy0gytRuikshkyJVr3LLLZiEo1C1K/qZzQvicTBY3BaeJqXlNfD6G0pQiI+rTPPMZX1v2PEzX+XvBHn6MrdBvMUQddH6eTg1FJPAqtT1SAJkES19VzRUZNfBMUmIRk3cbmWoaGr7IAouqent0lpfLLXivn3RMKHFMICXEEK0+eqP+NkhvMzSw19TaHz2V6u4kSKslZOymuHWQ6aYS03ah7JTDhd95sZIQTMirqOFRlDuDrfG5xZ3wVD9gfgAQlmjvfHQsNPdhM231On+VR9R73scI+hmd82/8VBGUHOxsy83biTQHO3z+SRb2NJpQB+vxGz5YxgIujJKK1BcWNfE39yUuBQWyPKEt2eS3H2YXr0GPPsdBCgps49zTHJ0uGPhgq9Sw5D+pQgjNndiBvKDFqCVAJ1LuGdmjfgNpVm4ndq6d+wXCAsvmhiQvhn6XxfEd7Sc5lD/wk10g13/XMhPNSccoEqZliwKiU3s0rUMdTXZ8M57BrP8LamtqWjT3YzBlT/BMMDxAPXHWR5KC+x6UZLAJYHvHg5WrC9HISdrxR1c/Hkd4HYswk/iSe1XasFVASIA/GhEUGhw2xA1WwnbfltnkLzaio1GFzEN6XItu9lBPPf4Q4wEFtrG3rFKR2gXnCLftzWIXwEfbsY/o= X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?OXorRFMwak9ZeE44a3VTZFZLSFJMMjR5NHdsN015ZUhoZGRzTld0a0NPZFQ5?= =?utf-8?B?eWppVFl6Q3hZOWdMT1JkMEJaQ0VCWmdtL3pZVGU2QzNOU1ZyL0lidTA2U1BY?= =?utf-8?B?NHVZTm5iZUFPblhRV2djbW1oVURDLy9uQ29jWkZsZjFweklRQjUyMTN1Tkhm?= =?utf-8?B?MUJ4cFh0RFdGZHcyTEVtNFp0bysyNWlxc2lVUjBLSU56dDBmM3hPZ29JVENu?= =?utf-8?B?MFJvVGlxamZyVm9ENkgyWldBb2V0MkpkcWRGUWcvZDZRNS9ua3VKN0pNSVR4?= =?utf-8?B?RGxNOWMyZEVzeWJYQ3ZZcnhtOWNZNFoxZ1c3RGZzTDJNYldhelozRGhScmor?= =?utf-8?B?Y0c4cERYSXBjenV6REdpM1ZTR3FwTTlUWk1sQSsxaTNhRk9TYmRuaUh4NjJF?= =?utf-8?B?dmVxU0xVVFBac1pkSFF2Yy9SSHBvN2VFQW1XQXU5MWdSbFNHYzBJZUpmN0U3?= =?utf-8?B?QktTUTF4d2NWMHp3YS9IMHp0STVmV24wVmVIckcrdFFwMENLcmJPa1J5RldN?= =?utf-8?B?Nkw2MXdQSUtlcDhiMk1vUXhoazd2SDRqYkZqVEo2MVRWaVliQ2lWalI4YWQ4?= =?utf-8?B?ZFpwRmRpSVlmZlRha2lNMUVTSUZXN0J5M0pMR2UzZkdLbUNXTlRIUXl3SVZ3?= =?utf-8?B?TTBiQTRrcXRBSzhtRVpPZERzcmNERFpBNzlXK0xNenNNeWJyV1IvU1R4bXpw?= =?utf-8?B?QzRwZ2dsN1FGQXBCdGNUVDVCbHpVQmhCTGJMd0tTV1lFYTYrM2thTDFvemVx?= =?utf-8?B?allKc003cTc4dkdkcjBydUxnSjZEVXZyNS9UelNIeFFXTWdLSS9nRTdlaUlC?= =?utf-8?B?RnBoRHFMSWR6NnpZdWZJKzFpMEJObndPQmxCZ1JsWHluOVR5TkNmR2w4Vms0?= =?utf-8?B?eER5OFBtV2VFWEx6Z2FCcU4vTUxsbFVvdVpBSmpBSEROOVE1aXhrVEdHSkRh?= =?utf-8?B?WVNwNGl0TkpqUU9MangwNW1WOE1FY3g0UmNYZEk3cjFXdFpBUmd3eE0vVDZy?= =?utf-8?B?dHMwYlZoYVhWdnJ3N2Y5NEdZMmN0bGd0bHA3UVRzUkdYeXVmWU9remsrbmts?= =?utf-8?B?eUEvSEUxSG9jY2tLV2FtVGNFckxwSDFCanVvQytYdzdqZXh3TmxPR2JVbUQv?= =?utf-8?B?YUx5RzZXUFNuZ0hsNkxFMVNJcU9MUTlQdGgxdG1JV29hcndTK1l2V0VFcTRM?= =?utf-8?B?ZVBublhYSTNZSFdINXVKejBvYmdtZ09DRkQvUnRJY2xSdmVSUUZ5SzA0L1pD?= =?utf-8?B?ZmxFTHRUdlNCWGF5alFVRzFhKy8xSEQvVDloR0NiRXRiK05TRnpkbi8rYjh1?= =?utf-8?B?UnJKUmdWL0xHUVNFaTNTSGdpV0JWazBzMHBmWjdqT2JKVkhHTnFrL0MxdUVz?= =?utf-8?B?QUpzUjhyUEc0QXlwdHJnV0tCYkZPMXMreWp6d2dHNVY0N1VVRzhObFI1MUZh?= =?utf-8?B?b0xpN1NhODJYemduWVorMTNpWXh6aFVwQ2hHakZxQWdEVEVaRkxrSERVSjVk?= =?utf-8?B?b1dZa2JkenVnaWpRQ01wajZWUlhZUFpDK0d0SlV1Nncvb2xvd0hYMFZkcGFl?= =?utf-8?B?di9CVEdpZlZCYTdpcHR6bkVqeWszS2JaSnZoNmdRdWFqUG4reGRDZ1p6ejJ3?= =?utf-8?B?cXZqUUdwbmxLbFRIcEhaQmpMQUM0bi9yOFV2Q1VYbi92blRnd1NzQTFtNzlE?= =?utf-8?B?K2ZhRHNxVFdHSkZvZDFYWlIxS1o1cUJmaWJ4aFBucXNsY2U4SWJFbnNzR3V0?= =?utf-8?Q?i+sJEgUXUQ3ylJXe/c=3D?= X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 824cb507-c405-412d-6b5b-08da6cf43ae7 X-MS-Exchange-CrossTenant-AuthSource: DB6PR0101MB2214.eurprd01.prod.exchangelabs.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Jul 2022 21:42:24.5536 (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: VI1PR01MB4784 Subject: Re: [FFmpeg-devel] [PATCH 14/18] avcodec/hevcdec: Don't allocate redundant HEVCContexts 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 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: Michael Niedermayer: > On Sat, Jul 23, 2022 at 07:44:40AM +0200, Andreas Rheinhardt wrote: >> Andreas Rheinhardt: >>> Michael Niedermayer: >>>> On Sat, Jul 02, 2022 at 08:32:06AM +0200, Andreas Rheinhardt wrote: >>>>> Michael Niedermayer: >>>>>> On Fri, Jul 01, 2022 at 12:29:45AM +0200, Andreas Rheinhardt wrote: >>>>>>> The HEVC decoder has both HEVCContext and HEVCLocalContext >>>>>>> structures. The latter is supposed to be the structure >>>>>>> containing the per-slicethread state. >>>>>>> >>>>>>> Yet up until now that is not how it is handled in practice: >>>>>>> Each HEVCLocalContext has a unique HEVCContext allocated for it >>>>>>> and each of these coincides except in exactly one field: The >>>>>>> corresponding HEVCLocalContext. This makes it possible to pass >>>>>>> the HEVCContext everywhere where logically a HEVCLocalContext >>>>>>> should be used. And up until recently, this is how it has been done. >>>>>>> >>>>>>> Yet the preceding patches changed this, making it possible >>>>>>> to avoid allocating redundant HEVCContexts. >>>>>>> >>>>>>> Signed-off-by: Andreas Rheinhardt >>>>>>> --- >>>>>>> libavcodec/hevcdec.c | 40 ++++++++++++++++------------------------ >>>>>>> libavcodec/hevcdec.h | 2 -- >>>>>>> 2 files changed, 16 insertions(+), 26 deletions(-) >>>>>>> >>>>>>> diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c >>>>>>> index 9d1241f293..048fcc76b4 100644 >>>>>>> --- a/libavcodec/hevcdec.c >>>>>>> +++ b/libavcodec/hevcdec.c >>>>>>> @@ -2548,13 +2548,12 @@ static int hls_decode_entry_wpp(AVCodecContext *avctxt, void *hevc_lclist, >>>>>>> { >>>>>>> HEVCLocalContext *lc = ((HEVCLocalContext**)hevc_lclist)[self_id]; >>>>>>> const HEVCContext *const s = lc->parent; >>>>>>> - HEVCContext *s1 = avctxt->priv_data; >>>>>>> - int ctb_size = 1<< s1->ps.sps->log2_ctb_size; >>>>>>> + int ctb_size = 1 << s->ps.sps->log2_ctb_size; >>>>>>> int more_data = 1; >>>>>>> int ctb_row = job; >>>>>>> - int ctb_addr_rs = s1->sh.slice_ctb_addr_rs + ctb_row * ((s1->ps.sps->width + ctb_size - 1) >> s1->ps.sps->log2_ctb_size); >>>>>>> - int ctb_addr_ts = s1->ps.pps->ctb_addr_rs_to_ts[ctb_addr_rs]; >>>>>>> - int thread = ctb_row % s1->threads_number; >>>>>>> + int ctb_addr_rs = s->sh.slice_ctb_addr_rs + ctb_row * ((s->ps.sps->width + ctb_size - 1) >> s->ps.sps->log2_ctb_size); >>>>>>> + int ctb_addr_ts = s->ps.pps->ctb_addr_rs_to_ts[ctb_addr_rs]; >>>>>>> + int thread = ctb_row % s->threads_number; >>>>>>> int ret; >>>>>>> >>>>>>> if(ctb_row) { >>>>>>> @@ -2572,7 +2571,7 @@ static int hls_decode_entry_wpp(AVCodecContext *avctxt, void *hevc_lclist, >>>>>>> >>>>>>> ff_thread_await_progress2(s->avctx, ctb_row, thread, SHIFT_CTB_WPP); >>>>>>> >>>>>>> - if (atomic_load(&s1->wpp_err)) { >>>>>>> + if (atomic_load(&s->wpp_err)) { >>>>>>> ff_thread_report_progress2(s->avctx, ctb_row , thread, SHIFT_CTB_WPP); >>>>>> >>>>>> the consts in "const HEVCContext *const " make clang version 6.0.0-1ubuntu2 unhappy >>>>>> (this was building shared libs) >>>>>> >>>>>> >>>>>> CC libavcodec/hevcdec.o >>>>>> src/libavcodec/hevcdec.c:2574:13: error: address argument to atomic operation must be a pointer to non-const _Atomic type ('const atomic_int *' (aka 'const _Atomic(int) *') invalid) >>>>>> if (atomic_load(&s->wpp_err)) { >>>>>> ^ ~~~~~~~~~~~ >>>>>> /usr/lib/llvm-6.0/lib/clang/6.0.0/include/stdatomic.h:134:29: note: expanded from macro 'atomic_load' >>>>>> #define atomic_load(object) __c11_atomic_load(object, __ATOMIC_SEQ_CST) >>>>>> ^ ~~~~~~ >>>>>> 1 error generated. >>>>>> src/ffbuild/common.mak:81: recipe for target 'libavcodec/hevcdec.o' failed >>>>>> make: *** [libavcodec/hevcdec.o] Error 1 >>>>>> >>>>>> thx >>>>>> >>>>> >>>>> Thanks for testing this. atomic_load is indeed declared without const in >>>>> 7.17.7.2: >>>>> >>>>> C atomic_load(volatile A *object); >>>>> >>>>> Upon reflection this makes sense, because if atomics are implemented via >>>>> mutexes, even a read may involve a preceding write. So I'll cast const >>>>> away here, too, and add a comment. (It works when casting const away, >>>>> doesn't it?) >>>> >>>> This doesnt feel "right". These pointers should not be coming from a const >>>> if they are written to >>>> >>> >>> The HEVCContext is not const because the underlying object is const; the >>> HEVCContext is const when accessed from any part of the code that may be >>> run from slice threads, because if a slice thread modifies it, you have >>> a data race in case any of the other slice threads reads this field or >>> modifies it itself. But this is by definition not true for atomic >>> operations, so casting const away for them is fine. >>> >>>> The compiler accepts it with an explicit cast though. With an implicit cast >>>> it produces a warning >>>> >> >> Did the above explanation satisfy you? Or do you want something else? > > sure, ok > > [...] > Good to hear. This patchset (namely patch 11/18: "avcodec/hevcpred: Pass HEVCLocalContext when slice-threading") includes modifications to mips code that I created blindly. Can you please test it? Here is a branch of this rebased on top of current git master: https://github.com/mkver/FFmpeg/commits/hevc_wpp (Said branch actually contains a bit of further work which also modifies mips code (in particular, https://github.com/mkver/FFmpeg/commit/cf441e559b8d4bf2c05c29483ccf49e82fc6b863 does so); you may also test this.) - Andreas _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".