Message ID | 20200314144221.17112-1-brice@waegenei.re |
---|---|
State | Accepted |
Headers | show |
Series | youtube-dl add ffmpeg, pycryptodome and zsh-completion | expand |
Context | Check | Description |
---|---|---|
cbaines/comparison | success | View comparision |
cbaines/git branch | success | View Git branch |
cbaines/applying patch | success | View Laminar job |
On Sat, Mar 14, 2020 at 03:42:20PM +0100, Brice Waegeneire wrote: > * gnu/packages/video.scm (youtube-dl)[arguments]: Add phase > wrap-executable. > [inputs]: Add ffmpeg. > [propagated-inputs]: Add python-pycryptodome. What do these added dependencies do?
On 2020-03-14 17:21, Leo Famulari wrote: > On Sat, Mar 14, 2020 at 03:42:20PM +0100, Brice Waegeneire wrote: >> * gnu/packages/video.scm (youtube-dl)[arguments]: Add phase >> wrap-executable. >> [inputs]: Add ffmpeg. >> [propagated-inputs]: Add python-pycryptodome. > > What do these added dependencies do? I should have written this in the cover-letter, my bad. pycryptodome is needed for the hlsative downloader, ffmpeg adds the ability to merge video and audio files downloaded separately by youtube-dl and removes the following warning: WARNING: You have requested multiple formats but ffmpeg or avconv are not installed. The formats won't be merged. Have a look in the NixOS derivation for more information on dependencies: https://github.com/NixOS/nixpkgs/blob/86a085cd4690d497c487d5090fabb0bbf093e7a2/pkgs/tools/misc/youtube-dl/default.nix#L33
On Sat, Mar 14, 2020 at 05:39:50PM +0000, Brice Waegeneire wrote: > I should have written this in the cover-letter, my bad. > pycryptodome is needed for the hlsative downloader, ffmpeg adds the ability > to merge video and audio files downloaded separately by youtube-dl and > removes the following warning: > WARNING: You have requested multiple formats but ffmpeg or avconv are not > installed. The formats won't be merged. What is hlsative? I guess that for FFmpeg, it looks in $PATH? This has always worked for me. But I think it's a good idea to bind them together.
Hullo Brice, Thanks for the patch! Brice Waegeneire 写道: > I should have written this in the cover-letter, my bad. > pycryptodome is needed for the hlsative downloader, ffmpeg adds > the > ability to merge video and audio files downloaded separately by > youtube-dl and removes the following warning: > WARNING: You have requested multiple formats but ffmpeg or > avconv are > not installed. The formats won't be merged. This message is one of the best I've seen. It clearly explains to the user what's (not) going to happen, and what they can do to change that *if* they want to. Hence I think adding ffmpeg as a hard dependency is incorrect. (I'd also oppose a youtube-dl-full variant, by the way. Packages aren't the right place for this; profiles are.) Does youtube-dl print a similarly clear message when pycryptodome is needed but missing? If not, that addition LGTM with a ("pycryptodome" ,pycryptodome) ; for the hlsnative downloader comment. Cover letters & commit messages age badly. Kind regards, T G-R
Leo Famulari 写道:
> What is hlsative?
hlsnative, it's a ‘limited [HLS] implementation that does not
require ffmpeg.’[0]
Kind regards,
T G-R
[0]:
https://github.com/ytdl-org/youtube-dl/blob/master/youtube_dl/downloader/hls.py#L25
On Sat, Mar 14, 2020 at 11:17:42PM +0100, Tobias Geerinckx-Rice wrote: > Brice Waegeneire 写道: > > I should have written this in the cover-letter, my bad. > > pycryptodome is needed for the hlsative downloader, ffmpeg adds the > > ability to merge video and audio files downloaded separately by > > youtube-dl and removes the following warning: > > WARNING: You have requested multiple formats but ffmpeg or avconv are > > not installed. The formats won't be merged. > > This message is one of the best I've seen. It clearly explains to the user > what's (not) going to happen, and what they can do to change that *if* they > want to. Hence I think adding ffmpeg as a hard dependency is incorrect. I see your point of view, and I also have FFmpeg installed manually because I use it frequently in my work. I guess it depends on how many people are experiencing the issue that Brice is having.
On 2020-03-14 22:17, Tobias Geerinckx-Rice via Guix-patches via wrote: > Brice Waegeneire 写道: >> WARNING: You have requested multiple formats but ffmpeg or avconv are >> not installed. The formats won't be merged. NOTE: This message appear when using a format option like this ='bestvideo[height<=320]+bestaudio'=. > This message is one of the best I've seen. It clearly explains to the > user what's (not) going to happen, and what they can do to change that > *if* they want to. Hence I think adding ffmpeg as a hard dependency > is incorrect. What about this one? #+begin_src sh $ youtue-dl https://www.youtube.com/watch\?v\=dp8PhLsUcFE [youtube] dp8PhLsUcFE: Downloading webpage [youtube] dp8PhLsUcFE: Downloading m3u8 information [youtube] dp8PhLsUcFE: Downloading MPD manifest [download] Destination: Bloomberg Global Financial News-dp8PhLsUcFE.mp4 ERROR: m3u8 download detected but ffmpeg or avconv could not be found. Please install one. #+end_src When downloading a live stream from youtube.com (and proabably others), ~youtube-dl~ needs ffmpeg to download the HLS stream – I tried with just ~pycryptodome~ and it doesn't work. > (I'd also oppose a youtube-dl-full variant, by the way. Packages > aren't the right place for this; profiles are.) > > Does youtube-dl print a similarly clear message when pycryptodome is > needed but missing? If not, that addition LGTM with a > > ("pycryptodome" ,pycryptodome) ; for the hlsnative downloader #+begin_src sh $ youtube-dl http://www.ivi.ru/watch/146500 [ivi] 146500: Downloading video JSON ERROR: pycryptodomex not found. Please install it. #+end_src #+begin_src python self.report_error('pycrypto not found. Please install it.') #+end_src I digged a little deeper about this dependency, ~pycryptodome~ replace the deprecated ~pycrypto~ library with the same name space (=Crypto=) while ~pycryptodomex~ uses it's own name space (=Cryptodome=) to provide similar functionality. #+begin_src python self.report_warning( 'hlsnative has detected features it does not support, ' 'extraction will be delegated to ffmpeg') #+end_src So I'm not sure if adding ~pycryptodome(x)~ is that useful after all. Especially if it can't totally replace ~ffmpeg~ when donating HLS streams. > comment. Cover letters & commit messages age badly. Noted. I think ~ffmpeg~ should be an input of ~youtube-dl~ while ~pycrytodome(x)~ don't need to since most (except the ivi downloader it seems) functionality can be achieved with ~ffmpeg~. WDYT?
diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm index 074a1235a3..4ca037532f 100644 --- a/gnu/packages/video.scm +++ b/gnu/packages/video.scm @@ -37,6 +37,7 @@ ;;; Copyright © 2019 Riku Viitanen <riku.viitanen@protonmail.com> ;;; Copyright © 2020 Oleg Pykhalov <go.wigust@gmail.com> ;;; Copyright © 2020 Josh Holland <josh@inv.alid.pw> +;;; Copyright © 2020 Brice Waegeneire <brice@waegenei.re> ;;; ;;; This file is part of GNU Guix. ;;; @@ -1580,7 +1581,19 @@ To load this plugin, specify the following option when starting mpv: (string-append "'" prefix "/etc/")) (("'share/") (string-append "'" prefix "/share/"))) - #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))) + (propagated-inputs + `(("python-pycryptodome" ,python-pycryptodome))) (synopsis "Download videos from YouTube.com and other sites") (description "Youtube-dl is a small command-line program to download videos from