From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.ffmpeg.org (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTPS id 2F8634D722 for ; Sun, 1 Jun 2025 21:42:02 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTP id 29CA568DC09; Mon, 2 Jun 2025 00:41:58 +0300 (EEST) Received: from NAM04-MW2-obe.outbound.protection.outlook.com (mail-mw2nam04olkn2103.outbound.protection.outlook.com [40.92.46.103]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTPS id 2FB4C68D41C for ; Mon, 2 Jun 2025 00:41:51 +0300 (EEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=lDMfAuwue4qtqIiDmqlpBH+iF0E0IQpFwIEqcKG82c4Dt0CwBqf3zj+4yOt5FBkBNlRSUsEYArffV0uWdEi9h3Rcfg9VvrEv4mk0vd36A5F2RS5Vp/i70y1UWPQQs8cG7cl1TTrIgEPSGsv/p3vW7yfHkt+DTz2XKiDHSKCaJKTxMPe9a7B8Szmf/rYPSGWNuwRHq98u8/cvMGKeIYbGzmkHzr9ZE4aAbrz0Dqv1jxcjX3DFcxgLeTFI5yyXOX7+yr/3fnJoedvYYMZGr1sQ/HeHon7y/nJzhe9mTKzJbJJH4tBuTbj1OR+LLA2eNqitjtjzL1TmTKKy12wmD61anQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; 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=yd13lFcOV9XKFcQ1PKe5F0saci9S4sUpWoSFyPApKSw=; b=bRdiPWa9LzDeSvlLZuMq8riEYMuqBKU6AtNQ9YTJ89En1oUZDZmsuZ6T+4LO2QjwI/Bp89G6il7Duax/PVOlpvZY/Z65JEt1a104LeXl0JKMiryYSanlI5JgGhDM/5NzsoZbKigzZ8ZNkimxZ5Q83N8K9CskDSd+I0uvB6C8MqWwo+a1fbaC55pv+77VavjzdswgISRvkEFichmSPp19NnRq4j2b/wE17GrUv3HSMY9U99CMZ15SAcuKHR/aIPvdlXCOakHv9Yyuy34TjMwdhEsAj+Y5dXHV4te8KQzefJS+b//RFOZtrC5AECM0opmXbhfVo86Oo61xLAA+Xh4UdA== 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=hotmail.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=yd13lFcOV9XKFcQ1PKe5F0saci9S4sUpWoSFyPApKSw=; b=a9CBSdb8oQZ1ZDR2QLn6ZR081/ELE/UFDk3iy6beRaPuSKLIQy65OsuZsKSui/VGIJ2Of8jSQ4y8dRWMZkyHnwZC5gk0NjHnl/1bawf1ATBSRq8chpY3JxZSMyOXiTwalU6wwjv4WjCnUf5VffjPZB/WUsBNxAKzEkddBywTn0AD6Eq2Jil6qemuCgocuQ+a4es41aLdf/nd241up952lYVa2LQaKUbJHapbCXhP1iRWfP1hMb2PaJMfbi30b57S3euHmS2/dKKrVeCvNQe5Ls7wrrHqQEEGU1ikMNGXzvKRy082j6G4ydW56r5/cUwTmvUI4C1uXj/Gu9no5c6jtA== Received: from DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM (2603:10b6:8:b::20) by EA2P223MB1120.NAMP223.PROD.OUTLOOK.COM (2603:10b6:303:25e::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.8769.26; Sun, 1 Jun 2025 21:41:47 +0000 Received: from DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM ([fe80::bf09:8e9:b07f:98a7]) by DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM ([fe80::bf09:8e9:b07f:98a7%7]) with mapi id 15.20.8769.031; Sun, 1 Jun 2025 21:41:47 +0000 From: "softworkz ." To: FFmpeg development discussions and patches Thread-Topic: [FFmpeg-devel] [PATCH 01/11] fftools/graph/graphprint: Fix races when initializing graphprint Thread-Index: AQHb0qUjQJixtXGMNUqVWA513ZB097PtrunwgABuyACAAJCM0A== Date: Sun, 1 Jun 2025 21:41:47 +0000 Message-ID: References: In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-publictraffictype: Email x-ms-traffictypediagnostic: DM8P223MB0365:EE_|EA2P223MB1120:EE_ x-ms-office365-filtering-correlation-id: 2f37b6f4-0d42-479b-9528-08dda1551c21 x-ms-exchange-slblob-mailprops: Cq7lScuPrnoSu68Y5FdRDlCPOBk43FEtC66ibW2S+2K/daHgP61pVuUCWX8YBAr7hRKQ5Bm5oQEdtJoPAsNCITs2NeJJ0GLszpZ75xTXNet/XN9367r5HSiYuNvm0nrnH4GG7KijZzrQHEpThakhKR5y1w6UfZgzAj/w4LsTj/AVulr9bdS8pU6rwA6Zh7G8eSYhYAB59vopglk5al8tOVCrjqBoiCCLVR/5ZcPtykoY+Qewx6EJWQ2ruXi25PV49xzmR2w4KRu82xqvfmSAdfoE3VKxFmQCrzHhx3KZti/y6AUPldtNNchBeN6E3FUt0Lj/v5GGNxn7AMaZ6CNQF8QxKRjxDbjJ3c6fm0GQ7NlXdobQOudq0vQHTrq8DWT1jakU636gnFmpvTwdL+AzNqeTgzf/QfyDMDsG1iZXRHhMx/RxRdmST2/4c7lxH64/RhDfJ7NHILOQs131awonwTtvi6tqoHRiy2n4P0qegMm/eaztd2FgyEI962VPo3K4TqvtDAjslsOfs0ZydUBuwjYPYwd2yVRMGWdMhT2G7cj05uUYUc5sa4kU6RJ6rTOcW3Rc80KQwY8fkmTLOTbxad/Dc0NSp0Lt4Dh+UZS1fRGTZJ150f6PR2MmX8aTrrfGRXBKlHj2Epq+JMBWRBjlxZWXqYmk62/RyZ3RtV0PTyzSs0z9OWoFvF96Q2IbcZlYCrWtuhRBurtNa8gg1iIVRCJi95GePmd2IZgyT5SeAzCrIhNn4jKeX/m6N5O0FAlX2oJZ2YZa3Qs= x-microsoft-antispam: BCL:0; ARA:14566002|15080799009|19110799006|7092599006|8060799009|461199028|8062599006|3412199025|440099028|102099032; x-microsoft-antispam-message-info: =?us-ascii?Q?MxNEU5cGONYmvgAnyag+IE4LnVBD5haq/ByMuOHBHNWLvq2tbE0G+hM8546p?= =?us-ascii?Q?wu6OXMoyGXtL5zNmT5j8xPN+5djvKcDxhH4D+vz7WplJEvKaBx9omH5qTT7X?= =?us-ascii?Q?SlXKyefyFK/Enjkt88O/LMYB3gzBoxAfbgiYukANj5De8YaA4U8A1RpjkU7B?= =?us-ascii?Q?hGfuiGQ6qD2ulUVGuZ+N1jhVbo9uwYpXsn5cdBX3GwKaUW0g+MMv6UjW43vu?= =?us-ascii?Q?0v18s1eFQuNQZ/OtYafDFMDxJjRsJDy9xGJ/Lfx7VkQ3XYx8qTGoB//25Ebf?= =?us-ascii?Q?pI4qi1wId8w1WcfP3iTDuYmxMGxj0H4wNrxLdYEzj1jdJIV0uD8Z2CbMlX2f?= =?us-ascii?Q?c1JwO94VU4fDXsQ7y39V4rXsxQYBXCxPxt3Do+U+2/md/1CF9KFXWwucVVr3?= =?us-ascii?Q?L47wPPbbwzSXm6UKPwgCx19IvNlQfUAfxfMC9XWAUqdifriWELfj6cqNGC38?= =?us-ascii?Q?RxmpIyIceembJPDtqL43FsYzzz61c6MiPIrt+WIl+c62pI9nrPkO7ZeAT4UD?= =?us-ascii?Q?ltoN0NT0mLktxE4foaa3KKJXtuWFMBSilR3JXfNf7YxbWtQu7yJMMQbJfM/B?= =?us-ascii?Q?IKrGUNh/PIr0579zIksl3iYbl2Q1zQq7oN3jRx2Wm+dqyFqDCsBN0wIgfUWw?= =?us-ascii?Q?tG/3Xg/G8LDKUypXmyVBC3e/D55zOsXg6zgxGmT2YTPR8HqcVpkFg4nnSVTC?= =?us-ascii?Q?7cIEWp8zMI4hEGB4V/LyUuob4oxAcf8BC4o66/2jTVybXzBFndBSTt/F0VQw?= =?us-ascii?Q?7p1Vetv4fKZhakd1997nuP2APInCMJuORaqfXM9kq14TuD+Si4w+2Mb071sO?= =?us-ascii?Q?5qKsMc2JXZ/ET7w90t3w9hQMS19i/ZXa+4rwTeF9jnZI+bsaWW9HFKRrUnc6?= =?us-ascii?Q?wGW+f2OLAOPY92QNyEOIbJPSiLnM2T+K6umGWc5psPe/1kZsKzbTSkzRaf8/?= =?us-ascii?Q?jAVr+UirQ8C1tD6qWpNtlLbaBZWkl4C59eK9/Sj+i13fk9gJT1Lo0epSoZpQ?= =?us-ascii?Q?WfrrrDFdi48IYJYRID0pT7Q2cTc+qEBTz0G49EJejbOsbRkozVhhx79gB4uV?= =?us-ascii?Q?mO+CvekW4bCTOHWg4IhOEZ3jhXmgIiQRTnkfvOl9GFRBS9rveuA=3D?= x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?jAfv8atm2CeJmOWQd+Wj8fYDgz9cuHcEm/8sIeg5Hsyw0cQZNB/HLTfV9f9f?= =?us-ascii?Q?hvFdOgE2EK/O362H4wY2g7byQK9Y7OMzzXGxYpOqjG4klkoLkEv1l8EBoTKe?= =?us-ascii?Q?7grJt8/pjGjfcxsZbD0dLFtciT2uXRsfxo7jTmyQlr80XKzXSSlMGBFpICmn?= =?us-ascii?Q?R04GimMdtaYWNA9nxIBVZtu0lKJr2gBlzmvzeqcHlEBgTPJgWev4NI3tZ1rn?= =?us-ascii?Q?FX0hjfhsOm3p9S0msoKMlCibSknzC/uKnqE0Wyu+bCanF1mswuLbGEvMRAFJ?= =?us-ascii?Q?tZSESRZQ4JRNi7p5CgV6s7R4u4htYZWS9aFHEveOVHKixUyMmGM9z35F5Meq?= =?us-ascii?Q?Ams56z3R5rgX9Gb8tTQfxKkGYWk/d0bKYcyjcbBsBM4na2GOXRfjRXpRtwAw?= =?us-ascii?Q?A7qoK7NNMv7nTk3jy2TG4A/HX7voG0znFvyu3qVWrMgai34E6P8bd4RAi7yI?= =?us-ascii?Q?wz5xw0cTJvHF+nlfxlAuNXNpPHGilh0ujbesBn5p8SgxZiZnlSdEhXQeU170?= =?us-ascii?Q?qaY0fFSyGgNiZ16U2UKamByS53TBsjUGaClIg9M9Gy0StM2QdZ6xn1lv9NLc?= =?us-ascii?Q?6u9hTPDUHNliQdEkSeBKw6709fBQUqp5TIU0zsi/L9uIBxFFRatyl5lgeVIs?= =?us-ascii?Q?1PdQR2V20X2Rx8VstEBEVS1xN20ys13eGTFBscRx5NTlZ8ULl/VPG0ou9LDQ?= =?us-ascii?Q?GbWP/iVNjYQGKND8/C6QQ63LuCqtik5mJqe/5/we42XgxLktLXA72IDyseAH?= =?us-ascii?Q?Q5Nn0nthdthDDrSM0smMb5IbIod6tXuw4GOf1anI+dbVeIo4i5W79c9xnWme?= =?us-ascii?Q?Rj2T1Zk/IJmWemvajHLbkmwuic9KAfKHFV6+XI3nupEw6yRdaE7EW5riXhMi?= =?us-ascii?Q?L+DleFDg/B8VR0c5krcBcehQHQTgh34joW48eGOZ01FHTXw4ddVdB7fDuOWT?= =?us-ascii?Q?zXGSWsJmhStEPWizoz5SYnQf1MUZ2kUdiH4/lmQJ76EfoPPuxksXxy+t6kNR?= =?us-ascii?Q?IbBOM6InZ8OAv5rtRgE1TCeZIinbjqYbLy5WMmLovS5T5h/bzjc/Dw1ahUPz?= =?us-ascii?Q?ebKdRe8ApFQgRo95qoELJMgAlQKjDP2drnkU8kebjPLgQuALOMnWvezwWPUL?= =?us-ascii?Q?6o1z9rCTqBj3YuGIsWEVKaekoTYLHgFifk/4TBWefBgCw5tZhfMJNOpmeAgZ?= =?us-ascii?Q?pOx1UGqSlMIjwMAjLnOKlpj6FbXTuolun9Wgt4zIjGX4boM4zcwqStUeVvQ?= =?us-ascii?Q?=3D?= MIME-Version: 1.0 X-OriginatorOrg: sct-15-20-8534-20-msonline-outlook-c7cf3.templateTenant X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-CrossTenant-Network-Message-Id: 2f37b6f4-0d42-479b-9528-08dda1551c21 X-MS-Exchange-CrossTenant-originalarrivaltime: 01 Jun 2025 21:41:47.2239 (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: EA2P223MB1120 Subject: Re: [FFmpeg-devel] [PATCH 01/11] fftools/graph/graphprint: Fix races when initializing graphprint 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: > -----Original Message----- > From: ffmpeg-devel On Behalf Of Andreas > Rheinhardt > Sent: Sonntag, 1. Juni 2025 12:40 > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH 01/11] fftools/graph/graphprint: Fix races > when initializing graphprint Hi Andreas, thanks for v2. > > - Why do you want to remove the ResourceManager AVClass? > > Because I think callers should provide their own logcontext Do we have any precedent cases for this? What would be the parameter type for passing it - AVClass* ? > > It wasn't unused. Now the prefix is gone for log entries in > > decompress_gzip() > > True, this patch is just wrong (in case of decompression errors, > av_log() will treat ResourceManagerContext as a logging context, > although it is no more; this would lead to segfaults if AVDictionary is > already set, i.e. when initializing the second resource). Yes, I had seen it crashing once as well when moving debugger execution to an av_log line during the second execution. All good now! > > Actually, all av_log() calls should include the resman_ctx > > Seems this has been forgotten (well..by me) Thanks for fixing! > > - For the registered_formatters initialization: > > I used to have initialization order issues when I had tried > > with static initialization. That's the reason for those functions > > Probably you've done it differently as it seems to work so far > > What issues? Here's my comment from an earlier review: ---- > > +static const AVTextFormatter *registered_formatters[7+1]; > > maybe use a const here, also I'd be more happy if we had a more > dynamic registration system to avoid the hardcoded bits While trying this, I remembered that I had tried this before already, but it causes trouble with static initialization order. Probably that's the reason why it has been like this before already, I'm not sure? ---- What I had seen: The registered_formatters array was initialized before static initialization of all its members had been done. It happened at a rather early stage, so it is well possible that it is resolved by changes that were made meanwhile. FATE tests are passing, so it's probably fine, now. Here's my full review: v2-0001-fftools-graph-graphprint-Fix-races-when-initializ LGTM, I wasn't aware that av_strtok() is destructive, maybe the doc text should mention it. s param being non-const might give a hint, but it's still a bit unexpected for anybody who doesn't know all the APIs by heart. Now that it's no longer updated, should print_graphs_format be made const? Same for print_graphs_file? Could also be in a future patch, though. v2-0002-fftools-textformat-avtextformat-Separate-mutable- I am aware of the mixed mutability in the section definitions. The general idea of separation and making the section definitions immutable is right of course. Yet, the general goal is to build a versatile API for other use cases, so we should not make changes based on "x is currently used by a only, not by b, so make it local to a". The ability to limit the fields that should be output for each section is an essential feature that should be generally available for all usages, that's why it should rather go in the opposite direction - i.e. from FFprobe (where it's specific atm) to the API. I haven't come to do that yet. We should also hear what Stefano will say about it, as far as I understood he sees it similar, but I might be wrong. v2-0003-fftools-textformat-avtextformat-Avoid-relocations LGTM v2-0004-fftools-resources-resman-Don-t-alloc-ResourceMana LGTM v2-0005-fftools-resources-resman-Use-assert-for-always-fa LGTM v2-0006-fftools-resources-resman-Use-proper-logcontext LGTM, thanks! v2-0007-fftools-textformat-avtextformat-Fix-races-when-in LGTM v2-0008-fftools-ffprobe-Factor-writing-common-side-data-t LGTM v2-0009-fftools-ffprobe-Fix-indentation LGTM v2-0010-fftools-textformat-avtextformat-Move-avtext_print avtext_print_integers() allows to print binary data in a tabular layout and various formats. It is generally useful and not specific to FFprobe's usage. AFAIR, Stefano had seen it the same way. v2-0011-fftools-graph-graphprint-Remove-redundant-avio_fl LGTM In summary v2-0002-fftools-textformat-avtextformat-Separate-mutable- v2-0010-fftools-textformat-avtextformat-Move-avtext_print Should wait for Stefano to comment. All others are good to go. Thanks sw _______________________________________________ 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".