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

firefox-beta-bin: 79.0b7 -> 80.0b5 #95023

Merged
merged 1 commit into from Aug 14, 2020
Merged

firefox-beta-bin: 79.0b7 -> 80.0b5 #95023

merged 1 commit into from Aug 14, 2020

Conversation

Atemu
Copy link
Member

@Atemu Atemu commented Aug 10, 2020 •
edited

Motivation for this change

FF80 brings VAAPI decoding to X11 which I can confirm to be working.

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.

samuelgrf
samuelgrf suggested changes Aug 10, 2020
View reviewed changes
Copy link
Member

@samuelgrf samuelgrf left a comment •
edited

Choose a reason for hiding this comment

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

I've just tested this and VAAPI decoding doesn't work for me.

For VAAPI to work correctly libva needs to be added to libPath and ffmpeg_3 needs to be changed to ffmpeg_4.

These changes are also mentioned in #81917

@Atemu
Copy link
Member Author

Atemu commented Aug 10, 2020

libva and mesa are in the path already thanks to the changes for VAAPI on wayland and it works with ffmpeg_3 on my machine. Does it work if you s/ffmpeg_3/ffmpeg/g?

Firefox quite a bit of setup to actually use VAAPI, have you verified that your profile works on wayland?
What's the output of MOZ_LOG="PlatformDecoderModule:3" MOZ_X11_EGL=1 firefox when you open a video?

samuelgrf
samuelgrf approved these changes Aug 10, 2020
View reviewed changes
Copy link
Member

@samuelgrf samuelgrf left a comment

Choose a reason for hiding this comment

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

Forget the last review. I was accidentally building firefox-beta-bin from the stable branch, where ffmpeg_4 and libva aren't part of the wrapper.

[Child 27413: MediaPDecoder #2]: D/PlatformDecoderModule VA-API FFmpeg init successful shows up in the log when building firefox from unstable with this pr.

I think all the wayland vaapi changes and this pr should be backported to 20.03 once this is merged.

@samuelgrf
Copy link
Member

samuelgrf commented Aug 10, 2020 •
edited

But we should probably still change ffmpeg_3 to ffmpeg in firefox-bin/default.nix, so the version is consistent with the wrapper.

@Atemu
Copy link
Member Author

Atemu commented Aug 10, 2020

I'd leave that for a separate PR though as it works fine without and the change would affect packages other than firefox-beta-bin; dropbox apparently being one of them.

@samuelgrf
Copy link
Member

Makes sense, I'll open a separate PR for that later.

@Atemu Atemu requested a review from adisbladis August 14, 2020 10:37
@adisbladis adisbladis merged commit 3b3a080 into NixOS:master Aug 14, 2020
@Atemu Atemu deleted the firefox-beta-bin-update branch August 14, 2020 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samuelgrf samuelgrf samuelgrf approved these changes

@adisbladis adisbladis Awaiting requested review from adisbladis

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Atemu @samuelgrf @adisbladis