Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion demo/full/scripts/controllers/Player.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,9 @@ function Player(): JSX.Element {
) {
return;
}
if (playerModule) {
playerModule.destroy();
}
const playerMod = new PlayerModule(
Object.assign(
{},
Expand All @@ -166,7 +169,7 @@ function Player(): JSX.Element {
);
setPlayerModule(playerMod);
return playerMod;
}, [playerOpts]);
}, [playerOpts, playerModule]);

const onVideoClick = useCallback(() => {
if (playerModule === null) {
Expand Down
51 changes: 51 additions & 0 deletions src/main_thread/api/__tests__/public_api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -440,5 +440,56 @@ describe("API - Public API", () => {
expect(player.getMinimumPosition()).toBe(null);
});
});

describe("Player instantiation", () => {
it("should log a warning if creating two players attached to the same video element", () => {
const PublicAPI = jest.requireActual("../public_api").default;
const warn = jest.spyOn(console, "warn").mockImplementation(jest.fn());
const videoElement = document.createElement("video");
const player1 = new PublicAPI({ videoElement });
expect(player1.getVideoElement()).toBe(videoElement);

const errorMessage =
"The video element is already attached to another RxPlayer instance." +
"\nMake sure to dispose the previous instance with player.dispose() before creating" +
" a new player instance attaching that video element.";

new PublicAPI({ videoElement });
expect(warn).toHaveBeenCalledWith(errorMessage);
expect(warn).toHaveBeenCalledTimes(1);

warn.mockClear();
/*
* TODO: for next major version 5.0: this need to throw an error instead of just logging
* this was not done for minor version as it could be considerated a breaking change
*
* expect(() => {
* new PublicAPI({ videoElement });
* }).toThrow(errorMessage);
*/
});

it(`should not log a warning if creating a player attached to
the same video element after the previous one was disposed`, () => {
const PublicAPI = jest.requireActual("../public_api").default;
const warn = jest.spyOn(console, "warn").mockImplementation(jest.fn());
const videoElement = document.createElement("video");
const player1 = new PublicAPI({ videoElement });
expect(player1.getVideoElement()).toBe(videoElement);

player1.dispose();
expect(warn).not.toHaveBeenCalled();
/*
* TODO: for next major version 5.0: this need to throw an error instead of just logging
* this was not done for minor version as it could be considerated a breaking change.
*
* expect(() => {
* new PublicAPI({ videoElement });
* }).not.toThrow();
*
*/
warn.mockClear();
});
});
});
});
43 changes: 42 additions & 1 deletion src/main_thread/api/public_api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,13 @@ class Player extends EventEmitter<IPublicAPIEvent> {
/** Current version of the RxPlayer. */
public readonly version: string;

/**
* Store all video elements currently in use by an RxPlayer instance.
* This is used to check that a video element is not shared between multiple instances.
* Use of a WeakSet ensure the object is garbage collected if it's not used anymore.
*/
private static _priv_currentlyUsedVideoElements = new WeakSet<HTMLMediaElement>();

/**
* Media element attached to the RxPlayer.
* Set to `null` when the RxPlayer is disposed.
Expand Down Expand Up @@ -326,6 +333,39 @@ class Player extends EventEmitter<IPublicAPIEvent> {
addFeatures(featureList);
}

/**
* Register the video element to the set of elements currently in use.
* @param videoElement the video element to register.
* @throws Error - Throws if the element is already used by another player instance.
*/
private static _priv_registerVideoElement(videoElement: HTMLMediaElement) {
if (Player._priv_currentlyUsedVideoElements.has(videoElement)) {
const errorMessage =
"The video element is already attached to another RxPlayer instance." +
"\nMake sure to dispose the previous instance with player.dispose() before creating" +
" a new player instance attaching that video element.";
// eslint-disable-next-line no-console
console.warn(errorMessage);
/*
* TODO: for next major version 5.0: this need to throw an error instead of just logging
* this was not done for minor version as it could be considerated a breaking change.
*
* throw new Error(errorMessage);
*/
}
Player._priv_currentlyUsedVideoElements.add(videoElement);
}

/**
* Deregister the video element of the set of elements currently in use.
* @param videoElement the video element to deregister.
*/
static _priv_deregisterVideoElement(videoElement: HTMLMediaElement) {
if (Player._priv_currentlyUsedVideoElements.has(videoElement)) {
Player._priv_currentlyUsedVideoElements.delete(videoElement);
}
}

/**
* @constructor
* @param {Object} options
Expand All @@ -350,7 +390,7 @@ class Player extends EventEmitter<IPublicAPIEvent> {
this.log = log;
this.state = "STOPPED";
this.videoElement = videoElement;

Player._priv_registerVideoElement(this.videoElement);
const destroyCanceller = new TaskCanceller();
this._destroyCanceller = destroyCanceller;

Expand Down Expand Up @@ -567,6 +607,7 @@ class Player extends EventEmitter<IPublicAPIEvent> {
this.stop();

if (this.videoElement !== null) {
Player._priv_deregisterVideoElement(this.videoElement);
// free resources used for decryption management
disposeDecryptionResources(this.videoElement).catch((err: unknown) => {
const message = err instanceof Error ? err.message : "Unknown error";
Expand Down