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

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

[Ramiro Polla]

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.

Ramiro
_______________________________________________
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".

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

[softworkz .]

> -----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?

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-request@ffmpeg.org with subject "unsubscribe".

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

[Ramiro Polla]

On Fri, May 16, 2025 at 12:00 AM softworkz .
<softworkz-at-hotmail.com@ffmpeg.org> wrote:
> > 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?

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.

Ramiro
_______________________________________________
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".

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Ramiro Polla
> Sent: Freitag, 16. Mai 2025 00:13
> 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:00 AM softworkz .
> <softworkz-at-hotmail.com@ffmpeg.org> wrote:
> > > 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?
> 
> 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

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

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of softworkz .
> Sent: Freitag, 16. Mai 2025 00:19
> 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 Ramiro
> Polla
> > Sent: Freitag, 16. Mai 2025 00:13
> > 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:00 AM softworkz .
> > <softworkz-at-hotmail.com@ffmpeg.org> wrote:
> > > > 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?
> >
> > 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
> 
> _______________________________________________

So, in case you have just seen those few lines and not looked at the whole
patchset: 

This is creating filtergraph visualizations like this:
https://softworkz.github.io/ffmpeg_output_apis/2_hwa_qsv.html

The html and css are trusted because they are included included as
compressed resources in the binary and the rest is built dynamically
in code from the filtergraph objects at runtime, so this can all be
considered as trusted. And only this very specific shown launched
for viewing in a browser.

I totally agree that this should never be done for arbitrary html
content that is not under our control.

Best
softworkz






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

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

[Mark Thompson]

On 15/05/2025 23:19, softworkz . wrote:
> 
> 
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Ramiro Polla
>> Sent: Freitag, 16. Mai 2025 00:13
>> 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:00 AM softworkz .
>> <softworkz-at-hotmail.com@ffmpeg.org> wrote:
>>>> 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?
>>
>> 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.

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

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@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-bounces@ffmpeg.org> On Behalf Of Ramiro
> Polla
> >> Sent: Freitag, 16. Mai 2025 00:13
> >> 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:00 AM softworkz .
> >> <softworkz-at-hotmail.com@ffmpeg.org> wrote:
> >>>> 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?
> >>
> >> 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?

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-request@ffmpeg.org with subject "unsubscribe".

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

[Ramiro Polla]

On Fri, May 16, 2025 at 12:43 AM softworkz .
<softworkz-at-hotmail.com@ffmpeg.org> wrote:
>
>
>
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@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-bounces@ffmpeg.org> On Behalf Of Ramiro
> > Polla
> > >> Sent: Freitag, 16. Mai 2025 00:13
> > >> 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:00 AM softworkz .
> > >> <softworkz-at-hotmail.com@ffmpeg.org> wrote:
> > >>>> 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?
> > >>
> > >> 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?

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.

Ramiro
_______________________________________________
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".

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@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.com@ffmpeg.org> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@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-bounces@ffmpeg.org> On Behalf Of
> Ramiro
> > > Polla
> > > >> Sent: Freitag, 16. Mai 2025 00:13
> > > >> 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:00 AM softworkz .
> > > >> <softworkz-at-hotmail.com@ffmpeg.org> wrote:
> > > >>>> 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?
> > > >>
> > > >> 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-request@ffmpeg.org with subject "unsubscribe".

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

[Ramiro Polla]

On Fri, May 16, 2025 at 1:04 AM softworkz .
<softworkz-at-hotmail.com@ffmpeg.org> wrote:
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Ramiro Polla
> > Sent: Freitag, 16. Mai 2025 00:49
[...]
> > 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.

I understand that very few users will remember the proper invocation
off the top of their heads.

<ChatGPT>
But at the same time, a malicious user crafting a script, wrapper, or
even just pasting shell commands into a terminal can absolutely be
expected to find and exploit any flaw we introduce, especially if it's
a call to system() with file paths involved. So while the feature is
aimed at convenience for a large group of users, it also creates a
non-trivial risk vector that a very small number of malicious users
could exploit in subtle and damaging ways. And historically, these are
exactly the kind of paths that get targeted over time.
</ChatGPT>

I very much appreciate the filtergraph visualizations that you linked
to (it *is* really useful), but I just don’t think ffmpeg should try
to launch the browser for us.

Ramiro
_______________________________________________
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".

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Ramiro Polla
> Sent: Freitag, 16. Mai 2025 01:30
> 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 1:04 AM softworkz .
> <softworkz-at-hotmail.com@ffmpeg.org> wrote:
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Ramiro
> Polla
> > > Sent: Freitag, 16. Mai 2025 00:49
> [...]
> > > 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.
> 
> I understand that very few users will remember the proper invocation
> off the top of their heads.
> 
> <ChatGPT>
> But at the same time, a malicious user crafting a script, wrapper, or
> even just pasting shell commands into a terminal can absolutely be
> expected to find and exploit any flaw we introduce, especially if it's
> a call to system() with file paths involved. So while the feature is
> aimed at convenience for a large group of users, it also creates a
> non-trivial risk vector that a very small number of malicious users
> could exploit in subtle and damaging ways. And historically, these are
> exactly the kind of paths that get targeted over time.
> </ChatGPT>

This is just bla bla.

Please explain how you believe this could be exploited.

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-request@ffmpeg.org with subject "unsubscribe".

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@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-bounces@ffmpeg.org> On Behalf Of Ramiro
> Polla
> >> Sent: Freitag, 16. Mai 2025 00:13
> >> 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:00 AM softworkz .
> >> <softworkz-at-hotmail.com@ffmpeg.org> wrote:
> >>>> 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?
> >>
> >> 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.

I just wanted to note that this is not the normal way how it works.
The normal way is that you provide the target output file on the command 
line, so that part exists already.

"-sg" is just on top of it which saves you from dealing with file names
and manual opening. Instead, you can run command after command and the
graph will open in the browser in one tab after another, so that you can
compare easily across different runs without any manual work.


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-request@ffmpeg.org with subject "unsubscribe".

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

[Rémi Denis-Courmont]

Le perjantaina 16. toukokuuta 2025, 1.19.15 Itä-Euroopan kesäaika softworkz . 
a écrit :
> of course I understand that. 
> But it isn't constructed from untrusted input.

You're being ridiculous. `system()` has a long history of causign bugs, many 
of them security related, and many not fixable.

If you were implementing a command line interface that needs to process 
trusted input like the shell would, you would want to use `wordexp()`.

As you merely need to spawn a child process, use the `posix_spawn`*`()` where 
available, and `fork()` then `exec`*`()` elsewhere. We don't want to spawn a 
shell just to start a well-known executable (other than the shell itself).

-- 
德尼-库尔蒙‧雷米
Tapio's place new town, former Finnish Republic of Uusimaa



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

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Rémi Denis-
> Courmont
> Sent: Samstag, 24. Mai 2025 17:55
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make it a
> Killer-Feature!
> 
> Le perjantaina 16. toukokuuta 2025, 1.19.15 Itä-Euroopan kesäaika softworkz .
> a écrit :
> > of course I understand that.
> > But it isn't constructed from untrusted input.
> 
> You're being ridiculous. `system()` has a long history of causign bugs, many
> of them security related, and many not fixable.
> 
> If you were implementing a command line interface that needs to process
> trusted input like the shell would, you would want to use `wordexp()`.
> 
> As you merely need to spawn a child process, use the `posix_spawn`*`()`
> available, and `fork()` then `exec`*`()` elsewhere. 


glibc's system() implementation is using posix_spawn internally since 2.34 and
before that, it is using fork() and execve() to launch sh.


> We don't want to spawn a
> shell just to start a well-known executable (other than the shell itself).

And yet, exactly the latter is happening, because the code is
invoking a shell script (xdg-open) - it doesn't launch a browser
executable.
Sadly, this has been misunderstood by many - who commented 
without even looking at the code.

Sure - we could invoke the script as an executable - that would
give us a single advantage: we would then supply the html file
path as an argument rather than in a command string. This 
prevents injection attacks that try to escape to the shell,
but that's just one possible attack vector. Just because we 
supply it as an argument to the script doesn't mean it's 
safe. The xdg-open scripts can differ by platform and can have
their own vulnerabilities. And since xdg-open is redirecting
to a variety of applications - from which every single one can 
have its own vulnerabilities, there is not much safety we 
would have gained by that.

It all burns down to this: 

It is our responsibility to make sure that the path we are
passing over is safe. No matter how we are calling xdg-open.

That path is constructed programmatically, it doesn't depend
on user input. It is constructed from the temp folder path
combined with a file name that has a fixed format generated 
from the time of execution.

