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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Lynne
> Sent: Freitag, 16. Mai 2025 02:45
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make it a
> Killer-Feature!
> 
> On 16/05/2025 02:39, James Almer wrote:
> > On 5/15/2025 9:36 PM, softworkz . wrote:
> >>
> >>
> >>> -----Original Message-----
> >>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> >>> softworkz .
> >>> Sent: Freitag, 16. Mai 2025 02:33
> >>> 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!
> >>>
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of James
> >>> Almer
> >>>> Sent: Freitag, 16. Mai 2025 02:28
> >>>> To: ffmpeg-devel at ffmpeg.org
> >>>> Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now,
> >>>> make it
> >>> a
> >>>> Killer-Feature!
> >>>>
> >>>> On 5/15/2025 9:17 PM, softworkz . wrote:
> >>>>>
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> >>>>>> Marton
> >>>>>> Balint
> >>>>>> Sent: Freitag, 16. Mai 2025 02:00
> >>>>>> 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!
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> On Thu, 15 May 2025, softworkz . wrote:
> >>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>> -----Original Message-----
> >>>>>>>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> >>> Ramiro
> >>>>>> Polla
> >>>>>>>> Sent: Donnerstag, 15. Mai 2025 23:50
> >>>>>>>> To: ffmpeg-devel at ffmpeg.org
> >>>>>>>> Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint:
> >>>>>>>> Now,
> >>> make
> >>>>>> it a
> >>>>>>>> Killer-Feature!
> >>>>>>>>
> >>>>>>>> Hi,
> >>>>>>>>
> >>>>>>>> On Thu, May 15, 2025 at 11:11 PM softworkz <git at 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?
> >>>>>>
> >>>>>> Because ffmpeg is not a browser opener tool, but a transcoding
> >>>>>> tool. An
> >>>>>> argument can be made for every feature you can think of (Why not
> >>>>>> add an
> >>>>>> option which shuts down a computer when the transcoding is done?
> >>>>>> Why not
> >>>>>> add a playable DOOM implementation so the user will not be bored when
> >>>>>> waiting for the transcode to finish).
> >>>>>>
> >>>>>> Let's just revert this. The many ffmpeg cli frontends can open
> >>>>>> browsers
> >>> if
> >>>>>> they want.
> >>>>>
> >>>>> Many good arguments can be found for both sides.
> >>>>
> >>>> We don't launch external applications from the CLI, ever, under no
> >>>> circumstances. This is not an exception.
> >>>>
> >>>>>
> >>>>>> Because ffmpeg is not a browser opener tool
> >>>>>
> >>>>> By all respect, this isn't one.
> >>>>>
> >>>>>
> >>>>> Anyway, I will let the TC decide about this, then.
> >>>>
> >>>> No, there's no need to involve the TC when everyone is telling you that
> >>>> something is wrong.
> >>>
> >>> I think this is exactly the kind of case which the TC has been installed
> >>> for to handle.
> >>
> >> To clarify: I'm not seeking a yes/no decision but rather the question of
> >> "How it can be safely delivered while still being convenient".
> >
> > We don't launch a damn video player when transcoding finishes. There's
> > no reason to launch a browser to display some generated webpage with a
> > graph just like there's no reason to do it for any other kind of output.
> >
> > Print the name of the html, with a path if one was given, and that's
> > enough.
> 
> Exactly. We never open external applications

Almost every new feature adds some behavior that has never been there
before. I'm totally open for doing it in a different or in a more "safe"
way. 
Also - to be precise, it doesn't launch a browser directly, but the 
associated application for .html files (if any).

I believe that it should possible to find a way that is acceptable for 
the majority while still being convenient.

Maybe something like on first use of "-sg", it outputs a script as 
text that the user needs to save or setup in some way in order to get 
auto-launch.

Or an environment variable that needs to be set to enabled it.

Regards
sw