diff mbox series

[bug#61117] Update svtplay-dl to 4.18

Message ID 5646122.DvuYhMxLoT@localhost.localdomain
State New
Headers show
Series [bug#61117] Update svtplay-dl to 4.18 | expand

Commit Message

Jessica Tallon Jan. 28, 2023, 11:29 a.m. UTC
Hello,

I've attached a patch to upgrade svtplay-dl from 4.17 to 4.18. I've also moved 
ffmpeg form inputs to propagated-inputs as it's needed while the program is 
running to merge two files, not while building.

Thanks,
Jessica.

Comments

Tobias Geerinckx-Rice Jan. 28, 2023, 12:33 p.m. UTC | #1
Jessica,

Jessica Tallon 写道:
> I've attached a patch to upgrade svtplay-dl from 4.17 to 4.18.

Thanks!

> I've also moved ffmpeg form inputs to propagated-inputs

Please avoid propagation whenever possible; it breaks all kinds of 
nice things.

Propagating A from B means that Guix will effectively ‘install’ A 
alongside B when the user installs only B.  Or how classical 
package managers bluntly handle ‘dependencies’.  Not good!

Here's what I'd do instead [untested]:

    (arguments
     (list #:phases
           #~(modify-phases %standard-phases
               (add-after 'wrap 'wrap-harder
                 (lambda* (#:key inputs outputs 
                 #:allow-other-keys)
                   (wrap-script (string-append (assoc-ref outputs 
                   "out")
                                               "/bin/svtplay-dl")
                     `("PATH" ":" prefix
                       (,(dirname (search-input-file inputs
                                                     "bin/ffmpeg"))))))))))
    (native-inputs (list guile-3.0))    ; for wrap-script

> it's needed while the program is 
> running to merge two files, not while building.

So 4.17 did not need or use ffmpeg this way?

Otherwise, put each unrelated changes into a separate commit: one 
to improve the ffmpeg situation, and one to update svtplay-dl. 
You can send multiple related commits as one patch series to one 
bug number, though.

Kind regards,

T G-R
Tobias Geerinckx-Rice Jan. 28, 2023, 12:58 p.m. UTC | #2
Tobias Geerinckx-Rice via Guix-patches via 写道:
>                 (lambda* (#:key inputs outputs
>                 #:allow-other-keys)           ^

Dunno why mu4e added a hard newline here.  There shouldn't be one.

Kind regards,

T G-R
Tobias Geerinckx-Rice Jan. 28, 2023, 1:02 p.m. UTC | #3
Tobias Geerinckx-Rice via Guix-patches via 写道:
>                   (wrap-script (string-append (assoc-ref outputs
>                   "out") 
>                   ^

And here.  All the others are good.  Sorry for the mess.

Kind regards,

T G-R
Leo Famulari March 5, 2023, 8:13 p.m. UTC | #4
Thanks again for working on this package!

On Sat, Jan 28, 2023 at 01:33:44PM +0100, Tobias Geerinckx-Rice via Guix-patches via wrote:
> > I've also moved ffmpeg form inputs to propagated-inputs
> 
> Please avoid propagation whenever possible; it breaks all kinds of nice
> things.

I'm here to express my weak preference for dynamically binding FFmpeg in
use cases like this one.

That means, I prefer if packages like svtplay-dl do not depend on FFmpeg
at all, but rather expect the user to install FFmpeg alongside them.

I prefer this because I use a custom FFmpeg professionally as a video
engineer, and it's easier to use it with Guix packages if the dependency
is resolved at run-time rather than at build-time.

I'd expect that many people like me also use a variety of custom FFmpeg
builds for different use cases.

Like I said, it's a weak preference. And I probably wouldn't use
svtplay-dl at work in the US, although I do use youtube-dl / yt-dlp. Let
me know what you think.
Jessica Tallon March 5, 2023, 8:15 p.m. UTC | #5
Leo Famulari <leo@famulari.name> writes:

> Thanks again for working on this package!
>
> On Sat, Jan 28, 2023 at 01:33:44PM +0100, Tobias Geerinckx-Rice via Guix-patches via wrote:
>> > I've also moved ffmpeg form inputs to propagated-inputs
>> 
>> Please avoid propagation whenever possible; it breaks all kinds of nice
>> things.
>
> I'm here to express my weak preference for dynamically binding FFmpeg in
> use cases like this one.
>
> That means, I prefer if packages like svtplay-dl do not depend on FFmpeg
> at all, but rather expect the user to install FFmpeg alongside them.
>
> I prefer this because I use a custom FFmpeg professionally as a video
> engineer, and it's easier to use it with Guix packages if the dependency
> is resolved at run-time rather than at build-time.
>
> I'd expect that many people like me also use a variety of custom FFmpeg
> builds for different use cases.
>
> Like I said, it's a weak preference. And I probably wouldn't use
> svtplay-dl at work in the US, although I do use youtube-dl / yt-dlp. Let
> me know what you think.

Hello,

Thanks for sharing your experiance, especially as a professional video
engineer :)

I think I have a mild opinion for having it bring in ffmpeg, this is
mostly for three reasons:

- There isn't a good way to communicate to users that they might wish to
  pull in ffmpeg.
  
- svtplay-dl will download both audio and video parts and then realise
  it doesn't have ffmpeg and will give up producing a warning. This
  leaves it up to the user to fix, while a merging both files is a
  simple job it often has me searching through a manual as I'm not well
  versed in ffmpeg or similar.

- I like it when things work consistantly (if x version of svtplay-dl
  works a specific way on my machine, it should on another machine, not
  dependent on my profile)

My preference is mild though, let me know what you think!

Thanks,
Jessica.
Leo Famulari March 5, 2023, 8:58 p.m. UTC | #6
On Sun, Mar 05, 2023 at 09:15:14PM +0100, Jessica Tallon wrote:
> I think I have a mild opinion for having it bring in ffmpeg, this is
> mostly for three reasons:
> 
> - There isn't a good way to communicate to users that they might wish to
>   pull in ffmpeg.
>   
> - svtplay-dl will download both audio and video parts and then realise
>   it doesn't have ffmpeg and will give up producing a warning. This
>   leaves it up to the user to fix, while a merging both files is a
>   simple job it often has me searching through a manual as I'm not well
>   versed in ffmpeg or similar.
> 
> - I like it when things work consistantly (if x version of svtplay-dl
>   works a specific way on my machine, it should on another machine, not
>   dependent on my profile)
> 
> My preference is mild though, let me know what you think!

I think you should go with your method!
diff mbox series

Patch

From f487a0b2b9433d3151782e187fa9b69e13b9e0c3 Mon Sep 17 00:00:00 2001
Message-Id: <f487a0b2b9433d3151782e187fa9b69e13b9e0c3.1674905145.git.tsyesika@tsyesika.se>
From: Jessica Tallon <tsyesika@tsyesika.se>
Date: Sat, 28 Jan 2023 11:50:32 +0100
Subject: [PATCH] * gnu/packages/video.scm (svtplay-dl) update to 4.18

---
 gnu/packages/video.scm | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
index 60d39c38dc..4f2012ef3c 100644
--- a/gnu/packages/video.scm
+++ b/gnu/packages/video.scm
@@ -5635,7 +5635,7 @@  (define-public video-contact-sheet
 (define-public svtplay-dl
   (package
     (name "svtplay-dl")
-    (version "4.17")
+    (version "4.18")
     (source (origin
               (method git-fetch)
               (uri (git-reference
@@ -5644,10 +5644,11 @@  (define-public svtplay-dl
               (file-name (git-file-name name version))
               (sha256
                (base32
-                "0yjxmvldskw4pji3lg69pbx05izvxahz9my7z5p31mkiz6v33dmx"))))
+                "1xb2n3bwjddr86mjynbfd8m2g98gzqbj5mmsfc735q9xp14l30pf"))))
     (build-system python-build-system)
-    (inputs (list ffmpeg python-pyaml python-requests python-pysocks
+    (inputs (list python-pyaml python-requests python-pysocks
                   python-cryptography))
+    (propagated-inputs (list ffmpeg))
     (home-page "https://svtplay-dl.se/")
     (synopsis "Download or stream SVT Play's (and others) TV programmes")
     (description
-- 
2.39.1