There has been one comment (can't find it anymore) that I would
call the single most valid comment made in this regard, which 
was about the way how the temp path is determined on Linux,
and that's where I agree that it isn't safe enough in the way
how it was done.


Best regards,
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-request@ffmpeg.org with subject "unsubscribe".

Re: [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@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.

Thanks,
Marton
_______________________________________________
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".

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

[softworkz .]

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

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

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-request@ffmpeg.org with subject "unsubscribe".

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

[James Almer]

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

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. You pushed this set before even addressing all reviews.

Either send a new patch removing the offending code, or a git revert for 
the entire commit.


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

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

[softworkz .]

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

> You pushed this set before even addressing all reviews.

This is incorrect.

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-request@ffmpeg.org with subject "unsubscribe".

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

[softworkz .]

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

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-request@ffmpeg.org with subject "unsubscribe".

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

[James Almer]

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

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

[Lynne]

[-- Attachment #1.1.1.1: Type: text/plain, Size: 7229 bytes --]

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@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.

Exactly. We never open external applications

[-- Attachment #1.1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 637 bytes --]

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 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".

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Lynne
> Sent: Freitag, 16. Mai 2025 02:45
> To: ffmpeg-devel@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@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.
> 
> 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






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

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

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 6358 bytes --]

Hi

On Fri, May 16, 2025 at 12:17:14AM +0000, 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.
> 
> > 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.

Not speaking as a TC member here but IMHO

1. lets all calm down, so far we have a civil and productive discussion
   maybe we can simply find a solution everyone is happy with!

2. all security issues must be fixed if there are some

3. there should be a configure flag to enable/disable the browser opening feature
   if it remains

4. can system() be replaced by fork()+exec*() ? is that something people would
   prefer ?

5. this is a cool feature, i would use this if its available, that said
   if i had to manually open a browser with a given URL that would work
   for me too.

6. everyone who unconditionally hates this, please try it

7. as long as the discussion is nice and productive i think the TC is not the
   best path forward. If things degenerate into some mess then pas it to teh
   TC, yes

8. I think one part people should agree on first is if this should be disabled
   while it is being discussed (maybe a disabled by default configure switch
   would work?)

Anyway, iam a little sleepy maybe i have some better view after sleeping over
this :)

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship: All citizens are under surveillance, all their steps and
actions recorded, for the politicians to enforce control.
Democracy: All politicians are under surveillance, all their steps and
actions recorded, for the citizens to enforce control.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 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".

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Michael
> Niedermayer
> Sent: Freitag, 16. Mai 2025 02:54
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make it a
> Killer-Feature!
> 
> Hi
> 
> On Fri, May 16, 2025 at 12:17:14AM +0000, 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.
> >
> > > 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.
> 
> Not speaking as a TC member here but IMHO
> 
> 1. lets all calm down, so far we have a civil and productive discussion
>    maybe we can simply find a solution everyone is happy with!
> 
> 2. all security issues must be fixed if there are some
> 
> 3. there should be a configure flag to enable/disable the browser opening
> feature
>    if it remains
> 
> 4. can system() be replaced by fork()+exec*() ? is that something people would
>    prefer ?
> 
> 5. this is a cool feature, i would use this if its available, that said
>    if i had to manually open a browser with a given URL that would work
>    for me too.
> 
> 6. everyone who unconditionally hates this, please try it
> 
> 7. as long as the discussion is nice and productive i think the TC is not the
>    best path forward. If things degenerate into some mess then pas it to teh
>    TC, yes

I mentioned the TC, because there didn't seem to be any interest to even 
talk about it.

Questioning the security side is a reasonable concern IMO. Maybe it's possible
to find different APIs for doing the same or requiring the user to setup
a script or an environment variable to make it work.

Being feature doing something that hasn't been done before is not a 
reasonable concern IMO. Nobody is being forced to use it.

User convenience is important and beneficial for the project as it
makes it more attractive.
I thought about the generated output eventually having multiple tabs
where you can also view the log (with coloring) and another one
showing stats. Maybe also a breakdown of how the command line 
parameters were applied.

Finally, I want to repeat that I'm really open for discussion, I wish 
it would have been during the 15 revisions of the patchset and the three
reminders I had sent. 

Right now, I think it's really not fair to try to blame me as if I 
had done anything wrong.
I also want to stress that there wasn't a single review comment that
wouldn't have addressed. Those claims are not true.

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-request@ffmpeg.org with subject "unsubscribe".

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of softworkz .
> Sent: Freitag, 16. Mai 2025 03:27
> 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 Michael
> > Niedermayer
> > Sent: Freitag, 16. Mai 2025 02:54
> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make it


It's gone. I have reverted it.

Case closed. Apologies for the annoyance.

Regards
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-request@ffmpeg.org with subject "unsubscribe".

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of softworkz .
> Sent: Freitag, 16. Mai 2025 10:43
> 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 softworkz
> .
> > Sent: Freitag, 16. Mai 2025 03:27
> > 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 Michael
> > > Niedermayer
> > > Sent: Freitag, 16. Mai 2025 02:54
> > > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make
> it
> 
> 
> It's gone. I have reverted it.
> 
> Case closed. Apologies for the annoyance.
> 
> Regards
> sw


To all those who are disappointed now - or in the Future
========================================================

Sadly, this is once another feature from which FFmpeg 
developers are thinking that you should not have it.

I have tried my best, but I have no interest in endless 
arguing and upsetting everybody, so this feature dies 
here officially.

The good news is that when you compile for yourself, you
can still get it by reverting the revert commit, i.e. 
after checking out the current master, execute

git revert 79e2a845cd162696c7652bbb6cd407bfa24b738b


At some point, this may get broken, in that case you can
check out the previous commit:

git checkout 505510acdad5bc08b67e01d66c5b339c8fe27d39

but that will give you the state from May 16, 2025.


Eventually, I may check out the new "source plugin" 
mechanism, and provide the feature in a plugin branch,
so that you can add it via that way before compiling.


All the best,
softworkz

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

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

[Nicolas George]

softworkz . (HE12025-05-16):
> Sadly, this is once another feature from which FFmpeg 
> developers are thinking that you should not have it.
> 
> I have tried my best, but I have no interest in endless 
> arguing and upsetting everybody, so this feature dies 
> here officially.

Or you could find alternate method of implementing it that do not have
the flaws that your senior developers have pointed.

Your choice: play the victim or accept counsel.

-- 
  Nicolas George
_______________________________________________
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".

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Nicolas
> George
> Sent: Freitag, 16. Mai 2025 11:51
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make it a
> Killer-Feature!
> 
> softworkz . (HE12025-05-16):
> > Sadly, this is once another feature from which FFmpeg
> > developers are thinking that you should not have it.
> >
> > I have tried my best, but I have no interest in endless
> > arguing and upsetting everybody, so this feature dies
> > here officially.
> 
> Or you could find alternate method of implementing it that do not have
> the flaws that your senior developers have pointed.

> Your choice: play the victim or accept counsel.

Wake up and wait until you have a bright moment, then
read and realize that there was no counsel about launching the browser
in a safe way. In fact I was the only one making suggestions in this
regard.

The counsel from YOUR (not my) "senior developers" was not to launch
a browser at all - but that's what the feature is about.

I am not a victim in any way, I don't feel like a victim and neither 
do I pretend to be. 
Instead, I am reasonable and accept that a majority doesn't want it and
I think I pulled the plug at the right time.

Yet I know that the majority on the side of users is an entirely
different story and I think they deserve an explanation why it
happened. That's all.

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-request@ffmpeg.org with subject "unsubscribe".

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

[Nicolas George]

softworkz . (HE12025-05-16):
> The counsel from YOUR (not my) "senior developers" was not to launch
> a browser at all - but that's what the feature is about.

Counsel to do it properly has been given. Only in a few words, but your
repeated rudeness when offered counsel justifies people not wasting time
giving more details.

-- 
  Nicolas George
_______________________________________________
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".

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

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1359 bytes --]

On Fri, May 16, 2025 at 11:50:30AM +0200, Nicolas George wrote:
> softworkz . (HE12025-05-16):
> > Sadly, this is once another feature from which FFmpeg 
> > developers are thinking that you should not have it.
> > 
> > I have tried my best, but I have no interest in endless 
> > arguing and upsetting everybody, so this feature dies 
> > here officially.
> 
> Or you could find alternate method of implementing it that do not have
> the flaws

yes if thats possible, iam not sure everyone will agree though


> that your senior developers have pointed.

I think "your senior developers" is a bit provocative
and this mailing list has demonstrated its ability to melt down
quite spectacularly so lets just avoid being provocative in a
way thats not positive


> 
> Your choice: play the victim or accept counsel.

Can we please keep the thread technical

the patch was already reverted and theres already a clear majority
against system() calls. We are making good progress with everyone
working towards finding solutions for all issues uncovered
(some of which are in the review process that missed this patch
 which was posted 12 times over 1 month)

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Never trust a computer, one day, it may think you are the virus. -- Compn

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 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".

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

[Nicolas George]

Michael Niedermayer (HE12025-05-16):
> yes if thats possible, iam not sure everyone will agree though

That is possible and easy.

> I think "your senior developers" is a bit provocative

Maybe it is, but it plugs into a pattern of behavior by softworkz of
taking little heed of comments made by people with a lot more experience
working on FFmpeg and more importantly working as a team.

In particular I think you granted Git access too soon. A track record of
multiple non-trivial patches and useful reviews should have been
necessary.

> Can we please keep the thread technical

You should have answered that to “To all those who are disappointed now
- or in the Future”, because that is when the discussion stopped being
technical.

Regards,

-- 
  Nicolas George
_______________________________________________
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".

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Nicolas
> George
> Sent: Freitag, 16. Mai 2025 14:04
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make it a
> Killer-Feature!
> 
> Michael Niedermayer (HE12025-05-16):
> > yes if thats possible, iam not sure everyone will agree though
> 
> That is possible and easy.
> 
> > I think "your senior developers" is a bit provocative
> 
> Maybe it is, but it plugs into a pattern of behavior by softworkz of
> taking little heed of comments made by people with a lot more experience
> working on FFmpeg and more importantly working as a team.
> 
> In particular I think you granted Git access too soon. A track record of
> multiple non-trivial patches and useful reviews should have been
> necessary.


> 
> Regards,
> 
> --
>   Nicolas George
> _______________________________________________


For background and aftermath, please read: The "bad" Patch

https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2025-May/344274.html

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-request@ffmpeg.org with subject "unsubscribe".

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Michael
> Niedermayer
> Sent: Freitag, 16. Mai 2025 13: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 11:50:30AM +0200, Nicolas George wrote:
> > softworkz . (HE12025-05-16):
> > > Sadly, this is once another feature from which FFmpeg
> > > developers are thinking that you should not have it.
> > >
> > > I have tried my best, but I have no interest in endless
> > > arguing and upsetting everybody, so this feature dies
> > > here officially.
> >
> > Or you could find alternate method of implementing it that do not have
> > the flaws
> 
> yes if thats possible, iam not sure everyone will agree though

That's getting right to the point why I have given up on this and 
written the message "to those who are...":

Because it had become clear quite a few of those who are objecting
are not objecting the way how it is implemented technically - that could
have been solved - but rather the behavior that FFmpeg is launching a 
browser. And this is something that doesn't have a solution and so I 
have ended this.

No hard feeling except about being falsely blamed by 3 persons.

Not Nicolas, he just gets in return what he calls out for.

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-request@ffmpeg.org with subject "unsubscribe".

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

[Nicolas George]

softworkz . (HE12025-05-16):
> Because it had become clear quite a few of those who are objecting
> are not objecting the way how it is implemented technically - that could
> have been solved - but rather the behavior that FFmpeg is launching a 
> browser.

This is how it was implemented technically and this is what is being
criticized.

>	   And this is something that doesn't have a solution and so I 
> have ended this.

There is a solution, it was suggested to you.

-- 
  Nicolas George
_______________________________________________
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".

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

[Romain Beauxis]

Le jeu. 15 mai 2025 à 19:54, Michael Niedermayer <michael@niedermayer.cc> a
écrit :
>
> Hi
>
> On Fri, May 16, 2025 at 12:17:14AM +0000, 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.
> >
> > > 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.
>
> Not speaking as a TC member here but IMHO
>
> 1. lets all calm down, so far we have a civil and productive discussion
>    maybe we can simply find a solution everyone is happy with!
>
> 2. all security issues must be fixed if there are some
>
> 3. there should be a configure flag to enable/disable the browser opening
feature
>    if it remains
>
> 4. can system() be replaced by fork()+exec*() ? is that something people
would
>    prefer ?

Forked processes inherit opened file descriptors unless explicitly marked
as cloexec, this is another big can of worms..

> 5. this is a cool feature, i would use this if its available, that said
>    if i had to manually open a browser with a given URL that would work
>    for me too.

At the very least there should be a prompt asking the user to confirm
what's about to happen.

A user-friend way and also secure way could be to output something that is
readily usage, for instance:
bash -c `ffmpeg <options>`

> 6. everyone who unconditionally hates this, please try it
>
> 7. as long as the discussion is nice and productive i think the TC is not
the
>    best path forward. If things degenerate into some mess then pas it to
teh
>    TC, yes
>
> 8. I think one part people should agree on first is if this should be
disabled
>    while it is being discussed (maybe a disabled by default configure
switch
>    would work?)
>
> Anyway, iam a little sleepy maybe i have some better view after sleeping
over
> this :)
>
> thx
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Dictatorship: All citizens are under surveillance, all their steps and
> actions recorded, for the politicians to enforce control.
> Democracy: All politicians are under surveillance, all their steps and
> actions recorded, for the citizens to enforce control.
> _______________________________________________
> 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".
_______________________________________________
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".

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Romain
> Beauxis
> Sent: Freitag, 16. Mai 2025 05:40
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make it a
> Killer-Feature!
> 
> Le jeu. 15 mai 2025 à 19:54, Michael Niedermayer <michael@niedermayer.cc> a
> écrit :
> >
> > Hi
> >
> > On Fri, May 16, 2025 at 12:17:14AM +0000, 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.
> > >
> > > > 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.
> >
> > Not speaking as a TC member here but IMHO
> >
> > 1. lets all calm down, so far we have a civil and productive discussion
> >    maybe we can simply find a solution everyone is happy with!
> >
> > 2. all security issues must be fixed if there are some
> >
> > 3. there should be a configure flag to enable/disable the browser opening
> feature
> >    if it remains
> >
> > 4. can system() be replaced by fork()+exec*() ? is that something people
> would
> >    prefer ?
> 
> Forked processes inherit opened file descriptors unless explicitly marked
> as cloexec, this is another big can of worms..
> 
> > 5. this is a cool feature, i would use this if its available, that said
> >    if i had to manually open a browser with a given URL that would work
> >    for me too.
> 
> At the very least there should be a prompt asking the user to confirm
> what's about to happen.

I think that's a very good and agreeable point in general, independent
from everything else. I assume you mean at first-time use? Or first time
per day/month maybe?



> 
> A user-friend way and also secure way could be to output something that is
> readily usage, for instance:
> bash -c `ffmpeg <options>`

I'm not sure I understand the idea. ffmpeg should output another ffmpeg command?



Generally, I think something with a kind of "first-time setup" pattern would 
be great like:


