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

ffmpeg: misc cleanups #50220

Merged
merged 18 commits into from Jan 27, 2019
Merged

ffmpeg: misc cleanups #50220

merged 18 commits into from Jan 27, 2019

Conversation

pbogdan
Copy link
Member

@pbogdan pbogdan commented Nov 11, 2018

Motivation for this change

There are 5 versions of ffmpeg included in nixpkgs, 2 of which are not supported upstream as I understand it. This attempts to switch packages to a more up to date version where possible. As a result ffmpeg_1 can be dropped; ffmpeg_0 still remains :/ with a single reverse dependency gst-ffmpeg. The packages depending on gst-ffmpeg are:

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-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)
  • Fits CONTRIBUTING.md.

I verified things build on x86_64-linux but not tested much beyond that.

@jtojnar
Copy link
Contributor

jtojnar commented Nov 13, 2018

Related issues: #22372, #15930


👍 for dropping Miro, for #39975

url = "https://bitbucket.org/acoustid/acoustid-fingerprinter/commits/632e87969c3a5562a5d4842b03613267ba6236b2/raw";
sha256 = "15hm9knrpqn3yqrwyjz4zh2aypwbcycd0c5svrsy1fb2h2rh05jk";
})
./ffmpeg.patch
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add source of the path in a comment?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote it myself so this is the source 😆 I would probably need to re-work it a bit before sending upstream so that it works with older versions too.

@@ -23,7 +23,7 @@ let
curl
dbus
expat
ffmpeg_0_10
ffmpeg
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this really work (can local files be played)? Arch package requires https://aur.archlinux.org/packages/ffmpeg-compat-54/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the symlinks I added further down in this file it does, yes. I tested locally and playing files from a local library works fine.

@@ -9525,9 +9525,6 @@ with pkgs;
ffmpeg_0_10 = callPackage ../development/libraries/ffmpeg/0.10.nix {
inherit (darwin.apple_sdk.frameworks) Cocoa;
};
ffmpeg_1_2 = callPackage ../development/libraries/ffmpeg/1.2.nix {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you delete the file as well?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thanks for the catch.

@jtojnar jtojnar added this to To do in Picking up garbage via automation Nov 13, 2018
@jtojnar jtojnar moved this from To do to In progress in Picking up garbage Nov 13, 2018
@Mic92
Copy link
Member

Mic92 commented Nov 13, 2018

cc @rnhmjoj regarding gnash

@pbogdan pbogdan mentioned this pull request Nov 14, 2018
9 tasks
@rnhmjoj
Copy link
Contributor

rnhmjoj commented Nov 14, 2018

It should be safe to remove, it didn't really work anyway.

@Mic92
Copy link
Member

Mic92 commented Nov 14, 2018

I doubt that the tor-browser still uses gst-ffmpeg, because it is based on firefox. cc @joachifm

@pbogdan
Copy link
Member Author

pbogdan commented Nov 14, 2018

@Mic92 Yeah, I believe it should be safe to remove as gstreamer support seems to be gone (only looked into it today). I started working on a change that removes gstreamer dependencies from firefox and tor-browser expressions (FF doesn't use gst-ffmpeg but refers to some other legacy gstreamer packages).

@rnhmjoj rnhmjoj mentioned this pull request Nov 16, 2018
9 tasks
@veprbl
Copy link
Member

veprbl commented Dec 12, 2018

@pbogdan What is the status of this? What kind of testing did you do?

@infinisil infinisil added this to the 19.03 milestone Jan 26, 2019
@infinisil
Copy link
Member

Nice work! Can you fix the merge conflict? Post a comment when done (GitHub doesn't send out notifications for new commits), I can then quickly test some things myself and merge it after that.

@pbogdan
Copy link
Member Author

pbogdan commented Jan 27, 2019

@infinisil Thanks, conflicts resolved.

@infinisil
Copy link
Member

@GrahamcOfBorg build acoustidFingerprinter
@GrahamcOfBorg build cantata
@GrahamcOfBorg build renpy
@GrahamcOfBorg build ffmpegthumbnailer
@GrahamcOfBorg build ffms
@GrahamcOfBorg build xineLib
@GrahamcOfBorg build avxsynth
@GrahamcOfBorg build baresip
@GrahamcOfBorg build freerdp_legacy
@GrahamcOfBorg build guvcview

@infinisil infinisil merged commit fede414 into NixOS:master Jan 27, 2019
Picking up garbage automation moved this from In progress to Done Jan 27, 2019
@infinisil infinisil mentioned this pull request Jan 27, 2019
vcunat added a commit that referenced this pull request Feb 1, 2019
This fixes evaluation.  I'm not sure why pantheon.elementary-videos
was mixing gst 1 with old gst-ffmpeg.
/cc #48637, #50220.
dtzWill pushed a commit to dtzWill/nixpkgs that referenced this pull request Feb 5, 2019
This fixes evaluation.  I'm not sure why pantheon.elementary-videos
was mixing gst 1 with old gst-ffmpeg.
/cc NixOS#48637, NixOS#50220.
@pbogdan pbogdan deleted the ffmpeg-cleanup branch December 3, 2019 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

7 participants