Skip to content

Commit b2c8716

Browse files
committed
Make attachWorker return a promise by default
Last week, we've seen an issue on some Samsung and LG TVs when relying on the RxPlayer new experimental `MULTI_THREAD` feature due to specific opcodes found in our WebAssembly files which were not compatible with some of those TVs' browser. Though we isolated and fixed the issue in #1372, it might be better to also find a longer term solution to rollback the `MULTI_THREAD` feature when an issue is detected with it preventing us from playing. This could be done in several ways, from throwing errors, to new events, to just return a rejecting Promise in the `attachWorker` method. I chose to go with the latter of those solutions now because it appears logical API-wise and implementation-wise to have that method return a Promise which resolves only if we're able to communicate with a WebWorker (and reject either if the browser does not support it, if a security policy prevent from running the worker, if the request for the worker's code fail or if the code evualation itself fails). I've also added a specialized error just for that API to give more context about what failed (missing feature? etc.). I was afraid that relying on this new Promise to indicate an issue at WebAssembly-compilation-time for our MPD parser would bother us in the future if we ever add other WebAssembly modules (e.g. a smooth parser), which could also independently fail (should we reject the Promise when either compilation fails? Even if we could theoretically still play DASH contents? How would we mix this way with a potentially lazy-loading of features where we wouldn't be compiling right away? and so on...), but after exposing all the potential future paths I could see this `MULTI_THREAD` feature taking, I was able to find an adapted solution still compatible with returning a Promise on the `attachWorker` API. I also tried to automatically fallback from a "multithread mode" to the regular monothread one inside the RxPlayer but doing this was complex. So for now, if `attachWorker` fails, the RxPlayer will remove the worker from its state (new `loadVideo` calls won't depend on it) but it is the responsibility of the application to reload if a content was loaded in "multithread mode" was loaded in the meantime. If an application doesn't want to handle that supplementary complexity, it can just await the Promise returned by `attachWorker` before loading the first content (and catching eventual errors). As the RxPlayer automatically removes the worker if its initialization fails, this will lead to automatically fallback on main thread. At the cost of some time compared to load and initialize the worker parallely.
1 parent bec07b3 commit b2c8716

File tree

5 files changed

+148
-14
lines changed

5 files changed

+148
-14
lines changed

src/core/api/public_api.ts

