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

widevine-cdm: add aarch64 support #337261

Merged
merged 3 commits into from
Aug 29, 2024
Merged

Conversation

dxwil
Copy link
Contributor

@dxwil dxwil commented Aug 25, 2024

Description of changes

Adds support for arm64 widevine using code from widevine-installer. The package builds and outputs the relevant files in the same locations as the x64 version, although I cannot test it because I don't know how to make chromium.enableWideVine select a local version, it tries to install the x64 one (please help me on this one :).

There are also some things I would like input on from someone else:

  1. Versioning between the two builds. Currently arm is using an older version, it's probably a good idea to make them the same, but without being able to test, I can't do that, because I don't know if newer versions exist and can be successfully patched for arm. List of chromeos images containing arm64 widevine here.
  2. Firefox support. Shouldn't be hard to implement if we come up with a system. Chromium has the enableWideVine flag that as I'm aware does not exist for Firefox. All that Firefox needs are some options as well as one env variable, refer to this. I could build this into the package but that would mean installing the package would configure Firefox automatically, which is different from the behavior that exists with Chromium (where enableWideVine needs to be set).

Edit: after some googling I see that Firefox drm works out of the box for x64, but for arm the changes linked above are still necessary.

Edit 2: What I don't understand is why Firefox doesn't need a seperate flag for widevine like chromium, is it because Firefox only installs widevine if drm is explicitly enabled and chromium auto installs it on launch? Could it also have something to do with licenses, why isn't Firefox unfree when it has widevine and chromium is?

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/widevine-aarch64-input-needed/51209/1

@RossComputerGuy RossComputerGuy self-assigned this Aug 26, 2024
@RossComputerGuy
Copy link
Member

Cool, thank you for putting in the effort. I was meaning to get to this myself but just haven't found the time. I'll review and test this tonight.


runHook postInstall
'';

# Accoring to widevine-installer: "Hack because Chromium hardcodes a check for this right now..."
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cannot verify without being able to test

@RossComputerGuy
Copy link
Member

Result of nixpkgs-review pr 337261 run on aarch64-linux 1

1 package built:
  • widevine-cdm

Copy link
Member

@RossComputerGuy RossComputerGuy left a comment

Choose a reason for hiding this comment

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

Good start, got some changes.

'';
} else null;

