Release/v4.1.0 #1456
Merged
Release/v4.1.0 #1456
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
release
Root issue ---------- While working on code refactoring for better taking into account the new potentially multithread nature of the RxPlayer code (with some files only running in a WebWorker environment and others only running in a main thread environment), we recently refactored our file hierarchy (#1365) to better reflect that new situation. The idea is to make RxPlayer developpers more aware of what code is intended to run where. In that work, we had a remaining issue concerning the `PlaybackObserver`. This is the part of the code that is monitoring and advertising playback conditions to the rest of the RxPlayer code. Most core RxPlayer modules rely on it, in both main thread and WebWorker environments. The root issue is that the `PlaybackObserver` is a class and thus cannot be easily transmitted in-between environments (as the main thread and WebWorker only exchanges between one another through `postMessage` calls, which follows some rules preventing from doing that). As such, and only on a multithreaded scenario, the RxPlayer is serializing in some way the constructed source PlaybackObserver in the main thread to reconstruct one (the `WorkerPlaybackObserver`) on the worker. Because a whole complex PlaybackObserver-compatible structure has to both be constructed in the main thread and in the worker (yet only the main thread can act as the "true" source one, because the media element can only be accessed in main thread), we were asking ourselves where should we put the common utils (required by both the main thread and worker) needed to construct one (like the `ObservationPosition` class and the `generateReadOnlyObserver` util). Solution 1 ---------- This is a first solution proposal, which moves the `PlaybackObserver` directory outside of the `main_thread` and `core` directories. Instead, its code is now directly in `src/playback_observer`. This solution takes inspiration from the `src/mse` directory, which also exports both: 1. files intended to be imported when MSE API are present in the current environment (when in main thread or when in a WebWorker with the MSE-in-worker feature) and 2. files intended to be imported in environments without MSE. For example the `src/mse/main_media_source_interface.ts` should __ONLY__ be imported in environments with MSE capabilities. If you do not, you should import `src/mse/worker_media_source_interface.ts`. Likewise, I here added a `src/playback_observer/media_element_playback_observer.ts` file exporting a `MediaElementPlaybackObserver` structure (new name of the `PlaybackObserver`) which can only be imported in environements where the media element is available (so, only on main thread) and a `src/playback_observer/worker_playback_observer.ts` file which should only be imported in other environments. Doing this allows to easily share common utils, in a `src/playback_observer/utils` directory, without involving the rest of the RxPlayer code. Result ------ I'm quite happy with the result, though I get it may seem weird to have a complex core frequently-running logic part directly in `src` (and not in `main_thread` or `core` as was the norm) - though it is also the case of directories like `mse` or `transports`. RxPlayer code very rarely imported files inside other directories (which it does here with paths like `../playback_observer/media_element_playback_observer`) as a way to push better modularization. Though here it may seem like a feature because its unusual-ness forces the developper to double check if this is the right file that is imported.
…itial failed While doing some testing on `<UTCTiming>` elements in a DASH MPD, it has been brought to us that if attempts to load an URL linked to it for the initial fetch of the Manifest failed (in which case we fallback to a mode where we don't rely on that element) and if the Manifest has to be refreshed multiple times, then the URL will be accessed every time the Manifest is refreshed, even if just one sucessful attempt would be enough. The issue was very simple to fix, as it was just that a newly obtained `clockOffset` - the actual metadata derived from parsing an `<UTCTiming>` element, was just not actually copied when updating the base Manifest by its new parsed version. Once that `clockOffset` is set to the base Manifest, that information will be communicated to our Manifest parser when refreshed, so it won't retry to load the clock and continue relying on the one fetched before.
maxDelay now restrict the value of the delay after that the fuzzFactor has been applied. This ensure that the returned delay is trully equal or inferior the the maxDelay. This would reduce from 3900ms to 3000ms the maxDefault based on our current config but updating the value does not seem necessary.
expected retryDelay
exporting a class with an IIFE change the exported type of the class from "RxPlayer" to "typeof RxPlayer". That will provoke typechecking errors for applications that used "RxPlayer" as a type. This does NOT mess-up with the sideEffects config because the global overrides, such as "patchWebkitSourceBuffer()" are executed at the entry point for RxPlayer class (src/index and src/minimal).
fix: types and lint on demo
peaBerberian
force-pushed
the
release/v4.1.0
branch
from
df1de31
to
0201055
Compare
We were relying on express only for serving static files for development builds and testing purposes - in a way that was kind of overkill considering that we need almost none of its features. To simplify maintainance, I decided to remove that dependency and just implement the bare minimum we need from it instead.
peaBerberian
force-pushed
the
release/v4.1.0
branch
from
0201055
to
164c5d7
Compare
added 3 commits
June 20, 2024 15:48
- A zero-filled keyId makes all streams undecipherable.
fix(isobmff): exclude zero-filled keyId from the init segment parsing
peaBerberian
force-pushed
the
release/v4.1.0
branch
from
f242b25
to
1367ae3
Compare
…en an error is thrown
…error-msg fix: on chrome display correctly the error message in the console when an error is thrown
This commit removes dependencies that should now be unneeded. I had to update the standalone demo's script to remove its dependency on webpack by copy-pasting most of its bundling code from the full demo's script. As we never really had a use for the standalone demo, I'll also propose in the future to just remove it, but I prefer to do that in a second step.
peaBerberian
force-pushed
the
release/v4.1.0
branch
2 times, most recently
from
8b178ca
to
99fb82d
Compare
peaBerberian
force-pushed
the
release/v4.1.0
branch
from
744240d
to
3bc00cd
Compare
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.
This is the PR for the future v4.1.0.
It contains a fair share of refactoring considering that we only merged most 4.0.0-era refactoring AFTER the v4.0.0 to ensure we don't break everything with that initial major release (which was in beta and alpha stages for multiple years).
To note that some Canal+'s applications rely on pre-releases and most of those refactoring were already tested, and delivered in production since january - so we're already relatively confident of its stablity.
We just didn't make any new release since because no huge issue or feature-need arised.
Among many other fixes and improvements mostly linked to device compatibility, this release also includes DASH "ContentProtection references" and key improvements for our experimental MULTI_THREAD features - which most notably does not require any WebAssembly code anymore.
A good news is that we didn't find any issue in production relative to that huge feature (to be perfectly honest we saw a DRM-related race condition on a very specific device only used at Canal+, yet our debugging sessions seems to indicate the device's lower-level software is at fault and that it is not inherently linked to the MULTI_THREAD feature, only easier to reproduce with it).
Note that this PR is only for visibility, features, bug fixes and improvements are not all merged yet and consequently this branch does not contain everything announced by the changelog.
Changelog
Features
ContentProtectionreferences [DASH: Implement Content Protection References #1439]Bug fixes
Resolve base url not correct when root MPD base url start with
/#1440]maxVideoBufferSizeAPI [When listing segments needed, stop filtering the current range #1421][Fix duplicate segment requests for low-latency contents #1422]
[fix(compat): on Edge test comprehensively KeySystems before considering them as usable #1434]
0x0key id found in DASH initialization segments are they are often linkedto unencrypted data. [fix(isobmff): exclude zero-filled keyId from the init segment parsing #1466, DRM: Zero filled TENC KID cause a NO_PLAYABLE_REPRESENTATION error #1458]
to prevent fatal errors [On the playstation 5, reload ASAP if a key become unusable #1399]
MULTI_THREADexperimental featurenow works on older browser and on the Playstation 4 [Fix Worker usage on PS4 browser #1401, Do not rely on performance.now if unavailable #1402]
autoPlayisset to
falseor not set and the video element has the attribute "playsinline" [fix(safari): V4 - no longer stuck in buffering on safari iOS when video has attribute "playsinline" #1408,Infinity loading state on Safari Mobile when autoplay is false #1390]
LOADINGstate if thedurationis set toNaN(rare issue in a normally-unsupportedmultiple RxPlayer-per-media-element scenario) [fix(directfile): media with autoplay:false is infinitly in loading state #1393]
Other improvements
normalizedproperty equal to"und"for better ISO 639-3 compatibility [fix: safari support audio lang "und" in audioTrack #1428]MULTI_THREADfeature does not need adashWasmUrlanymore nor compatibility to WebAssembly [DASH: rely on "txml" parser for Multithreading usages #1384]
DEBUG_ELEMENTfeature now allows to display all debug informationeven under the "multithreading" mode [feat:(worker) debug element display buffer in worker mode #1438]
performance.nowAPI if not available [Do not rely on performance.now if unavailable #1402]typekeyword at type imports to be sure they have no codeimpact on our final build [[WIP] v4 code reorganization #1365]
src/main_threadandsrc/corerespectivelyfor main thread and worker code in a "multithread" mode [[WIP] v4 code reorganization #1365]
prettierandrustfmtformatting tools in the codebase [Release/v4.0.0 #1387]webpack[ [Proposal] Stop transpiling some builds to ES5 #1435, Proposal: Stop relying on webpack for legacy bundles generation #1425, Proposal: remove the need to rely on webpack for es5 worker generation #1420]vitestframework to simplify and unify test-relateddependencies and test writing [use vitest for integration tests #1444, Switch unit tests to use vitest #1445]