> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of Ramiro Polla
> Sent: Freitag, 16. Mai 2025 00:49
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make it a
> Killer-Feature!
>
> On Fri, May 16, 2025 at 12:43 AM softworkz .
> <softworkz-at-hotmail....@ffmpeg.org> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of Mark
> > > Thompson
> > > Sent: Freitag, 16. Mai 2025 00:35
> > > To: ffmpeg-devel@ffmpeg.org
> > > Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make
> it a
> > > Killer-Feature!
> > >
> > > On 15/05/2025 23:19, softworkz . wrote:
> > > >
> > > >
> > > >> -----Original Message-----
> > > >> From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of
> Ramiro
> > > Polla
> > > >> Sent: Freitag, 16. Mai 2025 00:13
> > > >> To: FFmpeg development discussions and patches <ffmpeg-
> de...@ffmpeg.org>
> > > >> Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now,
> make
> > > it a
> > > >> Killer-Feature!
> > > >>
> > > >> On Fri, May 16, 2025 at 12:00 AM softworkz .
> > > >> <softworkz-at-hotmail....@ffmpeg.org> wrote:
> > > >>>> On Thu, May 15, 2025 at 11:11 PM softworkz <g...@videolan.org> wrote:
> > > >>>> [...]
> > > >>>>> diff --git a/fftools/graph/filelauncher.c
> b/fftools/graph/filelauncher.c
> > > >>>>> new file mode 100644
> > > >>>>> index 0000000000..45514ca599
> > > >>>>> --- /dev/null
> > > >>>>> +++ b/fftools/graph/filelauncher.c
> > > >>>> [...]
> > > >>>>> +int ff_open_html_in_browser(const char *html_path)
> > > >>>>> +{
> > > >>>>> + if (!html_path || !*html_path)
> > > >>>>> + return -1;
> > > >>>>> +
> > > >>>>> +#if defined(_WIN32)
> > > >>>>> +
> > > >>>>> + // --- Windows ---------------------------------
> > > >>>>> + {
> > > >>>>> + HINSTANCE rc = ShellExecuteA(NULL, "open", html_path, NULL,
> > > >> NULL,
> > > >>>> SW_SHOWNORMAL);
> > > >>>>> + if ((UINT_PTR)rc <= 32) {
> > > >>>>> + // Fallback: system("start ...")
> > > >>>>> + char cmd[1024];
> > > >>>>> + _snprintf_s(cmd, sizeof(cmd), _TRUNCATE, "start \"\"
> > > >> \"%s\"",
> > > >>>> html_path);
> > > >>>>> + if (system(cmd) != 0)
> > > >>>>> + return -1;
> > > >>>>> + }
> > > >>>>> + return 0;
> > > >>>>> + }
> > > >>>>> +
> > > >>>>> +#elif defined(__APPLE__)
> > > >>>>> +
> > > >>>>> + // --- macOS -----------------------------------
> > > >>>>> + {
> > > >>>>> + // "open" is the macOS command to open a file/URL with the
> > > >> default
> > > >>>> application
> > > >>>>> + char cmd[1024];
> > > >>>>> + snprintf(cmd, sizeof(cmd), "open '%s' 1>/dev/null 2>&1 &",
> > > >>>> html_path);
> > > >>>>> + if (system(cmd) != 0)
> > > >>>>> + return -1;
> > > >>>>> + return 0;
> > > >>>>> + }
> > > >>>>> +
> > > >>>>> +#else
> > > >>>>> +
> > > >>>>> + // --- Linux / Unix-like -----------------------
> > > >>>>> + // We'll try xdg-open, then gnome-open, then kfmclient
> > > >>>>> + {
> > > >>>>> + // Helper macro to try one browser command
> > > >>>>> + // Returns 0 on success, -1 on failure
> > > >>>>> + #define TRY_CMD(prog) do {
> \
> > > >>>>> + char buf[1024];
> \
> > > >>>>> + snprintf(buf, sizeof(buf), "%s '%s' 1>/dev/null 2>&1
> &", \
> > > >>>>> + (prog), html_path);
> \
> > > >>>>> + int ret = system(buf);
> \
> > > >>>>> + /* On Unix: system() returns -1 if the shell can't run.
> */\
> > > >>>>> + /* Otherwise, check exit code in lower 8 bits.
> > > >> */\
> > > >>>>> + if (ret != -1 && WIFEXITED(ret) && WEXITSTATUS(ret) ==
> 0) \
> > > >>>>> + return 0;
> \
> > > >>>>> + } while (0)
> > > >>>>> +
> > > >>>>> + TRY_CMD("xdg-open");
> > > >>>>> + TRY_CMD("gnome-open");
> > > >>>>> + TRY_CMD("kfmclient exec");
> > > >>>>> +
> > > >>>>> + fprintf(stderr, "Could not open '%s' in a browser.\n",
> > > >> html_path);
> > > >>>>> + return -1;
> > > >>>>> + }
> > > >>>>> +
> > > >>>>> +#endif
> > > >>>>> +}
> > > >>>> [...]
> > > >>>>
> > > >>>> Sorry I didn't have a closer look at the patchset while it was under
> > > >>>> review, but system(cmd) is a big no-no. We could create a file with
> an
> > > >>>> explicit path passed by the user, but then it's up to the user to
> open
> > > >>>> it.
> > > >>>
> > > >>> What's bad about opening a file in the browser when that's the
> documented
> > > >>> behavior of the cli parameter?
> > > >>
> > > >> Straight out of ChatGPT:
> > > >> I understand the motivation — making the feature more user-friendly by
> > > >> launching the result directly is a nice touch. The concern isn't with
> > > >> the feature itself, but rather with the way it's implemented.
> > > >> Using system() to launch a browser introduces potential security
> > > >> risks, especially if the file path is ever constructed from untrusted
> > > >> input (e.g. future scripting, API wrapping, or unexpected shell
> > > >> expansion). It's generally discouraged in projects like FFmpeg, where
> > > >> robustness and security are critical.
> > > >
> > > > Hi,
> > > >
> > > > of course I understand that.
> > > > But it isn't constructed from untrusted input.
> > > >
> > > > Best regards
> > > > sw
> > >
> > > $ export TMPDIR="'; rm -rf / ;'\\\\"
> > > $ ./ffmpeg_g -sg -i /dev/null -f null -
> > >
> > > Calls to system are just not a good idea in general. Suggest printing the
> > > file name and let the user open the file however they choose to.
> >
> > How about some middle ground like where the user needs to confirm with
> > another keypress?
> >
> > Or maybe something where the user needs to setup a script or an environment
> > variable to confirm that the automatic opening is performed?
> >
> > Or when a user uses the option for the first time, show a prompt whether
> > they are sure they want this to be auto-opened?
>
> What about the user parsing the output from the cli, looking for a
> specific string (such as "graph file saved to [...]"), and opening
> that?
How many user will do that? 0.00001% ? And that's not necessary anyway,
You can already do
ffmpeg -print_graphs -print_graphs_format mermaidhtml -print_graphs_file x.html
But when you need that, you don't remember what exactly you need to
specify, and look it up and change the file name on each run and
launch the browser manually, etc.
The reason for the title of this commit is because of adding a highly useful
method to get insights into what ffmpeg is doing which everybody can
remember and quickly add to a command line without needing to jump through
any hoops.
> Another point I'd like to add is that for platform-specific code like
> this, in the long term, we get a bunch of patches to fix changes for
> Microsoft's or Apple's new way of doing things, or someone wants to
> add BeOS or TempleOS support, and they're rarely properly maintained.
> It's better not to set that precedent.
Such things exist in many places, I really do not see a problem here.
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-requ...@ffmpeg.org with subject "unsubscribe".
