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

[James Almer <jamrial@gmail.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 6401 bytes --]

On 5/15/2025 9:36 PM, softworkz . wrote:
> 
> 
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of softworkz .
>> Sent: Freitag, 16. Mai 2025 02:33
>> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
>> Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make it a
>> Killer-Feature!
>>
>>
>>
>>> -----Original Message-----
>>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of James
>> Almer
>>> Sent: Freitag, 16. Mai 2025 02:28
>>> To: ffmpeg-devel@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@ffmpeg.org> On Behalf Of Marton
>>>>> Balint
>>>>> Sent: Freitag, 16. Mai 2025 02:00
>>>>> 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 Thu, 15 May 2025, softworkz . wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
>> Ramiro
>>>>> Polla
>>>>>>> Sent: Donnerstag, 15. Mai 2025 23:50
>>>>>>> To: ffmpeg-devel@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@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.


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".