Demo: Only include DASH_WASM parser when explicitly asked for #1584
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
peaBerberian
added
the
proposal
In our demo, we have specific scripts to start that demo with our WebAssembly MPD parser, the `DASH_WASM` feature. What those do is to set a `__INCLUDE_WASM_PARSER__` boolean to `true` in global scope, which signals to the demo page that it should include the `DASH_WASM` feature and load the corresponding WebAssembly file. However I noticed that when enabling "multithreading" mode in our demo page, it tried to download the WebAssembly MPD parser regardless of the `__INCLUDE_WASM_PARSER__` boolean. This made sense pre-`v4.1.0` when we only relied on the WebAssembly parser to parse a DASH MPD in "multithreading" mode. But now that we also have a JS parser embedded inside our worker code, it seems unnecessary. It also led to some http 404 when testing the demo locally without having built the WebAssembly file. Those don't matter much though because the RxPlayer automatically fallbacks to the JS parser. Also, the demo built on our GitHub pages accessible on https://developers.canal-plus.com/rx-player/ do have the WebAssembly MPD parser file built. This means that when enabling the "multithreading" mode on that demo, we're actually loading and using that WebAssembly file to parse MPDs. Doing the change I'm proposing here of only using the WebAssembly parser if `__INCLUDE_WASM_PARSER__` is set to `true` will thus lead to a small change in the globally-accessible demo page, where we will be using the default JS parser instead in "multithreading" mode. We could also in the future add a checkbox in our demo specifically for the inclusion or not of the WebAssembly parser.
peaBerberian
force-pushed
the
misc/demo-wasm-opt
branch
from
5b9f79b to
002b868
Compare
Florent-Bouisset
approved these changes
Dec 11, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In our demo, we have specific scripts to start that demo with our WebAssembly MPD parser, the
DASH_WASMfeature.What those do is to set a
__INCLUDE_WASM_PARSER__boolean totruein global scope, which signals to the demo page that it should include theDASH_WASMfeature and load the corresponding WebAssembly file.However I noticed that when enabling "multithreading" mode in our demo page, it tried to download the WebAssembly MPD parser regardless of the
__INCLUDE_WASM_PARSER__boolean.This made sense pre-
v4.1.0when we only relied on the WebAssembly parser to parse a DASH MPD in "multithreading" mode. But now that we also have a JS parser embedded inside our worker code, it seems unnecessary.It also led to some http 404 when testing the demo locally without having built the WebAssembly file. Those don't matter much though because the RxPlayer automatically fallbacks to the JS parser.
Also, the demo built on our GitHub pages accessible on https://developers.canal-plus.com/rx-player/ do have the WebAssembly MPD parser file built. This means that when enabling the "multithreading" mode on that demo, we're actually loading and using that WebAssembly file to parse MPDs.
Doing the change I'm proposing here of only using the WebAssembly parser if
__INCLUDE_WASM_PARSER__is set totruewill thus lead to a small change in the globally-accessible demo page, where we will be using the default JS parser instead in "multithreading" mode.We could also in the future add a checkbox in our demo specifically for the inclusion or not of the WebAssembly parser.