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

[Marton Balint]

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.

Thanks,
Marton