> -----Original Message-----
> From: ffmpeg-devel <[email protected]> On Behalf Of Romain
> Beauxis
> Sent: Freitag, 16. Mai 2025 05:40
> To: FFmpeg development discussions and patches <[email protected]>
> Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make it a
> Killer-Feature!
>
> Le jeu. 15 mai 2025 à 19:54, Michael Niedermayer <[email protected]> a
> écrit :
> >
> > Hi
> >
> > On Fri, May 16, 2025 at 12:17:14AM +0000, softworkz . wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: ffmpeg-devel <[email protected]> On Behalf Of
> Marton
> > > > Balint
> > > > Sent: Freitag, 16. Mai 2025 02:00
> > > > To: FFmpeg development discussions and patches <
> [email protected]>
> > > > 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 <[email protected]> On Behalf Of
> Ramiro
> > > > Polla
> > > > >> Sent: Donnerstag, 15. Mai 2025 23:50
> > > > >> To: [email protected]
> > > > >> 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 <[email protected]>
> 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
[email protected]
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
[email protected] with subject "unsubscribe".
