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

digikam: fix media playback by adding ffmpeg to buildInputs #56144

Merged
merged 1 commit into from Mar 9, 2019

Conversation

callahad
Copy link
Member

Motivation for this change

Since Digikam 6.0.0, media playback requires ffmpeg as a direct dependency.

This shouldn't really change the closure size, as another of Digikam's dependencies, libqtav, already depends on ffmpeg.

The difference is evident during the cmakeConfigurePhase:

--- before.txt	2019-02-21 10:10:24.312731211 +0000
+++ after.txt	2019-02-21 10:01:43.642602245 +0000
@@ -1,13 +1,13 @@
--- Could NOT find FFmpeg (missing: FFMPEG_LIBRARIES FFMPEG_INCLUDE_DIRS AVCODEC_LIBRARIES AVCODEC_INCLUDE_DIRS AVFILTER_LIBRARIES AVFILTER_INCLUDE_DIRS AVFORMAT_LIBRARIES AVFORMAT_INCLUDE_DIRS AVUTIL_LIBRARIES AVUTIL_INCLUDE_DIRS SWSCALE_LIBRARIES SWSCALE_INCLUDE_DIRS) 
--- FFMPEG_FOUND        = FALSE
--- FFMPEG_INCLUDE_DIRS = 
--- FFMPEG_LIBRARIES    = 
--- FFMPEG_DEFINITIONS  = 
+-- Found FFmpeg: /nix/store/naf33s4w5kpxv060vmjv6cz5jvcqnxrb-ffmpeg-3.4.5/lib/libavcodec.so;/nix/store/naf33s4w5kpxv060vmjv6cz5jvcqnxrb-ffmpeg-3.4.5/lib/libavfilter.so;/nix/store/naf33s4w5kpxv060vmjv6cz5jvcqnxrb-ffmpeg-3.4.5/lib/libavformat.so;/nix/store/naf33s4w5kpxv060vmjv6cz5jvcqnxrb-ffmpeg-3.4.5/lib/libavutil.so;/nix/store/naf33s4w5kpxv060vmjv6cz5jvcqnxrb-ffmpeg-3.4.5/lib/libswscale.so
+-- FFMPEG_FOUND        = TRUE
+-- FFMPEG_INCLUDE_DIRS = /nix/store/bsgkvbqniycnf94xv8dmlszazqrlas7n-ffmpeg-3.4.5-dev/include
+-- FFMPEG_LIBRARIES    = /nix/store/naf33s4w5kpxv060vmjv6cz5jvcqnxrb-ffmpeg-3.4.5/lib/libavcodec.so;/nix/store/naf33s4w5kpxv060vmjv6cz5jvcqnxrb-ffmpeg-3.4.5/lib/libavfilter.so;/nix/store/naf33s4w5kpxv060vmjv6cz5jvcqnxrb-ffmpeg-3.4.5/lib/libavformat.so;/nix/store/naf33s4w5kpxv060vmjv6cz5jvcqnxrb-ffmpeg-3.4.5/lib/libavutil.so;/nix/store/naf33s4w5kpxv060vmjv6cz5jvcqnxrb-ffmpeg-3.4.5/lib/libswscale.so
+-- FFMPEG_DEFINITIONS  =
 -- QtAV search path: /nix/store/yjj7f7lypknwzbxvkvjkf8aa0av6ss0d-qtbase-5.12.0-dev/lib/cmake
 -- Found QtAV: /nix/store/ncxf8s0jlqmh1cvlp9day3byh45mxqb3-libqtav-1.12.0/lib/libQtAV.so;/nix/store/ncxf8s0jlqmh1cvlp9day3byh45mxqb3-libqtav-1.12.0/lib/libQtAVWidgets.so
 -- Found QtAV version 1.12.0
 -- QtAV_FOUND       = TRUE
 -- QtAV_INCLUDE_DIR = /nix/store/ncxf8s0jlqmh1cvlp9day3byh45mxqb3-libqtav-1.12.0/include/QtAV /nix/store/ncxf8s0jlqmh1cvlp9day3byh45mxqb3-libqtav-1.12.0/include/QtAVWidgets
 -- QtAV_LIBRARIES   = /nix/store/ncxf8s0jlqmh1cvlp9day3byh45mxqb3-libqtav-1.12.0/lib/libQtAV.so;/nix/store/ncxf8s0jlqmh1cvlp9day3byh45mxqb3-libqtav-1.12.0/lib/libQtAVWidgets.so
 -- QtAV_VERSION     = 1.12.0
--- ENABLE_MEDIAPLAYER option is enabled but FFMpeg cannot be found. Media player support is disabled.
+-- Media player support is enabled.

...and during program execution, in the Help → Components Information menu:

digikam-ffmpeg

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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Since version 6.0.0, media playback in Digikam requires ffmpeg.
@aanderse
Copy link
Member

aanderse commented Mar 6, 2019

@GrahamcOfBorg build digikam
@callahad next time no need for an issue if you already have a PR.

@callahad
Copy link
Member Author

callahad commented Mar 6, 2019

Gotcha. For next time, what's the best way to get a pull request reviewed after it's been sitting for a few days?

@aanderse
Copy link
Member

aanderse commented Mar 7, 2019

Timed out... I'll try this again then:

@GrahamcOfBorg build digikam

@callahad to get your PRs merged you want to make them more appealing to people with commit access. You should look for people who might not have commit access, but have been doing a fair bit of work and review on nixpkgs. If you get people like this to review and approve your PR then people with commit access will be more like to accept your PR as the PR has already been reviewed and the amount of work required has been lowered.

#50105 (comment)

Another effective way I've found has simply been to bug people on IRC or discourse until they merge your PRs 😃

@aanderse
Copy link
Member

aanderse commented Mar 7, 2019

@GrahamcOfBorg build enblend-enfuse vigra python27Packages.numpy openblas

@aanderse
Copy link
Member

aanderse commented Mar 7, 2019

The aarch64-linux issue seems to be with dependencies and unrelated to this PR.

EDIT: confirmed via #56949

Copy link
Member

@aanderse aanderse left a comment

Choose a reason for hiding this comment

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

This builds on my system.
LGTM 👍

@aanderse
Copy link
Member

aanderse commented Mar 9, 2019

@markuskowa seeing as you're aware of the openblas issue on aarch64 issue, is there any chance you'd be willing to merge?

@markuskowa markuskowa merged commit 2debad9 into NixOS:master Mar 9, 2019
@aanderse
Copy link
Member

aanderse commented Mar 9, 2019

@markuskowa Thanks! 🎉

@markuskowa
Copy link
Member

Thanks. LGTM. Let hydra do the build. This seems to be too big for OfBorg.

@callahad callahad deleted the digikam-media-player branch April 23, 2019 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants