-
Notifications
You must be signed in to change notification settings - Fork 132
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
fix(safari): video not starting because key are never considered usable for a track #1479
Conversation
Maybe related to issue #1326 |
To propose an alternative solution for this one: Maybe we could add something to the payload when receiving the event, like a Also maybe when doing this we should close the previous MediaKeySession or at least close it. I'm afraid that we could be mixing things up if we had two alive MediaKeySession linked to the same key id + init data. |
this.error = null; | ||
|
||
eme.onEncrypted( | ||
mediaElement, | ||
(evt) => { | ||
log.debug("DRM: Encrypted event received from media element."); | ||
const initData = getInitData(evt as MediaEncryptedEvent); | ||
const initData = getInitData(evt as ICustomMediaEncryptedEvent); |
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.
We may want to better type onEncrypted
instead?
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.
Is that still necessary if we update onEncrypted
's type in IEmeApiImplementation
like this?:
onEncrypted: (
target: IEventTargetLike,
listener: (evt: ICustomMediaEncryptedEvent) => void,
cancelSignal: CancellationSignal,
) => void;
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.
Yes because createCompatibleEventListener
returns a listener of type (event?: Event) => void
and will not satisfies type (evt: ICustomMediaEncryptedEvent) => void
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.
Event
should be compatible to an ICustomMediaEncryptedEvent
though as forceSessionRecreation
is optional no?
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.
Yet maybe Event
is not compatible with a MediaEncryptedEvent
, in which case we might improve on the type?
Through createCompatibleEventListener
event overloading?
Also, does it work? |
compatibleEventListener( | ||
target, | ||
(event?: Event) => { | ||
const patchedEvent = object_assign( |
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.
Weird that eslint is not telling us about this weird function naming
9fd3962
to
d563ea2
Compare
Revert #1479 (Safari DRM audio track issue)
We observed an issue with encrypted HLS playlist on Safari with the legacy EME API:
If the HLS playlist has multiple audio with the same encryption key, and the default lang of the playlist is NOT the lang of the OS, the video will stall.
This appears to be a race condition within Safari, that occurs when the licence is received at about the same time the audio tracks are added.
Safari will continuously fire events "webkitneedkey" for the init-data we just processed, the RxPlayer will not be re-creating a session for an already handled init-data.
Forcing to recreate a session even if the init-data is already handled fixed the issue.