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 ESMTPS id C09EA4E4EF for ; Wed, 12 Mar 2025 04:01:14 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 1FD7968DD68; Wed, 12 Mar 2025 06:01:12 +0200 (EET) Received: from NAM02-BN1-obe.outbound.protection.outlook.com (mail-bn1nam02olkn2068.outbound.protection.outlook.com [40.92.15.68]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id E8B1168D450 for ; Wed, 12 Mar 2025 06:01:04 +0200 (EET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=vneylLZpgiTSIBGG/H55DOl/LGGkcsHCZqQR0Q63+dMfY0QxZGHuVayTJG+f4NVAUCzY6a+yNE6SPGDuiWh6kZFALeDBHkkqeFMAi8UTv5OpIUK3Ng2+/jawSwCjquUcDVHKeDxeNKkXvmMN9NFJdIZeh1oC983hSQCao+AczAkRfgklq/APkuQJGitKY7K9plCGw/D+S1OQ8QgsMCM0DzC9QrNGI/XfJ+yNC7YbBV0J7Lh6ougkO5+Kl8kjqDFooKIpBB8Q53x9x1nAW5WjfXqzGa9A2Xti+i5EhFepxda+HZtZnZUr3s+J/3ZExPKUpvvYQX4Wwye50ZUrTUwMlA== 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=UIGYOYCdY49v5XROU8hiBvjuuiKV7GNs82s9nYXRxFU=; b=Trp/QaxLpvdgqdD9z/qdy2mTnmPgD0urqJR9EBovFmvzSFUAOqpHOcgi7WsIv7My6Q9DTMsZ0Lwg0tq1Zg0pTlcqRmdIXWnxoGVTfQ5r5fBg9Q9VW++ZzNpeMUdpQyWQ69MUT0taoTW1i1xvsON6T4LTLqqfBhTng1+MiXsEyk995m5JF8pW4wUH2gzddQNzOAnKpV3Y0ynXGN3gVFFWfdaVRR16yf7LKOUx3IQ3e/YJrAudqVmiQxNwm6pEKLeQV6QfI6U0MZIkWjmR9irjrIBQXCl1mfKk7C9wQFHR7zVtquACEHR+Z5KIcThi4qA9E3boAHC1CRLoih6AxVM6+w== 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=UIGYOYCdY49v5XROU8hiBvjuuiKV7GNs82s9nYXRxFU=; b=MwlHYXS4hIpWbsjrRbEjQBR/yTCM4qhdP6rhtOsBllQyBwWxDPj2A0po4WZD3yWqcGYWIgB05y1QPEz9Z/STAetT6SOAlNNuFCNnZveFuxV+DdArdR7xmkxMAEZr4JpvxkeQ5QWO3EaUjiN6tQfb5Q4+/cCsxqgLFkvA/KPQCVep4L6hKs6fDq24vQXzQYXcuEWZOmCVrF9GYmKxjGcQWGSXF1NRydOzB5OX4L1NKaFzYRT4n1TmCKegVlpEmNV5hlFHhKBkNo37iFoebHLMUA8bG6rzbp56xbxOli7poJhLfxM37AuJzgfoP4o52evnaA8saOejfcLvT7xdduXAOw== Received: from DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM (2603:10b6:8:b::20) by LV8P223MB1112.NAMP223.PROD.OUTLOOK.COM (2603:10b6:408:1c1::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.8511.27; Wed, 12 Mar 2025 04:01:01 +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.8511.026; Wed, 12 Mar 2025 04:01:01 +0000 From: Soft Works To: Stefano Sabatini , FFmpeg development discussions and patches Thread-Topic: [FFmpeg-devel] [PATCH v6 6/8] fftools/ffmpeg_graphprint: Add options for filtergraph printing Thread-Index: AQHbkGcV7czitumF4023r7PbG7V0LLNtAGCAgAHi2HA= Date: Wed, 12 Mar 2025 04:01:01 +0000 Message-ID: References: <8ae08a700697d4c6c350939896c6a75405057883.1741464994.git.ffmpegagent@gmail.com> 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_|LV8P223MB1112:EE_ x-ms-office365-filtering-correlation-id: aae673b6-808f-41c0-3d18-08dd611a80e7 x-microsoft-antispam: BCL:0; ARA:14566002|461199028|8062599003|7092599003|8060799006|15080799006|19110799003|10035399004|440099028|3412199025|41001999003|102099032; x-microsoft-antispam-message-info: =?iso-8859-1?Q?AgOM/HhplLBUC7EmPoDBPH2KFaq7FztVyG284HY7POez9Z1+OOCMAwE1Jl?= =?iso-8859-1?Q?gtHX+h/EgqwX1518AZVx5vtuMwsWer2P9SNvC6QHfj/XEN716Rr/t5L5XF?= =?iso-8859-1?Q?1gSFVOSqgXmhhib1f1L32QICqBR3qPsdHlycZBrtDaCtUnfs7sihZ3w7Mg?= =?iso-8859-1?Q?BnFL+vsILCDX7o+QlerItMwcQ4sfw6rhRlU5vaukWUOpDk8n5uDxBo/YAd?= =?iso-8859-1?Q?oFA80RHnhNzUtKeToi48i7DFgNAFJxZLgxB5y9FXa+USH+CCaHO3wTkquB?= =?iso-8859-1?Q?I1BRh6mXL9CGal135YdhZEKZh/KsPCvz/8LvuZCjlgUmr6FGUbIFVFOyUs?= =?iso-8859-1?Q?UmsrM7EV8qk4MaHOs5GcdQFIfdRu4EyZctufUZsDNUWgY6YR8L3nkKIHPp?= =?iso-8859-1?Q?5GxTV9TMclpeUc7koMeYO1TBq02C/h2cf3/xwrzDcd81fD1SjmwfZOr09N?= =?iso-8859-1?Q?RBUGitg5jLsKU8Gud57yJCYPC6TZAbQY9a+4e3yNvmxCInZAZ55FtS7vNx?= =?iso-8859-1?Q?4YCPoQxKFq0pJ6CZJgk9VeJ5bW3xL7xZ9D8uaSgH/ROmmjPhbUefljZywv?= =?iso-8859-1?Q?Qpv4QDhwCXijlBtTwzXJpmtYGKAtHy8ajunZZHzwVxV7NyIxtyG3n6RXee?= =?iso-8859-1?Q?/1YYkbIcRCdEJIiqfQWSKnfYHJPVunWphUKogyRcIu4uxW5aHCuGXfX6zs?= =?iso-8859-1?Q?fm5Edt+HNc8yv1QSKEq4su9Hscbd6mH1KaDR6ZWDPOfak8kiAsnd3qH+oJ?= =?iso-8859-1?Q?x0EwzhBcrJCmLorBb2EAx85V2PZKWQ1t592i92U4zlVmgHIePDHiXJg4E5?= =?iso-8859-1?Q?jCpFvb6gnP+qk5oKv0JzJkVHBP2ohODU4MbMy5XKay3uHqz1qfeQHupIkS?= =?iso-8859-1?Q?BBdSrg0MKBoKOYJLgROs38B5a14kjxUr1ToAKrrkhmiKFC+HPGRe+jAQOA?= =?iso-8859-1?Q?rFZGyxc8KNsgTpoTiREf7X14259wLvlt9DI+CuQYLyxZ08FDmUwcuWzsp7?= =?iso-8859-1?Q?fSdcb9PoOez0IaXaq9PBfxDzNkPfwE2U6ljnBQIXC7PySKN7z17bG3K7Lc?= =?iso-8859-1?Q?bYn7NEMetkKgLJMDmSsf3B21hH03NRuE185GGE28VXL0NmnOamoqZxncP4?= =?iso-8859-1?Q?tsr0OiEXJpJE2GIlfVFerG7YRdM/TOhE7+fXEqBAhCf5dNlRA2FRupu57N?= =?iso-8859-1?Q?0VpTXfARJl5cAakVOLXsfi0tBsRmNTmYpAk=3D?= x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?iso-8859-1?Q?xoA6uf958XApnwTcWpCiwFzTwnnsEgyyjRBOpHLQcvqalXCaqWZoYseaFU?= =?iso-8859-1?Q?E3ITAyTU5lb25R51zd8XCjR+b3h+SbjcRzKVT+fFLVijQkDs3IEZW1A3te?= =?iso-8859-1?Q?Lj2SCpAhbaV+kCLVi5RZ+iXtUXhCW1N3o/DzVgZrP1IvIIa8ibytuEs8Vx?= =?iso-8859-1?Q?nkFHZQn3I/Exlp+qfn46yz2wIS/Vs7heMjypLmKcHv1zwvu582FtRPjEGw?= =?iso-8859-1?Q?Tpss7I3EAAlniYIYIrOx85I6pIBVFLYtwsBiRHQzwpLMEHhSIgVxUaOdQd?= =?iso-8859-1?Q?7BthYhORfzWGbFonIN7t14IQx5DvFIzStB6ASc1lRn4KXFfiz8c08dqbLK?= =?iso-8859-1?Q?m9d5gj6+NINZ0xEDWD9Mcx5GZ0o1EFvqH/dz1NEoXeOD2e8idm3xFq9Pmt?= =?iso-8859-1?Q?pDJKu5b9J8a4Fw45eXkXS1VtwgrNJ0Rngkf1kyzP8rEEVY8QQ4IFwS5uIb?= =?iso-8859-1?Q?VR1q4GezNc4Sr+ok8iPV86iQyRt/XnHHLINML/RMi2JBczRP8C8q8VVo0s?= =?iso-8859-1?Q?Y9gYo/mlK7+wTE6DrQE606JL+98tI9YlMhjEha2jaYquDztHdLjAiDizS7?= =?iso-8859-1?Q?RWCv6pxdH8qljZEB764qpHzZSa+XPNcSdTViNDbOGr43t6tawSPlZoHeYN?= =?iso-8859-1?Q?SHOpkCnuHrbgRUGgDtLn10sOwSiLX210EWb2qeX+SDjHJCavLKwBqu1hEP?= =?iso-8859-1?Q?74QTtAWkQ7BQigItojxSVOXBwjDhDR0v4ad8jQoNrwSLmrQR1iO+ZPwa+M?= =?iso-8859-1?Q?KW8XZdEYKlNrPrkMzbJP1QtF66N7IT7iqR8cpjWAVw0QyBGEEavJ0Dep6r?= =?iso-8859-1?Q?1NT7W7nlYk21AaMJ9q2HYBdZq/r7hVTBtQDEspwpZUWphfsAxPLJpbjAEA?= =?iso-8859-1?Q?zlFdpOJo+nAIsjDlNWN5zKVHq9K5MQsxrquRHHkEkeIJpEILnWksEY+g0v?= =?iso-8859-1?Q?fxGkhiJDt5C1ecyEeMPtmCdCK7i5wHyyzx/aOLLp2HiR3QErtgfI8j2SIo?= =?iso-8859-1?Q?K4/5PCoZ43605o8ntAXUV3NwG5HBzRNNSz+Ng0aInq0BquujEpZz3jokfO?= =?iso-8859-1?Q?FTBSCUd8VK5hsaAzmIGIeJqQYLq6r/+wgutGFbaIDypIWdgFC5uMLJ7nFK?= =?iso-8859-1?Q?vh7TZ6+jGUa7DYsT7f5hGhLO1XR2gjAZFF9QX3G1gOX7n5NisdEwCV091l?= =?iso-8859-1?Q?wA0roViiyqg0nEAeZcMuSrkzaHh1WgKaCVwo+DO4OSNJI/gcGVeBI/1Vex?= =?iso-8859-1?Q?dwCpRISXd3Ayoi2uGmczS3kcqy527SYlfFDsUClVU=3D?= MIME-Version: 1.0 X-OriginatorOrg: sct-15-20-4755-11-msonline-outlook-1ff67.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: aae673b6-808f-41c0-3d18-08dd611a80e7 X-MS-Exchange-CrossTenant-originalarrivaltime: 12 Mar 2025 04:01:01.5534 (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: LV8P223MB1112 Subject: Re: [FFmpeg-devel] [PATCH v6 6/8] fftools/ffmpeg_graphprint: Add options for filtergraph printing 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="iso-8859-1" Content-Transfer-Encoding: quoted-printable Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: > -----Original Message----- > From: Stefano Sabatini > Sent: Dienstag, 11. M=E4rz 2025 00:03 > To: FFmpeg development discussions and patches > Cc: softworkz > Subject: Re: [FFmpeg-devel] [PATCH v6 6/8] fftools/ffmpeg_graphprint: > Add options for filtergraph printing Hi Stefano, I've made all of your changes unless mentioned below, ran patcheck, found t= wo or three more bits, but a lot of its output is not applicable I suppose. Here are some notes: > > + [SECTION_ID_HWFRAMESCONTEXT] =3D { SECTION_ID_HWFRAMESCONTEXT, = "HwFramesContext", 0, { SECTION_ID_ERROR, -1 }, }, > > + > > + [SECTION_ID_ERROR] =3D { SECTION_ID_ERROR, "Error", 0= , { -1 } }, > = > > + [SECTION_ID_LOGS] =3D { SECTION_ID_LOGS, "Log", AV_T= EXTFORMAT_SECTION_FLAG_IS_ARRAY, { SECTION_ID_LOG, -1 } }, > > + [SECTION_ID_LOG] =3D { SECTION_ID_LOG, "LogEntry", = 0, { -1 }, }, > = > Why not Logs/Log? It's OK but I'd keep ID and names in synch to > simplify reading/editing. The last three ones weren't even used - removed. > > +static void print_hwdevicecontext(AVTextFormatContext *w, const AVHWDe= viceContext *hw_device_context) > > +{ > > + avtext_print_section_header(w, NULL, SECTION_ID_HWDEViCECONTEXT); > > + > = > > + print_int("HasHwDeviceContext", 1); > > + print_str("DeviceType", av_hwdevice_get_type_name(hw_device_contex= t->type)); > = > = > Is this even needed? If missing I'd expect type to be none. Or not? av_hwdevice_get_type_name does not return "none" but NULL for the "none" en= um id. The reasoning was slightly different, though: Typically, you deserialize JS= ON (or XML) into an object model. If the DeviceType is not present in the t= arget object model (missing enum member), the cannot be deserialized which = means that it gets lost - including the information that there was a device= context in place. The HasHwDeviceContext (and similar members) prevents th= at by separating presence and type information. But I have reduced these that now and also dropped the extra section for hw= _device_context. > > + print_str("SwPixelFormatAlias", pixdescSw->alias); > > + } > > + > = > > + print_int("Width", hw_frames_context->width); > > + print_int("Height", hw_frames_context->height); > = > is this meaningful in case of no context? Or should we rather skip them? What do you mean by "no context"?. The function is only entered if a hwfram= es context exists. In that case it's interesting to know width and height of the frames contex= t as it may differ from the presentation frame size. > > + ////case AVMEDIA_TYPE_SUBTITLE: > > + //// print_str("Format", av_x_if_null(av_get_subtitle_fmt_= name(link->format), "?")); > > + //// print_int("Width", link->w); > > + //// print_int("Height", link->h); > > + //// print_q("TimeBase", link->time_base, '/'); > > + //// break; > = > I guess this does not exist yet right? This does exist and is working, just not in ffmpeg yet (https://github.com/= ffstaging/FFmpeg/pull/18) But it's only av_get_subtitle_fmt_name(), so I left just that line commente= d. (I can also remove it if it should) > > + avtext_print_section_header(w, NULL, SECTION_ID_FILTER); > > + > > + print_str("Name", filter->name); > > + > > + if (filter->filter) { > = > > + print_str("Name2", filter->filter->name); > = > something more descriptive such as "class name"? = I've made it more clear now. There's a filter_id and a filter_name, no more= name2/3. > > + print_str("DestName", link->dst->name); > > + print_str("DestPadName", avfilter_pad_get_name(link->dstpad, 0= )); > > + print_str("SourceName", link->src->name); > = > possibly unrelated, but I'm a bit surprised by the asymmetry In case you meant why there's no SourcePadName in this snippet, that's beca= use the source is the filter under which these ouputs are shown. I have cha= nged that now, so there's pad_index, pad_name, dest_filter_id and dest_pad_= name for outputs and similar for inputs. Generally, there's asymmetry in quite a number of cases. What might explain= it somewhat is that those graphs have a fixed direction. > > + if ((ret =3D avtext_context_open(&tctx, text_formatter, wctx, w_ar= gs, sections, FF_ARRAY_ELEMS(sections), 0, 0, 0, 0, -1, NULL)) >=3D 0) { > = > > + avtext_print_section_header(tctx, NULL, SECTION_ID_ROOT); > > + avtext_print_section_header(tctx, NULL, SECTION_ID_FILTERGRAPH= S); > > + avtext_print_section_header(tctx, NULL, SECTION_ID_FILTERGRAPH= ); > > + > > + av_bprint_clear(target_buf); > = > If I understand this is printing the sections and then discarding the > generated buffer, right? Maybe add a not to explain why this is done Done: // Due to the threading model each graph needs to print itself into a buffer // from its own thread. The actual printing happens short before cleanup in= ffmpeg.c // where all grahps are assembled together. To make this work, we need to p= ut the // formatting context into the same state like it would be when printing al= l at once, // so here we print the section headers and clear the buffer to get into th= e right state. > > + if (print_graphs) { > > + printf("%s", target_buf.str); > = > > + av_log(NULL, AV_LOG_INFO, "%s %c", target_buf.str, '\n'= ); > = > why it not hardcoding the newline in the message? I can't remember, not sure which or whether there was a good reason. > > + { "print_graphs_format", OPT_TYPE_STRING, 0, > > + { &print_graphs_format }, > = > > + "set the output printing format (available formats are: default,= compact, csv, flat, ini, json, xml)", "format" }, > = > non blocking but it would be good to avoid the hardcode in a case a > new format is added (or we'll skip update) There's again the problem of static initialization order. While I'm sure th= at this could be worked out in some tricky way, I wonder whether that's wor= th the effort. When was the last time that a new text format has been added= ? And when some new format gets added, it will need to be "hard-coded" in t= he doc files and at various other places anyway. > About the output style, this is using FooBar style against foo_bar > employed by ffprobe itself. I'm not against this, but I wonder if this > was considered and if using a more consistent style across tools is > helpful. Back then, I hadn't thought much about it. I just did what I thought is nat= ural for JSON and XML documents. = Even though it will cause me some headaches, your point is undeniably valid= , so I've reworked everything to snake case and also adjusted the naming of= some elements for better clarity. Thanks again for your reviews, best wishes, 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".