Skip to content

Commit 280df78

Browse files
committed
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" (initial release of the feature). 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 rust-lang/rust#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 print a deprecation notice. 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. 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 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 both the deprecation notices and the fact that another rustc flag (-C target=mvp) didn't have the expected effect. 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.
1 parent d134224 commit 280df78

File tree

1 file changed

+1
-1
lines changed

1 file changed

+1
-1
lines changed

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@
132132
"build:dev": "./scripts/generate_build.mjs --dev-mode",
133133
"build:all": "npm run build:wasm:release && npm run bundle && npm run bundle:min && npm run build",
134134
"build:wasm:debug": "mkdir -p dist && cd ./src/parsers/manifest/dash/wasm-parser && cargo build --target wasm32-unknown-unknown && cp target/wasm32-unknown-unknown/debug/mpd_node_parser.wasm ../../../../../dist/mpd-parser.wasm",
135-
"build:wasm:release": "mkdir -p dist && cd ./src/parsers/manifest/dash/wasm-parser && cargo build --target wasm32-unknown-unknown --release && node ../../../../../scripts/wasm-optimize.mjs target/wasm32-unknown-unknown/release/mpd_node_parser.wasm ../../../../../dist/mpd-parser.wasm && cd ../../../../../ && npm run wasm-strip",
135+
"build:wasm:release": "mkdir -p dist && cd ./src/parsers/manifest/dash/wasm-parser && cargo build --target wasm32-unknown-unknown --release && wasm-opt target/wasm32-unknown-unknown/release/mpd_node_parser.wasm --signext-lowering -O4 -o ../../../../../dist/mpd-parser.wasm && cd ../../../../../ && npm run wasm-strip",
136136
"bundle": "webpack --progress --config webpack.config.mjs --env production",
137137
"bundle:min": "webpack --progress --config webpack.config.mjs --env minify --env production",
138138
"bundle:min:watch": "webpack --progress --config webpack.config.mjs -w --env production --env minify",

0 commit comments

Comments
 (0)