Skip to content

Commit

Permalink
[Proposal] Solution 1: PlaybackObserver is moved in src
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
peaBerberian committed Feb 20, 2024
1 parent 1d732f4 commit 25ee193
Show file tree
Hide file tree
Showing 40 changed files with 726 additions and 661 deletions.
10 changes: 5 additions & 5 deletions src/core/adaptive/adaptive_representation_selector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,17 @@

import config from "../../config";
import log from "../../log";
import type {
IObservationPosition,
IReadOnlyPlaybackObserver,
} from "../../main_thread/types";
import type {
IAdaptation,
IManifest,
IPeriod,
IRepresentation,
ISegment,
} from "../../manifest";
import type {
ObservationPosition,
IReadOnlyPlaybackObserver,
} from "../../playback_observer";
import isNullOrUndefined from "../../utils/is_null_or_undefined";
import noop from "../../utils/noop";
import type { IRange } from "../../utils/ranges";
Expand Down Expand Up @@ -604,7 +604,7 @@ export interface IRepresentationEstimatorPlaybackObservation {
* Information on the current media position in seconds at the time of a
* Playback Observation.
*/
position : IObservationPosition;
position : ObservationPosition;
/**
* Last "playback rate" set by the user. This is the ideal "playback rate" at
* which the media should play.
Expand Down
2 changes: 1 addition & 1 deletion src/core/main/common/DecipherabilityFreezeDetector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

import log from "../../../log";
import type { IFreezingStatus, IRebufferingStatus } from "../../../main_thread/types";
import type { IFreezingStatus, IRebufferingStatus } from "../../../playback_observer";
import getMonotonicTimeStamp from "../../../utils/monotonic_timestamp";
import type SegmentSinksStore from "../../segment_sinks";

Expand Down
2 changes: 1 addition & 1 deletion src/core/main/common/content_time_boundaries_observer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@ import type {
IBufferType,
} from "../../../core/types";
import { MediaError } from "../../../errors";
import type { IReadOnlyPlaybackObserver } from "../../../main_thread/types";
import type {
IManifest,
IAdaptation,
IRepresentationIndex,
IPeriod,
} from "../../../manifest";
import type { IReadOnlyPlaybackObserver } from "../../../playback_observer";
import type { IPlayerError } from "../../../public_types";
import EventEmitter from "../../../utils/event_emitter";
import isNullOrUndefined from "../../../utils/is_null_or_undefined";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ import type {
IStreamOrchestratorPlaybackObservation,
} from "../../../core/types";
import log from "../../../log";
import type { IReadOnlyPlaybackObserver } from "../../../main_thread/types";
import type {
IManifest,
IPeriod,
} from "../../../manifest";
import type { IMediaSourceInterface } from "../../../mse";
import type { IReadOnlyPlaybackObserver } from "../../../playback_observer";
import type { IPlayerError } from "../../../public_types";
import type { CancellationSignal } from "../../../utils/task_canceller";
import ContentTimeBoundariesObserver from "./content_time_boundaries_observer";
Expand Down
1 change: 0 additions & 1 deletion src/core/main/index.ts

This file was deleted.

18 changes: 9 additions & 9 deletions src/core/main/worker/worker_main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import {
} from "../../../errors";
import features from "../../../features";
import log from "../../../log";
// XXX TODO
import { ObservationPosition } from "../../../main_thread/api/playback_observer";
import Manifest, {
Adaptation,
Period,
Expand All @@ -24,6 +22,11 @@ import {
WorkerMessageType,
} from "../../../multithread_types";
import DashWasmParser from "../../../parsers/manifest/dash/wasm-parser";
import { ObservationPosition } from "../../../playback_observer";
import type {
IWorkerPlaybackObservation,
} from "../../../playback_observer/worker_playback_observer";
import WorkerPlaybackObserver from "../../../playback_observer/worker_playback_observer";
import type { IPlayerError, ITrackType } from "../../../public_types";
import createDashPipelines from "../../../transports/dash";
import arrayFind from "../../../utils/array_find";
Expand Down Expand Up @@ -60,10 +63,6 @@ import {
import sendMessage, {
formatErrorForSender,
} from "./send_message";
import type {
ICorePlaybackObservation,
} from "./worker_playback_observer";
import WorkerPlaybackObserver from "./worker_playback_observer";

export default function initializeWorkerMain() {
/**
Expand Down Expand Up @@ -97,7 +96,7 @@ export default function initializeWorkerMain() {
/**
* When set, emit playback observation made on the main thread.
*/
let playbackObservationRef : SharedReference<ICorePlaybackObservation> | null = null;
let playbackObservationRef : SharedReference<IWorkerPlaybackObservation> | null = null;

onmessage = function (e: MessageEvent<IMainThreadMessage>) {
log.debug("Worker: received message", e.data.type);
Expand Down Expand Up @@ -161,7 +160,7 @@ export default function initializeWorkerMain() {

const currentCanceller = new TaskCanceller();
const currentContentObservationRef = new SharedReference<
ICorePlaybackObservation
IWorkerPlaybackObservation
>(objectAssign(msg.value.initialObservation, {
position: new ObservationPosition(...msg.value.initialObservation.position),
}));
Expand Down Expand Up @@ -475,7 +474,7 @@ interface IBufferingInitializationInformation {
function loadOrReloadPreparedContent(
val : IBufferingInitializationInformation,
contentPreparer : ContentPreparer,
playbackObservationRef : IReadOnlySharedReference<ICorePlaybackObservation>,
playbackObservationRef : IReadOnlySharedReference<IWorkerPlaybackObservation>,
parentCancelSignal : CancellationSignal
) {
const currentLoadCanceller = new TaskCanceller();
Expand Down Expand Up @@ -544,6 +543,7 @@ function loadOrReloadPreparedContent(

const playbackObserver = new WorkerPlaybackObserver(playbackObservationRef,
contentId,
sendMessage,
currentLoadCanceller.signal);

const contentTimeBoundariesObserver = createContentTimeBoundariesObserver(
Expand Down
90 changes: 0 additions & 90 deletions src/core/main/worker/worker_playback_observer.ts

This file was deleted.

2 changes: 1 addition & 1 deletion src/core/segment_sinks/garbage_collector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

import log from "../../log";
import type { IReadOnlyPlaybackObserver } from "../../main_thread/types";
import type { IReadOnlyPlaybackObserver } from "../../playback_observer";
import isNullOrUndefined from "../../utils/is_null_or_undefined";
import type { IRange } from "../../utils/ranges";
import { getInnerAndOuterRanges } from "../../utils/ranges";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,8 @@
*/

import config from "../../../config";
import type { IReadOnlyPlaybackObserver } from "../../../main_thread/types";
import type {
IAdaptation,
IPeriod,
} from "../../../manifest";
import type { IAdaptation, IPeriod } from "../../../manifest";
import type { IReadOnlyPlaybackObserver } from "../../../playback_observer";
import arrayIncludes from "../../../utils/array_includes";
import type {
IRange } from "../../../utils/ranges";
Expand Down
2 changes: 1 addition & 1 deletion src/core/stream/adaptation/types.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import type { IReadOnlyPlaybackObserver } from "../../../main_thread/types";
import type {
IManifest,
IAdaptation,
IPeriod,
IRepresentation,
} from "../../../manifest";
import type { IReadOnlyPlaybackObserver } from "../../../playback_observer";
import type {
IAudioTrackSwitchingMode,
IVideoTrackSwitchingMode,
Expand Down
2 changes: 1 addition & 1 deletion src/core/stream/orchestrator/stream_orchestrator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@
import config from "../../../config";
import { MediaError } from "../../../errors";
import log from "../../../log";
import type { IReadOnlyPlaybackObserver } from "../../../main_thread/types";
import type {
IManifest,
IDecipherabilityUpdateElement,
IPeriod,
} from "../../../manifest";
import type { IReadOnlyPlaybackObserver } from "../../../playback_observer";
import isNullOrUndefined from "../../../utils/is_null_or_undefined";
import queueMicrotask from "../../../utils/queue_microtask";
import type {
Expand Down
10 changes: 3 additions & 7 deletions src/core/stream/period/period_stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,9 @@ import {
MediaError,
} from "../../../errors";
import log from "../../../log";
import type { IReadOnlyPlaybackObserver } from "../../../main_thread/types";
import type {
IAdaptation,
IPeriod } from "../../../manifest";
import {
toTaggedTrack,
} from "../../../manifest";
import type { IAdaptation, IPeriod } from "../../../manifest";
import { toTaggedTrack } from "../../../manifest";
import type { IReadOnlyPlaybackObserver } from "../../../playback_observer";
import type { ITrackType } from "../../../public_types";
import arrayFind from "../../../utils/array_find";
import objectAssign from "../../../utils/object_assign";
Expand Down
10 changes: 5 additions & 5 deletions src/core/stream/period/types.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import type {
IObservationPosition,
IReadOnlyPlaybackObserver,
} from "../../../main_thread/types";
import type {
IManifest,
IAdaptation,
IPeriod,
} from "../../../manifest";
import type {
ObservationPosition,
IReadOnlyPlaybackObserver,
} from "../../../playback_observer";
import type { ITrackType } from "../../../public_types";
import type { IRange } from "../../../utils/ranges";
import type {
Expand Down Expand Up @@ -94,7 +94,7 @@ export interface IPeriodStreamPlaybackObservation {
* Information on the current media position in seconds at the time of the
* Observation.
*/
position : IObservationPosition;
position : ObservationPosition;
/** `duration` property of the HTMLMediaElement. */
duration : number;
/** `readyState` property of the HTMLMediaElement. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
*/

import config from "../../../../config";
import type { IReadOnlyPlaybackObserver } from "../../../../main_thread/types";
import type { IAdaptation, IPeriod } from "../../../../manifest";
import type { IReadOnlyPlaybackObserver } from "../../../../playback_observer";
import areCodecsCompatible from "../../../../utils/are_codecs_compatible";
import type { IRange } from "../../../../utils/ranges";
import {
Expand Down
12 changes: 6 additions & 6 deletions src/core/stream/representation/types.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
import type {
IContentProtection,
IObservationPosition,
IReadOnlyPlaybackObserver,
} from "../../../main_thread/types";
import type { IContentProtection } from "../../../main_thread/types";
import type {
IManifest,
IAdaptation,
Expand All @@ -11,6 +7,10 @@ import type {
IRepresentation,
} from "../../../manifest";
import type { IEMSG } from "../../../parsers/containers/isobmff";
import type {
ObservationPosition,
IReadOnlyPlaybackObserver,
} from "../../../playback_observer";
import type {
IAudioRepresentationsSwitchingMode,
IPlayerError,
Expand Down Expand Up @@ -185,7 +185,7 @@ export interface IRepresentationStreamPlaybackObservation {
* Information on the current media position in seconds at the time of a
* Playback Observation.
*/
position : IObservationPosition;
position : ObservationPosition;
/**
* Information on whether the media element was paused at the time of the
* Observation.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ import {
SourceBufferError,
} from "../../../../errors";
import log from "../../../../log";
import type { IReadOnlyPlaybackObserver } from "../../../../main_thread/types";
import { toTaggedTrack } from "../../../../manifest";
import type { IReadOnlyPlaybackObserver } from "../../../../playback_observer";
import type { IRange } from "../../../../utils/ranges";
import type { IReadOnlySharedReference } from "../../../../utils/reference";
import sleep from "../../../../utils/sleep";
Expand Down
2 changes: 1 addition & 1 deletion src/core/stream/representation/utils/get_buffer_status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@
*/

import config from "../../../../config";
import type { IReadOnlyPlaybackObserver } from "../../../../main_thread/types";
import type {
IManifest,
IAdaptation,
IPeriod,
IRepresentation,
} from "../../../../manifest";
import type { IReadOnlyPlaybackObserver } from "../../../../playback_observer";
import isNullOrUndefined from "../../../../utils/is_null_or_undefined";
import type {
IBufferedChunk,
Expand Down
Loading

0 comments on commit 25ee193

Please sign in to comment.