Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refacto RxJS-based ABRManager into a SharedReference-based AdaptiveRepresentationSelector #1091

Merged
merged 5 commits into from
Jun 8, 2022

Conversation

peaBerberian
Copy link
Collaborator

@peaBerberian peaBerberian commented Mar 21, 2022

After #962 and #1042, this is another huge pull request which tries to progressively reduce our dependency on the RxJS library to improve code "approachability". You can look at the two previous (since-merged) PR for reasons leading to this.

Here RxJS and associated concepts have been completely removed from the src/core/abr directory, which concerns our Adaptive BitRate-linked code.
On that subject, in the same vein than the eme directory has been renamed decrypt and the EMEManager the ContentDecryptor - so its more explicit and clear to newcomers and external contributors - the abr directory has been renamed adaptive and the ABRManager, the AdaptiveRepresentationSelector.

The key points here are:

  • Because the adaptive code use many event listeners, a lot of code in compat/event_listeners.ts has been updated so it returns IReadOnlySharedReference s instead of RxJS Observables.
    Shared references can be seen as [very] simplified BehaviorSubjects when compared to RxJS. Their read-only flavor cannot update the value, they can only get the current value or "listen" (subscribe) to value changes (because the event listener should not have the need to emit them).

    Simple shared reference description for those not that familiar with BehaviorSubjects: SharedReferences are JS objects with a value inside them (any value: a number, boolean, object, another shared reference etc.), that can be updated and read at any time (by anyone in possession of this object), with also the possibility to register a callback so it is called each time its stored value is updated.

  • adaptive estimates are also returned as an IReadOnlySharedReference - so the AdaptationStream can easily create a RepresentationStream each time the Representation estimate changes through a callback.

  • A third argument has been added to our EventEmitter's addEventListener method: an optional CancellationSignal.
    This allows to easily remove the listener when the signal emits - CancellationSignal being the preferred solutions for now for cancellation handling in the player (previously happening implicitly through Observable unsubscription).
    Because we do not want to leak that supplementary argument to the API/TypeScript types, the public API (core/api/public_api.ts) redefines its own addEventListener method on top so it can explicitly remove the third argument.

@peaBerberian peaBerberian added ABR Relative to adaptive streaming (Adaptive BitRate) Refacto This Pull Request changes a lot of RxPlayer's code and/or logic skip-performance-checks Performance tests are not run on this PR Priority: 3 (Low) This issue or PR has a low priority. and removed skip-performance-checks Performance tests are not run on this PR labels Mar 21, 2022
@peaBerberian peaBerberian added this to the 3.28.0 milestone May 9, 2022
@peaBerberian peaBerberian force-pushed the next branch 2 times, most recently from 989ffe8 to c8154ed Compare June 7, 2022 13:54
@peaBerberian peaBerberian merged commit b67e176 into next Jun 8, 2022
peaBerberian added a commit that referenced this pull request Jun 8, 2022
Refacto RxJS-based `ABRManager` into a SharedReference-based `AdaptiveRepresentationSelector`
peaBerberian added a commit that referenced this pull request Jun 13, 2022
Refacto RxJS-based `ABRManager` into a SharedReference-based `AdaptiveRepresentationSelector`
peaBerberian added a commit that referenced this pull request Jun 14, 2022
Refacto RxJS-based `ABRManager` into a SharedReference-based `AdaptiveRepresentationSelector`
peaBerberian added a commit that referenced this pull request Jun 15, 2022
Refacto RxJS-based `ABRManager` into a SharedReference-based `AdaptiveRepresentationSelector`
peaBerberian added a commit that referenced this pull request Jun 21, 2022
Refacto RxJS-based `ABRManager` into a SharedReference-based `AdaptiveRepresentationSelector`
peaBerberian added a commit that referenced this pull request Jun 23, 2022
Refacto RxJS-based `ABRManager` into a SharedReference-based `AdaptiveRepresentationSelector`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ABR Relative to adaptive streaming (Adaptive BitRate) Priority: 3 (Low) This issue or PR has a low priority. Refacto This Pull Request changes a lot of RxPlayer's code and/or logic skip-performance-checks Performance tests are not run on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants