-
Notifications
You must be signed in to change notification settings - Fork 124
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
[Proposal] Forbid usage of the HTMLMediaElement and MSE TS types #1397
base: dev
Are you sure you want to change the base?
Conversation
9a07885
to
1d267d6
Compare
8e84506
to
3872914
Compare
function assetCompatibleIMediaElement(_x: IMediaElement) { | ||
// Noop | ||
} | ||
// @ts-expect-error unused function, just used for compile-time typechecking | ||
function testMediaSource(x: MediaSource) { | ||
assetCompatibleIMediaSource(x); | ||
} | ||
function assetCompatibleIMediaSource(_x: IMediaSource) { | ||
// Noop | ||
} | ||
// @ts-expect-error unused function, just used for compile-time typechecking | ||
function testSourceBuffer(x: SourceBuffer) { | ||
assetCompatibleISourceBuffer(x); | ||
} | ||
function assetCompatibleISourceBuffer(_x: ISourceBuffer) { | ||
// Noop | ||
} | ||
// @ts-expect-error unused function, just used for compile-time typechecking | ||
function testSourceBufferList(x: SourceBufferList) { | ||
assetCompatibleISourceBufferList(x); | ||
} | ||
function assetCompatibleISourceBufferList(_x: ISourceBufferList) { | ||
// Noop |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you meant assert
instead of asset
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
nodeName: string; | ||
paused: boolean; | ||
playbackRate: number; | ||
preload: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
preload can be of type:
preload: "none" | "metadata" | "auto" | ""
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
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); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this file relates to the PR ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed
export default function findCompleteBox(buf: Uint8Array, wantedName: number): number { | ||
const len = buf.length; | ||
let i = 0; | ||
while (i + 8 <= len) { | ||
let size = be4toi(buf, i); | ||
|
||
if (size === 0) { | ||
size = len - i; | ||
} else if (size === 1) { | ||
if (i + 16 > len) { | ||
return -1; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
related to the PR ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed
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.
3872914
to
dff24b8
Compare
2dd7c59
to
bc8db34
Compare
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
andSourceBuffer
) 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 ofHTMLMediaElement
), I added the vendor-prefixed events and methods that may be used by the RxPlayer (that were previously in theICompatHTMLMediaElement
type) - such as thewebkitSetMediaKeys
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.