diff mbox series

[bug#54744] Update gstreamer and its families to 1.20.1.

Message ID 86bkxeop1g.fsf@163.com
State Accepted
Headers show
Series [bug#54744] Update gstreamer and its families to 1.20.1. | expand

Checks

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

Commit Message

Zhu Zihao April 6, 2022, 3:38 a.m. UTC
Here's a series of patches that updates gstreamer to 1.20.1.

`guix refresh gstreamer -l` shows that triggers 900+ rebuilds, I'm not
sure every packages depends on gstreamer is tested and worked well.
Currently only webkitgtk is tested.

Comments

Liliana Marie Prikler April 6, 2022, 5:56 a.m. UTC | #1
Am Mittwoch, dem 06.04.2022 um 11:38 +0800 schrieb Zhu Zihao:
> 
> Here's a series of patches that updates gstreamer to 1.20.1.
> 
> `guix refresh gstreamer -l` shows that triggers 900+ rebuilds, I'm not
> sure every packages depends on gstreamer is tested and worked well. 
> Currently only webkitgtk is tested.
Indeed, gstreamer updates go to staging.

> * gnu/packages/gstreamer.scm (%gstreamer-version): New variable.
I don't think that's necessary or even beneficial.  Drop it.
Ditto for all the packages referencing it.

> * gnu/packages/gstreamer.scm (gst-plugins-good): Update to 1.20.1.
> [...]
> [propagated-inputs]: Remove unnecessary propagation.
Do gst-plugins-good really no longer depend on the base plugins?

> * gnu/packages/gstreamer.scm (gst-plugins-ugly): Update to 1.20.1.
> [...]
> [propagated-inputs]: Remove unnecessary propagation.
Idem.

> * gnu/packages/gstreamer.scm (gst-libav): Update to 1.20.1.
> [...]
> [propagated-inputs]: Remove unnecessary propagation.
Ibidem.

> (gst-plugins/selection): Remove because it's not useful in packaging
> and requires extra maintenance.
> * gnu/packages/video.scm (pitivi)[inputs]: Use gst-plugins-bad.
Packages that only require some bad plugins and can exactly state which
should not be forced to pull in all of them.  The bad plugins are
explicitly named bad, because they're bad.  Enabling any of them beyond
necessity should be an active choice of the user.

> * gnu/packages/webkit.scm (webkitgtk):
> [inputs]: Add gst-plugins-bad. It provides gstreamer-parsecodec.
My, what a perfect use case for gst-plugins/selection that would be. 
Note, that gratuitous media codecs are an additional attack surface.

Cheers
Zhu Zihao April 6, 2022, 9:16 a.m. UTC | #2
Liliana Marie Prikler <liliana.prikler@ist.tugraz.at> writes:
>> * gnu/packages/gstreamer.scm (%gstreamer-version): New variable.
> I don't think that's necessary or even beneficial.  Drop it.
> Ditto for all the packages referencing it.

Good. From my view this can ensure packager to update every package of
gstreamer, not only a part of them. Gstreamer now use a monorepo for all
its components, they'll also have regular release cycle for all
components.

>> * gnu/packages/gstreamer.scm (gst-plugins-good): Update to 1.20.1.
>> [...]
>> [propagated-inputs]: Remove unnecessary propagation.
> Do gst-plugins-good really no longer depend on the base plugins?
>
>> * gnu/packages/gstreamer.scm (gst-plugins-ugly): Update to 1.20.1.
>> [...]
>> [propagated-inputs]: Remove unnecessary propagation.
> Idem.
>
>> * gnu/packages/gstreamer.scm (gst-libav): Update to 1.20.1.
>> [...]
>> [propagated-inputs]: Remove unnecessary propagation.
> Ibidem.

I checked the ugly and good and gst-libav, they do rely on
gst-plugins-base, but no propagation needed. I don't find a reason that
they really requires propagation, because their content just a so file.
Gstreamer in Nixpkgs also don't have propagations for above packages.

For necessary propgations like gst-plugins-bad(header or gi), I've added
some comments.

>> (gst-plugins/selection): Remove because it's not useful in packaging
>> and requires extra maintenance.
>> * gnu/packages/video.scm (pitivi)[inputs]: Use gst-plugins-bad.
> Packages that only require some bad plugins and can exactly state which
> should not be forced to pull in all of them.  The bad plugins are
> explicitly named bad, because they're bad.  Enabling any of them beyond
> necessity should be an active choice of the user.
>
>> * gnu/packages/webkit.scm (webkitgtk):
>> [inputs]: Add gst-plugins-bad. It provides gstreamer-parsecodec.
> My, what a perfect use case for gst-plugins/selection that would be. 
> Note, that gratuitous media codecs are an additional attack surface.
>
> Cheers

I'll investigate into it.

Currently I only see pitivi use gst-plugins/selection. Many package use
gst-plugins-bad directly. 

BTW, how to maintain changes for a long patches series like this in the
review phase? It's quite annoying when I make some minor changes but I
have to re-sent all patches to the mail list.
Liliana Marie Prikler April 6, 2022, 10:05 a.m. UTC | #3
Am Mittwoch, dem 06.04.2022 um 17:16 +0800 schrieb Zhu Zihao:
> 
> Liliana Marie Prikler <liliana.prikler@ist.tugraz.at> writes:
> > > * gnu/packages/gstreamer.scm (%gstreamer-version): New variable.
> > I don't think that's necessary or even beneficial.  Drop it.
> > Ditto for all the packages referencing it.
> 
> Good. From my view this can ensure packager to update every package
> of gstreamer, not only a part of them. Gstreamer now use a monorepo
> for all its components, they'll also have regular release cycle for
> all components.
Can't really say that the monorepo is a good idea nor condemn it
without knowing the motivations behind, but let's hope that things
still build in isolation.  As for updating, the new variable does more
harm than good.  It forces every build to break when actually the build
would have been sane.  IMHO you should instead leave compatibility
breaking changes to upstream.

> > > * gnu/packages/gstreamer.scm (gst-plugins-good): Update to
> > > 1.20.1.
> > > [...]
> > > [propagated-inputs]: Remove unnecessary propagation.
> > Do gst-plugins-good really no longer depend on the base plugins?
> > 
> > > * gnu/packages/gstreamer.scm (gst-plugins-ugly): Update to
> > > 1.20.1.
> > > [...]
> > > [propagated-inputs]: Remove unnecessary propagation.
> > Idem.
> > 
> > > * gnu/packages/gstreamer.scm (gst-libav): Update to 1.20.1.
> > > [...]
> > > [propagated-inputs]: Remove unnecessary propagation.
> > Ibidem.
> 
> I checked the ugly and good and gst-libav, they do rely on
> gst-plugins-base, but no propagation needed. I don't find a reason
> that they really requires propagation, because their content just a
> so file.
> Gstreamer in Nixpkgs also don't have propagations for above packages.
I personally found that missing these GstElements at runtime can screw
you up.  If that is no longer the case, then fair enough, but you need
to prove that by launching a gst pipeline using a plugin from e.g. gst-
plugins-good without having gst-plugins-base in your GST paths.

> For necessary propgations like gst-plugins-bad(header or gi), I've
> added some comments.
That is always welcome.

> > 
> > > (gst-plugins/selection): Remove because it's not useful in
> > > packaging and requires extra maintenance.
> > > * gnu/packages/video.scm (pitivi)[inputs]: Use gst-plugins-bad.
> > Packages that only require some bad plugins and can exactly state
> > which should not be forced to pull in all of them.  The bad plugins
> > are explicitly named bad, because they're bad.  Enabling any of
> > them beyond necessity should be an active choice of the user.
> > 
> > 
> > > * gnu/packages/webkit.scm (webkitgtk):
> > > [inputs]: Add gst-plugins-bad. It provides gstreamer-parsecodec.
> > My, what a perfect use case for gst-plugins/selection that would
> > be.  Note, that gratuitous media codecs are an additional attack
> > surface.
> > 
> > Cheers
> 
> I'll investigate into it.
> 
> Currently I only see pitivi use gst-plugins/selection.  Many packages
> use gst-plugins-bad directly. 
Whether to use gst-plugins-bad or a selection of it is a per-package
decision.  It depends on whether upstream communicates clearly which
plugins they need or whether you'd have to pull hairs out of their
noses to do so.  In the latter case, it's likelier that they include it
just because they can and will introduce more dependencies on it as
time marches forward.
For most packages not using it, there might however just be a
historical reason – gst-plugins/selection has not existed for that
long.  If you feel it is underused, you might want to search for
packages that (like Pitivi) provide a clear description of what they
need.

> BTW, how to maintain changes for a long patches series like this in
> the review phase? It's quite annoying when I make some minor changes
> but I have to re-sent all patches to the mail list.
Ideally, you would send them one by one using `git send-email'.  Then
you can send a v2 for an individual patch.  Note that excessive use of
minor changes might however annoy both reviewers and infrastructure (I
hear there's a patchwork instance somewhere actually building these
packages).  In my opinion it would be wiser to let it sit for a while,
collect opinions, and then formulate a v2 with all of those in mind.

Note that in your case, v2 should also be sent to staging, i.e. [PATCH
staging v2].

Cheers
Zhu Zihao April 6, 2022, 3:34 p.m. UTC | #4
Liliana Marie Prikler <liliana.prikler@ist.tugraz.at> writes:

>> I checked the ugly and good and gst-libav, they do rely on
>> gst-plugins-base, but no propagation needed. I don't find a reason
>> that they really requires propagation, because their content just a
>> so file.
>> Gstreamer in Nixpkgs also don't have propagations for above packages.
> I personally found that missing these GstElements at runtime can screw
> you up.  If that is no longer the case, then fair enough, but you need
> to prove that by launching a gst pipeline using a plugin from e.g. gst-
> plugins-good without having gst-plugins-base in your GST paths.

Re-check the source code of good,ugly,gst-libav. It looks that some good
plugins will use gst_element_factory_make to create element in base,
It's reasonable for gst-plugins-good to propagates
gst-plugins-base(propagation of gstreamer is not neeeded, I think)

But for ugly and gst-libav. I don't see such code in their source code.
So It's better to remove their propagated-inputs.
Liliana Marie Prikler April 7, 2022, 6:27 a.m. UTC | #5
Am Mittwoch, dem 06.04.2022 um 23:34 +0800 schrieb Zhu Zihao:
> Re-check the source code of good,ugly,gst-libav. It looks that some
> good plugins will use gst_element_factory_make to create element in
> base, It's reasonable for gst-plugins-good to propagates
> gst-plugins-base(propagation of gstreamer is not neeeded, I think)
The explicit propagation of gstreamer from any of the plugin paths is
so that GST_PLUGIN_SYSTEM_PATH is set.  However, it might be a little
too restrictive – it does prohibit combination of any of the plugins
with a slightly differently built gstreamer.

> But for ugly and gst-libav. I don't see such code in their source
> code.  So It's better to remove their propagated-inputs.
Static analysis is not enough here, you need integration tests.
diff mbox series

Patch

From 70ea4b4c8085fce957557a9df2a44ab217ffaf78 Mon Sep 17 00:00:00 2001
From: Zhu Zihao <all_but_last@163.com>
Date: Wed, 6 Apr 2022 11:31:00 +0800
Subject: [PATCH 10/10] gnu: gst-editing-services: Update to 1.20.1.

* gnu/packages/gstreamer.scm (gst-editing-services): Update to 1.20.1.
[version]: Use `%gstreamer-version` variable.
[source]: Generate download URL by package name.
[arguments]: Use G-expressions.
[native-inputs]: Use label-less input style.
---
 gnu/packages/gstreamer.scm | 35 +++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/gnu/packages/gstreamer.scm b/gnu/packages/gstreamer.scm
index d96a07cc33..ef0b1d1ead 100644
--- a/gnu/packages/gstreamer.scm
+++ b/gnu/packages/gstreamer.scm
@@ -1002,35 +1002,38 @@  (define-public gst-libav
 (define-public gst-editing-services
   (package
     (name "gst-editing-services")
-    (version "1.18.5")
+    (version %gstreamer-version)
     (source (origin
               (method url-fetch)
               (uri (string-append
                     "https://gstreamer.freedesktop.org/src/" name "/"
-                    "gst-editing-services-" version ".tar.xz"))
+                    name "-" version ".tar.xz"))
               (sha256
                (base32
-                "1x8db4021qv4ypq1g6n5q2awrb7glr4xp1h650c3w7q59lwsix4a"))))
+                "1ps887yyj6jkj8a2613n43b4fbvynxwryinxvavi00cfnlhipkka"))))
     (build-system meson-build-system)
     (arguments
-     ;; FIXME: 16/22 failing tests.
-     `(#:tests? #f
-       #:glib-or-gtk? #t     ; To wrap binaries and/or compile schemas
-       #:phases (modify-phases %standard-phases
-                  ,@%common-gstreamer-phases)))
+     (list
+      ;; FIXME: 16/22 failing tests.
+      #:tests? #f
+      #:glib-or-gtk? #t              ; To wrap binaries and/or compile schemas
+      #:phases
+      #~(modify-phases %standard-phases
+          #$@%common-gstreamer-phases)))
     (propagated-inputs
      (list gstreamer gst-plugins-base))
     (inputs
      (list glib glib-networking gtk+ libxml2))
     (native-inputs
-     `(("flex" ,flex)
-       ("gobject-introspection" ,gobject-introspection)
-       ("glib:bin" ,glib "bin")
-       ("gst-plugins-bad" ,gst-plugins-bad)
-       ("gst-plugins-good" ,gst-plugins-good)
-       ("perl" ,perl)
-       ("pkg-config" ,pkg-config)
-       ("python" ,python-wrapper)))
+     (list flex
+           gobject-introspection
+           `(,glib "bin")
+           ;; gst-devtools
+           gst-plugins-bad
+           gst-plugins-good
+           perl
+           pkg-config
+           python-wrapper))
     (home-page "https://gstreamer.freedesktop.org/")
     (synopsis "GStreamer library for non-linear editors")
     (description
-- 
2.34.0