Commits on Feb 5, 2024

1. 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
   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 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.

   

   peaBerberian committed Feb 5, 2024
   Configuration menu

   • View commit details
   • Copy full SHA for 631ff14
   • Browse repository at this point

   Copy the full SHA

   631ff14 View commit details

   Browse the repository at this point in the history