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.

Sign up for GitHub

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

Jump to bottom

libsForQt5.qtwebengine: use ffmpeg instead of ffmpeg_3 #120646

Merged
merged 2 commits into from
May 6, 2021
Merged

libsForQt5.qtwebengine: use ffmpeg instead of ffmpeg_3 #120646

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6de7e0d
libsForQt5.qtwebengine: use ffmpeg instead of ffmpeg_3
dotlambda Apr 25, 2021
6ba8371
libsForQt5.qtwebengine: always use system ffmpeg
dotlambda May 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 4 additions & 6 deletions pkgs/development/libraries/qt-5/modules/qtwebengine.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
, enableProprietaryCodecs ? true
, gn
, cups, darwin, openbsm, runCommand, xcbuild, writeScriptBin
, ffmpeg_3 ? null
, ffmpeg ? null
, lib, stdenv, fetchpatch
, version ? null
, qtCompatVersion
Expand Down Expand Up @@ -140,9 +140,8 @@ qtModule {
fi
'';

qmakeFlags = if stdenv.hostPlatform.isAarch32 || stdenv.hostPlatform.isAarch64
then [ "--" "-system-ffmpeg" ] ++ optional enableProprietaryCodecs "-proprietary-codecs"
else optional enableProprietaryCodecs "-- -proprietary-codecs";
qmakeFlags = [ "--" "-system-ffmpeg" ]
++ optional enableProprietaryCodecs "-proprietary-codecs";

propagatedBuildInputs = [
# Image formats
Expand All @@ -158,8 +157,7 @@ qtModule {
harfbuzz icu

libevent
] ++ optionals (stdenv.hostPlatform.isAarch32 || stdenv.hostPlatform.isAarch64) [
ffmpeg_3
ffmpeg
Copy link
Member Author

Choose a reason for hiding this comment

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

Does anyone know why this is only used on ARM?
Do we use a bundled Ffmpeg on other platforms? If so, we shouldn't.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at qmakeFlags above:

  qmakeFlags = if stdenv.hostPlatform.isAarch32 || stdenv.hostPlatform.isAarch64
    then [ "--" "-system-ffmpeg" ] ++ optional enableProprietaryCodecs "-proprietary-codecs"
    else optional enableProprietaryCodecs "-- -proprietary-codecs";

Maybe we shouldn't? Looks like it was changed in e01c613, but only done for aarch64 to avoid a rebuild.

It's interesting that OfBorg claims this causes 0 rebuilds...

Copy link
Member Author

@dotlambda dotlambda May 1, 2021 •
edited

Choose a reason for hiding this comment

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

Maybe we shouldn't? Looks like it was changed in e01c613, but only done for aarch64 to avoid a rebuild.

Yes, let's use system ffmpeg on all platforms.

It's interesting that OfBorg claims this causes 0 rebuilds...

That's because ofborg only checks on x86_64.

Copy link
Member

@samueldr samueldr May 1, 2021 •
edited

Choose a reason for hiding this comment

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

Nope, ofborg checks aarch64-linux too. The ^.^ check includes the list of changed paths.

The reported count, though, might be x86_64 only.

Copy link
Member Author

Choose a reason for hiding this comment

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

The reported count, though, might be x86_64 only.

That's what I meant.

] ++ optionals (!stdenv.isDarwin) [
dbus zlib minizip snappy nss protobuf jsoncpp

Expand Down