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 06AA847788 for ; Thu, 21 Sep 2023 23:06:52 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 77CED68C912; Fri, 22 Sep 2023 02:06:48 +0300 (EEST) Received: from EUR05-DB8-obe.outbound.protection.outlook.com (mail-db8eur05olkn2069.outbound.protection.outlook.com [40.92.89.69]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id E0F7968C118 for ; Fri, 22 Sep 2023 02:06:41 +0300 (EEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=L5TiuTIQigjtyCP6bhKo/Cii7d4S3QUY20Q1veWtI+EmHIi5rXm6mxEVwPHgVZyML5nPCQd8+CWOAEBtfAOSfrKdQA7mYQ22OkxPA3/kUohzGm/N/gvQHXV8brlOyLOrE5izVQUQC+R17vxRFOtkmbhGLXODb14sH/5yYVP54f/vYbOfcUA3Hcvvpzg5ASQh0IlHWhR1i2jWxSP4flHKXRXtLnS4dBXtpDk3GgZk74LgKQcWgH2VmYcaSEvTgK6jwX7r7VcFFG/PTgUerlF+7i+0y35riFO4KgXIiUw2cKpGIFtRokrQ4xILpZSO5whqiq/V0WxikTLthu2CDu5y0g== 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=jn3ZlLIgze3sGuKpm6ZGNHrepk8SJaypnxIA6lLDbRc=; b=OtX6zbkz8ByiwOSln38HXrBBrrwGAO843Ip4hoZOQhjfbcmoC9wR2zZvPXnsZTj0xjK0W+FH0bdRvt3v+sAsbVi/z21IHZ5YCTLnNAQiR+f17myeyLvoLluaJDDfHwhftnuG8hzajCoDeHbiUw+Cjv3oczPBwR1LiaU2oOUupsl9uZoZS+iZP7HhlHK/9k4YvIOtNZyu49AH/7cxPiOPOuKmE/lEN5aOm3AObBewJnhxM0ZwFcTKgOqCKDC+TYwPk+FGSEfalFoFEuYVd2Gu87bkijVUKnTzRnu15r5AKNFX1CP9HQQBxDvdWrElpUXtEFSo31WW2BX79WvH04golw== 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=jn3ZlLIgze3sGuKpm6ZGNHrepk8SJaypnxIA6lLDbRc=; b=Srdvl4SSpNnPwpeMkRXxaH9LOsC4FVPvFBqEVUplB/NpQ63JKKD/7cnMGkLEQ6RRzdLZFl5cIek1vxz8d38X0+1sLGYlT7QYohopn4q4Tzl5/opz4cTxDorbaNoh4IPQmD7rvbysXmNMwFRRXgM2q52uqUMngTqQVMm1/CX5so/93K3eLFIP0VhFUNs59MFP59vdPSpm4hEn6V86T85GlQEyRZPqBsQWWR+/ixwq78dg9FeGYs1snRZrJeIYIDRM/UgRWuFTVo4UsCEB/8AHRzW7HIBQ+0gEH2T6ckvuMHSvui3hmnzl5uBrrpX4ktvr3WQqM9XoHQcXCXHrkXi0hw== Received: from AS8P250MB0744.EURP250.PROD.OUTLOOK.COM (2603:10a6:20b:541::14) by AM8P250MB0246.EURP250.PROD.OUTLOOK.COM (2603:10a6:20b:329::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6813.19; Thu, 21 Sep 2023 23:06:39 +0000 Received: from AS8P250MB0744.EURP250.PROD.OUTLOOK.COM ([fe80::5e01:aea5:d3a8:cafa]) by AS8P250MB0744.EURP250.PROD.OUTLOOK.COM ([fe80::5e01:aea5:d3a8:cafa%3]) with mapi id 15.20.6792.026; Thu, 21 Sep 2023 23:06:39 +0000 Message-ID: Date: Fri, 22 Sep 2023 01:07:52 +0200 Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: From: Andreas Rheinhardt In-Reply-To: X-TMN: [yezhNYt9WLWHr26FmXjdWrFBB0xjoy3v] X-ClientProxiedBy: FR0P281CA0063.DEUP281.PROD.OUTLOOK.COM (2603:10a6:d10:49::11) To AS8P250MB0744.EURP250.PROD.OUTLOOK.COM (2603:10a6:20b:541::14) X-Microsoft-Original-Message-ID: MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: AS8P250MB0744:EE_|AM8P250MB0246:EE_ X-MS-Office365-Filtering-Correlation-Id: 8be12ea2-e6d8-4fdc-4d9f-08dbbaf7693a X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: fgxmcW+X1ppgex0S4lDA0s3poqjIbSJGg+JsKiHS0ZdxRHl4dRknBofbvLiaAlYynffGHEtMgXscE5DOK1alhIZ3FAQM/aCxWhfjT0liAxH/6AHsZFaOPUjLRZUa3IgXa7ajigoK8xFiH5TX+Bw6njIR8i2x0axjCvJGoddDWGZpxdBegeoG17spoQHMoHJ7yhSd3DcoQ+SHTfmQC+cFwmFKlCVzQVNZD2wbMrZDuXa+l8Mpkeosjs5Zf8EkkP9bnPq8N72BZ5+MJRlE7qM9LLGp179df61pTSG5INximgoZeOazZ2xX+d8i8SDH7AkyIYWgLWvYFX5i2DdPwvOmeZFd+nshIXLJcdOZzLg2+nJJeWuuuqTG3QV8jRgaDXaw808Wpiz+3kfWFNBQE92PzZtWkGpj60Uczgu2yB0nP+ZmDBgTrTrt3ftjtw3zY9SJfnIBbN+4lRGVLeiZuSKXeS6bYOmL+gV64FE/ANMto4kw3FocraW7Wqo7RDVQmK7yjKVhviG4tR+emKrj6cXmg1Bzxd4/I6KeJK9BveYyDup79vsq5shIcxz2/w4WoJ2nregjPm8Tco649SNNy25PU+X7BVytpjoVduvjiID+TPA= X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?WEE3d0lmZjY5eVNKZXNlcWhuWTAybkxvN0dVSm1aZlR2UHhtdlpPQ0FhVUhF?= =?utf-8?B?N3ZaY2g5NGtqa3FFNlNlL3NkeGZaanNBcm1kc1ZXUXV0UDRkd1hPQlErYzB4?= =?utf-8?B?SkpuTjcwMEFCTklNaHlxVjRkdkZsQnI4Qkc1WWxGSU9yQ2kwdHMwNHEwallE?= =?utf-8?B?MHFQTUNYc1BsbCt3dkoxeWJLNnR3cWlEVldnbWxadnpXRFhpb0NHM3NkeTMr?= =?utf-8?B?YXQ4QzNoNHlkUjhUaDl6MTkyU2E1U3dMTWlaTTc0Um1WSXRiV25LK1lOSW9r?= =?utf-8?B?NXh0U1V1alZlM3lKVnR2cDRMZjNGa0ppN0lnM1RhZ2ZHQkdCU3hkVG1BUGxp?= =?utf-8?B?Q2dVOHJSWTl6a0w3M2ErcXFCTFZoYU5DL3poaURzVW9XYzVJN1ZIbk1NWlBr?= =?utf-8?B?bUJvOW1hNm52a0VwV291NFlIbDlTcThKU2JFOG5CZ1NtNVl0V2tOZ25KdFl5?= =?utf-8?B?NGVWZkVRT29ON3ZodzVLUDc3T2RPVi8xMEdoUTFwLzhTMDdMbTVNRGZ2TGkx?= =?utf-8?B?OHJpcC9SMWdQaVpBNmtUWEFxU1dnWmdwSml2dDh1a00yN2svcjNjNGY4YWhC?= =?utf-8?B?bTU2UityUXZaSVhCSHNoajlvaHJRMXVhek1ua2wwRWZ0dUVMcG5ZMVJhbjVC?= =?utf-8?B?M3lQUGpuRkR3TWhnUXlzQ3pKZkxuWFRZWFlRZXZqcWk1Y3o3Ymttc0ZvUi9i?= =?utf-8?B?MWpRcjRUN1JGd1g3K0dxbGZOU0I2SjFSMVEySzcwOXJSZ3doWTkwZXI5ZjBG?= =?utf-8?B?Qlp5a1NCVzNsblk1T1RxWjFvUXhmUkhlNnR2MEVpTVF2ZUlJWCtRV0RFdlFS?= =?utf-8?B?STZOdGEyS3ppcmRTTjljQ1B5MVpyZjRnU25Gd2dTaTdjdXdFU0hHbU5KTzVp?= =?utf-8?B?MlVNUnkxZHQ3U3ZjSEhWTGFkM0dqRHJTanJjeU1DcStMRkJFd055M0JRTWN4?= =?utf-8?B?Q0JsVFdOL0pEeWZkOEo4RTFPb0RXUmpZSnlrU1U5MURUK0xvNTRCSVFrN0Zv?= =?utf-8?B?bmNqRGxnVjJGUzlWcFRBRkJWcVJNb1NpZFBNVTB6SGpUOVBEMkcySERtdXNo?= =?utf-8?B?T29JS083MFNsWkd2WUZ1VC9wV0ZuTzVjakMxNy9pM2N0THR6ZDNEN3g5dWVB?= =?utf-8?B?TjVBbUdZVWJtK2xsdFo3MUJYMDJ0bXM2UnU4cHZUcVViOW1mME5jeDZpQVFS?= =?utf-8?B?YklkZW9odkxkTHBubkpsSU5NNjIyOFRsYVo0NjhJR2MrM2R1VGQ5NUFXc1M5?= =?utf-8?B?Wkc2VHlJTHFtOG1qKzduRlRyMXRvUVhRNGNQZ3hmUVhuWGFwcUVmU1MzeFRR?= =?utf-8?B?RHU1K0dyZXVKUGxUQnZ6b0pjdlpwNk4vMHI3WWNRUFE3MjU2aHdJek4zcytk?= =?utf-8?B?TGcxdlIybUtXZzBIdkNPT21NMzUrbDJQcnQwNUV4a2loVFl3U2FIV1p0Y0FY?= =?utf-8?B?cklNWndxQUZITWZEcVRyT1FVRFMvaExMVGl6SXo5WTNRY0lVNFAxZkhSYnAz?= =?utf-8?B?MTBzcVVVaHkvTVJGQlcyT1EwaVMva1NrUysyR0FRYnJEZUREbGovR1ljWmJI?= =?utf-8?B?Qkg4TysrUzVsS3BmQ3I2bFNIeWpZS0VzZzdqTHZsNWNlYkRMUlByZ1RKWnNt?= =?utf-8?B?Z3VZM21NbTZVZUR4aTBQZWtRRUJ0Z21HVWFiM0lnekh2M0V2YWl3dWk1cms2?= =?utf-8?Q?hObflgWqRiS0gtt0k+CI?= X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 8be12ea2-e6d8-4fdc-4d9f-08dbbaf7693a X-MS-Exchange-CrossTenant-AuthSource: AS8P250MB0744.EURP250.PROD.OUTLOOK.COM X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Sep 2023 23:06:39.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: AM8P250MB0246 Subject: Re: [FFmpeg-devel] [PATCH 02/42] avcodec/refstruct: Add simple API for refcounted objects 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: Nicolas George: > Andreas Rheinhardt (12023-09-19): >> For now, this API is supposed to replace all the internal uses >> of reference counted objects in libavcodec; "internal" here >> means that the object is created in libavcodec and is never >> put directly in the hands of anyone outside of it. >> >> It is intended to be made public eventually, but for now >> I enjoy the ability to modify it freely. >> >> Several shortcomings of the AVBuffer API motivated this API: > > > > Thanks for stating qualms that I have had for a long time about this > API. > > An API like this one would have been really useful when the new channel > layout structure was integrated. On a brighter note, it will be really > useful if we finally turn the library's global state into a structure > that can exist in multiple instances. > >> This brings with it >> one further downside: It is not apparent from the pointer itself >> whether the underlying object is managed by the refstruct API >> or whether this pointer is a reference to it (or merely a pointer >> to it). > > I do not count it as a downside: it is just how the C language is > supposed to work. When we get a pointer, there is nothing written on it > that says whether we are supposed to free(), av_free(), g_object_unref() > it or just do nothing. People who cannot track static pointer ownership > in their sleep should do Java. > I agree that this is not a major issue; I just wanted to be honest and mention it in the commit message. > Also, you probably do not remember, three years ago I started proposing > a similar idea, and it got bikeshedded to death: > > http://ffmpeg.org/pipermail/ffmpeg-devel/2020-June/265227.html > > The difference is: > > You make this API universal using void pointers and callbacks, and you > make it convenient by hiding the internals using pointer arithmetic. > > I achieve more or less the same using a template to generate code, with > the extra benefit that it is type-safe. > 1. I do not agree that your approach simplifies creating refcounted structures. Think of e.g. the H.264 and HEVC parameter sets patches in this series: Using your approach I would need to include this header five times, each time preceded by custom defines. I would also need to give the referencing and unreferencing functions external linkage and therefore add them to the headers (this is not true for init_ref_count and get_ref_count (which would be unused in this scenario), so your template would need to be updated to enable this usecase, further complicating it). This is more boilerplate code than both current master and vastly more than my approach. (Also notice that in this case the references are of type pointer to const, so the ref and unref functions would need to accept such pointers, yet the init_ref_count and free functions would not, so this means your template would need to be a bit more complex again (although with sane defaults this can be made to affect only users that const-references).) 2. Your solution lacks universality and this makes it even more unusable for e.g. CBS: To support it with your approach one would basically have to add a structure like from Anton's counterproposal (or a structure like RefCount from refstruct.c) to the beginning of every CBS unit context, which would be very instrusive and ugly (unless you hide it like RefStruct does). 3. Your solution to the indirection caused by the custom free callbacks basically amounts to "Add custom unref function for every refcounted structure and inline the freeing code into it". I don't think that this indirection matters and agree with Anton's "If you constantly alloc and free objects in tight loops then you have way bigger problems than a single pointer dereference", but I also want to add that this adds more code into the binary. 4. I assume that if you extended your proposal to pools, we would end up with typed pools where every callback is inlined and lots of code in the binary will be duplicated. I don't like this result. 5. With both your and Anton's approach one would need to allocate a structure oneself, followed by initializing the refcount. In this API this is only one action, making every creation simpler. 6. I also like that RefStruct completely hides its internals, whereas your solution needs an atomic integer in the structure. To sum it up: I don't consider indirection to be an issue and the type-unsafety is also not really worrisome. The gain in type-safety from custom referencing and unreferencing functions would be far outweighed by the boilerplate code to create them as well as the space said boilerplate code will take up in the binary. > I see a few places in the code below where you pay the price for your > design choice. I think you should consider taking a few ideas from my > version. Could you be more specific? Is it just the inherent type-unsafety and the indirection caused by function pointers? - 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".