[Proposal] Forbid usage of the HTMLMediaElement and MSE TS types #1397

peaBerberian · 2024-03-06T19:03:59Z

In a recent PoC (Proof Of Concept), I attempted to replicate a subset of the HTMLMediaElement and MSE APIs without the decoding part to facilitate the implementation of some advanced features and integration tests.

It has shown potential, and though it may be a little too soon to merge and rely on that development, I propose here to merge a component of it that can be useful in multiple ways.

The idea is to restrict some key browser API (HTMLMediaElement, MediaSource and SourceBuffer) types by providing our own browser-compatible (this compatibilty is checked at compile-time through some TypeScript trick) type definitions but with either optional supplementary methods and attributes or compatible updated definitions (e.g. a method whose return type was only an enumeration of some values could now return even more values).

For example, for IMediaElement (the redefinition of HTMLMediaElement), I added the vendor-prefixed events and methods that may be used by the RxPlayer (that were previously in the ICompatHTMLMediaElement type) - such as the webkitSetMediaKeys method.
Here this allows to make TypeScript nicer to us when we're exploiting webkit/moz/ms-prefixed API for example.

Another key advantage is that the subset of MSE-related API that are relied-on by the RxPlayer are now clearly listed in a single file, which can be useful when debugging, making API-interaction changes and/or when refactoring.

src/compat/browser_compatibility_types.ts

Florent-Bouisset · 2024-03-26T14:35:25Z

src/transports/utils/__tests__/find_complete_box.test.ts

