[bug#63117] [PATCH] gnu: yt-dlp: Change input.

[bug#63117] [PATCH] gnu: yt-dlp: Change input.

[Dominik Delgado Steuter via Guix-patches via]

yt-dlp complained when the "--add-metadata" flag was used.
youtube-dl does not need ffmpeg as a propagated-input, though.

* gnu/packages/video.scm (yt-dlp)[inputs]: Remove ffmpeg.
[propagated-inputs]: Add ffmpeg.
---
 gnu/packages/video.scm | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
index 838dfe7..df6bbc5 100644
--- a/gnu/packages/video.scm
+++ b/gnu/packages/video.scm
@@ -2598,6 +2598,7 @@ (define-public yt-dlp
                 (when tests?
                   (invoke "pytest" "-k" "not download"))))))))
     (inputs (modify-inputs (package-inputs youtube-dl)
+              (delete ffmpeg) ;yt-dlp needs it as propagated-input
               (append python-brotli
                       python-certifi
                       python-mutagen
@@ -2611,6 +2612,8 @@ (define-public yt-dlp
          (list pandoc)
          '())
        (list python-pytest zip)))
+    (propagated-inputs
+     (list ffmpeg))
     (description
      "yt-dlp is a small command-line program to download videos from
 YouTube.com and many more sites.  It is a fork of youtube-dl with a

base-commit: fa685c87eaa9888a4278f39bb2b815673589dced
-- 
2.39.2

[bug#63117] [PATCH] gnu: yt-dlp: Change input.

[Jack Hill]

On Thu, 27 Apr 2023, Dominik Delgado Steuter via Guix-patches via wrote:

> yt-dlp complained when the "--add-metadata" flag was used.
> youtube-dl does not need ffmpeg as a propagated-input, though.

Dominik,

Thanks for working on improving our yt-dlp package. Can you explain a 
little more about why yt-dlp needs ffmpeg to be propagated? I haven't 
looked too closely, but I suspect yt-dlp just want to be able to call 
ffmpeg at runtime. If that's true, then I think it would be better to use 
substitute* in a phase so that yt-dlp can use the full path to ffmpeg, and 
not clutter folks' profiles.

If I'm correct, can you provide an updated patch that does that? I'm happy 
to provide more guidance if you need it.

If I'm wrong and ffmpeg needs to be propagated for some other reason, can 
you add a comment in the code explaining the need?

Best,
Jack

[bug#63117] [PATCH] gnu: yt-dlp: Change input.

[Dominik Delgado Steuter via Guix-patches via]

Hi Jack,

thanks for reviewing my patch. You are probably right that yt-dlp only 
needs the path and not necessarily the propagated-input.

I will try to figure out how exactly to do that and then send the 
updated patch.

Regards,
Dominik

Am 29.04.23 um 06:36 schrieb Jack Hill:
> On Thu, 27 Apr 2023, Dominik Delgado Steuter via Guix-patches via wrote:
> 
>> yt-dlp complained when the "--add-metadata" flag was used.
>> youtube-dl does not need ffmpeg as a propagated-input, though.
> 
> Dominik,
> 
> Thanks for working on improving our yt-dlp package. Can you explain a 
> little more about why yt-dlp needs ffmpeg to be propagated? I haven't 
> looked too closely, but I suspect yt-dlp just want to be able to call 
> ffmpeg at runtime. If that's true, then I think it would be better to 
> use substitute* in a phase so that yt-dlp can use the full path to 
> ffmpeg, and not clutter folks' profiles.
> 
> If I'm correct, can you provide an updated patch that does that? I'm 
> happy to provide more guidance if you need it.
> 
> If I'm wrong and ffmpeg needs to be propagated for some other reason, 
> can you add a comment in the code explaining the need?
> 
> Best,
> Jack

[bug#63117] [PATCH] gnu: yt-dlp: Fix substitution for ffmpeg path.

[Dominik Delgado Steuter via Guix-patches via]

The old expression did not work; ffmpeg was not found
when using the --add-metadata flag.

* gnu/packages/video.scm (yt-dlp)
[arguments]: Adjust substitution in custom 'default-to-the-ffmpeg-input phase.
---
 gnu/packages/video.scm | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
index 65fd92e..1301b79 100644
--- a/gnu/packages/video.scm
+++ b/gnu/packages/video.scm
@@ -64,6 +64,7 @@
 ;;; Copyright © 2022 Chadwain Holness <chadwainholness@gmail.com>
 ;;; Copyright © 2022 Andy Tai <atai@atai.org>
 ;;; Copyright © 2023 Ott Joon <oj@vern.cc>
+;;; Copyright © 2023 Dominik Delgado Steuter <dds@disroot.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -2570,8 +2571,8 @@ (define-public yt-dlp
             (replace 'default-to-the-ffmpeg-input
               (lambda _
                 (substitute* "yt_dlp/postprocessor/ffmpeg.py"
-                  (("\\.get_param\\('ffmpeg_location'\\)" match)
-                   (format #f "~a or '~a'" match (which "ffmpeg"))))))
+                  (("location = self.get_param(.*)$")
+                   (string-append "location = '" #$ffmpeg "/bin'\n")))))
             (replace 'build-generated-files
               (lambda* (#:key inputs #:allow-other-keys)
                 (if (assoc-ref inputs "pandoc")

base-commit: 6922069bcbe5c08da09c00e5aad44e390ebd1cc7
-- 
2.39.2

[bug#63117] [PATCH] gnu: yt-dlp: Fix substitution for ffmpeg path.

[Liliana Marie Prikler]

Am Freitag, dem 05.05.2023 um 21:50 +0200 schrieb Dominik Delgado
Steuter:
> The old expression did not work; ffmpeg was not found
> when using the --add-metadata flag.
> 
> * gnu/packages/video.scm (yt-dlp)
> [arguments]: Adjust substitution in custom 'default-to-the-ffmpeg-
> input phase.
> ---
>  gnu/packages/video.scm | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
> index 65fd92e..1301b79 100644
> --- a/gnu/packages/video.scm
> +++ b/gnu/packages/video.scm
> @@ -64,6 +64,7 @@
>  ;;; Copyright © 2022 Chadwain Holness <chadwainholness@gmail.com>
>  ;;; Copyright © 2022 Andy Tai <atai@atai.org>
>  ;;; Copyright © 2023 Ott Joon <oj@vern.cc>
> +;;; Copyright © 2023 Dominik Delgado Steuter <dds@disroot.org>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -2570,8 +2571,8 @@ (define-public yt-dlp
>              (replace 'default-to-the-ffmpeg-input
>                (lambda _
>                  (substitute* "yt_dlp/postprocessor/ffmpeg.py"
> -                  (("\\.get_param\\('ffmpeg_location'\\)" match)
> -                   (format #f "~a or '~a'" match (which
> "ffmpeg"))))))
> +                  (("location = self.get_param(.*)$")
> +                   (string-append "location = '" #$ffmpeg
> "/bin'\n")))))
The proper expression would be 
  (dirname (search-input-file inputs "bin/ffmpeg"))
You also need to make the (lambda _ ...) into a 
(lambda* (#:key inputs #:allow-other-keys) ...)
>              (replace 'build-generated-files
>                (lambda* (#:key inputs #:allow-other-keys)
>                  (if (assoc-ref inputs "pandoc")
> 
> base-commit: 6922069bcbe5c08da09c00e5aad44e390ebd1cc7

Cheers

[bug#63117] [PATCH] gnu: yt-dlp: Fix substitution for ffmpeg path.

[Dominik Delgado Steuter via Guix-patches via]

The old expression did not work; ffmpeg was not found
when using the --add-metadata flag.

* gnu/packages/video.scm (yt-dlp)
[arguments]: Adjust substitution in custom 'default-to-the-ffmpeg-input phase.
---
 gnu/packages/video.scm | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
index e6c437f..6d2824f 100644
--- a/gnu/packages/video.scm
+++ b/gnu/packages/video.scm
@@ -64,6 +64,7 @@
 ;;; Copyright © 2022 Chadwain Holness <chadwainholness@gmail.com>
 ;;; Copyright © 2022 Andy Tai <atai@atai.org>
 ;;; Copyright © 2023 Ott Joon <oj@vern.cc>
+;;; Copyright © 2023 Dominik Delgado Steuter <dds@disroot.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -2569,10 +2570,13 @@ (define-public yt-dlp
         #~(modify-phases #$phases
             ;; See the comment for the corresponding phase in youtube-dl.
             (replace 'default-to-the-ffmpeg-input
-              (lambda _
+              (lambda* (#:key inputs #:allow-other-keys)
                 (substitute* "yt_dlp/postprocessor/ffmpeg.py"
-                  (("\\.get_param\\('ffmpeg_location'\\)" match)
-                   (format #f "~a or '~a'" match (which "ffmpeg"))))))
+                  (("location = self.get_param(.*)$")
+                   (string-append
+                     "location = '"
+                     (dirname (search-input-file inputs "bin/ffmpeg"))
+                     "'\n")))))
             (replace 'build-generated-files
               (lambda* (#:key inputs #:allow-other-keys)
                 (if (assoc-ref inputs "pandoc")

base-commit: 24b6f94cf9b4ab97ef2eb70d05b2104a06776e62
-- 
2.40.1

bug#63117: [PATCH] gnu: yt-dlp: Change input.

[Ludovic Courtès]

Hi Dominik,

Dominik Delgado Steuter <dds@disroot.org> skribis:

> The old expression did not work; ffmpeg was not found
> when using the --add-metadata flag.
>
> * gnu/packages/video.scm (yt-dlp)
> [arguments]: Adjust substitution in custom 'default-to-the-ffmpeg-input phase.

Applied.  Thank you and thanks to Liliana for reviewing!

Ludo’.