- ffmpeg -sg
  - if "first-time use" (=detect whether the "setup" has been made)
    => don't execute actual cmd but show message explaining what -sg does, and 
                  offer to set it up for the user (in whichever way is needed on the platform)  or tell 
                  the user what's needed to be done if very simple
                  if user chooses "no", it prints a message asking to remove the -sg param and exits
    - otherwise use the existing "setup" to auto-launch the browser

On the technical side, I don't have a specific way in mind yet, but I think that opens up more ways 
how the auto-launch could happen. It would be very explicit for once and also more secure
because it doesn't work out-of-the box without "setup". When FFmpeg can perform the setup upon 
user confirmation, then it would still be very convenient.

Best
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-request@ffmpeg.org with subject "unsubscribe".

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of softworkz .
> Sent: Freitag, 16. Mai 2025 06:16
> 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 Romain
> > Beauxis
> > Sent: Freitag, 16. Mai 2025 05:40
> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make it
> a
> > Killer-Feature!
> >
> > Le jeu. 15 mai 2025 à 19:54, Michael Niedermayer <michael@niedermayer.cc> a
> > écrit :
> > >
> > > Hi
> > >
> > > On Fri, May 16, 2025 at 12:17:14AM +0000, 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!
> > > > > >>


[...]


> > > Not speaking as a TC member here but IMHO
> > >
> > > 1. lets all calm down, so far we have a civil and productive discussion
> > >    maybe we can simply find a solution everyone is happy with!
> > >
> > > 2. all security issues must be fixed if there are some
> > >
> > > 3. there should be a configure flag to enable/disable the browser opening
> > feature
> > >    if it remains
> > >
> > > 4. can system() be replaced by fork()+exec*() ? is that something people
> > would
> > >    prefer ?
> >
> > Forked processes inherit opened file descriptors unless explicitly marked
> > as cloexec, this is another big can of worms..
> >
> > > 5. this is a cool feature, i would use this if its available, that said
> > >    if i had to manually open a browser with a given URL that would work
> > >    for me too.
> >
> > At the very least there should be a prompt asking the user to confirm
> > what's about to happen.
> 
> I think that's a very good and agreeable point in general, independent
> from everything else. I assume you mean at first-time use? Or first time
> per day/month maybe?
> 
> 
> 
> >
> > A user-friend way and also secure way could be to output something that is
> > readily usage, for instance:
> > bash -c `ffmpeg <options>`
> 
> I'm not sure I understand the idea. ffmpeg should output another ffmpeg
> command?
> 
> 
> 
> Generally, I think something with a kind of "first-time setup" pattern would
> be great like:
> 
> 
> - ffmpeg -sg
>   - if "first-time use" (=detect whether the "setup" has been made)
>     => don't execute actual cmd but show message explaining what -sg does, and
>                   offer to set it up for the user (in whichever way is needed
> on the platform)  or tell
>                   the user what's needed to be done if very simple
>                   if user chooses "no", it prints a message asking to remove
> the -sg param and exits
>     - otherwise use the existing "setup" to auto-launch the browser
> 
> On the technical side, I don't have a specific way in mind yet, but I think
> that opens up more ways
> how the auto-launch could happen. It would be very explicit for once and also
> more secure
> because it doesn't work out-of-the box without "setup". When FFmpeg can
> perform the setup upon
> user confirmation, then it would still be very convenient.


A possible alternative on the code-side might be SDL_OpenURL(), but I haven't tried 
yet. It would be dependent on SDL2 then, but afaik, that's typically included 
in builds (please correct me if wrong). 
Whether one would find that better, equal or worse, probably depends on everybody's
real motivations and there appear to be different ones, so I'm just mentioning
it to see what people think.

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-request@ffmpeg.org with subject "unsubscribe".

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

[Marton Balint]

On Fri, 16 May 2025, Michael Niedermayer wrote:

> Hi
>
> On Fri, May 16, 2025 at 12:17:14AM +0000, 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.
>>
>>> 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.
>
> Not speaking as a TC member here but IMHO
>
> 1. lets all calm down, so far we have a civil and productive discussion
>   maybe we can simply find a solution everyone is happy with!
>
> 2. all security issues must be fixed if there are some
>
> 3. there should be a configure flag to enable/disable the browser opening feature
>   if it remains

Disagree. We should not clutter our code with this feature, it is out of 
our scope.

>
> 4. can system() be replaced by fork()+exec*() ? is that something people would
>   prefer ?

No.

>
> 5. this is a cool feature, i would use this if its available, that said
>   if i had to manually open a browser with a given URL that would work
>   for me too.

Exactly.

>
> 6. everyone who unconditionally hates this, please try it
>
> 7. as long as the discussion is nice and productive i think the TC is not the
>   best path forward. If things degenerate into some mess then pas it to teh
>   TC, yes
>
> 8. I think one part people should agree on first is if this should be disabled
>   while it is being discussed (maybe a disabled by default configure switch
>   would work?)

Yes. Vast majority of the people dislike this.

Regards,
Marton
_______________________________________________
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".

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

[Rémi Denis-Courmont]

Le perjantaina 16. toukokuuta 2025, 3.54.21 Itä-Euroopan kesäaika Michael 
Niedermayer a écrit :
> 1. lets all calm down, so far we have a civil and productive discussion
>    maybe we can simply find a solution everyone is happy with!
> 
> 2. all security issues must be fixed if there are some

system() is security issue incarnate.

> 3. there should be a configure flag to enable/disable the browser opening
> feature if it remains

I don't see the point in having a configure flag. Either the TC finds the feature 
should not exist at all, and it should not exist at all. Or else, the feature 
can stay as a disabled-by-default *run-time* flag and with the security issues 
fixed.

> 4. can system() be replaced by fork()+exec*() ? is that something people
> would prefer ?

That is obviously far more adequate, though certain precautions are 
potentially necessary such as resetting signal handling.

> 5. this is a cool feature, i would use this if its available, that said
>    if i had to manually open a browser with a given URL that would work
>    for me too.

If the feature outputs at a stable URL, keeping a tab open with auto or manual 
refresh would work just as well, indeed, without the controversial feature.

> 7. as long as the discussion is nice and productive i think the TC is not
> the best path forward. If things degenerate into some mess then pas it to
> teh TC, yes

It's been 6 days, and I don't see how to resolve this other than with a TC 
decision.

-- 
德尼-库尔蒙‧雷米
Hagalund ny stad, f.d. Finska republik Nylands



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

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Rémi Denis-
> Courmont
> Sent: Samstag, 24. Mai 2025 18:02
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make it a
> Killer-Feature!
> 
> Le perjantaina 16. toukokuuta 2025, 3.54.21 Itä-Euroopan kesäaika Michael
> Niedermayer a écrit :
> > 1. lets all calm down, so far we have a civil and productive discussion
> >    maybe we can simply find a solution everyone is happy with!
> >
> > 2. all security issues must be fixed if there are some
> 
> system() is security issue incarnate.
> 
> > 3. there should be a configure flag to enable/disable the browser opening
> > feature if it remains
> 
> I don't see the point in having a configure flag. Either the TC finds the
> feature
> should not exist at all, and it should not exist at all. Or else, the feature
> can stay as a disabled-by-default *run-time* flag and with the security issues
> fixed.
> 
> > 4. can system() be replaced by fork()+exec*() ? is that something people
> > would prefer ?
> 
> That is obviously far more adequate, though certain precautions are
> potentially necessary such as resetting signal handling.

Which is what system() does already.

The funny thing is, that if I would have copied glibc's implementation of
system() - nobody would have objected.


> > 5. this is a cool feature, i would use this if its available, that said
> >    if i had to manually open a browser with a given URL that would work
> >    for me too.
> 
> If the feature outputs at a stable URL, keeping a tab open with auto or manual
> refresh would work just as well, indeed, without the controversial feature.

The idea is that each ffmpeg execution opens a new tab, so that you can 
compare between them.


> > 7. as long as the discussion is nice and productive i think the TC is not
> > the best path forward. If things degenerate into some mess then pas it to
> > teh TC, yes
> 
> It's been 6 days, and I don't see how to resolve this other than with a TC
> decision.

It was reverted on the same day already.

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-request@ffmpeg.org with subject "unsubscribe".

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

[James Almer]

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