+import findCompleteBox from "../find_complete_box";
+
+describe("transports utils - findCompleteBox", () => {
+  it("should return -1 if the box is not found", () => {
+    const byteArr = new Uint8Array([
+      0, 0, 0, 9, 0x64, 0x67, 0x32, 0x55, 4, 0, 0, 0, 10, 0x88, 0x68, 0x47, 0x53, 12, 88,
+    ]);
+
+    expect(findCompleteBox(byteArr, 0x75757575)).toEqual(-1);
+    expect(findCompleteBox(byteArr, 0x99999999)).toEqual(-1);
+    expect(findCompleteBox(byteArr, 0x99999)).toEqual(-1);
+  });
+
+  it("should return its index if the box is found", () => {
+    const byteArr = new Uint8Array([
+      0, 0, 0, 9, 0x64, 0x67, 0x32, 0x55, 4, 0, 0, 0, 10, 0x88, 0x68, 0x47, 0x53, 12, 88,
+    ]);
+
+    expect(findCompleteBox(byteArr, 0x64673255)).toEqual(0);


does this file relates to the PR ?

I still can see them in the Pull request

I guess I re-added them during a rebase, I re-removed it

src/transports/utils/find_complete_box.ts

src/compat/browser_compatibility_types.ts

Based on #1397. In the idea of including fake encrypted contents in our integration tests by mocking MSE+EME+HTML 5 media API (not as complex as it sounds) to increase by a lot the types of issues our CI is capable to catch, I noticed an opportunity to even improve on the current code. Like in #1397, the idea is there to provide typings subset of the various EME API and to rely on them instead in the RxPlayer code (and enforcing this through our linter). This allows to: - much simplify EME API mocking by not having to implement the full extent of the EME API - though almost all of it is implemented instead (exceptions are the `EventTarget`'s third `options` optional parameter which we never use, `dispatchEvent`, and the `onevent` methods that we never rely on). - Allow the definition of environment-specific APIs - Be more aware of which EME API we currently use The gain is here much more evident than in #1397 as we already had some kind of sub-typings with the `ICustom...` types (e.g. `ICustomMediaKeys`). By renaming those `I...` (e.g. `IMediaKeys`) and ensuring they are actually compatible with the base type (e.g. `MediaKeys`), we end up in my opinion with simpler code.

This is a proof-of-concept to see if adding a mock implementation of the video HTML5 API and MSE API subset used by the RxPlayer inside the code of the RxPlayer seems useful and maintainable. Motivation ---------- Initially, the idea was to implement one of the several solutions we have in mind to pre-load a future content, this was the most far-fetched solution, but I thought that it could still make enough sense to be tried. In this solution, the application would create two player instances: 1. One linked to the true media element on the page, as usual 2. The Other linked to our dummy media element which would preload and store locally (in memory? through storage APIs when available?) loaded contents. Note that here nothing will change in the RxPlayer API, it is just that the application will have provided to that instance our mocked media element - which would implement all that storage logic in its implementation of MSE API - instead of a regular one. This is to ensure a very minimal modification of the core RxPlayer code. When the application judged that playback should begin, it can get the preloaded data through an API of that dummy media element, call `stop` on the RxPlayer instance with that dummy element and give the preloaded data as a supplementary `loadVideo` option (`preloadedData`?) to the RxPlayer instance with the real media element. There are several complexities that are not yet handled here: most of all we need to be careful as segments on that dummy implementation will for now be stored in memory. Also, we will also need to provide segment-identification metadata alongside the preloaded data so the "real" RxPlayer is able to recognize which Adaptation/Representation has already been loaded so that instance doesn't try to replace it. Also a potential use case asked by applications is to preload a content while another one is already loading. With how this solution is currently implemented, this wouldn't be efficient, as both instances could be loading media data at the same time without priorization mechanisms (e.g. we would imagine that the currently-loaded content is more important) - though I imagine this could be implemented in some way. Other uses ---------- While doing a skeleton of it, I've realized that the MSE API outside of segment parsing and decoding was relatively straightforward: throw when the state or arguments is not right, send the right events etc., so I thought that a second use case (which may well in final be our first use case!) would be for testing. Thanks to this implementation: 1. we could just push fake generated content to facilitate writing tests (no need to generate a real content linked to the wanted behavior for each test) 2. we could more easily replicate and test MSE implementation bugs seen on other devices and ensure they keep being tested. Another nice use of it is that it implement a good chunk of the API used by the RxPlayer that are only found in browser environments, which may simplify PoCs in more restricted environments which can still run JS.

In a recent PoC (Proof Of Concept), I attempted to replicate a subset of the HTMLMediaElement and MSE APIs without the decoding part to facilitate the implementation of some advanced features and integration tests. It has shown potential, and though it may be a little too soon to merge and rely on that development, I propose here to merge a component of it that can be useful in multiple ways. The idea is to restrict some key browser API (`HTMLMediaElement`, `MediaSource` and `SourceBuffer`) types by providing our own browser-compatible (this compatibilty is checked at compile-time through some TypeScript trick) type definitions but with either optional supplementary methods and attributes or compatible updated definitons (e.g. a method whose return type was only an enumeration of some values could now return even more values). For example, for `IMediaElement` (the redefinition of `HTMLMediaElement`), I added the vendor-prefixed events and methods that may be used by the RxPlayer (that were previously in the `ICompatHTMLMediaElement` type) - such as the `webkitSetMediaKeys` method. Here this allows to make TypeScript nicer to us when we're exploiting webkit/moz/ms-prefixed API for example. I also added to it the optional `FORCED_MEDIA_SOURCE` attribute allowing to define a custom MSE implementation when relying on a given media element, though we could also remove that part for this PR. Another key advantage is that the subset of MSE-related API that are relied-on by the RxPlayer are now clearly listed in a single file, which can be useful when debugging, making API-interaction changes and/or when refactoring.

Based on #1397. In the idea of including fake encrypted contents in our integration tests by mocking MSE+EME+HTML 5 media API (not as complex as it sounds) to increase by a lot the types of issues our CI is capable to catch, I noticed an opportunity to even improve on the current code. Like in #1397, the idea is there to provide typings subset of the various EME API and to rely on them instead in the RxPlayer code (and enforcing this through our linter). This allows to: - much simplify EME API mocking by not having to implement the full extent of the EME API - though almost all of it is implemented instead (exceptions are the `EventTarget`'s third `options` optional parameter which we never use, `dispatchEvent`, and the `onevent` methods that we never rely on). - Allow the definition of environment-specific APIs - Be more aware of which EME API we currently use The gain is here much more evident than in #1397 as we already had some kind of sub-typings with the `ICustom...` types (e.g. `ICustomMediaKeys`). By renaming those `I...` (e.g. `IMediaKeys`) and ensuring they are actually compatible with the base type (e.g. `MediaKeys`), we end up in my opinion with simpler code.

In a recent PoC (Proof Of Concept), I attempted to replicate a subset of the HTMLMediaElement and MSE APIs without the decoding part to facilitate the implementation of some advanced features and integration tests. It has shown potential, and though it may be a little too soon to merge and rely on that development, I propose here to merge a component of it that can be useful in multiple ways. The idea is to restrict some key browser API (`HTMLMediaElement`, `MediaSource` and `SourceBuffer`) types by providing our own browser-compatible (this compatibilty is checked at compile-time through some TypeScript trick) type definitions but with either optional supplementary methods and attributes or compatible updated definitons (e.g. a method whose return type was only an enumeration of some values could now return even more values). For example, for `IMediaElement` (the redefinition of `HTMLMediaElement`), I added the vendor-prefixed events and methods that may be used by the RxPlayer (that were previously in the `ICompatHTMLMediaElement` type) - such as the `webkitSetMediaKeys` method. Here this allows to make TypeScript nicer to us when we're exploiting webkit/moz/ms-prefixed API for example. I also added to it the optional `FORCED_MEDIA_SOURCE` attribute allowing to define a custom MSE implementation when relying on a given media element, though we could also remove that part for this PR. Another key advantage is that the subset of MSE-related API that are relied-on by the RxPlayer are now clearly listed in a single file, which can be useful when debugging, making API-interaction changes and/or when refactoring.

Based on #1397. In the idea of including fake encrypted contents in our integration tests by mocking MSE+EME+HTML 5 media API (not as complex as it sounds) to increase by a lot the types of issues our CI is capable to catch, I noticed an opportunity to even improve on the current code. Like in #1397, the idea is there to provide typings subset of the various EME API and to rely on them instead in the RxPlayer code (and enforcing this through our linter). This allows to: - much simplify EME API mocking by not having to implement the full extent of the EME API - though almost all of it is implemented instead (exceptions are the `EventTarget`'s third `options` optional parameter which we never use, `dispatchEvent`, and the `onevent` methods that we never rely on). - Allow the definition of environment-specific APIs - Be more aware of which EME API we currently use The gain is here much more evident than in #1397 as we already had some kind of sub-typings with the `ICustom...` types (e.g. `ICustomMediaKeys`). By renaming those `I...` (e.g. `IMediaKeys`) and ensuring they are actually compatible with the base type (e.g. `MediaKeys`), we end up in my opinion with simpler code.

peaBerberian added the proposal This Pull Request or Issue is only a proposal for a change with the expectation of a debate on it label Mar 6, 2024

peaBerberian force-pushed the misc/imediaelement branch 5 times, most recently from 9a07885 to 1d267d6 Compare March 11, 2024 14:22

peaBerberian force-pushed the dev branch from 1f20b46 to d71f841 Compare March 11, 2024 14:49

peaBerberian force-pushed the misc/imediaelement branch 2 times, most recently from 8e84506 to 3872914 Compare March 14, 2024 13:16

Florent-Bouisset reviewed Mar 26, 2024

View reviewed changes

peaBerberian force-pushed the misc/imediaelement branch 2 times, most recently from 2dd7c59 to bc8db34 Compare May 2, 2024 14:35

peaBerberian force-pushed the dev branch from 682d938 to 424ff17 Compare June 13, 2024 15:20

peaBerberian force-pushed the misc/imediaelement branch from bc8db34 to 7994c64 Compare June 13, 2024 15:34

peaBerberian added the Priority: 2 (Medium) This issue or PR has a medium priority. label Jun 18, 2024

peaBerberian force-pushed the misc/imediaelement branch from 7994c64 to 0d544d4 Compare June 20, 2024 12:19

Florent-Bouisset reviewed Jun 25, 2024

View reviewed changes

src/compat/browser_compatibility_types.ts Show resolved Hide resolved

peaBerberian added this to the 4.2.0 milestone Jun 27, 2024

peaBerberian force-pushed the misc/imediaelement branch from 0d544d4 to 4e190ac Compare June 28, 2024 14:49

peaBerberian mentioned this pull request Jul 5, 2024

[Proposal] Forbid usage of the MediaKeys type and other EME TS types #1477

Open

peaBerberian force-pushed the misc/imediaelement branch from 4e190ac to 6e4f495 Compare July 9, 2024 14:57

peaBerberian mentioned this pull request Jul 9, 2024

[Proposal] Add hidden "Dummy media API" experimental feature and add integration tests linked to DRM #1478

Open

peaBerberian added 2 commits July 24, 2024 18:35

peaBerberian added 6 commits July 24, 2024 18:36

Remove FORCED_MEDIA_SOURCE

d8999cf

Rewrite some details on JSDOC for media element types redefinitions

5718491

Compat: fix name asset to assert

5d0c7b0

Compat: Update HTMLMediaElement.prototype.preload type

9121466

Remove duplicate find_complete_box file in transports code

21d0d72

IMediaElement: Add needkey / webkitneedkey

ee99661

peaBerberian force-pushed the misc/imediaelement branch from 2e8a86a to ee99661 Compare July 24, 2024 16:37

peaBerberian added 2 commits July 24, 2024 18:56

Fix find_complete_box.test.ts

7128374

Remove unnecessary find_complete_box files

d6feb0d

Florent-Bouisset approved these changes Jul 30, 2024

View reviewed changes

peaBerberian merged commit c7c2c60 into dev Jul 31, 2024
6 checks passed

peaBerberian deleted the misc/imediaelement branch November 9, 2024 21:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Proposal] Forbid usage of the HTMLMediaElement and MSE TS types #1397

[Proposal] Forbid usage of the HTMLMediaElement and MSE TS types #1397

peaBerberian commented Mar 6, 2024 •

edited

Loading

Florent-Bouisset Mar 26, 2024

peaBerberian May 2, 2024

Florent-Bouisset Jul 25, 2024

peaBerberian Jul 26, 2024

[Proposal] Forbid usage of the HTMLMediaElement and MSE TS types #1397

[Proposal] Forbid usage of the HTMLMediaElement and MSE TS types #1397

Conversation

peaBerberian commented Mar 6, 2024 • edited Loading

Florent-Bouisset Mar 26, 2024

Choose a reason for hiding this comment

peaBerberian May 2, 2024

Choose a reason for hiding this comment

Florent-Bouisset Jul 25, 2024

Choose a reason for hiding this comment

peaBerberian Jul 26, 2024

Choose a reason for hiding this comment

peaBerberian commented Mar 6, 2024 •

edited

Loading