From 7e484e2ae6c0d91e246c080d9e56568fcdb32ba7 Mon Sep 17 00:00:00 2001 From: Paul Berberian Date: Thu, 1 Feb 2024 16:03:26 +0100 Subject: [PATCH] WASM: Temporarily rollback on wasm-opt dependency for release builds to make it work on some lg TVs After testing at large scale our `MULTI_THREAD` feature, we noticed that some LG and samsung TVs had issues running it. The problem in question was due to an unrecognized WebAssembly Op Code by the device linked to the "sign-extensions" proposal (https://github.com/WebAssembly/sign-extension-ops/blob/master/proposals/sign-extension-ops/Overview.md) which was not introduced initially with the so-called "WebAssembly MVP" (the initial release of WebAssembly). It turns out that rustc (the Rust compiler) targets a WebAssembly version that is a little further than the MVP by default (it seems that this decision is actually taken by the LLVM project from what I understand from an answers in https://github.com/rust-lang/rust/issues/109807) and amongst the included features is the sign-extensions feature (so the rest of included feature is totally unclear and I could not find any resource on the web listing them. From there, I was able to remove the sign-extensions Op Code from the built WebAssembly file by doing either one of these two ways: 1. Through a rustc flag (`-C target-feature=-sign-ext`). It should be noted that the inclusion of this flag prints a warning notice indicating that the flag is "unstable" Interestingly, a `-C target-cpu=mvp` flag, which looks like it is exactly what we want, is also listed in that page yet it doesn't seem to remove the sign-extensions opcode for me. Maybe because it takes an already-compiled stdlib and the problematic opcode is from there? We could re-build stdlib through another flag but this latter one is marked as "unstable" so I didn't want to adventure too far into this theory and I didn't either understand why the other flag would have an effect in that case. 2. By adding the `--signext-lowering` flag to binaryen's wasm-opt tool which is documented at: > lower sign-ext operations to wasm mvp and disable the sign > extension feature That solution is nice but I couldn't rely on that flag with the [binaryen npm module](https://www.npmjs.com/package/binaryen) we now rely on (because RxPlayer developers most likely have npm installed so it was seen as a simpler dependency in the project than binaries brought by a binaryen project that has to be installed separately). This means that to add this feature, I have to bring back the full `binaryen` package as a dependency of the RxPlayer which isn't nice. Even with its drawbacks, I chose to go with the second solution here because I was afraid due to its warning notice, the fact that the other rustc flag (-C target=mvp) didn't have the expected effect and the hint that this might not work when the feature is in stdlib. We may come back to this in the future. Still, this only fix the issue with the `sign-extensions` opcode, and not the real larger issue which is to either properly handle all devices supporting WebAssembly or to be able to detect and fallback when a device fails to compile it. This hypotetical scenario (for now, though may be more common in the future), will be handled in a future PR. It only acts on that feature. Future WebAssembly builds could break if other new features are not handled by the device. --- .github/workflows/checks.yml | 1 + package-lock.json | 17 ----- package.json | 1 - scripts/wasm-optimize.mjs | 63 ------------------- .../manifest/dash/wasm-parser/README.md | 11 ++-- 5 files changed, 8 insertions(+), 85 deletions(-) delete mode 100644 scripts/wasm-optimize.mjs diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 6ce203daa6c..5f6898a9682 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -66,6 +66,7 @@ jobs: cache: 'npm' # needed for integration & memory tests codecs support - run: sudo add-apt-repository multiverse && sudo apt update && sudo apt install -y ubuntu-restricted-extras + - run: sudo apt-get -y install binaryen - run: npm install - run: npm run build # Firefox seems to have issue with integration tests on GitHub actions only diff --git a/package-lock.json b/package-lock.json index 84d205a5f00..5e48892bae5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -25,7 +25,6 @@ "@typescript-eslint/parser": "6.16.0", "arraybuffer-loader": "1.0.8", "babel-loader": "9.1.3", - "binaryen": "116.0.0", "chai": "4.3.10", "core-js": "3.34.0", "esbuild": "0.19.10", @@ -4825,16 +4824,6 @@ "node": ">=8" } }, - "node_modules/binaryen": { - "version": "116.0.0", - "resolved": "https://registry.npmjs.org/binaryen/-/binaryen-116.0.0.tgz", - "integrity": "sha512-Hp0dXC6Cb/rTwWEoUS2BRghObE7g/S9umKtxuTDt3f61G6fNTE/YVew/ezyy3IdHcLx3f17qfh6LwETgCfvWkQ==", - "dev": true, - "bin": { - "wasm-opt": "bin/wasm-opt", - "wasm2js": "bin/wasm2js" - } - }, "node_modules/body-parser": { "version": "1.20.1", "resolved": "https://registry.npmjs.org/body-parser/-/body-parser-1.20.1.tgz", @@ -18190,12 +18179,6 @@ "integrity": "sha512-jDctJ/IVQbZoJykoeHbhXpOlNBqGNcwXJKJog42E5HDPUwQTSdjCHdihjj0DlnheQ7blbT6dHOafNAiS8ooQKA==", "dev": true }, - "binaryen": { - "version": "116.0.0", - "resolved": "https://registry.npmjs.org/binaryen/-/binaryen-116.0.0.tgz", - "integrity": "sha512-Hp0dXC6Cb/rTwWEoUS2BRghObE7g/S9umKtxuTDt3f61G6fNTE/YVew/ezyy3IdHcLx3f17qfh6LwETgCfvWkQ==", - "dev": true - }, "body-parser": { "version": "1.20.1", "resolved": "https://registry.npmjs.org/body-parser/-/body-parser-1.20.1.tgz", diff --git a/package.json b/package.json index 863acfb8c91..ab18b453687 100644 --- a/package.json +++ b/package.json @@ -217,7 +217,6 @@ "@typescript-eslint/parser": "6.16.0", "arraybuffer-loader": "1.0.8", "babel-loader": "9.1.3", - "binaryen": "116.0.0", "chai": "4.3.10", "core-js": "3.34.0", "esbuild": "0.19.10", diff --git a/scripts/wasm-optimize.mjs b/scripts/wasm-optimize.mjs deleted file mode 100644 index 0a3216fe4f3..00000000000 --- a/scripts/wasm-optimize.mjs +++ /dev/null @@ -1,63 +0,0 @@ -/** - * ============= wasm-optimize util ============= - * - * == What is this? - * - * This file allows to optimize a WebAssembly binary file (`.wasm`) by running - * binaryen's wasm-opt tool on it, through its JavaScript API. - * - * To run it, provide the source WebAssembly file as first argument and the - * output as second: - * ``` - * node wasm-optimize.mjs source.wasm dest.wasm - * ``` - * - * == Why? - * - * As the WebAssembly file produced by the RxPlayer is mostly for performance - * enhancement, it is important that we squeeze the most performance out of it - * at compile-time. - * - * The wasm-opt tool specically optimizes WebAssembly files, it is different - * from performance improvements made by the initial source compiler (e.g. the - * Rust compiler) which may not have the same constraints. Whether this script - * brings or not an improvement in comparison to the source WebAssembly file - * still should probably be regularly checked in real-life scenarios. - */ - -import binaryen from "binaryen"; -import * as fs from "fs"; - -run(); -function run() { - let inputFileName; - let outputFileName; - - if (process.argv.length < 3) { - console.error("Error: missing input file as first argument"); - process.exit(1); - } - if (process.argv.length < 4) { - console.error("Error: missing output file as second argument"); - process.exit(1); - } - inputFileName = process.argv[2]; - outputFileName = process.argv[3]; - - console.log("Starting logic to optimize wasm file:", inputFileName); - - let dataU8; - try { - const data = fs.readFileSync(inputFileName); - dataU8 = new Uint8Array(data.buffer); - binaryen.setOptimizeLevel(4); - const module = binaryen.readBinary(dataU8); - module.optimize(); - const output = module.emitBinary(); - fs.writeFileSync(outputFileName, output); - } catch (err) { - console.error("Error:", err?.message ?? "Unknown"); - process.exit(1); - } - console.log("WASM successfuly optimized!"); -} diff --git a/src/parsers/manifest/dash/wasm-parser/README.md b/src/parsers/manifest/dash/wasm-parser/README.md index 74b781b3aff..01d950c093b 100644 --- a/src/parsers/manifest/dash/wasm-parser/README.md +++ b/src/parsers/manifest/dash/wasm-parser/README.md @@ -251,12 +251,13 @@ fear that they may change in the future, in which case this documentation could easily be outdated. To be able to call those scripts, you will need to have the Rust compiler -toolchain installed and ready to compile to WebAssembly. +toolchain installed and ready to compile to WebAssembly as well as `binaryen`, a +WebAssembly-specialized toolbox. -To do this, the easiest way would be to rely on `rustup`, a tool to install and -update Rust toolchains: +There are several ways this can be done, with the easiest way generally being: - 1. Install [rustup](https://rustup.rs/) + 1. Install [rustup](https://rustup.rs/), which is a tool intended to + facilitate the installation of Rust toolchains 2. Install and rely on the stable toolchain: ```sh @@ -268,4 +269,6 @@ update Rust toolchains: rustup target add wasm32-unknown-unknown ``` + 4. Now install [binaryen](https://github.com/WebAssembly/binaryen) + That should be it!