[FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make it a Killer-Feature!
Michael Niedermayer
michael at niedermayer.cc
Fri May 16 03:54:21 EEST 2025
Hi
On Fri, May 16, 2025 at 12:17:14AM +0000, softworkz . wrote:
>
>
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Marton
> > Balint
> > Sent: Freitag, 16. Mai 2025 02:00
> > To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make it a
> > Killer-Feature!
> >
> >
> >
> > On Thu, 15 May 2025, softworkz . wrote:
> >
> > >
> > >
> > >> -----Original Message-----
> > >> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Ramiro
> > Polla
> > >> Sent: Donnerstag, 15. Mai 2025 23:50
> > >> To: ffmpeg-devel at ffmpeg.org
> > >> Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make
> > it a
> > >> Killer-Feature!
> > >>
> > >> Hi,
> > >>
> > >> On Thu, May 15, 2025 at 11:11 PM softworkz <git at videolan.org> wrote:
> > >> [...]
> > >>> diff --git a/fftools/graph/filelauncher.c b/fftools/graph/filelauncher.c
> > >>> new file mode 100644
> > >>> index 0000000000..45514ca599
> > >>> --- /dev/null
> > >>> +++ b/fftools/graph/filelauncher.c
> > >> [...]
> > >>> +int ff_open_html_in_browser(const char *html_path)
> > >>> +{
> > >>> + if (!html_path || !*html_path)
> > >>> + return -1;
> > >>> +
> > >>> +#if defined(_WIN32)
> > >>> +
> > >>> + // --- Windows ---------------------------------
> > >>> + {
> > >>> + HINSTANCE rc = ShellExecuteA(NULL, "open", html_path, NULL, NULL,
> > >> SW_SHOWNORMAL);
> > >>> + if ((UINT_PTR)rc <= 32) {
> > >>> + // Fallback: system("start ...")
> > >>> + char cmd[1024];
> > >>> + _snprintf_s(cmd, sizeof(cmd), _TRUNCATE, "start \"\" \"%s\"",
> > >> html_path);
> > >>> + if (system(cmd) != 0)
> > >>> + return -1;
> > >>> + }
> > >>> + return 0;
> > >>> + }
> > >>> +
> > >>> +#elif defined(__APPLE__)
> > >>> +
> > >>> + // --- macOS -----------------------------------
> > >>> + {
> > >>> + // "open" is the macOS command to open a file/URL with the
> > default
> > >> application
> > >>> + char cmd[1024];
> > >>> + snprintf(cmd, sizeof(cmd), "open '%s' 1>/dev/null 2>&1 &",
> > >> html_path);
> > >>> + if (system(cmd) != 0)
> > >>> + return -1;
> > >>> + return 0;
> > >>> + }
> > >>> +
> > >>> +#else
> > >>> +
> > >>> + // --- Linux / Unix-like -----------------------
> > >>> + // We'll try xdg-open, then gnome-open, then kfmclient
> > >>> + {
> > >>> + // Helper macro to try one browser command
> > >>> + // Returns 0 on success, -1 on failure
> > >>> + #define TRY_CMD(prog) do { \
> > >>> + char buf[1024]; \
> > >>> + snprintf(buf, sizeof(buf), "%s '%s' 1>/dev/null 2>&1 &", \
> > >>> + (prog), html_path); \
> > >>> + int ret = system(buf); \
> > >>> + /* On Unix: system() returns -1 if the shell can't run. */\
> > >>> + /* Otherwise, check exit code in lower 8 bits. */\
> > >>> + if (ret != -1 && WIFEXITED(ret) && WEXITSTATUS(ret) == 0) \
> > >>> + return 0; \
> > >>> + } while (0)
> > >>> +
> > >>> + TRY_CMD("xdg-open");
> > >>> + TRY_CMD("gnome-open");
> > >>> + TRY_CMD("kfmclient exec");
> > >>> +
> > >>> + fprintf(stderr, "Could not open '%s' in a browser.\n",
> > html_path);
> > >>> + return -1;
> > >>> + }
> > >>> +
> > >>> +#endif
> > >>> +}
> > >> [...]
> > >>
> > >> Sorry I didn't have a closer look at the patchset while it was under
> > >> review, but system(cmd) is a big no-no. We could create a file with an
> > >> explicit path passed by the user, but then it's up to the user to open
> > >> it.
> > >
> > > What's bad about opening a file in the browser when that's the documented
> > > behavior of the cli parameter?
> >
> > Because ffmpeg is not a browser opener tool, but a transcoding tool. An
> > argument can be made for every feature you can think of (Why not add an
> > option which shuts down a computer when the transcoding is done? Why not
> > add a playable DOOM implementation so the user will not be bored when
> > waiting for the transcode to finish).
> >
> > Let's just revert this. The many ffmpeg cli frontends can open browsers if
> > they want.
>
> Many good arguments can be found for both sides.
>
> > 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20250516/8ac974f3/attachment.sig>
More information about the ffmpeg-devel
mailing list