[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 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". 

Thanks,
sw