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