Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gstreamer: 1.16.2 -> 1.18.0 #99345

Merged
merged 5 commits into from Oct 29, 2020
Merged

gstreamer: 1.16.2 -> 1.18.0 #99345

merged 5 commits into from Oct 29, 2020

Conversation

@nh2
Copy link
Contributor

@nh2 nh2 commented Oct 2, 2020

Motivation for this change

https://gstreamer.freedesktop.org/releases/1.18/

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Note that building docs is disabled with this upgrade; somebody needs to package hotdoc for that to work again.

Feel free to push fixes straight into my branch, because I only did this work to try something out with srt, I do not actually have a lot of time to work on gstreamer upgrades currently.

@nh2 nh2 requested a review from jtojnar Oct 2, 2020
@nh2
Copy link
Contributor Author

@nh2 nh2 commented Oct 2, 2020

@GrahamcOfBorg build gst_all_1.gst-editing-services gst_all_1.gst-plugins-bad gst_all_1.gst-plugins-good gst_all_1.gst-rtsp-server gst_all_1.gst-validate gst_all_1.gst-libav gst_all_1.gst-plugins-base gst_all_1.gst-plugins-ugly gst_all_1.gst-vaapi gst_all_1.gstreamer

@ajs124
Copy link
Member

@ajs124 ajs124 commented Oct 2, 2020

@nh2 nh2 force-pushed the nh2:gstreamer-1.18.0 branch 2 times, most recently from e887ca8 to b0c2b0f Oct 2, 2020
@worldofpeace
Copy link
Member

@worldofpeace worldofpeace commented Oct 2, 2020

Quick question, now that we don't have ancient gstreamer in top-level could we move everything out of gst_all_1?

@nh2
Copy link
Contributor Author

@nh2 nh2 commented Oct 3, 2020

Quick question, now that we don't have ancient gstreamer in top-level could we move everything out of gst_all_1?

I'd be in favour of that, either making all packages top-level, or renaming gst_all_1 to gstreamer_all, or maybe even better, both (so that you can conveniently build build the entire gstreamer attrset with an easy-to-remember name).

@doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Oct 3, 2020

I would support this as well. IIRC I once wanted to patch a package to use gst v1.0+ plugins and the default attributes in the top level were for the deprecated packages (now no longer in Nixpkgs), and it took me some time to realize why won't it work.

@nh2
Copy link
Contributor Author

@nh2 nh2 commented Oct 4, 2020

Quick question, now that we don't have ancient gstreamer in top-level could we move everything out of gst_all_1?

Just for clarification, I think this should be done in a separate PR independent from version upgrades (and I don't have the bandwidth for it).

@nh2
Copy link
Contributor Author

@nh2 nh2 commented Oct 16, 2020

@nh2
Copy link
Contributor Author

@nh2 nh2 commented Oct 22, 2020

@GrahamcOfBorg build gst_all_1 fractal pitivi

@nh2 nh2 force-pushed the nh2:gstreamer-1.18.0 branch from af1fae0 to e2f5082 Oct 22, 2020
@nh2 nh2 changed the base branch from master to staging Oct 22, 2020
@nh2
Copy link
Contributor Author

@nh2 nh2 commented Oct 22, 2020

This should be more finished now.

gst_all_1 and pitivi now build fine when based on master, and I just switched the PR to staging.

@jtojnar
Copy link
Contributor

@jtojnar jtojnar commented Oct 22, 2020

We should also update gst-python, notably

  • Python 2.x is no longer supported

Regarding hotdoc, we should remove docs:

  • API and plugin documentation are no longer built with gtk_doc . The gtk_doc documentation has been removed in favour of a new unified documentation module built with hotdoc (also see "Documentation improvements" section below). Distributors should use the documentation release tarball instead of trying to package hotdoc and building the documentation from scratch.

Next

  • gst-plugins-bad now includes an internal copy of libusrsctp, as there are problems in usrsctp with global shared state, lack of API stability guarantees, and the absence of any kind of release process. We also can't rely on distros shipping a version with the fixes we need. Both firefox and Chrome bundle their own copies too. It is still possible to build against an external copy of usrsctp if so desired.

sctp is disabled so that does not affect us.

  • nvcodec no longer needs the NVIDIA NVDEC/NVENC SDKs available at build time, only at runtime. This allows distributions to ship this plugin by default and it will just start to work when the required run-time SDK libraries are installed by the user, without users needing to build and install the plugin from source.

We are disabling nvdec & nvenc at the moment; figuring out how to load them at runtime would be painful so let’s go on with it.

  • the gst-editing-services tarball is now named gst-editing-services for consistency (used to be gstreamer-editing-services ).

not packaged.

  • the gst-validate tarball has been superseded by the gst-devtools tarball for consistency with the git module name.

✔️

@nh2 nh2 force-pushed the nh2:gstreamer-1.18.0 branch from e2f5082 to 6c6704b Oct 22, 2020
@jtojnar
Copy link
Contributor

@jtojnar jtojnar commented Oct 22, 2020

What’s up with the fixDarwinDylibNames commits?

@nh2 nh2 force-pushed the nh2:gstreamer-1.18.0 branch from 6c6704b to eba8b33 Oct 22, 2020
@nh2
Copy link
Contributor Author

@nh2 nh2 commented Oct 22, 2020

What’s up with the fixDarwinDylibNames commits?

@jtojnar Not sure what you mean, perhaps you got an email from Github in between I rebased onto staging and changed the github merge base?

@nh2
Copy link
Contributor Author

@nh2 nh2 commented Oct 22, 2020

On staging, the builds fail due to dependency ghostscript failing:

./base/fapi_ft.c:1909:1: error: control reaches end of non-void function [-Werror=return-type]
 1909 | }
      | ^