src = if stdenv.isAarch64 then fetchurl {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
src = if stdenv.isAarch64 then fetchurl {
src = fetchurl (if stdenv.isAarch64 then {
url = "https://commondatastorage.googleapis.com/chromeos-localmirror/distfiles/chromeos-lacros-arm64-squash-zstd-${lacrosVersion}";
hash = "sha256-OKV8w5da9oZ1oSGbADVPCIkP9Y0MVLaQ3PXS3ZBLFXY=";
} else {
url = "https://dl.google.com/widevine-cdm/${version}-linux-x64.zip";
hash = "sha256-lGTrSzUk5FluH1o4E/9atLIabEpco3C3gZw+y6H6LJo=";
stripRoot = false;
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

from my testing fetchurl doesn't know how to unpack a zip file without specifying an unpackPhase

@dxwil dxwil force-pushed the widevine-cdm-arm64 branch 3 times, most recently from 5921521 to 9a83b97 Compare August 27, 2024 07:40
@dxwil
Copy link
Contributor Author

dxwil commented Aug 27, 2024

I messed up something with the commits... now the add maintainers one is called the same as the other ones

@dxwil dxwil force-pushed the widevine-cdm-arm64 branch 3 times, most recently from 27af374 to 9bf71ba Compare August 27, 2024 08:34
@dxwil
Copy link
Contributor Author

dxwil commented Aug 27, 2024

@RossComputerGuy do you know how I can get enableWideVine in chromium to select a local package so I can test

@emilylange
Copy link
Member

emilylange commented Aug 27, 2024

Unsure if I understand the question correctly, but to get chromium with enableWideVine = true from your local nixpkgs checkout, you can use

NIXPKGS_ALLOW_UNFREE=1 nix-build -E "(import ./. {}).chromium.override { enableWideVine = true; }"

@dxwil
Copy link
Contributor Author

dxwil commented Aug 27, 2024

Unsure if I understand the question correctly, but to get chromium with enableWideVine = true from your local nixpkgs checkout, you can use

NIXPKGS_ALLOW_UNFREE=1 nix-build -E "(import ./. {}).chromium.override { enableWideVine = true; }"

This is the part that's responsible for enabling widevine in chromium, it would fetch ${widevine-cdm} from upstream. How can I make it use a locally built version of widevine because upstream does not support aarch64, and would that also require for me to rebuild chromium?

@RossComputerGuy
Copy link
Member

it would fetch ${widevine-cdm} from upstream.

Where does it do that?

How can I make it use a locally built version of widevine because upstream does not support aarch64

You can run that command in your branch.

and would that also require for me to rebuild chromium?

Any dependency or derivation changes to Chromium would cause a rebuild.

@RossComputerGuy
Copy link
Member

That command works and took very little time on my own hardware (note the time nom-build outputted was because I had built it before).
image

@RossComputerGuy
Copy link
Member

I did a quick look at Tidal HiFi and we'll have to build from source instead of the prebuilt x86_64 binary. However, I did find this issue when looking into their issues. castlabs/electron-releases#23 (comment)

  1. The CDM contains video decoders, and thus requires MPEG-LA licensing. As long as the CDM is hosted by Google and dynamically installed from their servers, the license they have is enough. Hosting and/or bundling the CDM outside of Google could incur license requirements, and thus have legal consequences.

I am not 100% certain if this could cause us any legal issues. Just bringing this up so more knowledable individuals could say whether this is a problem or not.

@dxwil
Copy link
Contributor Author

dxwil commented Aug 28, 2024

I did a quick look at Tidal HiFi and we'll have to build from source instead of the prebuilt x86_64 binary.

Could u elaborate?

@dxwil
Copy link
Contributor Author

dxwil commented Aug 28, 2024

I am not 100% certain if this could cause us any legal issues. Just bringing this up so more knowledable individuals could say whether this is a problem or not.

Comparing to the x64 version, we're only applying one patch, that's the whole difference. Is the patch something that could bring trouble?

@RossComputerGuy
Copy link
Member

Could u elaborate?

Can't easily run x64 binaries on arm64

Comparing to the x64 version, we're only applying one patch, that's the whole difference.

Well, it's to a proprietary executable. That executable contains video decoders which are proprietary as well. As long as the binary isn't redistributed there doesn't seem to be any legal concerns.

@dxwil
Copy link
Contributor Author

dxwil commented Aug 29, 2024

I tried tidal-hifi and it obviously doesn't launch because it's only downloading the x64 binaries, I don't know why the nix package shows support for aarch64, it shouldn't in the first place.

Would you say the chrome install as done now is figured out and good enough to merge? I would still like to come up with something for Firefox as well as proper versioning though.

@RossComputerGuy
Copy link
Member

Would you say the chrome install as done now is figured out and good enough to merge?

Yeah, Chrome is good enough.

I would still like to come up with something for Firefox as well as proper versioning though.

I think we should bring the Firefox maintainers into the discussion. Maybe after merge, we create an issue and ping them so it doesn't clutter up this PR.

@dxwil
Copy link
Contributor Author

dxwil commented Aug 29, 2024

Would you say the chrome install as done now is figured out and good enough to merge?

Yeah, Chrome is good enough.

I would still like to come up with something for Firefox as well as proper versioning though.

I think we should bring the Firefox maintainers into the discussion. Maybe after merge, we create an issue and ping them so it doesn't clutter up this PR.

Yeah good idea, so anything else from me or can you merge?

Copy link
Member

@RossComputerGuy RossComputerGuy left a comment

Choose a reason for hiding this comment

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

LGTM

@RossComputerGuy RossComputerGuy merged commit 83ad471 into NixOS:master Aug 29, 2024
26 of 27 checks passed
@dxwil
Copy link
Contributor Author

dxwil commented Aug 29, 2024

PS. We cannot build tidal hifi from source because it uses Castlab's electron with widevine built-in which does not support aarch64. So I'll just open a PR to remove advertised aarch64 support.

@dxwil dxwil deleted the widevine-cdm-arm64 branch August 29, 2024 16:45
@dxwil
Copy link
Contributor Author

dxwil commented Aug 29, 2024

That command works and took very little time on my own hardware (note the time nom-build outputted was because I had built it before). image

Out of interest, what CPU are you using. I'm running the same command right now and it's been building for an hour and from the task number it looks like it's gonna take another 3 minimum. I'm on an Apple m1.

buildInputs = [ squashfsTools curl python3 ];

unpackPhase = if stdenv.isAarch64 then ''
curl -# -o lacros.squashfs "file://$src"
Copy link
Member

Choose a reason for hiding this comment

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

Why did you copy this from upstream? Just copy the file...?

Copy link
Contributor Author

@dxwil dxwil Aug 29, 2024

Choose a reason for hiding this comment

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

Could you elaborate, I don't think I understand? unsquashfs wouldn't work without renaming using curl like widevine-installer does, but I only tried a few times, maybe it makes sense to figure this part out and remove curl from the buildInputs if that's what you mean.

Copy link
Member

Choose a reason for hiding this comment

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

curl in this case is a glorified copy or symlink. It makes sense to use it upstream because they're downloading from a URL. Please try cp or lning the file to see if that works, it should.

@K900
Copy link
Contributor

K900 commented Aug 29, 2024

I see code copy-pasted without understanding from Asahi, I see comments marked as resolved without being addressed, this is not the kind of change we want to have. Reverting in #338262, please open a new PR and iterate.

RossComputerGuy pushed a commit to ExpidusOS/nixpkgs that referenced this pull request Sep 21, 2024
Credit to @dxwil for the original attempt in NixOS#337261, this is a
reattempt at adding aarch64-linux support to Widevine CDM based on how
Asahi Linux deals with it.

Signed-off-by: Tristan Ross <[email protected]>
RossComputerGuy pushed a commit to ExpidusOS/nixpkgs that referenced this pull request Sep 21, 2024
Credit to @dxwil for the original attempt in NixOS#337261, this is a
reattempt at adding aarch64-linux support to Widevine CDM based on how
Asahi Linux deals with it.

Signed-off-by: Tristan Ross <[email protected]>
RossComputerGuy pushed a commit to ExpidusOS/nixpkgs that referenced this pull request Sep 23, 2024
Credit to @dxwil for the original attempt in NixOS#337261, this is a
reattempt at adding aarch64-linux support to Widevine CDM based on how
Asahi Linux deals with it.

Signed-off-by: Tristan Ross <[email protected]>
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.

6 participants