unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#40060] [PATCH 0/2] youtube-dl add ffmpeg, pycryptodome and zsh-completion
@ 2020-03-14 14:34 Brice Waegeneire
  2020-03-14 14:42 ` [bug#40060] [PATCH 1/2] gnu: youtube-dl: Use ffmpeg and pycryptodome Brice Waegeneire
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Brice Waegeneire @ 2020-03-14 14:34 UTC (permalink / raw)
  To: 40060

Brice Waegeneire (2):
  gnu: youtube-dl: Use ffmpeg and pycryptodome
  gnu: youtube-dl: Add zsh completion

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

-- 
2.25.0

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [bug#40060] [PATCH 1/2] gnu: youtube-dl: Use ffmpeg and pycryptodome
  2020-03-14 14:34 [bug#40060] [PATCH 0/2] youtube-dl add ffmpeg, pycryptodome and zsh-completion Brice Waegeneire
@ 2020-03-14 14:42 ` Brice Waegeneire
  2020-03-14 17:21   ` Leo Famulari
  2020-03-14 14:42 ` [bug#40060] [PATCH 2/2] gnu: youtube-dl: Add zsh completion Brice Waegeneire
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 18+ messages in thread
From: Brice Waegeneire @ 2020-03-14 14:42 UTC (permalink / raw)
  To: 40060

* gnu/packages/video.scm (youtube-dl)[arguments]: Add phase
wrap-executable.
[inputs]: Add ffmpeg.
[propagated-inputs]: Add python-pycryptodome.
---
 gnu/packages/video.scm | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

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
-- 
2.25.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [bug#40060] [PATCH 2/2] gnu: youtube-dl: Add zsh completion
  2020-03-14 14:34 [bug#40060] [PATCH 0/2] youtube-dl add ffmpeg, pycryptodome and zsh-completion Brice Waegeneire
  2020-03-14 14:42 ` [bug#40060] [PATCH 1/2] gnu: youtube-dl: Use ffmpeg and pycryptodome Brice Waegeneire
@ 2020-03-14 14:42 ` Brice Waegeneire
  2020-03-23 17:15 ` [bug#40060] [PATCH v2 0/2] youtube-dl add ffmpeg and zsh-completion Brice Waegeneire
  2020-04-01 13:01 ` [bug#40060] [PATCH v3] gnu: youtube-dl: Add 'ffmpeg' as input Brice Waegeneire
  3 siblings, 0 replies; 18+ messages in thread
From: Brice Waegeneire @ 2020-03-14 14:42 UTC (permalink / raw)
  To: 40060

* gnu/packages/video.scm (youtube-dl)[arguments]: Add phase
install-completion.
---
 gnu/packages/video.scm | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
index 4ca037532f..38f376c7e4 100644
--- a/gnu/packages/video.scm
+++ b/gnu/packages/video.scm
@@ -1582,6 +1582,15 @@ To load this plugin, specify the following option when starting mpv:
                           (("'share/")
                            (string-append "'" prefix "/share/")))
                         #t)))
+                  (add-after 'install 'install-completion
+                    (lambda* (#:key outputs #:allow-other-keys)
+                      (let* ((out (assoc-ref outputs "out"))
+                             (zsh (string-append out
+                                                 "/share/zsh/site-functions")))
+                        (mkdir-p zsh)
+                        (copy-file "youtube-dl.zsh"
+                                   (string-append zsh "/_youtube-dl"))
+                        #t)))
                   (add-after 'install 'wrap-executable
                     (lambda* (#:key inputs outputs #:allow-other-keys)
                       (let ((out (assoc-ref outputs "out"))
-- 
2.25.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [bug#40060] [PATCH 1/2] gnu: youtube-dl: Use ffmpeg and pycryptodome
  2020-03-14 14:42 ` [bug#40060] [PATCH 1/2] gnu: youtube-dl: Use ffmpeg and pycryptodome Brice Waegeneire
@ 2020-03-14 17:21   ` Leo Famulari
  2020-03-14 17:39     ` Brice Waegeneire
  0 siblings, 1 reply; 18+ messages in thread
From: Leo Famulari @ 2020-03-14 17:21 UTC (permalink / raw)
  To: Brice Waegeneire; +Cc: 40060

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?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [bug#40060] [PATCH 1/2] gnu: youtube-dl: Use ffmpeg and pycryptodome
  2020-03-14 17:21   ` Leo Famulari
@ 2020-03-14 17:39     ` Brice Waegeneire
  2020-03-14 17:47       ` Leo Famulari
  2020-03-14 22:17       ` Tobias Geerinckx-Rice via Guix-patches via
  0 siblings, 2 replies; 18+ messages in thread
From: Brice Waegeneire @ 2020-03-14 17:39 UTC (permalink / raw)
  To: Leo Famulari; +Cc: 40060, Guix-patches

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [bug#40060] [PATCH 1/2] gnu: youtube-dl: Use ffmpeg and pycryptodome
  2020-03-14 17:39     ` Brice Waegeneire
@ 2020-03-14 17:47       ` Leo Famulari
  2020-03-14 22:20         ` Tobias Geerinckx-Rice via Guix-patches via
  2020-03-14 22:17       ` Tobias Geerinckx-Rice via Guix-patches via
  1 sibling, 1 reply; 18+ messages in thread
From: Leo Famulari @ 2020-03-14 17:47 UTC (permalink / raw)
  To: Brice Waegeneire; +Cc: 40060

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.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [bug#40060] [PATCH 1/2] gnu: youtube-dl: Use ffmpeg and pycryptodome
  2020-03-14 17:39     ` Brice Waegeneire
  2020-03-14 17:47       ` Leo Famulari
@ 2020-03-14 22:17       ` Tobias Geerinckx-Rice via Guix-patches via
  2020-03-15 17:25         ` Leo Famulari
  2020-03-17  9:06         ` Brice Waegeneire
  1 sibling, 2 replies; 18+ messages in thread
From: Tobias Geerinckx-Rice via Guix-patches via @ 2020-03-14 22:17 UTC (permalink / raw)
  To: Leo Famulari, 40060

[-- Attachment #1: Type: text/plain, Size: 1074 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [bug#40060] [PATCH 1/2] gnu: youtube-dl: Use ffmpeg and pycryptodome
  2020-03-14 17:47       ` Leo Famulari
@ 2020-03-14 22:20         ` Tobias Geerinckx-Rice via Guix-patches via
  0 siblings, 0 replies; 18+ messages in thread
From: Tobias Geerinckx-Rice via Guix-patches via @ 2020-03-14 22:20 UTC (permalink / raw)
  To: Leo Famulari; +Cc: 40060, Brice Waegeneire

[-- Attachment #1: Type: text/plain, Size: 256 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [bug#40060] [PATCH 1/2] gnu: youtube-dl: Use ffmpeg and pycryptodome
  2020-03-14 22:17       ` Tobias Geerinckx-Rice via Guix-patches via
@ 2020-03-15 17:25         ` Leo Famulari
  2020-03-17  9:06         ` Brice Waegeneire
  1 sibling, 0 replies; 18+ messages in thread
From: Leo Famulari @ 2020-03-15 17:25 UTC (permalink / raw)
  To: Tobias Geerinckx-Rice; +Cc: 40060

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.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [bug#40060] [PATCH 1/2] gnu: youtube-dl: Use ffmpeg and pycryptodome
  2020-03-14 22:17       ` Tobias Geerinckx-Rice via Guix-patches via
  2020-03-15 17:25         ` Leo Famulari
@ 2020-03-17  9:06         ` Brice Waegeneire
  1 sibling, 0 replies; 18+ messages in thread
From: Brice Waegeneire @ 2020-03-17  9:06 UTC (permalink / raw)
  To: me, 40060

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?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [bug#40060] [PATCH v2 0/2] youtube-dl add ffmpeg and zsh-completion
  2020-03-14 14:34 [bug#40060] [PATCH 0/2] youtube-dl add ffmpeg, pycryptodome and zsh-completion Brice Waegeneire
  2020-03-14 14:42 ` [bug#40060] [PATCH 1/2] gnu: youtube-dl: Use ffmpeg and pycryptodome Brice Waegeneire
  2020-03-14 14:42 ` [bug#40060] [PATCH 2/2] gnu: youtube-dl: Add zsh completion Brice Waegeneire
@ 2020-03-23 17:15 ` Brice Waegeneire
  2020-03-23 17:15   ` [bug#40060] [PATCH v2 1/2] gnu: youtube-dl: Add 'ffmpeg' as input Brice Waegeneire
                     ` (2 more replies)
  2020-04-01 13:01 ` [bug#40060] [PATCH v3] gnu: youtube-dl: Add 'ffmpeg' as input Brice Waegeneire
  3 siblings, 3 replies; 18+ messages in thread
From: Brice Waegeneire @ 2020-03-23 17:15 UTC (permalink / raw)
  To: 40060

Remove pycryptodome since ffmpeg is enought for accessing
HLS streams.

Brice Waegeneire (2):
  gnu: youtube-dl: Add 'ffmpeg' as input.
  gnu: youtube-dl: Add zsh completion.

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


base-commit: d3e7282ec35d87842e8143f0230e27fd5ec5e74d
-- 
2.25.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [bug#40060] [PATCH v2 1/2] gnu: youtube-dl: Add 'ffmpeg' as input.
  2020-03-23 17:15 ` [bug#40060] [PATCH v2 0/2] youtube-dl add ffmpeg and zsh-completion Brice Waegeneire
@ 2020-03-23 17:15   ` Brice Waegeneire
  2020-03-23 17:15   ` [bug#40060] [PATCH v2 2/2] gnu: youtube-dl: Add zsh completion Brice Waegeneire
  2020-03-23 19:28   ` [bug#40060] [PATCH v2 0/2] youtube-dl add ffmpeg and zsh-completion Mathieu Othacehe
  2 siblings, 0 replies; 18+ messages in thread
From: Brice Waegeneire @ 2020-03-23 17:15 UTC (permalink / raw)
  To: 40060

* gnu/packages/video.scm (youtube-dl)[arguments]: Add phase
wrap-executable.
[inputs]: Add ffmpeg.
---
 gnu/packages/video.scm | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
index 38e9d36307..3b64c435b7 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.
 ;;;
@@ -1671,7 +1672,17 @@ 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)))
     (synopsis "Download videos from YouTube.com and other sites")
     (description
      "Youtube-dl is a small command-line program to download videos from
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [bug#40060] [PATCH v2 2/2] gnu: youtube-dl: Add zsh completion.
  2020-03-23 17:15 ` [bug#40060] [PATCH v2 0/2] youtube-dl add ffmpeg and zsh-completion Brice Waegeneire
  2020-03-23 17:15   ` [bug#40060] [PATCH v2 1/2] gnu: youtube-dl: Add 'ffmpeg' as input Brice Waegeneire
@ 2020-03-23 17:15   ` Brice Waegeneire
  2020-03-23 19:28   ` [bug#40060] [PATCH v2 0/2] youtube-dl add ffmpeg and zsh-completion Mathieu Othacehe
  2 siblings, 0 replies; 18+ messages in thread
From: Brice Waegeneire @ 2020-03-23 17:15 UTC (permalink / raw)
  To: 40060

* gnu/packages/video.scm (youtube-dl)[arguments]: Add phase
install-completion.
---
 gnu/packages/video.scm | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
index 3b64c435b7..cb575a7375 100644
--- a/gnu/packages/video.scm
+++ b/gnu/packages/video.scm
@@ -1673,6 +1673,15 @@ To load this plugin, specify the following option when starting mpv:
                           (("'share/")
                            (string-append "'" prefix "/share/")))
                         #t)))
+                  (add-after 'install 'install-completion
+                    (lambda* (#:key outputs #:allow-other-keys)
+                      (let* ((out (assoc-ref outputs "out"))
+                             (zsh (string-append out
+                                                 "/share/zsh/site-functions")))
+                        (mkdir-p zsh)
+                        (copy-file "youtube-dl.zsh"
+                                   (string-append zsh "/_youtube-dl"))
+                        #t)))
                   (add-after 'install 'wrap-executable
                     (lambda* (#:key inputs outputs #:allow-other-keys)
                       (let ((out (assoc-ref outputs "out"))
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [bug#40060] [PATCH v2 0/2] youtube-dl add ffmpeg and zsh-completion
  2020-03-23 17:15 ` [bug#40060] [PATCH v2 0/2] youtube-dl add ffmpeg and zsh-completion Brice Waegeneire
  2020-03-23 17:15   ` [bug#40060] [PATCH v2 1/2] gnu: youtube-dl: Add 'ffmpeg' as input Brice Waegeneire
  2020-03-23 17:15   ` [bug#40060] [PATCH v2 2/2] gnu: youtube-dl: Add zsh completion Brice Waegeneire
@ 2020-03-23 19:28   ` Mathieu Othacehe
  2020-03-23 20:23     ` Brice Waegeneire
  2 siblings, 1 reply; 18+ messages in thread
From: Mathieu Othacehe @ 2020-03-23 19:28 UTC (permalink / raw)
  To: 40060; +Cc: 40060-done


Hello Brice,

> Remove pycryptodome since ffmpeg is enought for accessing
> HLS streams.

I could do not find any reference to pycryptodome, but pushed the serie
as is LTGM.

Thanks,

Mathieu

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [bug#40060] [PATCH v2 0/2] youtube-dl add ffmpeg and zsh-completion
  2020-03-23 19:28   ` [bug#40060] [PATCH v2 0/2] youtube-dl add ffmpeg and zsh-completion Mathieu Othacehe
@ 2020-03-23 20:23     ` Brice Waegeneire
  2020-03-23 20:43       ` Mathieu Othacehe
  0 siblings, 1 reply; 18+ messages in thread
From: Brice Waegeneire @ 2020-03-23 20:23 UTC (permalink / raw)
  To: Mathieu Othacehe; +Cc: 40060, me, leo

On 2020-03-23 19:28, Mathieu Othacehe wrote:
>> Remove pycryptodome since ffmpeg is enought for accessing
>> HLS streams.
> 
> I could do not find any reference to pycryptodome, but pushed the serie
> as is LTGM.

The first revision of this patch added pycryptodome as input, this 
version
doesn't. BTW Tobias and Leo were against adding ffmpeg to youtube-dl.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [bug#40060] [PATCH v2 0/2] youtube-dl add ffmpeg and zsh-completion
  2020-03-23 20:23     ` Brice Waegeneire
@ 2020-03-23 20:43       ` Mathieu Othacehe
  0 siblings, 0 replies; 18+ messages in thread
From: Mathieu Othacehe @ 2020-03-23 20:43 UTC (permalink / raw)
  To: Brice Waegeneire; +Cc: 40060, me, leo


> The first revision of this patch added pycryptodome as input, this version
> doesn't. BTW Tobias and Leo were against adding ffmpeg to youtube-dl.

Oops didn't see the small "v2". Well I'll revert this one until this is
clarified.

Sorry about that,

Mathieu

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [bug#40060] [PATCH v3] gnu: youtube-dl: Add 'ffmpeg' as input.
  2020-03-14 14:34 [bug#40060] [PATCH 0/2] youtube-dl add ffmpeg, pycryptodome and zsh-completion Brice Waegeneire
                   ` (2 preceding siblings ...)
  2020-03-23 17:15 ` [bug#40060] [PATCH v2 0/2] youtube-dl add ffmpeg and zsh-completion Brice Waegeneire
@ 2020-04-01 13:01 ` Brice Waegeneire
  2020-04-01 13:40   ` Tobias Geerinckx-Rice via Guix-patches via
  3 siblings, 1 reply; 18+ messages in thread
From: Brice Waegeneire @ 2020-04-01 13:01 UTC (permalink / raw)
  To: 40060

* 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(-)

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
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [bug#40060] [PATCH v3] gnu: youtube-dl: Add 'ffmpeg' as input.
  2020-04-01 13:01 ` [bug#40060] [PATCH v3] gnu: youtube-dl: Add 'ffmpeg' as input Brice Waegeneire
@ 2020-04-01 13:40   ` Tobias Geerinckx-Rice via Guix-patches via
  0 siblings, 0 replies; 18+ messages in thread
From: Tobias Geerinckx-Rice via Guix-patches via @ 2020-04-01 13:40 UTC (permalink / raw)
  To: 40060

[-- Attachment #1: Type: text/plain, Size: 1239 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2020-04-01 13:41 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-14 14:34 [bug#40060] [PATCH 0/2] youtube-dl add ffmpeg, pycryptodome and zsh-completion Brice Waegeneire
2020-03-14 14:42 ` [bug#40060] [PATCH 1/2] gnu: youtube-dl: Use ffmpeg and pycryptodome Brice Waegeneire
2020-03-14 17:21   ` Leo Famulari
2020-03-14 17:39     ` Brice Waegeneire
2020-03-14 17:47       ` Leo Famulari
2020-03-14 22:20         ` Tobias Geerinckx-Rice via Guix-patches via
2020-03-14 22:17       ` Tobias Geerinckx-Rice via Guix-patches via
2020-03-15 17:25         ` Leo Famulari
2020-03-17  9:06         ` Brice Waegeneire
2020-03-14 14:42 ` [bug#40060] [PATCH 2/2] gnu: youtube-dl: Add zsh completion Brice Waegeneire
2020-03-23 17:15 ` [bug#40060] [PATCH v2 0/2] youtube-dl add ffmpeg and zsh-completion Brice Waegeneire
2020-03-23 17:15   ` [bug#40060] [PATCH v2 1/2] gnu: youtube-dl: Add 'ffmpeg' as input Brice Waegeneire
2020-03-23 17:15   ` [bug#40060] [PATCH v2 2/2] gnu: youtube-dl: Add zsh completion Brice Waegeneire
2020-03-23 19:28   ` [bug#40060] [PATCH v2 0/2] youtube-dl add ffmpeg and zsh-completion Mathieu Othacehe
2020-03-23 20:23     ` Brice Waegeneire
2020-03-23 20:43       ` Mathieu Othacehe
2020-04-01 13:01 ` [bug#40060] [PATCH v3] gnu: youtube-dl: Add 'ffmpeg' as input Brice Waegeneire
2020-04-01 13:40   ` Tobias Geerinckx-Rice via Guix-patches via

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).