cc1: some warnings being treated as errors

I have a branch gstreamer-1.18.0-nonstaging that one can use to test non-staging.

Sounds like pitivi needs strictDeps = false;. I thought I already added that woman_shrugging

I also just figured that out, doing it in PR #101398 (and I rebased this PR on top of that commit).

With these fixes, pitivi starts fine on the new gstreamer.

@nh2 nh2 force-pushed the nh2:gstreamer-1.18.0 branch from eba8b33 to 79112e3 Oct 22, 2020
@nh2
Copy link
Contributor Author

@nh2 nh2 commented Oct 22, 2020

Regarding hotdoc, we should remove gtk:

@jtojnar I think I already did that everywhere, removing:

-    gtk-doc
-    docbook_xsl
-    docbook_xml_dtd_43

We are disabling nvdec & nvenc at the moment; figuring out how to load them at runtime would be painful so let’s go on with it.

Yes, I would like to eventually get that working to be able to encode h264 very efficiently on nvidia, so if you have an idea for it, please share :)

  • the gst-editing-services tarball is now named gst-editing-services for consistency (used to be gstreamer-editing-services ).

not packaged.

It is, pkgs/development/libraries/gstreamer/ges/default.nix. But I have updated it.

@ofborg ofborg bot added the 6.topic: python label Oct 22, 2020
Copy link
Contributor

@jtojnar jtojnar left a comment

Pitivi seems to work fine when I rebase this branch + the strictDeps patch onto unstable.

Build logs look fine too, except for unknown meson flags examples in ugly, and nvdec and nvenc in bad.

@nh2 nh2 force-pushed the nh2:gstreamer-1.18.0 branch from 79112e3 to f0d1274 Oct 22, 2020
@nh2
Copy link
Contributor Author

@nh2 nh2 commented Oct 22, 2020

Build logs look fine too, except for unknown meson flags examples in ugly, and nvdec and nvenc in bad.

Fixed.

Copy link
Contributor

@doronbehar doronbehar left a comment

Just a few formatting issues, otherwise LGTM. Thanks for working on this.

nh2 added 4 commits Oct 1, 2020
Fixes #98769.

Important changes from https://gstreamer.freedesktop.org/releases/1.18/:

* `gst-validate` was renamed to `gst-devtools` upstream:

    > * the `gst-validate` tarball has been superseded by
    >   the `gst-devtools` tarball for consistency with the git module name.

* `gst-python` is now Python 3 only:

    > * Python 2.x is no longer supported
@nh2 nh2 force-pushed the nh2:gstreamer-1.18.0 branch from d2e626c to 38c5299 Oct 24, 2020
@nh2
Copy link
Contributor Author

@nh2 nh2 commented Oct 29, 2020

I've run nixpkgs-review for a while on gstreamer-1.18.0-nonstaging; there are various failures there but I couldn't find any that are related to gstreamer.

Merging.

@nh2 nh2 merged commit e5b345c into NixOS:staging Oct 29, 2020
19 of 20 checks passed
19 of 20 checks passed
tests
Details
action
Details
gst_all_1.gst-plugins-bad, gst_all_1.gst-plugins-bad.passthru.tests, pitivi, pitivi.passthru.tests on x86_64-darwin
Details
gst_all_1.gst-plugins-bad, gst_all_1.gst-plugins-bad.passthru.tests, pitivi, pitivi.passthru.tests on aarch64-linux Failure
Details
gst_all_1.gst-plugins-bad, gst_all_1.gst-plugins-bad.passthru.tests, pitivi, pitivi.passthru.tests on x86_64-linux Failure
Details
Evaluation Performance Report Evaluator Performance Report
Details
Wait for ofborg
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="38c5299"; rev="38c52994a6334fb37b9009a638b1e5904ca4cb7f"; } ./pkgs/t
Details
grahamcofborg-eval-lib-tests nix-build --arg pkgs import ./. {} ./lib/tests/release.nix
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="38c5299"; rev="38c52994a6334fb37b9009a638b1e5904ca4cb7f"; } ./nixos/
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="38c5299"; rev="38c52994a6334fb37b9009a638b1e5904ca4cb7f"; } ./nixos/
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="38c5299"; rev="38c52994a6334fb37b9009a638b1e5904ca4cb7f"; } ./nixos/
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="38c5299"; rev="38c52994a6334fb37b9009a638b1e5904ca4cb7f"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="38c5299"; rev="38c52994a6334fb37b9009a638b1e5904ca4cb7f"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="38c5299"; rev="38c52994a6334fb37b9009a638b1e5904ca4cb7f"; } ./pkgs/t
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@nh2
Copy link
Contributor Author

@nh2 nh2 commented Nov 17, 2020

@samueldr found that this broke aarch64: https://hydra.nixos.org/build/130355045/nixlog/42

Has header "bcm_host.h" : NO 

sys/rpicamsrc/meson.build:30:4: ERROR: Problem encountered: Could not find bcm_host.h. Please pass the location of this header via -Drpi-header-dir=/path

I had tried to use ofborg to figure out whether the upgrade breaks other arches, but I couldn't tell, because staging was apparently broken on on aarch64 at the time:

https://github.com/NixOS/nixpkgs/pull/99345/checks?check_run_id=1300702358

and I don't have an aarch64 device on which I could test it well.

I think our CI story for this isn't good yet. I think ofborg should also test staging PRs against nixpkgs-unstable to be able to tell quickly whether they would work at all. Otherwise it'll always be a blind shot.


Extracted into #104099

@nh2
Copy link
Contributor Author

@nh2 nh2 commented Nov 17, 2020

It broke Darwin as well: #104062

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.