Lines changed: 51 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import {
3636
IErrorType,
3737
MediaError,
3838
} from "../../errors";
39+
import WorkerInitializationError from "../../errors/worker_initialization_error";
3940
import features, {
4041
addFeatures,
4142
IFeature,
@@ -54,7 +55,11 @@ import Manifest, {
5455
ManifestMetadataFormat,
5556
createRepresentationFilterFromFnString,
5657
} from "../../manifest";
57-
import { MainThreadMessageType } from "../../multithread_types";
58+
import {
59+
IWorkerMessage,
60+
MainThreadMessageType,
61+
WorkerMessageType,
62+
} from "../../multithread_types";
5863
import {
5964
IAudioRepresentation,
6065
IAudioRepresentationsSwitchingMode,
@@ -435,10 +440,18 @@ class Player extends EventEmitter<IPublicAPIEvent> {
435440
*/
436441
public attachWorker(
437442
workerSettings : IWorkerSettings
438-
) : void {
439-
if (!hasWebassembly) {
440-
log.warn("API: Cannot rely on a WebWorker: WebAssembly unavailable");
441-
} else {
443+
) : Promise<void> {
444+
return new Promise((res, rej) => {
445+
if (typeof Worker !== "function") {
446+
log.warn("API: Cannot rely on a WebWorker: Worker API unavailable");
447+
return rej(new WorkerInitializationError("INCOMPATIBLE_ERROR",
448+
"Worker unavailable"));
449+
}
450+
if (!hasWebassembly) {
451+
log.warn("API: Cannot rely on a WebWorker: WebAssembly unavailable");
452+
return rej(new WorkerInitializationError("INCOMPATIBLE_ERROR",
453+
"WebAssembly unavailable"));
454+
}
442455
if (typeof workerSettings.workerUrl === "string") {
443456
this._priv_worker = new Worker(workerSettings.workerUrl);
444457
} else {
@@ -448,10 +461,40 @@ class Player extends EventEmitter<IPublicAPIEvent> {
448461
}
449462

450463
this._priv_worker.onerror = (evt: ErrorEvent) => {
451-
this._priv_worker = null;
452-
log.error("Unexpected worker error",
464+
if (this._priv_worker !== null) {
465+
this._priv_worker.terminate();
466+
this._priv_worker = null;
467+
}
468+
log.error("API: Unexpected worker error",
453469
evt.error instanceof Error ? evt.error : undefined);
470+
rej(new WorkerInitializationError("UNKNOWN_ERROR",
471+
"Unexpected Worker \"error\" event"));
472+
};
473+
const handleInitMessages = (msg: MessageEvent) => {
474+
const msgData = msg.data as unknown as IWorkerMessage;
475+
if (msgData.type === WorkerMessageType.InitError) {
476+
log.warn("API: Processing InitError worker message: detaching worker");
477+
if (this._priv_worker !== null) {
478+
this._priv_worker.removeEventListener("message", handleInitMessages);
479+
this._priv_worker.terminate();
480+
this._priv_worker = null;
481+
}
482+
rej(
483+
new WorkerInitializationError(
484+
"SETUP_ERROR",
485+
"Worker parser initialization failed: " + msgData.value.errorMessage
486+
)
487+
);
488+
} else if (msgData.type === WorkerMessageType.InitSuccess) {
489+
log.info("API: InitSuccess received from worker.");
490+
if (this._priv_worker !== null) {
491+
this._priv_worker.removeEventListener("message", handleInitMessages);
492+
}
493+
res();
494+
}
454495
};
496+
this._priv_worker.addEventListener("message", handleInitMessages);
497+
455498
log.debug("---> Sending To Worker:", MainThreadMessageType.Init);
456499
this._priv_worker.postMessage({
457500
type: MainThreadMessageType.Init,
@@ -478,7 +521,7 @@ class Player extends EventEmitter<IPublicAPIEvent> {
478521
},
479522
});
480523
}, this._destroyCanceller.signal);
481-
}
524+
});
482525
}
483526

484527
/**

src/core/init/multithread/main_thread/multi_thread_content_initializer.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1031,6 +1031,11 @@ export default class MultiThreadContentInitializer extends ContentInitializer {
10311031
break;
10321032
}
10331033

1034+
case WorkerMessageType.InitSuccess:
1035+
case WorkerMessageType.InitError:
1036+
// Should already be handled by the API
1037+
break;
1038+
10341039
default:
10351040
assertUnreachable(msgData);
10361041
}

src/core/init/multithread/worker/worker_portal.ts

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -108,12 +108,20 @@ export default function initializeWorkerPortal() {
108108
const diffWorker = Date.now() - performance.now();
109109
mainThreadTimestampDiff.setValueIfChanged(diffWorker - diffMain);
110110
updateLoggerLevel(msg.value.logLevel, msg.value.sendBackLogs);
111-
dashWasmParser.initialize({ wasmUrl: msg.value.dashWasmUrl }).catch((err) => {
112-
const error = err instanceof Error ?
113-
err.toString() :
114-
"Unknown Error";
115-
log.error("Worker: Could not initialize DASH_WASM parser", error);
116-
});
111+
dashWasmParser.initialize({ wasmUrl: msg.value.dashWasmUrl }).then(
112+
() => {
113+
sendMessage({ type: WorkerMessageType.InitSuccess,
114+
value: null });
115+
}, (err) => {
116+
const error = err instanceof Error ?
117+
err.toString() :
118+
"Unknown Error";
119+
log.error("Worker: Could not initialize DASH_WASM parser", error);
120+
sendMessage({ type: WorkerMessageType.InitError,
121+
value: { errorMessage: error,
122+
kind: "dashWasmInitialization" } });
123+
124+
});
117125

118126
if (!msg.value.hasVideo || msg.value.hasMseInWorker) {
119127
contentPreparer.disposeCurrentContent();
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import errorMessage from "./error_message";
2+
3+
type IWorkerInitializationErrorCode = "UNKNOWN_ERROR" |
4+
"SETUP_ERROR" |
5+
"INCOMPATIBLE_ERROR";
6+
7+
/**
8+
* Error linked to the WebWorker initialization.
9+
*
10+
* @class WorkerInitializationError
11+
* @extends Error
12+
*/
13+
export default class WorkerInitializationError extends Error {
14+
public readonly name : "WorkerInitializationError";
15+
public readonly type : "WORKER_INITIALIZATION_ERROR";
16+
public readonly message : string;
17+
public readonly code : IWorkerInitializationErrorCode;
18+
19+
/**
20+
* @param {string} code
21+
* @param {string} message
22+
*/
23+
constructor(
24+
code : IWorkerInitializationErrorCode,
25+
message : string
26+
) {
27+
super();
28+
// @see https://stackoverflow.com/questions/41102060/typescript-extending-error-class
29+
Object.setPrototypeOf(this, WorkerInitializationError.prototype);
30+
31+
this.name = "WorkerInitializationError";
32+
this.type = "WORKER_INITIALIZATION_ERROR";
33+
this.code = code;
34+
this.message = errorMessage(this.code, message);
35+
}
36+
}

src/multithread_types.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -573,6 +573,44 @@ export type ISentError = ISerializedNetworkError |
573573
ISerializedEncryptedMediaError |
574574
ISerializedOtherError;
575575

576+
/**
577+
* Message sent by the WebWorker when its initialization, started implicitely
578+
* as soon as the `new Worker` call was made for it, has finished and succeeded.
579+
*
580+
* Once that message has been received, you can ensure that no
581+
* `IInitErrorWorkerMessage` will ever be received for the same worker.
582+
*
583+
* Note that receiving this message is not a requirement before preparing and
584+
* loading a content, both initialization and content loading can be started in
585+
* parallel.
586+
*/
587+
export interface IInitSuccessWorkerMessage {
588+
type: WorkerMessageType.InitSuccess;
589+
value: null;
590+
}
591+
592+
/**
593+
* Message sent by the WebWorker when its initialization, started implicitely
594+
* as soon as the `new Worker` call was made for it, has finished and failed.
595+
*
596+
* Once that message has been received, you can ensure that no
597+
* `IInitErrorWorkerMessage` will ever be received for the same worker.
598+
*
599+
* Note that you may received this message while preparing and/or loading a
600+
* content, both initialization and content loading can be started in
601+
* parallel.
602+
* As such, this message may be coupled with a content error.
603+
*/
604+
export interface IInitErrorWorkerMessage {
605+
type: WorkerMessageType.InitError;
606+
value: {
607+
/** A string describing the error encountered. */
608+
errorMessage: string;
609+
610+
kind: "dashWasmInitialization";
611+
};
612+
}
613+
576614
export interface INeedsBufferFlushWorkerMessage {
577615
type: WorkerMessageType.NeedsBufferFlush;
578616
contentId: string;
@@ -883,6 +921,8 @@ export const enum WorkerMessageType {
883921
EndOfStream = "end-of-stream",
884922
Error = "error",
885923
InbandEvent = "inband-event",
924+
InitError = "init-error",
925+
InitSuccess = "init-success",
886926
InterruptEndOfStream = "stop-end-of-stream",
887927
InterruptMediaSourceDurationUpdate = "stop-media-source-duration",
888928
LockedStream = "locked-stream",
@@ -921,6 +961,8 @@ export type IWorkerMessage = IAbortBufferWorkerMessage |
921961
IEndOfStreamWorkerMessage |
922962
IErrorWorkerMessage |
923963
IInbandEventWorkerMessage |
964+
IInitSuccessWorkerMessage |
965+
IInitErrorWorkerMessage |
924966
IInterruptMediaSourceDurationWorkerMessage |
925967
ILockedStreamWorkerMessage |
926968
ILogMessageWorkerMessage |

0 commit comments

Comments
 (0)