[DO NOT MERGE] DRM integration test for maintaining MediaKeySession across reload

[theRealRobG]

This commit introduces an integration test that tries to prove that it is possible to maintain an active MediaKeySession from one media source to another. It is not intended that this gets merged into main branch. The hope is that this test can get run across a large breadth of devices in order to test whether this usage of MSE+EME is universally valid.

This is in relation to: https://github.com/shaka-project/shaka-player/issues/4379

[github-actions[bot]]

Incremental code coverage: 100.00%

[joeyparrish]

Oh, looks like @avelad already triggered a lab run, and your test passed on all platforms!

[joeyparrish]

Okay, I think see one issue that might confound your test. It seems you set externalMediaKeys_, but I don't see where mediaKeys_ is set on the import case. So later, in attachMediaKeys_(), the call to this.video_.setMediaKeys(this.mediaKeys_); is setting it to null, right?

I don't see the purpose of having a separate member for externalMediaKeys_. Can you please reuse mediaKeys_ so that attach() works correctly? Then we'll re-run the tests to make sure we don't have any new errors.

[joeyparrish]

I'm heading out on vacation for a week, so @theodab, @avelad, other maintainers, please feel free to continue this in my absence.

[theRealRobG]

Okay, I think see one issue that might confound your test. It seems you set externalMediaKeys_, but I don't see where mediaKeys_ is set on the import case. So later, in attachMediaKeys_(), the call to this.video_.setMediaKeys(this.mediaKeys_); is setting it to null, right?

I don't see the purpose of having a separate member for externalMediaKeys_. Can you please reuse mediaKeys_ so that attach() works correctly? Then we'll re-run the tests to make sure we don't have any new errors.

externalMediaKeys_ is actually a boolean that indicates whether the DrmEngine was constructed using external media keys... Clearly it is badly named and I was confused initially about it as well. Its only purpose is so that later on in queryMediaKeys_ we can abandon before creating new media keys for the session in the case where we are re-using them from a previous session. mediaKeys_ is actually still set from whatever is provided in the constructor (defaulted to null when no existing key data is provided).

I'm going to give a quick look into whether I can re-use another property to decide whether new media keys need to be constructed or not... But at the very least I will update the variable name if I run into issues there.

[joeyparrish]

Okay, I see. I misread. Then the test results are likely valid, but we'll want to clear up the code before we try to land this. Thanks!

[theRealRobG]

Hi all,

Really sorry for leaving this open for so long with absolutely no communication on the intention here.

I've been pulled off onto other things and haven't had the time to get back to this, but I've been trying to organize someone to take over from where this PR has been left off, and I believe we will be back shortly to finish this off.

I'll close this PR now.

An outstanding discussion we were having internally is how we handle the different filtering requirements for track options; such that the ABR logic remains as is (and no risk of taking an ABR decision that leads to a codec switch), while allowing for the alternate codec options to be exposed on the public API. We'll work to come back with an implementation to discuss, but any suggestions on what makes sense as a direction that is more idiomatic to Shaka would be welcome.