From: James Almer <jamrial@gmail.com> To: ffmpeg-devel@ffmpeg.org Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/graphprint: Now, make it a Killer-Feature! Date: Thu, 15 May 2025 21:27:33 -0300 Message-ID: <fb9d50ee-2267-4260-8d97-a0dc6c25ce07@gmail.com> (raw) In-Reply-To: <BN0P223MB0358276CCC06346C2E48718EBA93A@BN0P223MB0358.NAMP223.PROD.OUTLOOK.COM> [-- 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".
next prev parent reply other threads:[~2025-05-16 0:27 UTC|newest] Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <20250515211148.6C91C4128B8@natalya.videolan.org> 2025-05-15 21:50 ` Ramiro Polla 2025-05-15 21:59 ` softworkz . 2025-05-15 22:13 ` Ramiro Polla 2025-05-15 22:19 ` softworkz . 2025-05-15 22:33 ` softworkz . 2025-05-15 22:34 ` Mark Thompson 2025-05-15 22:43 ` softworkz . 2025-05-15 22:49 ` Ramiro Polla 2025-05-15 23:04 ` softworkz . 2025-05-15 23:29 ` Ramiro Polla 2025-05-16 0:19 ` softworkz . 2025-05-15 22:49 ` softworkz . 2025-05-24 15:54 ` Rémi Denis-Courmont 2025-05-25 10:50 ` softworkz . 2025-05-16 0:00 ` Marton Balint 2025-05-16 0:17 ` softworkz . 2025-05-16 0:27 ` James Almer [this message] 2025-05-16 0:32 ` softworkz . 2025-05-16 0:36 ` softworkz . 2025-05-16 0:39 ` James Almer 2025-05-16 0:45 ` Lynne 2025-05-16 0:59 ` softworkz . 2025-05-16 0:54 ` Michael Niedermayer 2025-05-16 1:26 ` softworkz . 2025-05-16 8:43 ` softworkz . 2025-05-16 9:41 ` softworkz . 2025-05-16 9:50 ` Nicolas George 2025-05-16 10:10 ` softworkz . 2025-05-16 11:10 ` Nicolas George 2025-05-16 11:49 ` Michael Niedermayer 2025-05-16 12:03 ` Nicolas George 2025-05-31 21:38 ` softworkz . 2025-05-16 13:42 ` softworkz . 2025-05-16 13:45 ` Nicolas George 2025-05-16 3:39 ` Romain Beauxis 2025-05-16 4:15 ` softworkz . 2025-05-16 5:06 ` softworkz . 2025-05-16 8:11 ` Marton Balint 2025-05-24 16:01 ` Rémi Denis-Courmont 2025-05-25 11:04 ` softworkz . 2025-05-15 21:53 ` James Almer 2025-05-15 21:58 ` softworkz . 2025-05-15 22:00 ` James Almer 2025-05-15 22:02 ` softworkz . 2025-05-16 2:06 ` softworkz . 2025-05-31 21:38 ` softworkz . 2025-05-16 6:22 ` Martin Storsjö 2025-05-16 6:40 ` softworkz . 2025-05-16 7:50 ` softworkz . 2025-05-16 8:13 ` Gyan Doshi 2025-05-16 8:19 ` softworkz . 2025-05-16 8:19 ` Martin Storsjö 2025-05-16 8:25 ` softworkz . 2025-05-16 8:50 ` Martin Storsjö 2025-05-16 8:55 ` softworkz .
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=fb9d50ee-2267-4260-8d97-a0dc6c25ce07@gmail.com \ --to=jamrial@gmail.com \ --cc=ffmpeg-devel@ffmpeg.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel This inbox may be cloned and mirrored by anyone: git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \ ffmpegdev@gitmailbox.com public-inbox-index ffmpegdev Example config snippet for mirrors. AGPL code for this site: git clone https://public-inbox.org/public-inbox.git