[FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make it a Killer-Feature!

[James Almer]

On 5/15/2025 6:58 PM, softworkz . wrote:
> 
> 
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of James Almer
>> Sent: Donnerstag, 15. Mai 2025 23:53
>> To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
>> Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make it a
>> Killer-Feature!
>>
>>> ffmpeg | branch: master | softworkz <softworkz at hotmail.com
>> <https://ffmpeg.org/mailman/listinfo/ffmpeg-cvslog>> | Thu May 15 23:10:02
>> 2025 +0200| [1f2b8d7238eff4ab8a4d8d6177e250b8180d51f4] | committer: softworkz
>>>
>>> fftools/graphprint: Now, make it a Killer-Feature!
>>>
>>> remember this: -sg   <= means Show Graph
>>>
>>> Signed-off-by: softworkz <softworkz at hotmail.com
>> <https://ffmpeg.org/mailman/listinfo/ffmpeg-cvslog>>
>>>
>>>
>>> /http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=1f2b8d7238eff4ab8a
>> 4d8d6177e250b8180d51f4
>>> /---
>>>
>>>    doc/ffmpeg.texi              |   4 +
>>>    fftools/Makefile             |   1 +
>>>    fftools/ffmpeg.c             |   2 +-
>>>    fftools/ffmpeg.h             |   1 +
>>>    fftools/ffmpeg_filter.c      |   2 +-
>>>    fftools/ffmpeg_opt.c         |   4 +
>>>    fftools/graph/filelauncher.c | 205
>> +++++++++++++++++++++++++++++++++++++++++++
>>>    fftools/graph/graphprint.c   |  48 +++++++++-
>>>    fftools/graph/graphprint.h   |  32 +++++++
>>>    9 files changed, 296 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
>>> index 35675b5309..4bcb6d6a01 100644
>>> --- a/doc/ffmpeg.texi
>>> +++ b/doc/ffmpeg.texi
>>> @@ -1404,6 +1404,10 @@ Writes execution graph details to the specified file
>> in the format set via -prin
>>>    Sets the output format (available formats are: default, compact, csv,
>> flat, ini, json, xml, mermaid, mermaidhtml)
>>>    The default format is json.
>>>
>>> + at item <https://ffmpeg.org/mailman/listinfo/ffmpeg-cvslog> -sg
>> (@emph{global})
>>> +Writes the execution graph to a temporary html file (mermaidhtml format)
>> and
>>> +tries to launch it in the default browser.
>>> +
>>>    @item -progress @var{url} (@emph{global})
>>>    Send program-friendly progress information to @var{url}.
>>>
>>> diff --git a/fftools/Makefile b/fftools/Makefile
>>> index 361a4fd574..56a2910212 100644
>>> --- a/fftools/Makefile
>>> +++ b/fftools/Makefile
>>> @@ -22,6 +22,7 @@ OBJS-ffmpeg +=                  \
>>>        fftools/ffmpeg_opt.o        \
>>>        fftools/ffmpeg_sched.o      \
>>>        fftools/graph/graphprint.o        \
>>> +    fftools/graph/filelauncher.o      \
>>>        fftools/sync_queue.o        \
>>>        fftools/thread_queue.o      \
>>>        fftools/textformat/avtextformat.o \
>>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
>>> index 964770df23..6513e2129e 100644
>>> --- a/fftools/ffmpeg.c
>>> +++ b/fftools/ffmpeg.c
>>> @@ -309,7 +309,7 @@ const AVIOInterruptCB int_cb = { decode_interrupt_cb,
>> NULL };
>>>
>>>    static void ffmpeg_cleanup(int ret)
>>>    {
>>> -    if (print_graphs || print_graphs_file)
>>> +    if (print_graphs || print_graphs_file || show_graph)
>>>            print_filtergraphs(filtergraphs, nb_filtergraphs, input_files,
>> nb_input_files, output_files, nb_output_files);
>>>
>>>        if (do_benchmark) {
>>> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
>>> index 7fbf0ad532..49fea0307d 100644
>>> --- a/fftools/ffmpeg.h
>>> +++ b/fftools/ffmpeg.h
>>> @@ -721,6 +721,7 @@ extern int print_graphs;
>>>    extern char *print_graphs_file;
>>>    extern char *print_graphs_format;
>>>    extern int auto_conversion_filters;
>>> +extern int show_graph;
>>>
>>>    extern const AVIOInterruptCB int_cb;
>>>
>>> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
>>> index b774606562..e82e333b7f 100644
>>> --- a/fftools/ffmpeg_filter.c
>>> +++ b/fftools/ffmpeg_filter.c
>>> @@ -2985,7 +2985,7 @@ read_frames:
>>>
>>>    finish:
>>>
>>> -    if (print_graphs || print_graphs_file)
>>> +    if (print_graphs || print_graphs_file || show_graph)
>>>            print_filtergraph(fg, fgt.graph);
>>>
>>>        // EOF is normal termination
>>> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
>>> index 3d1efe32f9..24713d640f 100644
>>> --- a/fftools/ffmpeg_opt.c
>>> +++ b/fftools/ffmpeg_opt.c
>>> @@ -79,6 +79,7 @@ int vstats_version = 2;
>>>    int print_graphs = 0;
>>>    char *print_graphs_file = NULL;
>>>    char *print_graphs_format = NULL;
>>> +int show_graph = 0;
>>>    int auto_conversion_filters = 1;
>>>    int64_t stats_period = 500000;
>>>
>>> @@ -1748,6 +1749,9 @@ const OptionDef options[] = {
>>>        { "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, mermaid, mermaidhtml)", "format" },
>>> +    { "sg",   OPT_TYPE_BOOL, 0,
>>> +        { &show_graph },
>>> +        "create execution graph as temporary html file and try to launch it
>> in the default browser" },
>>
>> Absolutely not, wtf. Calling an external application like this?
>>
>> Revert this patch or remove this effect immediately.
> 
> 15 versions have been posted, I have sent 3 messages asking for comments
> before applying over the past 2 weeks.
> 
> sw

And there are still unresolved comments you didn't take into account 
before pushing this set.

This specific change is not acceptable, so it needs to be reverted.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 495 bytes
Desc: OpenPGP digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20250515/8aa09af6/attachment.sig>