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 273524262E for ; Sun, 24 Jul 2022 21:26:54 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id C6C1F68B7C9; Mon, 25 Jul 2022 00:26:51 +0300 (EEST) Received: from EUR05-VI1-obe.outbound.protection.outlook.com (mail-vi1eur05olkn2089.outbound.protection.outlook.com [40.92.90.89]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 6F3D968B094 for ; Mon, 25 Jul 2022 00:26:45 +0300 (EEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=fZKW5Rrs3w66YIZ8VDt/xZOBSGqEsAfQlbGprQMGOibU34h3vfYS02lmpLxN7HtDJ+Wk4ZVZVH8Ft9RnqlYVQo2fWO7JFtMA+J5Z4jDzCdcO8oLYTQv8f8RKlCmSTDn8/l5fdW6CPK5NHJLjTjbhZMzyNvlsYesKG/c4P3v7fvHNYGfHFvv4M5Bd6z1LNS0LWhls3fOADRq5APxSVAw+yeFNl03bE8NzcEY4sdJG1yoHQbpBmjzf5PCQjTwGikQAaLV/eAi2XFQiK2bHwXqVlW92AAIRGGG9+JeLKXtcfrKFZ5fSndNIJamnlGJFFRc3JwRM16wWNHD8tW2BsauRMw== 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=d5fte0ZADOfLP30wZyORQou7Ssyzil/t/8Biue8xmX4=; b=ahUJh89Nux+xsJUWAdCvzD3aI3D4cYTTk8b+FbIA/eA+HD5wfIZ6p8dQ/jBbSkpXR7yJ/7K70RPKKtY3rvM9v1FQXUxnu+OTsZPol5SqvcwSjs9M/kcz6VXOT+GxFyyoptG02/KM8v9rA9r+T+iYwG5v3NEIpjG3sZovHF1m2o/if74tzTMPHHAvks9n1t/0z6UsSIgSwRbrXBSRnMNtZrMlfirw8abCQbYuccNYmj25KraFTbDdPXUmX7WlUKpn4Pgo0nqXAMluS6qpbrkMc35HPKV74l/vnXoVkQ+FMEaCRJqwnaa+wuFa+IeoOqacrBPYujpjq41M46Y32urLJA== 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=d5fte0ZADOfLP30wZyORQou7Ssyzil/t/8Biue8xmX4=; b=ZD/v4TWuiW1uw34wApquUHy9ZJvKOC0Zk/x47hLw0HoYSqn8QxPVI3/Nem3LwZXAyXGR//cI3uGctzQF+PVEZVQ/7RIhfY9Kn1SCeX5QdUhjqcH30LjI2f2iTcFz+LWT0ueo9EQyEEuhHfXlT/RChcjZfzJGeFYFgOnP7gztiEGylU+uuLCcbROWAci74lznhw1yfXG5TdGNOBLqJz4Rxt6gUMgOJZBE9BuOn55/bdEWhPWKIB8ftELAKR15X5QJux0FMfKARVCPx5UNMFPJZsS6F03aWBExianrgF3KH8Zo2ZUK83+EL5k8WQLTnglgRZF7BeJS2PSnC6+XvJYSIg== Received: from DB6PR0101MB2214.eurprd01.prod.exchangelabs.com (2603:10a6:4:42::27) by DB7PR01MB5116.eurprd01.prod.exchangelabs.com (2603:10a6:10:84::24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5438.20; Sun, 24 Jul 2022 21:26:43 +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.024; Sun, 24 Jul 2022 21:26:43 +0000 Message-ID: Date: Sun, 24 Jul 2022 23:26:37 +0200 Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: <20220701212511.GY396728@pb2> <20220705222411.GM396728@pb2> <20220723143802.GR2088045@pb2> <20220724212314.GT2088045@pb2> From: Andreas Rheinhardt In-Reply-To: <20220724212314.GT2088045@pb2> X-TMN: [j2/HZYJTFOWW+NkGJspaP63oiqdldj3y] X-ClientProxiedBy: ZR0P278CA0123.CHEP278.PROD.OUTLOOK.COM (2603:10a6:910:20::20) To DB6PR0101MB2214.eurprd01.prod.exchangelabs.com (2603:10a6:4:42::27) X-Microsoft-Original-Message-ID: <78e163ba-1bc1-d310-6666-0c0d4f1bc7ee@outlook.com> MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: a5a0fad0-6711-45aa-c947-08da6dbb3238 X-MS-TrafficTypeDiagnostic: DB7PR01MB5116:EE_ X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: BbrE2TevdYfcmmzwBWSdMzOXmt0ClSipZG5Wkry+DHe1y57/EUJ3JTboxs2RI5LcJkES6Ahak8+IAwRppHVIobwywcPngPs5ETNZCCJqxPu6MbbblqApdIObBizpKcZv7kjjNhHEmrUBy8wezxEJkGZT5R9AGqf+ONmiLxt9Jnczaj6xLCHRYiinWcBjn5C8L3g+Gf4ZZzjLmxtt8tdyGTvLKU7/riJC6w6inIPJ//WnIT8zdI3eFutXieGdqKOVUwvMSAad3RJIZdesLs0sCIRUiHJ5lTgvAE5Bo5iQbWu/AZTKF+Kyp6z3jjFbyMIRQIZ0PXJBr5p1ubAQiizuYp7MKDJh9hwMz20AloNVqGtliUPqqoaQ5HnhDD5i7eLiJkJn5FFDd7AmCU5iSKcAuMTnA9Tf4MR4BUK8gi527zJPcsAMeQUK+qO7j51nSPxK+vJNRx+3cBTv6b/DUc2jxKGaykuY7wkK5T0880N1kOIl2nWnaaWf1M7i7MPnTRSFND3VAqTzEF/ai7d58+QpdiswL5L5Su7hPu2pwz3bXZG7puIQrRM6zXwjtAKTw6PCPS/glUJii6FRmlRll5I+WVi6hpplNWTnZaMLXQOLqj2YYJEjSF22eTXRqnhnna82F7WpfF2k9BvrGCLnWy5QMeZrxnPdDOYhZLWMj0wK24g+xgJP9/2+m6kS/jRtUVEYEXEkZVP0s465Hh0/pwyGtcR8aB/5wTWDfyOok+edzWw= X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?b2ppa0pwL0huZmM5WWlRUXpRVWxGL0hjRmx5U05KZTlYMkFMWUl5bUJTbGlW?= =?utf-8?B?N2k0TEw4cG52dmlhbW9NYm1PRlBhbkllS29aeG02S0pKOHVMbjVCYitORWd3?= =?utf-8?B?TlhRNFRyN05Ma2NiR1EwMDVaMXlqWlgwbTN5TUU2VWE5eUpTb2VxTkhBR294?= =?utf-8?B?bDluUC9Yc2had1ZCWkFCSlZDbmtYd2hrQWlaNWQ4NUxUOEdaQXRaNS9kek1E?= =?utf-8?B?ZW1NRFhxZHlKWDVWbFk5NmVuU0IvZzhUS1B0L3UzenRvdU1aSHNrV2o3QU5j?= =?utf-8?B?QlQ2dFZTN0RjTy9nN3Z1YUZSM2RXZXorbXplNUVMdHBnK0ZrMzdIazA4MUtB?= =?utf-8?B?eFdrbldzcmx6a000Y25oYXVlM0tETEJ1T055VEZXYkdkV25FM29WUWNYL0hO?= =?utf-8?B?SzhCTndlREFLNWlhYUZxUGxaVHZHUnM1ZVlzd0F1eVplM1Q3UEVQeWkvUTR4?= =?utf-8?B?UzczRzZPR0t1a3hBNVk4c2tuL0pxM0JhSVF1RHc0cGpVeDd4TGVRLytPaGt1?= =?utf-8?B?RGd0YkVRRU9OVDcvWXprbVllWmhhSk1EYzFoTnFjaWlha1ZHMWQ2SUtFSndi?= =?utf-8?B?TmRWNlFvZ1pJRDQralk0OWxOQWI5dmM3amI5Qk9rdXRkUVAwYURydWlHT2xh?= =?utf-8?B?b2pCR1A5bndNN3VqdXR2REVTQTFKc2t5ajU2UnV1V1ltMFlQaEl4QS9vZm0y?= =?utf-8?B?Smx6eGd3RjYwekpoZjZoTWpoSVQzYkw1RDJiSFZTRUFtdWNBV0k3Q3FXNmln?= =?utf-8?B?V1NvQ1JWVXk5WGdUY2c3SmQ3SjB3blhhbFR5bVEvcXdtRnJUTk1OUlpSY3gr?= =?utf-8?B?bDlTTGc0S2pYdzZBSVc2dmVuZFVMbUJmSVdheDhYdEs5dmhraFVsZklDU1JU?= =?utf-8?B?cWMxenNxQXlBQ2VDd2RPdVQ0M3JXYWRwd2FmM0FvSnZmQjVEc1ZQQWs0U2hu?= =?utf-8?B?cGFzZXR3QW05SFRIbDFFMzk4ZEZOS1FSN2VIeTM5RHdWVSsyWFM4WEtFNFYr?= =?utf-8?B?ajVJdEdwQkpVRVBXRlptcnc0cENxbHFlelhCZkJGQ2tQK3FKWmdhMVhTd3Vh?= =?utf-8?B?TU9uRWh0RHBIR1c3bXoranFUQjlCUkNlekVSMHJsSHI2c21JM1g0VWVQNzVN?= =?utf-8?B?TzBNbkR6YnpNYXZJbDdWdmxFenAybE91SkhLdG8wZ1BQblVUb1R4OE96YXNK?= =?utf-8?B?VWZoMm5CSkFxelZLV3VkKzB6dk9UajNzU0pPN0JlMHpHU1FDN0VYR2JDUnZv?= =?utf-8?B?ZXp3WEJxeGJ6NFpBaW1vRGxISEpEeWcrcEdHREhXUnJmcDFTQmszSVYrMDlh?= =?utf-8?B?emt5WVIvTktnYzlwRWlySDFEdUJMMjcxb0QyUE1aMlhOYjBDZ2JWUDBhVnBI?= =?utf-8?B?WDBNQkdmVk1VTi9XdzJGQmZvSnk3WEhwc3QrbXdia3Q0RjdLRk8rRnhRWGxz?= =?utf-8?B?a0wzcnFJbktqTm5BQWhkVEFZREtVRVBEWlB2Ris2OEV6SmIrbXB2R0JhYmg0?= =?utf-8?B?ell2NWJIQWZrTmhNUldEQkFGYWJhUHp0MkEwRVVlYWp2cU1wTXJYNW5HT0tC?= =?utf-8?B?RkZBRDFIY0hBOUhadzNNaXFIYW15ZElqSk5kN0taVTVEczBzQTN6QVk4dFB4?= =?utf-8?B?WnpnQ1l4MForZm9lekhNTTZmNjdwcEFabnVya2VZMUJwR0VqdFpXUmRoYUY5?= =?utf-8?B?aWQvN2hObkhyMUlPak5WL3Y0YlJVcW4raElZWk4yYktFWGlOZWl2QzBVR0U3?= =?utf-8?Q?RW2UIsLWZMsmCiYyz8=3D?= X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: a5a0fad0-6711-45aa-c947-08da6dbb3238 X-MS-Exchange-CrossTenant-AuthSource: DB6PR0101MB2214.eurprd01.prod.exchangelabs.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Jul 2022 21:26:43.0550 (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: DB7PR01MB5116 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 11:42:23PM +0200, Andreas Rheinhardt wrote: >> 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.) > > what exact tests do we need ? > simple fate ? any specific thread type count ? > also note i can only test qemu mips not real hw. > I stopped maintaining the MIPS hw and sofar noone volunteered to take its > maintaince over > Compiling (in particular being on the lookout for new warnings) and a simple fate should be enough. - 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".