shaka-project / shaka-player

JavaScript player library / DASH & HLS client / MSE-EME player
Apache License 2.0
7.11k stars 1.33k forks source link

Encrypted media playback freezes on documentPip if pssh is obtained from the media #6279

Open fernandoneira opened 7 months ago

fernandoneira commented 7 months ago

Have you read the FAQ and checked for duplicate open issues? Yes

If the problem is related to FairPlay, have you read the tutorial?

What version of Shaka Player are you using? latest

Can you reproduce the issue with our latest release version? Yes

Can you reproduce the issue with the latest code from main? Yes

Are you using the demo app or your own custom app? Custom App

If custom app, can you reproduce the issue using our demo app? Can't due to requiring a custom manifest parser OR a specific media asset

What browser and OS are you using? Chrome latest

For embedded devices (smart TVs, etc.), what model and firmware version are you using?

What are the manifest and license server URIs?

What configuration are you using? What is the output of player.getConfiguration()?

What did you do? Provided that:

One can reproduce a bug doing the following:

  1. Load a site with:
    • A video element inside a container div.
    • A button to enable documentPip and move the container div to the documentPip window.
    • A button to load a particular media asset into the video element using shaka player and starts playing it.
  2. Click button to launch a documentPip window.
  3. Click button to load media.

What did you expect to happen?

What actually happened?

The bug is caused by this line: https://github.com/shaka-project/shaka-player/blob/bb712c02835f1214be5f23c7f37891eb206ee8e1/lib/util/buffer_utils.js#L65

Using instanceof ArrayBuffer on the initData obtained from the player's encrypted event while on documentPip will actually return false, I presume due to the fact that the ArrayBuffer instance is different if coming from the documentPip context, and is being compared with the one from the main window.

Removing that check and building shaka fixed the issue.

Sorry for not providing a sample atm but I can't easily share the custom parser or media required, and can't set up a codepen with documentPip since it won't work on iframes.

joeyparrish commented 7 months ago

@fernandoneira, thank you for the analysis! This is very helpful.

Surely there will be other places in the code that follow a similar pattern, so any number of instanceof checks could fail in document PIP mode.

Are you aware of an alternative way to make those kinds of checks that would work in all modes?

joeyparrish commented 7 months ago

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/instanceof#instanceof_and_multiple_realms describes the situation succinctly, but provides no suggestions for alternatives.

joeyparrish commented 7 months ago

Array.isArray seems to be one of very few builtins for type checking, and we're already using it for arrays.

It looks like some projects are doing some weird runtime checks to see what a thing is. For example: https://github.com/inspect-js/is-date-object/blob/main/index.js I came across this while looking at changes another project made for "cross-realm compatibility". They replaced instanceof Date with this module. I don't love it.

This is a more promising alternative: https://jakearchibald.com/2017/arrays-symbols-realms/#symbols-and-realms

It would definitely work for our own classes. (We have a lot of instanceof shaka.util.Error and instanceof shaka.media.InitSegmentReference checks.) I'm not certain yet if we can use it to augment builtins like ArrayBuffer.

Excluding tests, for things outside instance of shaka..., we have:

git grep instanceof lib/ ui/ | grep -v 'instanceof shaka' | sed -e 's/.*\(instanceof [^ ),]*\).*/\1/' | sort -u
instanceof ArrayBuffer
instanceof Error
instanceof google.ima.dai.api.LiveStreamRequest
instanceof HTMLCanvasElement
instanceof HTMLMediaElement
instanceof Map
instanceof Object
instanceof TextTrack
instanceof TimeRanges
instanceof Uint8Array
joeyparrish commented 7 months ago

We still do not have a good way to replace all these instanceof checks with something that works cross-realm, specifically the checks for browser builtins. We can add is() methods to our own classes easily.