diff mbox series

[bug#40060,v3] gnu: youtube-dl: Add 'ffmpeg' as input.

Message ID 20200401130152.1871-1-brice@waegenei.re
State Accepted
Headers show
Series [bug#40060,v3] gnu: youtube-dl: Add 'ffmpeg' as input. | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch success View Laminar job

Commit Message

Brice Waegeneire April 1, 2020, 1:01 p.m. UTC 
* gnu/packages/video.scm (youtube-dl)[arguments]: Add phase
wrap-executable.
[inputs]: Add ffmpeg.
---

This version is rebased on top of master and the commit adding zsh completion
has been removed from the patch set since it has already been merged.

 gnu/packages/video.scm | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Comments

guix--- via Guix-patches via April 1, 2020, 1:40 p.m. UTC | #1 
Brice,

Sorry for the delay!  However, I don't really know what more to 
say.

I'm still against pulling in ffmpeg for everyone, for the reason 
given in my last[0] mail.  Keep in mind:

  $ guix size youtube-dl | tail -n1
  total: 184.0 MiB

  $ guix size youtube-dl ffmpeg | tail -n1
  total: 804.7 MiB

If youtube-dl silently failed or produced worse video without 
ffmpeg, things'd clearly be different.  Same if it disabled 
features at build time if ffmpeg is not found.  I don't think it 
does…

One could argue that those who really don't need or want ffmpeg 
can simply customise their youtube-dl package because Guix is 
awesome.  Both true!  But it's the asymmetry between opt-in

  $ guix install youtube-dl
  $ youtube-dl https://hotvidz.gnu/10h-of-rms-dancing.oggv
  Note: you'll want to install ffmpeg to see more glorious detail
  $ guix install ffmpeg
  …

and opt-out, where the user has to learn about 
substitute-keyword-arguments & write a custom package just to 
avoid pulling in ffmpeg, that bugs me.

That's just me.  If you convince enough people to merge this I 
certainly won't mind!

Kind regards,

T G-R

[0]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=40060#23
diff mbox series

Patch

diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
index 03796fd770..6ad46fb9fe 100644
--- a/gnu/packages/video.scm
+++ b/gnu/packages/video.scm
@@ -1680,7 +1680,17 @@  To load this plugin, specify the following option when starting mpv:
                         (mkdir-p zsh)
                         (copy-file "youtube-dl.zsh"
                                    (string-append zsh "/_youtube-dl"))
-                        #t))))))
+                        #t)))
+                 (add-after 'install 'wrap-executable
+                    (lambda* (#:key inputs outputs #:allow-other-keys)
+                      (let ((out (assoc-ref outputs "out"))
+                            (ffmpeg (assoc-ref inputs "ffmpeg")))
+                        (wrap-program (string-append out "/bin/youtube-dl")
+                          `("PATH" ":" prefix
+                            ,(list (string-append ffmpeg "/bin")))))
+                      #t)))))
+    (inputs
+     `(("ffmpeg" ,ffmpeg)))
     (synopsis "Download videos from YouTube.com and other sites")
     (description
      "Youtube-dl is a small command-line program to download videos from