> ffmpeg | branch: master | softworkz <softworkz at hotmail.com <https://ffmpeg.org/mailman/listinfo/ffmpeg-cvslog>> | Thu May 15 23:10:02 2025 +0200| [1f2b8d7238eff4ab8a4d8d6177e250b8180d51f4] | committer: softworkz
>
> fftools/graphprint: Now, make it a Killer-Feature!
>
> remember this: -sg   <= means Show Graph
>
> Signed-off-by: softworkz <softworkz at hotmail.com <https://ffmpeg.org/mailman/listinfo/ffmpeg-cvslog>>
>
> >/http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=1f2b8d7238eff4ab8a4d8d6177e250b8180d51f4 
> /---
>
>   doc/ffmpeg.texi              |   4 +
>   fftools/Makefile             |   1 +
>   fftools/ffmpeg.c             |   2 +-
>   fftools/ffmpeg.h             |   1 +
>   fftools/ffmpeg_filter.c      |   2 +-
>   fftools/ffmpeg_opt.c         |   4 +
>   fftools/graph/filelauncher.c | 205 +++++++++++++++++++++++++++++++++++++++++++
>   fftools/graph/graphprint.c   |  48 +++++++++-
>   fftools/graph/graphprint.h   |  32 +++++++
>   9 files changed, 296 insertions(+), 3 deletions(-)
>
> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> index 35675b5309..4bcb6d6a01 100644
> --- a/doc/ffmpeg.texi
> +++ b/doc/ffmpeg.texi
> @@ -1404,6 +1404,10 @@ Writes execution graph details to the specified file in the format set via -prin
>   Sets the output format (available formats are: default, compact, csv, flat, ini, json, xml, mermaid, mermaidhtml)
>   The default format is json.
>   
> + at item <https://ffmpeg.org/mailman/listinfo/ffmpeg-cvslog> -sg (@emph{global})
> +Writes the execution graph to a temporary html file (mermaidhtml format) and
> +tries to launch it in the default browser.
> +
>   @item -progress @var{url} (@emph{global})
>   Send program-friendly progress information to @var{url}.
>   
> diff --git a/fftools/Makefile b/fftools/Makefile
> index 361a4fd574..56a2910212 100644
> --- a/fftools/Makefile
> +++ b/fftools/Makefile
> @@ -22,6 +22,7 @@ OBJS-ffmpeg +=                  \
>       fftools/ffmpeg_opt.o        \
>       fftools/ffmpeg_sched.o      \
>       fftools/graph/graphprint.o        \
> +    fftools/graph/filelauncher.o      \
>       fftools/sync_queue.o        \
>       fftools/thread_queue.o      \
>       fftools/textformat/avtextformat.o \
> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> index 964770df23..6513e2129e 100644
> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -309,7 +309,7 @@ const AVIOInterruptCB int_cb = { decode_interrupt_cb, NULL };
>   
>   static void ffmpeg_cleanup(int ret)
>   {
> -    if (print_graphs || print_graphs_file)
> +    if (print_graphs || print_graphs_file || show_graph)
>           print_filtergraphs(filtergraphs, nb_filtergraphs, input_files, nb_input_files, output_files, nb_output_files);
>   
>       if (do_benchmark) {
> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> index 7fbf0ad532..49fea0307d 100644
> --- a/fftools/ffmpeg.h
> +++ b/fftools/ffmpeg.h
> @@ -721,6 +721,7 @@ extern int print_graphs;
>   extern char *print_graphs_file;
>   extern char *print_graphs_format;
>   extern int auto_conversion_filters;
> +extern int show_graph;
>   
>   extern const AVIOInterruptCB int_cb;
>   
> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> index b774606562..e82e333b7f 100644
> --- a/fftools/ffmpeg_filter.c
> +++ b/fftools/ffmpeg_filter.c
> @@ -2985,7 +2985,7 @@ read_frames:
>   
>   finish:
>   
> -    if (print_graphs || print_graphs_file)
> +    if (print_graphs || print_graphs_file || show_graph)
>           print_filtergraph(fg, fgt.graph);
>   
>       // EOF is normal termination
> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> index 3d1efe32f9..24713d640f 100644
> --- a/fftools/ffmpeg_opt.c
> +++ b/fftools/ffmpeg_opt.c
> @@ -79,6 +79,7 @@ int vstats_version = 2;
>   int print_graphs = 0;
>   char *print_graphs_file = NULL;
>   char *print_graphs_format = NULL;
> +int show_graph = 0;
>   int auto_conversion_filters = 1;
>   int64_t stats_period = 500000;
>   
> @@ -1748,6 +1749,9 @@ const OptionDef options[] = {
>       { "print_graphs_format", OPT_TYPE_STRING, 0,
>           { &print_graphs_format },
>         "set the output printing format (available formats are: default, compact, csv, flat, ini, json, xml, mermaid, mermaidhtml)", "format" },
> +    { "sg",   OPT_TYPE_BOOL, 0,
> +        { &show_graph },
> +        "create execution graph as temporary html file and try to launch it in the default browser" },

Absolutely not, wtf. Calling an external application like this?

Revert this patch or remove this effect immediately.


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

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of James Almer
> Sent: Donnerstag, 15. Mai 2025 23:53
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make it a
> Killer-Feature!
> 
> > ffmpeg | branch: master | softworkz <softworkz at hotmail.com
> <https://ffmpeg.org/mailman/listinfo/ffmpeg-cvslog>> | Thu May 15 23:10:02
> 2025 +0200| [1f2b8d7238eff4ab8a4d8d6177e250b8180d51f4] | committer: softworkz
> >
> > fftools/graphprint: Now, make it a Killer-Feature!
> >
> > remember this: -sg   <= means Show Graph
> >
> > Signed-off-by: softworkz <softworkz at hotmail.com
> <https://ffmpeg.org/mailman/listinfo/ffmpeg-cvslog>>
> >
> >
> >/http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=1f2b8d7238eff4ab8a
> 4d8d6177e250b8180d51f4
> > /---
> >
> >   doc/ffmpeg.texi              |   4 +
> >   fftools/Makefile             |   1 +
> >   fftools/ffmpeg.c             |   2 +-
> >   fftools/ffmpeg.h             |   1 +
> >   fftools/ffmpeg_filter.c      |   2 +-
> >   fftools/ffmpeg_opt.c         |   4 +
> >   fftools/graph/filelauncher.c | 205
> +++++++++++++++++++++++++++++++++++++++++++
> >   fftools/graph/graphprint.c   |  48 +++++++++-
> >   fftools/graph/graphprint.h   |  32 +++++++
> >   9 files changed, 296 insertions(+), 3 deletions(-)
> >
> > diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> > index 35675b5309..4bcb6d6a01 100644
> > --- a/doc/ffmpeg.texi
> > +++ b/doc/ffmpeg.texi
> > @@ -1404,6 +1404,10 @@ Writes execution graph details to the specified file
> in the format set via -prin
> >   Sets the output format (available formats are: default, compact, csv,
> flat, ini, json, xml, mermaid, mermaidhtml)
> >   The default format is json.
> >
> > + at item <https://ffmpeg.org/mailman/listinfo/ffmpeg-cvslog> -sg
> (@emph{global})
> > +Writes the execution graph to a temporary html file (mermaidhtml format)
> and
> > +tries to launch it in the default browser.
> > +
> >   @item -progress @var{url} (@emph{global})
> >   Send program-friendly progress information to @var{url}.
> >
> > diff --git a/fftools/Makefile b/fftools/Makefile
> > index 361a4fd574..56a2910212 100644
> > --- a/fftools/Makefile
> > +++ b/fftools/Makefile
> > @@ -22,6 +22,7 @@ OBJS-ffmpeg +=                  \
> >       fftools/ffmpeg_opt.o        \
> >       fftools/ffmpeg_sched.o      \
> >       fftools/graph/graphprint.o        \
> > +    fftools/graph/filelauncher.o      \
> >       fftools/sync_queue.o        \
> >       fftools/thread_queue.o      \
> >       fftools/textformat/avtextformat.o \
> > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> > index 964770df23..6513e2129e 100644
> > --- a/fftools/ffmpeg.c
> > +++ b/fftools/ffmpeg.c
> > @@ -309,7 +309,7 @@ const AVIOInterruptCB int_cb = { decode_interrupt_cb,
> NULL };
> >
> >   static void ffmpeg_cleanup(int ret)
> >   {
> > -    if (print_graphs || print_graphs_file)
> > +    if (print_graphs || print_graphs_file || show_graph)
> >           print_filtergraphs(filtergraphs, nb_filtergraphs, input_files,
> nb_input_files, output_files, nb_output_files);
> >
> >       if (do_benchmark) {
> > diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> > index 7fbf0ad532..49fea0307d 100644
> > --- a/fftools/ffmpeg.h
> > +++ b/fftools/ffmpeg.h
> > @@ -721,6 +721,7 @@ extern int print_graphs;
> >   extern char *print_graphs_file;
> >   extern char *print_graphs_format;
> >   extern int auto_conversion_filters;
> > +extern int show_graph;
> >
> >   extern const AVIOInterruptCB int_cb;
> >
> > diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> > index b774606562..e82e333b7f 100644
> > --- a/fftools/ffmpeg_filter.c
> > +++ b/fftools/ffmpeg_filter.c
> > @@ -2985,7 +2985,7 @@ read_frames:
> >
> >   finish:
> >
> > -    if (print_graphs || print_graphs_file)
> > +    if (print_graphs || print_graphs_file || show_graph)
> >           print_filtergraph(fg, fgt.graph);
> >
> >       // EOF is normal termination
> > diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> > index 3d1efe32f9..24713d640f 100644
> > --- a/fftools/ffmpeg_opt.c
> > +++ b/fftools/ffmpeg_opt.c
> > @@ -79,6 +79,7 @@ int vstats_version = 2;
> >   int print_graphs = 0;
> >   char *print_graphs_file = NULL;
> >   char *print_graphs_format = NULL;
> > +int show_graph = 0;
> >   int auto_conversion_filters = 1;
> >   int64_t stats_period = 500000;
> >
> > @@ -1748,6 +1749,9 @@ const OptionDef options[] = {
> >       { "print_graphs_format", OPT_TYPE_STRING, 0,
> >           { &print_graphs_format },
> >         "set the output printing format (available formats are: default,
> compact, csv, flat, ini, json, xml, mermaid, mermaidhtml)", "format" },
> > +    { "sg",   OPT_TYPE_BOOL, 0,
> > +        { &show_graph },
> > +        "create execution graph as temporary html file and try to launch it
> in the default browser" },
> 
> Absolutely not, wtf. Calling an external application like this?
> 
> Revert this patch or remove this effect immediately.

15 versions have been posted, I have sent 3 messages asking for comments
before applying over the past 2 weeks.

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-request@ffmpeg.org with subject "unsubscribe".

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

[James Almer]

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

On 5/15/2025 6:58 PM, softworkz . wrote:
> 
> 
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of James Almer
>> Sent: Donnerstag, 15. Mai 2025 23:53
>> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
>> Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make it a
>> Killer-Feature!
>>
>>> ffmpeg | branch: master | softworkz <softworkz at hotmail.com
>> <https://ffmpeg.org/mailman/listinfo/ffmpeg-cvslog>> | Thu May 15 23:10:02
>> 2025 +0200| [1f2b8d7238eff4ab8a4d8d6177e250b8180d51f4] | committer: softworkz
>>>
>>> fftools/graphprint: Now, make it a Killer-Feature!
>>>
>>> remember this: -sg   <= means Show Graph
>>>
>>> Signed-off-by: softworkz <softworkz at hotmail.com
>> <https://ffmpeg.org/mailman/listinfo/ffmpeg-cvslog>>
>>>
>>>
>>> /http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=1f2b8d7238eff4ab8a
>> 4d8d6177e250b8180d51f4
>>> /---
>>>
>>>    doc/ffmpeg.texi              |   4 +
>>>    fftools/Makefile             |   1 +
>>>    fftools/ffmpeg.c             |   2 +-
>>>    fftools/ffmpeg.h             |   1 +
>>>    fftools/ffmpeg_filter.c      |   2 +-
>>>    fftools/ffmpeg_opt.c         |   4 +
>>>    fftools/graph/filelauncher.c | 205
>> +++++++++++++++++++++++++++++++++++++++++++
>>>    fftools/graph/graphprint.c   |  48 +++++++++-
>>>    fftools/graph/graphprint.h   |  32 +++++++
>>>    9 files changed, 296 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
>>> index 35675b5309..4bcb6d6a01 100644
>>> --- a/doc/ffmpeg.texi
>>> +++ b/doc/ffmpeg.texi
>>> @@ -1404,6 +1404,10 @@ Writes execution graph details to the specified file
>> in the format set via -prin
>>>    Sets the output format (available formats are: default, compact, csv,
>> flat, ini, json, xml, mermaid, mermaidhtml)
>>>    The default format is json.
>>>
>>> + at item <https://ffmpeg.org/mailman/listinfo/ffmpeg-cvslog> -sg
>> (@emph{global})
>>> +Writes the execution graph to a temporary html file (mermaidhtml format)
>> and
>>> +tries to launch it in the default browser.
>>> +
>>>    @item -progress @var{url} (@emph{global})
>>>    Send program-friendly progress information to @var{url}.
>>>
>>> diff --git a/fftools/Makefile b/fftools/Makefile
>>> index 361a4fd574..56a2910212 100644
>>> --- a/fftools/Makefile
>>> +++ b/fftools/Makefile
>>> @@ -22,6 +22,7 @@ OBJS-ffmpeg +=                  \
>>>        fftools/ffmpeg_opt.o        \
>>>        fftools/ffmpeg_sched.o      \
>>>        fftools/graph/graphprint.o        \
>>> +    fftools/graph/filelauncher.o      \
>>>        fftools/sync_queue.o        \
>>>        fftools/thread_queue.o      \
>>>        fftools/textformat/avtextformat.o \
>>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
>>> index 964770df23..6513e2129e 100644
>>> --- a/fftools/ffmpeg.c
>>> +++ b/fftools/ffmpeg.c
>>> @@ -309,7 +309,7 @@ const AVIOInterruptCB int_cb = { decode_interrupt_cb,
>> NULL };
>>>
>>>    static void ffmpeg_cleanup(int ret)
>>>    {
>>> -    if (print_graphs || print_graphs_file)
>>> +    if (print_graphs || print_graphs_file || show_graph)
>>>            print_filtergraphs(filtergraphs, nb_filtergraphs, input_files,
>> nb_input_files, output_files, nb_output_files);
>>>
>>>        if (do_benchmark) {
>>> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
>>> index 7fbf0ad532..49fea0307d 100644
>>> --- a/fftools/ffmpeg.h
>>> +++ b/fftools/ffmpeg.h
>>> @@ -721,6 +721,7 @@ extern int print_graphs;
>>>    extern char *print_graphs_file;
>>>    extern char *print_graphs_format;
>>>    extern int auto_conversion_filters;
>>> +extern int show_graph;
>>>
>>>    extern const AVIOInterruptCB int_cb;
>>>
>>> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
>>> index b774606562..e82e333b7f 100644
>>> --- a/fftools/ffmpeg_filter.c
>>> +++ b/fftools/ffmpeg_filter.c
>>> @@ -2985,7 +2985,7 @@ read_frames:
>>>
>>>    finish:
>>>
>>> -    if (print_graphs || print_graphs_file)
>>> +    if (print_graphs || print_graphs_file || show_graph)
>>>            print_filtergraph(fg, fgt.graph);
>>>
>>>        // EOF is normal termination
>>> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
>>> index 3d1efe32f9..24713d640f 100644
>>> --- a/fftools/ffmpeg_opt.c
>>> +++ b/fftools/ffmpeg_opt.c
>>> @@ -79,6 +79,7 @@ int vstats_version = 2;
>>>    int print_graphs = 0;
>>>    char *print_graphs_file = NULL;
>>>    char *print_graphs_format = NULL;
>>> +int show_graph = 0;
>>>    int auto_conversion_filters = 1;
>>>    int64_t stats_period = 500000;
>>>
>>> @@ -1748,6 +1749,9 @@ const OptionDef options[] = {
>>>        { "print_graphs_format", OPT_TYPE_STRING, 0,
>>>            { &print_graphs_format },
>>>          "set the output printing format (available formats are: default,
>> compact, csv, flat, ini, json, xml, mermaid, mermaidhtml)", "format" },
>>> +    { "sg",   OPT_TYPE_BOOL, 0,
>>> +        { &show_graph },
>>> +        "create execution graph as temporary html file and try to launch it
>> in the default browser" },
>>
>> Absolutely not, wtf. Calling an external application like this?
>>
>> Revert this patch or remove this effect immediately.
> 
> 15 versions have been posted, I have sent 3 messages asking for comments
> before applying over the past 2 weeks.
> 
> sw

And there are still unresolved comments you didn't take into account 
before pushing this set.

This specific change is not acceptable, so it needs to be reverted.


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

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of James Almer
> Sent: Freitag, 16. Mai 2025 00:00
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make it a
> Killer-Feature!
> 
> On 5/15/2025 6:58 PM, softworkz . wrote:
> >
> >
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of James
> Almer
> >> Sent: Donnerstag, 15. Mai 2025 23:53
> >> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> >> Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make
> it a
> >> Killer-Feature!
> >>
> >>> ffmpeg | branch: master | softworkz <softworkz at hotmail.com
> >> <https://ffmpeg.org/mailman/listinfo/ffmpeg-cvslog>> | Thu May 15 23:10:02
> >> 2025 +0200| [1f2b8d7238eff4ab8a4d8d6177e250b8180d51f4] | committer:
> softworkz
> >>>
> >>> fftools/graphprint: Now, make it a Killer-Feature!
> >>>
> >>> remember this: -sg   <= means Show Graph
> >>>
> >>> Signed-off-by: softworkz <softworkz at hotmail.com
> >> <https://ffmpeg.org/mailman/listinfo/ffmpeg-cvslog>>
> >>>
> >>>
> >>>
> /http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=1f2b8d7238eff4ab8a
> >> 4d8d6177e250b8180d51f4
> >>> /---
> >>>
> >>>    doc/ffmpeg.texi              |   4 +
> >>>    fftools/Makefile             |   1 +
> >>>    fftools/ffmpeg.c             |   2 +-
> >>>    fftools/ffmpeg.h             |   1 +
> >>>    fftools/ffmpeg_filter.c      |   2 +-
> >>>    fftools/ffmpeg_opt.c         |   4 +
> >>>    fftools/graph/filelauncher.c | 205
> >> +++++++++++++++++++++++++++++++++++++++++++
> >>>    fftools/graph/graphprint.c   |  48 +++++++++-
> >>>    fftools/graph/graphprint.h   |  32 +++++++
> >>>    9 files changed, 296 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> >>> index 35675b5309..4bcb6d6a01 100644
> >>> --- a/doc/ffmpeg.texi
> >>> +++ b/doc/ffmpeg.texi
> >>> @@ -1404,6 +1404,10 @@ Writes execution graph details to the specified
> file
> >> in the format set via -prin
> >>>    Sets the output format (available formats are: default, compact, csv,
> >> flat, ini, json, xml, mermaid, mermaidhtml)
> >>>    The default format is json.
> >>>
> >>> + at item <https://ffmpeg.org/mailman/listinfo/ffmpeg-cvslog> -sg
> >> (@emph{global})
> >>> +Writes the execution graph to a temporary html file (mermaidhtml format)
> >> and
> >>> +tries to launch it in the default browser.
> >>> +
> >>>    @item -progress @var{url} (@emph{global})
> >>>    Send program-friendly progress information to @var{url}.
> >>>
> >>> diff --git a/fftools/Makefile b/fftools/Makefile
> >>> index 361a4fd574..56a2910212 100644
> >>> --- a/fftools/Makefile
> >>> +++ b/fftools/Makefile
> >>> @@ -22,6 +22,7 @@ OBJS-ffmpeg +=                  \
> >>>        fftools/ffmpeg_opt.o        \
> >>>        fftools/ffmpeg_sched.o      \
> >>>        fftools/graph/graphprint.o        \
> >>> +    fftools/graph/filelauncher.o      \
> >>>        fftools/sync_queue.o        \
> >>>        fftools/thread_queue.o      \
> >>>        fftools/textformat/avtextformat.o \
> >>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> >>> index 964770df23..6513e2129e 100644
> >>> --- a/fftools/ffmpeg.c
> >>> +++ b/fftools/ffmpeg.c
> >>> @@ -309,7 +309,7 @@ const AVIOInterruptCB int_cb = { decode_interrupt_cb,
> >> NULL };
> >>>
> >>>    static void ffmpeg_cleanup(int ret)
> >>>    {
> >>> -    if (print_graphs || print_graphs_file)
> >>> +    if (print_graphs || print_graphs_file || show_graph)
> >>>            print_filtergraphs(filtergraphs, nb_filtergraphs, input_files,
> >> nb_input_files, output_files, nb_output_files);
> >>>
> >>>        if (do_benchmark) {
> >>> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> >>> index 7fbf0ad532..49fea0307d 100644
> >>> --- a/fftools/ffmpeg.h
> >>> +++ b/fftools/ffmpeg.h
> >>> @@ -721,6 +721,7 @@ extern int print_graphs;
> >>>    extern char *print_graphs_file;
> >>>    extern char *print_graphs_format;
> >>>    extern int auto_conversion_filters;
> >>> +extern int show_graph;
> >>>
> >>>    extern const AVIOInterruptCB int_cb;
> >>>
> >>> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> >>> index b774606562..e82e333b7f 100644
> >>> --- a/fftools/ffmpeg_filter.c
> >>> +++ b/fftools/ffmpeg_filter.c
> >>> @@ -2985,7 +2985,7 @@ read_frames:
> >>>
> >>>    finish:
> >>>
> >>> -    if (print_graphs || print_graphs_file)
> >>> +    if (print_graphs || print_graphs_file || show_graph)
> >>>            print_filtergraph(fg, fgt.graph);
> >>>
> >>>        // EOF is normal termination
> >>> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> >>> index 3d1efe32f9..24713d640f 100644
> >>> --- a/fftools/ffmpeg_opt.c
> >>> +++ b/fftools/ffmpeg_opt.c
> >>> @@ -79,6 +79,7 @@ int vstats_version = 2;
> >>>    int print_graphs = 0;
> >>>    char *print_graphs_file = NULL;
> >>>    char *print_graphs_format = NULL;
> >>> +int show_graph = 0;
> >>>    int auto_conversion_filters = 1;
> >>>    int64_t stats_period = 500000;
> >>>
> >>> @@ -1748,6 +1749,9 @@ const OptionDef options[] = {
> >>>        { "print_graphs_format", OPT_TYPE_STRING, 0,
> >>>            { &print_graphs_format },
> >>>          "set the output printing format (available formats are: default,
> >> compact, csv, flat, ini, json, xml, mermaid, mermaidhtml)", "format" },
> >>> +    { "sg",   OPT_TYPE_BOOL, 0,
> >>> +        { &show_graph },
> >>> +        "create execution graph as temporary html file and try to launch
> it
> >> in the default browser" },
> >>
> >> Absolutely not, wtf. Calling an external application like this?
> >>
> >> Revert this patch or remove this effect immediately.
> >
> > 15 versions have been posted, I have sent 3 messages asking for comments
> > before applying over the past 2 weeks.
> >
> > sw
> 
> And there are still unresolved comments you didn't take into account
> before pushing this set.

No there aren't. But there's follow-up work for which I already have another
patchset ready.

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-request@ffmpeg.org with subject "unsubscribe".

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of James Almer
> Sent: Freitag, 16. Mai 2025 00:00
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make it a
> Killer-Feature!
> 
> On 5/15/2025 6:58 PM, softworkz . wrote:
> >
> >
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of James
> Almer
> >> Sent: Donnerstag, 15. Mai 2025 23:53
> >> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> >> Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make
> it a
> >> Killer-Feature!
> >>
> >>> ffmpeg | branch: master | softworkz <softworkz at hotmail.com
> >> <https://ffmpeg.org/mailman/listinfo/ffmpeg-cvslog>> | Thu May 15 23:10:02
> >> 2025 +0200| [1f2b8d7238eff4ab8a4d8d6177e250b8180d51f4] | committer:
> softworkz
> >>>
> >>> fftools/graphprint: Now, make it a Killer-Feature!
> >>>
> >>> remember this: -sg   <= means Show Graph
> >>>
> >>> Signed-off-by: softworkz <softworkz at hotmail.com
> >> <https://ffmpeg.org/mailman/listinfo/ffmpeg-cvslog>>
> >>>
> >>>
> >>>
> /http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=1f2b8d7238eff4ab8a
> >> 4d8d6177e250b8180d51f4
> >>> /---
> >>>
> >>>    doc/ffmpeg.texi              |   4 +
> >>>    fftools/Makefile             |   1 +
> >>>    fftools/ffmpeg.c             |   2 +-
> >>>    fftools/ffmpeg.h             |   1 +
> >>>    fftools/ffmpeg_filter.c      |   2 +-
> >>>    fftools/ffmpeg_opt.c         |   4 +
> >>>    fftools/graph/filelauncher.c | 205
> >> +++++++++++++++++++++++++++++++++++++++++++
> >>>    fftools/graph/graphprint.c   |  48 +++++++++-
> >>>    fftools/graph/graphprint.h   |  32 +++++++
> >>>    9 files changed, 296 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> >>> index 35675b5309..4bcb6d6a01 100644
> >>> --- a/doc/ffmpeg.texi
> >>> +++ b/doc/ffmpeg.texi
> >>> @@ -1404,6 +1404,10 @@ Writes execution graph details to the specified
> file
> >> in the format set via -prin
> >>>    Sets the output format (available formats are: default, compact, csv,
> >> flat, ini, json, xml, mermaid, mermaidhtml)
> >>>    The default format is json.
> >>>
> >>> + at item <https://ffmpeg.org/mailman/listinfo/ffmpeg-cvslog> -sg
> >> (@emph{global})
> >>> +Writes the execution graph to a temporary html file (mermaidhtml format)
> >> and
> >>> +tries to launch it in the default browser.
> >>> +
> >>>    @item -progress @var{url} (@emph{global})
> >>>    Send program-friendly progress information to @var{url}.
> >>>
> >>> diff --git a/fftools/Makefile b/fftools/Makefile
> >>> index 361a4fd574..56a2910212 100644
> >>> --- a/fftools/Makefile
> >>> +++ b/fftools/Makefile
> >>> @@ -22,6 +22,7 @@ OBJS-ffmpeg +=                  \
> >>>        fftools/ffmpeg_opt.o        \
> >>>        fftools/ffmpeg_sched.o      \
> >>>        fftools/graph/graphprint.o        \
> >>> +    fftools/graph/filelauncher.o      \
> >>>        fftools/sync_queue.o        \
> >>>        fftools/thread_queue.o      \
> >>>        fftools/textformat/avtextformat.o \
> >>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> >>> index 964770df23..6513e2129e 100644
> >>> --- a/fftools/ffmpeg.c
> >>> +++ b/fftools/ffmpeg.c
> >>> @@ -309,7 +309,7 @@ const AVIOInterruptCB int_cb = { decode_interrupt_cb,
> >> NULL };
> >>>
> >>>    static void ffmpeg_cleanup(int ret)
> >>>    {
> >>> -    if (print_graphs || print_graphs_file)
> >>> +    if (print_graphs || print_graphs_file || show_graph)
> >>>            print_filtergraphs(filtergraphs, nb_filtergraphs, input_files,
> >> nb_input_files, output_files, nb_output_files);
> >>>
> >>>        if (do_benchmark) {
> >>> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> >>> index 7fbf0ad532..49fea0307d 100644
> >>> --- a/fftools/ffmpeg.h
> >>> +++ b/fftools/ffmpeg.h
> >>> @@ -721,6 +721,7 @@ extern int print_graphs;
> >>>    extern char *print_graphs_file;
> >>>    extern char *print_graphs_format;
> >>>    extern int auto_conversion_filters;
> >>> +extern int show_graph;
> >>>
> >>>    extern const AVIOInterruptCB int_cb;
> >>>
> >>> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> >>> index b774606562..e82e333b7f 100644
> >>> --- a/fftools/ffmpeg_filter.c
> >>> +++ b/fftools/ffmpeg_filter.c
> >>> @@ -2985,7 +2985,7 @@ read_frames:
> >>>
> >>>    finish:
> >>>
> >>> -    if (print_graphs || print_graphs_file)
> >>> +    if (print_graphs || print_graphs_file || show_graph)
> >>>            print_filtergraph(fg, fgt.graph);
> >>>
> >>>        // EOF is normal termination
> >>> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> >>> index 3d1efe32f9..24713d640f 100644
> >>> --- a/fftools/ffmpeg_opt.c
> >>> +++ b/fftools/ffmpeg_opt.c
> >>> @@ -79,6 +79,7 @@ int vstats_version = 2;
> >>>    int print_graphs = 0;
> >>>    char *print_graphs_file = NULL;
> >>>    char *print_graphs_format = NULL;
> >>> +int show_graph = 0;
> >>>    int auto_conversion_filters = 1;
> >>>    int64_t stats_period = 500000;
> >>>
> >>> @@ -1748,6 +1749,9 @@ const OptionDef options[] = {
> >>>        { "print_graphs_format", OPT_TYPE_STRING, 0,
> >>>            { &print_graphs_format },
> >>>          "set the output printing format (available formats are: default,
> >> compact, csv, flat, ini, json, xml, mermaid, mermaidhtml)", "format" },
> >>> +    { "sg",   OPT_TYPE_BOOL, 0,
> >>> +        { &show_graph },
> >>> +        "create execution graph as temporary html file and try to launch
> it
> >> in the default browser" },
> >>
> >> Absolutely not, wtf. Calling an external application like this?
> >>
> >> Revert this patch or remove this effect immediately.
> >
> > 15 versions have been posted, I have sent 3 messages asking for comments
> > before applying over the past 2 weeks.
> >
> > sw

Two times you have (incorrectly) claimed this:

FIRST

> And there are still unresolved comments you didn't take into account
> before pushing this set.

SECOND

> You pushed this set before even addressing all reviews.


So, you didn't know the patchset but you want to know that I wouldn't have
addressed all review comments. How can you know that then? 

Guess what? I used to make all the changes mentioned, including those to
which I didn't immediately agree in my responses. And do you know why 
I did that? I did that exactly for the reason that nobody can come later
and make any argument of it - like you did. 
Anyway, it's not on you to decide whether the review of someone else has 
been addressed, especially not when the one is satisfied.

You are very welcome to do your own reviews of my patches, but please 
double-check in the future before making false claims that are 
discrediting me.

Thank you
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-request@ffmpeg.org with subject "unsubscribe".

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of James Almer
> Sent: Freitag, 16. Mai 2025 00:00
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make it a
> Killer-Feature!

> >>
> >> Absolutely not, wtf. Calling an external application like this?
> >>
> >> Revert this patch or remove this effect immediately.
> >
> > 15 versions have been posted, I have sent 3 messages asking for comments
> > before applying over the past 2 weeks.
> >
> > sw
> 
> And there are still unresolved comments you didn't take into account
> before pushing this set.
> 
> This specific change is not acceptable, so it needs to be reverted.


For background and aftermath, please read: The "bad" Patch

https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2025-May/344274.html

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-request@ffmpeg.org with subject "unsubscribe".

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

[Martin Storsjö]

On Thu, 15 May 2025, softworkz wrote:

> ffmpeg | branch: master | softworkz <softworkz@hotmail.com> | Thu May 15 23:10:02 2025 +0200| [1f2b8d7238eff4ab8a4d8d6177e250b8180d51f4] | committer: softworkz
>
> fftools/graphprint: Now, make it a Killer-Feature!
>
> remember this: -sg   <= means Show Graph
>
> Signed-off-by: softworkz <softworkz@hotmail.com>
>
>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=1f2b8d7238eff4ab8a4d8d6177e250b8180d51f4
> ---

This broke compilation in a number of configurations:

Windows UWP/Phone: 
https://fate.ffmpeg.org/report.cgi?slot=arm-msvc2022-winphone&time=20250516053009

filelauncher.o : error LNK2019: unresolved external symbol ShellExecuteA 
referenced in function ff_open_html_in_browser
ffmpeg_g.exe : fatal error LNK1120: 1 unresolved externals

iOS: 
https://fate.ffmpeg.org/report.cgi?slot=aarch64-apple-darwin-ios18&time=20250516011754

src/fftools/graph/filelauncher.c:61:13: error: 'system' is unavailable: 
not available on iOS
    61 |         if (system(cmd) != 0)
       |             ^
/Users/mstorsjo/Xcode_16.2.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS.sdk/usr/include/_stdlib.h:184:6: 
note: 'system' has been explicitly marked unavailable here
   184 | int      system(const char *) __DARWIN_ALIAS_C(system);
       |          ^
1 error generated.

tvOS: 
https://fate.ffmpeg.org/report.cgi?slot=aarch64-apple-darwin-tvos18&time=20250516014542

src/fftools/graph/filelauncher.c:61:13: error: 'system' is unavailable: 
not available on tvOS
    61 |         if (system(cmd) != 0)
       |             ^
/Users/mstorsjo/Xcode_16.2.app/Contents/Developer/Platforms/AppleTVOS.platform/Developer/SDKs/AppleTVOS.sdk/usr/include/_stdlib.h:184:6: 
note: 'system' has been explicitly marked unavailable here
   184 | int      system(const char *) __DARWIN_ALIAS_C(system);
       |          ^
1 error generated.

// Martin

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

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Martin
> Storsjö
> Sent: Freitag, 16. Mai 2025 08:22
> To: 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:
> 
> > ffmpeg | branch: master | softworkz <softworkz@hotmail.com> | Thu May 15
> 23:10:02 2025 +0200| [1f2b8d7238eff4ab8a4d8d6177e250b8180d51f4] | committer:
> softworkz
> >
> > fftools/graphprint: Now, make it a Killer-Feature!
> >
> > remember this: -sg   <= means Show Graph
> >
> > Signed-off-by: softworkz <softworkz@hotmail.com>
> >
> >>
> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=1f2b8d7238eff4ab8a4d
> 8d6177e250b8180d51f4
> > ---
> 
> This broke compilation in a number of configurations:
> 
> Windows UWP/Phone:
> https://fate.ffmpeg.org/report.cgi?slot=arm-msvc2022-
> winphone&time=20250516053009
> 
> filelauncher.o : error LNK2019: unresolved external symbol ShellExecuteA
> referenced in function ff_open_html_in_browser
> ffmpeg_g.exe : fatal error LNK1120: 1 unresolved externals
> 
> iOS:
> https://fate.ffmpeg.org/report.cgi?slot=aarch64-apple-darwin-
> ios18&time=20250516011754
> 
> src/fftools/graph/filelauncher.c:61:13: error: 'system' is unavailable:
> not available on iOS
>     61 |         if (system(cmd) != 0)
>        |             ^
> /Users/mstorsjo/Xcode_16.2.app/Contents/Developer/Platforms/iPhoneOS.platform/
> Developer/SDKs/iPhoneOS.sdk/usr/include/_stdlib.h:184:6:
> note: 'system' has been explicitly marked unavailable here
>    184 | int      system(const char *) __DARWIN_ALIAS_C(system);
>        |          ^
> 1 error generated.
> 
> tvOS:
> https://fate.ffmpeg.org/report.cgi?slot=aarch64-apple-darwin-
> tvos18&time=20250516014542
> 
> src/fftools/graph/filelauncher.c:61:13: error: 'system' is unavailable:
> not available on tvOS
>     61 |         if (system(cmd) != 0)
>        |             ^
> /Users/mstorsjo/Xcode_16.2.app/Contents/Developer/Platforms/AppleTVOS.platform
> /Developer/SDKs/AppleTVOS.sdk/usr/include/_stdlib.h:184:6:
> note: 'system' has been explicitly marked unavailable here
>    184 | int      system(const char *) __DARWIN_ALIAS_C(system);
>        |          ^
> 1 error generated.
> 
> // Martin
> 
> _______________________________________________

Hi Martin,

thanks for the log snippets.

I've setup so many CI builds but obviously still not enough 😊

I'll submit a fix.

Thank you
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-request@ffmpeg.org with subject "unsubscribe".

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Martin
> Storsjö
> Sent: Freitag, 16. Mai 2025 08:22
> To: 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:
> 
> > ffmpeg | branch: master | softworkz <softworkz@hotmail.com> | Thu May 15
> 23:10:02 2025 +0200| [1f2b8d7238eff4ab8a4d8d6177e250b8180d51f4] | committer:
> softworkz
> >
> > fftools/graphprint: Now, make it a Killer-Feature!
> >
> > remember this: -sg   <= means Show Graph
> >
> > Signed-off-by: softworkz <softworkz@hotmail.com>
> >
> >>
> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=1f2b8d7238eff4ab8a4d
> 8d6177e250b8180d51f4
> > ---
> 
> This broke compilation in a number of configurations:
> 
> Windows UWP/Phone:
> https://fate.ffmpeg.org/report.cgi?slot=arm-msvc2022-
> winphone&time=20250516053009
> 
> filelauncher.o : error LNK2019: unresolved external symbol ShellExecuteA
> referenced in function ff_open_html_in_browser
> ffmpeg_g.exe : fatal error LNK1120: 1 unresolved externals
> 
> iOS:
> https://fate.ffmpeg.org/report.cgi?slot=aarch64-apple-darwin-
> ios18&time=20250516011754
> 
> src/fftools/graph/filelauncher.c:61:13: error: 'system' is unavailable:
> not available on iOS
>     61 |         if (system(cmd) != 0)
>        |             ^
> /Users/mstorsjo/Xcode_16.2.app/Contents/Developer/Platforms/iPhoneOS.platform/
> Developer/SDKs/iPhoneOS.sdk/usr/include/_stdlib.h:184:6:
> note: 'system' has been explicitly marked unavailable here
>    184 | int      system(const char *) __DARWIN_ALIAS_C(system);
>        |          ^
> 1 error generated.
> 
> tvOS:
> https://fate.ffmpeg.org/report.cgi?slot=aarch64-apple-darwin-
> tvos18&time=20250516014542
> 
> src/fftools/graph/filelauncher.c:61:13: error: 'system' is unavailable:
> not available on tvOS
>     61 |         if (system(cmd) != 0)
>        |             ^
> /Users/mstorsjo/Xcode_16.2.app/Contents/Developer/Platforms/AppleTVOS.platform
> /Developer/SDKs/AppleTVOS.sdk/usr/include/_stdlib.h:184:6:
> note: 'system' has been explicitly marked unavailable here
>    184 | int      system(const char *) __DARWIN_ALIAS_C(system);
>        |          ^
> 1 error generated.
> 
> // Martin
> 

Hi Martin,

do you think this is a reasonable condition for Apple in configure?

        enable browser_launch
        if test -n "$sysroot"; then
            case "$sysroot" in
                */iPhone*.sdk|\
                */AppleTV*.sdk|\
                */WatchOS*.sdk|\
                */BridgeOS*.sdk)
                    disable browser_launch
                    ;;
            esac
        fi

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-request@ffmpeg.org with subject "unsubscribe".

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

[Gyan Doshi]

On 2025-05-16 01:20 pm, softworkz . wrote:
>
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Martin
>> Storsjö
>> Sent: Freitag, 16. Mai 2025 08:22
>> To: 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:
>>
>>> ffmpeg | branch: master | softworkz <softworkz@hotmail.com> | Thu May 15
>> 23:10:02 2025 +0200| [1f2b8d7238eff4ab8a4d8d6177e250b8180d51f4] | committer:
>> softworkz
>>> fftools/graphprint: Now, make it a Killer-Feature!
>>>
>>> remember this: -sg   <= means Show Graph
>>>
>>> Signed-off-by: softworkz <softworkz@hotmail.com>
>>>
>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=1f2b8d7238eff4ab8a4d
>> 8d6177e250b8180d51f4
>>> ---
>> This broke compilation in a number of configurations:
>>
>> Windows UWP/Phone:
>> https://fate.ffmpeg.org/report.cgi?slot=arm-msvc2022-
>> winphone&time=20250516053009
>>
>> filelauncher.o : error LNK2019: unresolved external symbol ShellExecuteA
>> referenced in function ff_open_html_in_browser
>> ffmpeg_g.exe : fatal error LNK1120: 1 unresolved externals
>>
>> iOS:
>> https://fate.ffmpeg.org/report.cgi?slot=aarch64-apple-darwin-
>> ios18&time=20250516011754
>>
>> src/fftools/graph/filelauncher.c:61:13: error: 'system' is unavailable:
>> not available on iOS
>>      61 |         if (system(cmd) != 0)
>>         |             ^
>> /Users/mstorsjo/Xcode_16.2.app/Contents/Developer/Platforms/iPhoneOS.platform/
>> Developer/SDKs/iPhoneOS.sdk/usr/include/_stdlib.h:184:6:
>> note: 'system' has been explicitly marked unavailable here
>>     184 | int      system(const char *) __DARWIN_ALIAS_C(system);
>>         |          ^
>> 1 error generated.
>>
>> tvOS:
>> https://fate.ffmpeg.org/report.cgi?slot=aarch64-apple-darwin-
>> tvos18&time=20250516014542
>>
>> src/fftools/graph/filelauncher.c:61:13: error: 'system' is unavailable:
>> not available on tvOS
>>      61 |         if (system(cmd) != 0)
>>         |             ^
>> /Users/mstorsjo/Xcode_16.2.app/Contents/Developer/Platforms/AppleTVOS.platform
>> /Developer/SDKs/AppleTVOS.sdk/usr/include/_stdlib.h:184:6:
>> note: 'system' has been explicitly marked unavailable here
>>     184 | int      system(const char *) __DARWIN_ALIAS_C(system);
>>         |          ^
>> 1 error generated.
>>
>> // Martin
>>
> Hi Martin,
>
> do you think this is a reasonable condition for Apple in configure?
>
>          enable browser_launch
>          if test -n "$sysroot"; then
>              case "$sysroot" in
>                  */iPhone*.sdk|\
>                  */AppleTV*.sdk|\
>                  */WatchOS*.sdk|\
>                  */BridgeOS*.sdk)
>                      disable browser_launch
>                      ;;
>              esac
>          fi

Can you add an option to avoid launching browser, and just gen html?

Regards,
Gyan

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

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Gyan Doshi
> Sent: Freitag, 16. Mai 2025 10:14
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make it a
> Killer-Feature!
> 
> 
> 
> On 2025-05-16 01:20 pm, softworkz . wrote:
> >
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Martin
> >> Storsjö
> >> Sent: Freitag, 16. Mai 2025 08:22
> >> To: 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:
> >>
> >>> ffmpeg | branch: master | softworkz <softworkz@hotmail.com> | Thu May 15
> >> 23:10:02 2025 +0200| [1f2b8d7238eff4ab8a4d8d6177e250b8180d51f4] |
> committer:
> >> softworkz
> >>> fftools/graphprint: Now, make it a Killer-Feature!
> >>>
> >>> remember this: -sg   <= means Show Graph
> >>>
> >>> Signed-off-by: softworkz <softworkz@hotmail.com>
> >>>
> >>
> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=1f2b8d7238eff4ab8a4d
> >> 8d6177e250b8180d51f4
> >>> ---
> >> This broke compilation in a number of configurations:
> >>
> >> Windows UWP/Phone:
> >> https://fate.ffmpeg.org/report.cgi?slot=arm-msvc2022-
> >> winphone&time=20250516053009
> >>
> >> filelauncher.o : error LNK2019: unresolved external symbol ShellExecuteA
> >> referenced in function ff_open_html_in_browser
> >> ffmpeg_g.exe : fatal error LNK1120: 1 unresolved externals
> >>
> >> iOS:
> >> https://fate.ffmpeg.org/report.cgi?slot=aarch64-apple-darwin-
> >> ios18&time=20250516011754
> >>
> >> src/fftools/graph/filelauncher.c:61:13: error: 'system' is unavailable:
> >> not available on iOS
> >>      61 |         if (system(cmd) != 0)
> >>         |             ^
> >>
> /Users/mstorsjo/Xcode_16.2.app/Contents/Developer/Platforms/iPhoneOS.platform/
> >> Developer/SDKs/iPhoneOS.sdk/usr/include/_stdlib.h:184:6:
> >> note: 'system' has been explicitly marked unavailable here
> >>     184 | int      system(const char *) __DARWIN_ALIAS_C(system);
> >>         |          ^
> >> 1 error generated.
> >>
> >> tvOS:
> >> https://fate.ffmpeg.org/report.cgi?slot=aarch64-apple-darwin-
> >> tvos18&time=20250516014542
> >>
> >> src/fftools/graph/filelauncher.c:61:13: error: 'system' is unavailable:
> >> not available on tvOS
> >>      61 |         if (system(cmd) != 0)
> >>         |             ^
> >>
> /Users/mstorsjo/Xcode_16.2.app/Contents/Developer/Platforms/AppleTVOS.platform
> >> /Developer/SDKs/AppleTVOS.sdk/usr/include/_stdlib.h:184:6:
> >> note: 'system' has been explicitly marked unavailable here
> >>     184 | int      system(const char *) __DARWIN_ALIAS_C(system);
> >>         |          ^
> >> 1 error generated.
> >>
> >> // Martin
> >>
> > Hi Martin,
> >
> > do you think this is a reasonable condition for Apple in configure?
> >
> >          enable browser_launch
> >          if test -n "$sysroot"; then
> >              case "$sysroot" in
> >                  */iPhone*.sdk|\
> >                  */AppleTV*.sdk|\
> >                  */WatchOS*.sdk|\
> >                  */BridgeOS*.sdk)
> >                      disable browser_launch
> >                      ;;
> >              esac
> >          fi
> 
> Can you add an option to avoid launching browser, and just gen html?

This is always possible:

ffmpeg -print_graphs -print_graphs_format mermaidhtml -print_graphs_file graph.html


or just the mermaid without html around:

ffmpeg -print_graphs -print_graphs_format mermaid -print_graphs_file graph.mermaid


or just print

ffmpeg -print_graphs -print_graphs_format mermaid

then you can copy paste the mermaid output somewhere. It's supported by most 
Markdown environments (incl. GitHub) when pasting as code block with 'mermaid'
as language.



And you can of course get the graph data in all formats that FFprobe supports, e.g:

ffmpeg -print_graphs -print_graphs_format json


Best
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-request@ffmpeg.org with subject "unsubscribe".

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

[Martin Storsjö]

On Fri, 16 May 2025, softworkz . wrote:

>
>
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Martin
>> Storsjö
>> Sent: Freitag, 16. Mai 2025 08:22
>> To: 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:
>>
>>> ffmpeg | branch: master | softworkz <softworkz@hotmail.com> | Thu May 15
>> 23:10:02 2025 +0200| [1f2b8d7238eff4ab8a4d8d6177e250b8180d51f4] | committer:
>> softworkz
>>>
>>> fftools/graphprint: Now, make it a Killer-Feature!
>>>
>>> remember this: -sg   <= means Show Graph
>>>
>>> Signed-off-by: softworkz <softworkz@hotmail.com>
>>>
>>>>
>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=1f2b8d7238eff4ab8a4d
>> 8d6177e250b8180d51f4
>>> ---
>>
>> This broke compilation in a number of configurations:
>>
>> Windows UWP/Phone:
>> https://fate.ffmpeg.org/report.cgi?slot=arm-msvc2022-
>> winphone&time=20250516053009
>>
>> filelauncher.o : error LNK2019: unresolved external symbol ShellExecuteA
>> referenced in function ff_open_html_in_browser
>> ffmpeg_g.exe : fatal error LNK1120: 1 unresolved externals
>>
>> iOS:
>> https://fate.ffmpeg.org/report.cgi?slot=aarch64-apple-darwin-
>> ios18&time=20250516011754
>>
>> src/fftools/graph/filelauncher.c:61:13: error: 'system' is unavailable:
>> not available on iOS
>>     61 |         if (system(cmd) != 0)
>>        |             ^
>> /Users/mstorsjo/Xcode_16.2.app/Contents/Developer/Platforms/iPhoneOS.platform/
>> Developer/SDKs/iPhoneOS.sdk/usr/include/_stdlib.h:184:6:
>> note: 'system' has been explicitly marked unavailable here
>>    184 | int      system(const char *) __DARWIN_ALIAS_C(system);
>>        |          ^
>> 1 error generated.
>>
>> tvOS:
>> https://fate.ffmpeg.org/report.cgi?slot=aarch64-apple-darwin-
>> tvos18&time=20250516014542
>>
>> src/fftools/graph/filelauncher.c:61:13: error: 'system' is unavailable:
>> not available on tvOS
>>     61 |         if (system(cmd) != 0)
>>        |             ^
>> /Users/mstorsjo/Xcode_16.2.app/Contents/Developer/Platforms/AppleTVOS.platform
>> /Developer/SDKs/AppleTVOS.sdk/usr/include/_stdlib.h:184:6:
>> note: 'system' has been explicitly marked unavailable here
>>    184 | int      system(const char *) __DARWIN_ALIAS_C(system);
>>        |          ^
>> 1 error generated.
>>
>> // Martin
>>
>
> Hi Martin,
>
> do you think this is a reasonable condition for Apple in configure?
>
>        enable browser_launch
>        if test -n "$sysroot"; then
>            case "$sysroot" in
>                */iPhone*.sdk|\
>                */AppleTV*.sdk|\
>                */WatchOS*.sdk|\
>                */BridgeOS*.sdk)
>                    disable browser_launch
>                    ;;
>            esac
>        fi

This is not how we normally do it. String matching trying to detect things 
is generally brittle. If the problem is that we can't compile a call to 
system(), the configure check should try to compile a call to system(), 
and if that doesn't succeed, avoid compiling the code that does that. That 
also works for e.g. ShellExecuteA.

That said, I also agree with everybody else that I'd rather not have 
ffmpeg do this at all.

// Martin
_______________________________________________
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".

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Martin
> Storsjö
> Sent: Freitag, 16. Mai 2025 10:19
> 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, 16 May 2025, softworkz . wrote:
> 
> >
> >
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Martin
> >> Storsjö
> >> Sent: Freitag, 16. Mai 2025 08:22
> >> To: 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:
> >>
> >>> ffmpeg | branch: master | softworkz <softworkz@hotmail.com> | Thu May 15
> >> 23:10:02 2025 +0200| [1f2b8d7238eff4ab8a4d8d6177e250b8180d51f4] |
> committer:
> >> softworkz
> >>>
> >>> fftools/graphprint: Now, make it a Killer-Feature!
> >>>
> >>> remember this: -sg   <= means Show Graph
> >>>
> >>> Signed-off-by: softworkz <softworkz@hotmail.com>
> >>>
> >>>>
> >>
> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=1f2b8d7238eff4ab8a4d
> >> 8d6177e250b8180d51f4
> >>> ---
> >>
> >> This broke compilation in a number of configurations:
> >>
> >> Windows UWP/Phone:
> >> https://fate.ffmpeg.org/report.cgi?slot=arm-msvc2022-
> >> winphone&time=20250516053009
> >>
> >> filelauncher.o : error LNK2019: unresolved external symbol ShellExecuteA
> >> referenced in function ff_open_html_in_browser
> >> ffmpeg_g.exe : fatal error LNK1120: 1 unresolved externals
> >>
> >> iOS:
> >> https://fate.ffmpeg.org/report.cgi?slot=aarch64-apple-darwin-
> >> ios18&time=20250516011754
> >>
> >> src/fftools/graph/filelauncher.c:61:13: error: 'system' is unavailable:
> >> not available on iOS
> >>     61 |         if (system(cmd) != 0)
> >>        |             ^
> >>
> /Users/mstorsjo/Xcode_16.2.app/Contents/Developer/Platforms/iPhoneOS.platform/
> >> Developer/SDKs/iPhoneOS.sdk/usr/include/_stdlib.h:184:6:
> >> note: 'system' has been explicitly marked unavailable here
> >>    184 | int      system(const char *) __DARWIN_ALIAS_C(system);
> >>        |          ^
> >> 1 error generated.
> >>
> >> tvOS:
> >> https://fate.ffmpeg.org/report.cgi?slot=aarch64-apple-darwin-
> >> tvos18&time=20250516014542
> >>
> >> src/fftools/graph/filelauncher.c:61:13: error: 'system' is unavailable:
> >> not available on tvOS
> >>     61 |         if (system(cmd) != 0)
> >>        |             ^
> >>
> /Users/mstorsjo/Xcode_16.2.app/Contents/Developer/Platforms/AppleTVOS.platform
> >> /Developer/SDKs/AppleTVOS.sdk/usr/include/_stdlib.h:184:6:
> >> note: 'system' has been explicitly marked unavailable here
> >>    184 | int      system(const char *) __DARWIN_ALIAS_C(system);
> >>        |          ^
> >> 1 error generated.
> >>
> >> // Martin
> >>
> >
> > Hi Martin,
> >
> > do you think this is a reasonable condition for Apple in configure?
> >
> >        enable browser_launch
> >        if test -n "$sysroot"; then
> >            case "$sysroot" in
> >                */iPhone*.sdk|\
> >                */AppleTV*.sdk|\
> >                */WatchOS*.sdk|\
> >                */BridgeOS*.sdk)
> >                    disable browser_launch
> >                    ;;
> >            esac
> >        fi
> 
> This is not how we normally do it. String matching trying to detect things
> is generally brittle. If the problem is that we can't compile a call to
> system(), the configure check should try to compile a call to system(),
> and if that doesn't succeed, avoid compiling the code that does that. That
> also works for e.g. ShellExecuteA.

I'm preparing a patch that aims to limit this to Windows, Linux and Mac
(regardless of API availability) and I'm not sure how to determine whether 
mac or the others above in configure.

Is there a better way?

> That said, I also agree with everybody else that I'd rather not have
> ffmpeg do this at all.
This is meant as a quick fix to unbreak the builds, for the final feature 
behavior, I'm open to discussion as mentioned.

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-request@ffmpeg.org with subject "unsubscribe".

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

[Martin Storsjö]

On Fri, 16 May 2025, softworkz . wrote:

>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Martin
>> Storsjö
>> Sent: Freitag, 16. Mai 2025 10:19
>> 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, 16 May 2025, softworkz . wrote:
>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Martin
>>>> Storsjö
>>>> Sent: Freitag, 16. Mai 2025 08:22
>>>> To: 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:
>>>>
>>>>> ffmpeg | branch: master | softworkz <softworkz@hotmail.com> | Thu May 15
>>>> 23:10:02 2025 +0200| [1f2b8d7238eff4ab8a4d8d6177e250b8180d51f4] |
>> committer:
>>>> softworkz
>>>>>
>>>>> fftools/graphprint: Now, make it a Killer-Feature!
>>>>>
>>>>> remember this: -sg   <= means Show Graph
>>>>>
>>>>> Signed-off-by: softworkz <softworkz@hotmail.com>
>>>>>
>>>>>>
>>>>
>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=1f2b8d7238eff4ab8a4d
>>>> 8d6177e250b8180d51f4
>>>>> ---
>>>>
>>>> This broke compilation in a number of configurations:
>>>>
>>>> Windows UWP/Phone:
>>>> https://fate.ffmpeg.org/report.cgi?slot=arm-msvc2022-
>>>> winphone&time=20250516053009
>>>>
>>>> filelauncher.o : error LNK2019: unresolved external symbol ShellExecuteA
>>>> referenced in function ff_open_html_in_browser
>>>> ffmpeg_g.exe : fatal error LNK1120: 1 unresolved externals
>>>>
>>>> iOS:
>>>> https://fate.ffmpeg.org/report.cgi?slot=aarch64-apple-darwin-
>>>> ios18&time=20250516011754
>>>>
>>>> src/fftools/graph/filelauncher.c:61:13: error: 'system' is unavailable:
>>>> not available on iOS
>>>>     61 |         if (system(cmd) != 0)
>>>>        |             ^
>>>>
>> /Users/mstorsjo/Xcode_16.2.app/Contents/Developer/Platforms/iPhoneOS.platform/
>>>> Developer/SDKs/iPhoneOS.sdk/usr/include/_stdlib.h:184:6:
>>>> note: 'system' has been explicitly marked unavailable here
>>>>    184 | int      system(const char *) __DARWIN_ALIAS_C(system);
>>>>        |          ^
>>>> 1 error generated.
>>>>
>>>> tvOS:
>>>> https://fate.ffmpeg.org/report.cgi?slot=aarch64-apple-darwin-
>>>> tvos18&time=20250516014542
>>>>
>>>> src/fftools/graph/filelauncher.c:61:13: error: 'system' is unavailable:
>>>> not available on tvOS
>>>>     61 |         if (system(cmd) != 0)
>>>>        |             ^
>>>>
>> /Users/mstorsjo/Xcode_16.2.app/Contents/Developer/Platforms/AppleTVOS.platform
>>>> /Developer/SDKs/AppleTVOS.sdk/usr/include/_stdlib.h:184:6:
>>>> note: 'system' has been explicitly marked unavailable here
>>>>    184 | int      system(const char *) __DARWIN_ALIAS_C(system);
>>>>        |          ^
>>>> 1 error generated.
>>>>
>>>> // Martin
>>>>
>>>
>>> Hi Martin,
>>>
>>> do you think this is a reasonable condition for Apple in configure?
>>>
>>>        enable browser_launch
>>>        if test -n "$sysroot"; then
>>>            case "$sysroot" in
>>>                */iPhone*.sdk|\
>>>                */AppleTV*.sdk|\
>>>                */WatchOS*.sdk|\
>>>                */BridgeOS*.sdk)
>>>                    disable browser_launch
>>>                    ;;
>>>            esac
>>>        fi
>>
>> This is not how we normally do it. String matching trying to detect things
>> is generally brittle. If the problem is that we can't compile a call to
>> system(), the configure check should try to compile a call to system(),
>> and if that doesn't succeed, avoid compiling the code that does that. That
>> also works for e.g. ShellExecuteA.
>
> I'm preparing a patch that aims to limit this to Windows, Linux and Mac
> (regardless of API availability) and I'm not sure how to determine whether
> mac or the others above in configure.
>
> Is there a better way?

Test compiling code and checking if the expected things are defined. You 
can include TargetConditionals.h and check if TARGET_OS_OSX evaluates to 
nonzero; that's what we do in code elsewhere anyway - grep for 
TARGET_OS_OSX.

// Martin
_______________________________________________
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".

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Martin
> Storsjö
> Sent: Freitag, 16. Mai 2025 10:51
> 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, 16 May 2025, softworkz . wrote:
> 
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Martin
> >> Storsjö
> >> Sent: Freitag, 16. Mai 2025 10:19
> >> 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, 16 May 2025, softworkz . wrote:
> >>
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Martin
> >>>> Storsjö
> >>>> Sent: Freitag, 16. Mai 2025 08:22
> >>>> To: 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:
> >>>>
> >>>>> ffmpeg | branch: master | softworkz <softworkz@hotmail.com> | Thu May 15
> >>>> 23:10:02 2025 +0200| [1f2b8d7238eff4ab8a4d8d6177e250b8180d51f4] |
> >> committer:
> >>>> softworkz
> >>>>>
> >>>>> fftools/graphprint: Now, make it a Killer-Feature!
> >>>>>
> >>>>> remember this: -sg   <= means Show Graph
> >>>>>
> >>>>> Signed-off-by: softworkz <softworkz@hotmail.com>
> >>>>>
> >>>>>>
> >>>>
> >>
> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=1f2b8d7238eff4ab8a4d
> >>>> 8d6177e250b8180d51f4
> >>>>> ---
> >>>>
> >>>> This broke compilation in a number of configurations:
> >>>>
> >>>> Windows UWP/Phone:
> >>>> https://fate.ffmpeg.org/report.cgi?slot=arm-msvc2022-
> >>>> winphone&time=20250516053009
> >>>>
> >>>> filelauncher.o : error LNK2019: unresolved external symbol ShellExecuteA
> >>>> referenced in function ff_open_html_in_browser
> >>>> ffmpeg_g.exe : fatal error LNK1120: 1 unresolved externals
> >>>>
> >>>> iOS:
> >>>> https://fate.ffmpeg.org/report.cgi?slot=aarch64-apple-darwin-
> >>>> ios18&time=20250516011754
> >>>>
> >>>> src/fftools/graph/filelauncher.c:61:13: error: 'system' is unavailable:
> >>>> not available on iOS
> >>>>     61 |         if (system(cmd) != 0)
> >>>>        |             ^
> >>>>
> >>
> /Users/mstorsjo/Xcode_16.2.app/Contents/Developer/Platforms/iPhoneOS.platform/
> >>>> Developer/SDKs/iPhoneOS.sdk/usr/include/_stdlib.h:184:6:
> >>>> note: 'system' has been explicitly marked unavailable here
> >>>>    184 | int      system(const char *) __DARWIN_ALIAS_C(system);
> >>>>        |          ^
> >>>> 1 error generated.
> >>>>
> >>>> tvOS:
> >>>> https://fate.ffmpeg.org/report.cgi?slot=aarch64-apple-darwin-
> >>>> tvos18&time=20250516014542
> >>>>
> >>>> src/fftools/graph/filelauncher.c:61:13: error: 'system' is unavailable:
> >>>> not available on tvOS
> >>>>     61 |         if (system(cmd) != 0)
> >>>>        |             ^
> >>>>
> >>
> /Users/mstorsjo/Xcode_16.2.app/Contents/Developer/Platforms/AppleTVOS.platform
> >>>> /Developer/SDKs/AppleTVOS.sdk/usr/include/_stdlib.h:184:6:
> >>>> note: 'system' has been explicitly marked unavailable here
> >>>>    184 | int      system(const char *) __DARWIN_ALIAS_C(system);
> >>>>        |          ^
> >>>> 1 error generated.
> >>>>
> >>>> // Martin
> >>>>
> >>>
> >>> Hi Martin,
> >>>
> >>> do you think this is a reasonable condition for Apple in configure?
> >>>
> >>>        enable browser_launch
> >>>        if test -n "$sysroot"; then
> >>>            case "$sysroot" in
> >>>                */iPhone*.sdk|\
> >>>                */AppleTV*.sdk|\
> >>>                */WatchOS*.sdk|\
> >>>                */BridgeOS*.sdk)
> >>>                    disable browser_launch
> >>>                    ;;
> >>>            esac
> >>>        fi
> >>
> >> This is not how we normally do it. String matching trying to detect things
> >> is generally brittle. If the problem is that we can't compile a call to
> >> system(), the configure check should try to compile a call to system(),
> >> and if that doesn't succeed, avoid compiling the code that does that. That
> >> also works for e.g. ShellExecuteA.
> >
> > I'm preparing a patch that aims to limit this to Windows, Linux and Mac
> > (regardless of API availability) and I'm not sure how to determine whether
> > mac or the others above in configure.
> >
> > Is there a better way?
> 
> Test compiling code and checking if the expected things are defined. You
> can include TargetConditionals.h and check if TARGET_OS_OSX evaluates to
> nonzero; that's what we do in code elsewhere anyway - grep for
> TARGET_OS_OSX.

Okay got it. Thanks a lot for your help. Even though it's not needed 
anymore now. The next FATE builds for those platforms should be green 
again.

Thank you
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-request@ffmpeg.org with subject "unsubscribe".