-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Re-query MediaKeys to handle codec profile changes #1567
Comments
Added a test to show how we broke Drm Engine for non-encrypted content in change id I0558de50552a614692488a6c78d46b5ea345bc7b Issue #1567 Change-Id: I2aab52c79eea1887b7f4281e1b8dcbbc0f3cd25e
Thank you very much for your quick fix, @vaage . I appreciated. However, there is an edge case and your fix does not cover it unless I am wrong. If a stream starts with clear content and switches to encrypted content later, it will filter out the encrypted contents because the clear segments have different codecs. Do you want me create another ticket for this? Apologies for this. I should have mentioned this earlier. Thank you, |
We'll re-open the issue while we figure out if it's a separate issue. This issue was more about a regression during a encrypted to clear transition. @SemihGk if I understand correctly, you are now asking about the reverse (the clear to encrypted transition). Right now media source does not support codec switching, so the content would still need to have the same codec (that would be filtered outside of DrmEngine). So is the problem that the codec stays the same but the profile changes causing us to reject it? |
Apologies for the misexplanation, @vaage . When I said different codecs, I meant the transition between mp4a.40.2 and mp4a.40.5 as the screenshots. With your fix, now the stream is able to switch from encrypted (mp4a.40.5) to clear (mp4a.40.2) content (I tested it works fine). However, the reverse case cannot be switched because you only filter encrypted segments at here. Now, imagine the clear content to encrypted content switch case. Clear content codec types will be stored in here. When the stream becomes encrypted, those encrypted segments will be filtered again due to the supportedTypes array will include clear content codec types only. Just a friendly reminder we initiate drmEngine even though stream starts with clear content at here . So, Please let me know if I missed something. Thank you. |
@SemihGk , this case came up in a conversation with @joeyparrish yesterday. He is out of the office until next Friday, but I will sync-up with him again once here is back and we will figure out how we want to approach this issue (we came up with 5 or 6 different cases so it gets messy fast). I will assign this to myself so that it won't fall between the cracks. |
I totally understand your concern. I also agree on the switch between clear and encrypted content is a kind of hack-ish workaround on the drm flow anyway. Figuring out the best solution would be better to avoid any temporary workaround. Thank you very much. |
Hi @SemihGk, @vaage. To clarify: The difference between mp4a.40.2 and mp4a.40.5 has nothing to do with clear vs encrypted. These are different profiles of AAC. Either could be clear, and either could be encrypted. Encryption is signalled outside of the codec string. And since these are two different AAC profiles, this shouldn't count as "codec switching" in MSE. In general, we can switch profiles of the same codec. The problem is this: When we negotiate with the browser for a CDM, we have to supply a list of MIME types we want to decode. The browser then reports back which ones can be decoded in context of the CDM. We use this information to filter out things the CDM can't decode. For example, if your content has both VP9 and H.264, you would ask for both. The browser may then report back that the CDM only supports H.264, so you would filter out VP9 as unusable. Failing to do so could result in a playback failure later. Or, your content may contain multiple profiles of H.264, such as a baseline profile, a main profile, and a UHD profile. You would ask for all of the ones you have in your content, and the browser may report back that the CDM only supports a subset of them, such as baseline & main, so you would filter out the UHD profile. Failing to do so could result in a playback failure later. Now in your scenario, the main content has mp4a.40.5. If we start playback during a main content period, we ask for a CDM that can decode this. We hear back from the browser that yes, it can decode mp4a.40.5. Later, during an ad break, we see mp4a.40.2 for the first time. Since we never asked for a CDM that could decode that, it's not in the list from the browser. So we're left assuming that the CDM can't decode it. (In fact, in Chrome, the Widevine CDM can handle both just fine.) I see a few options right away:
Back to you, @vaage! |
Thank you very much for your detailed reply, @joeyparrish . It is quite clear now. So, a stream should be able to switch codec profiles even though the stream does not have any clear segment. I would like to speak out about some of those options:
This option seems the easiest solution. In addition to this, we could implement a 'soft' restriction as we have it on AbrManager. Let's say, we can still filter out the unsupported codec profiles, but if there is no available variant left, we can leave one type of codec profiled variant which we did not ask it to CDM yet.
This one sounds the most robust solution for me. Calling
I checked some versions of EME specs out, but I could not find any information about which drafts have EME is singleton. I checked 0.1b Feb draft, that's not a singleton. I assume previous versions does not support this
This sounds a platform CDM implementation mistake. If different CDMs return report different results, I personally believe we should report this bug to the platform owners. I am just curios do you know any platform which has multiple CDMs for the same keySystem?
I can see two issues on this solution. First one as you pointed out, there would be an interruption and this is definitely a bad experience for users. Second one, the implementation would require a bit work. The last one is that the stream might have more than 2 codec profile changes. For our case, we do not know all the ads have the same codec profiles. We need to discuss this with backend team.
This might be tough. As for our case, none of our ad segment may have the same codec profile. It may vary as the ad urls are inserted by a 3rd party company. If you think other options are not solid, we will need to discuss this with out back-end team. I really appreciate for your considerations. Please let me know if I can help you to figure out a possible solution. Thanks a lot. |
Easy, yes. But I don't think it's a good idea, because of the risk of playback failures. The risk is this: if we assume the CDM supports anything we didn't ask about, we could fail playback in some cases. What if your content goes up to UHD resolutions, or uses an HDR color space, but those features aren't supported by the CDM? If we join during an ad break, we would perhaps only query SD & SDR profiles, then fail to decode later when the content is resumed. Worse, it might be non-deterministic, because only the users with the most bandwidth would attempt playback of those streams.
It's not about the EME spec. MediaKeys is never a singleton, but there is no requirement that a user agent support multiple concurrent instances of MediaKeys in the same page. In the past, I have worked on embedded HTML5 platforms which can only support a single video context at a time. So it's not a stretch to imagine an embedded system that might only be able to support one CDM instance at a time. After all, a CDM instance would consume limited hardware resources for hardware-based DRM. If
This is the case on Android, in which two CDM implementations can exist in parallel: hardware-backed, and pure-software implementations. Both use the same key system ID, but have different robustness strings. It's not a mistake, and it's not against the spec. If we can assume that robustness strings or key system IDs will always differentiate between them, then we can make sure we are querying the same one as before. I don't know if this assumption will hold on all platforms and with all key systems, but it might work.
I think we can abandon this idea, for all the reasons we discussed.
Understood. It sounds like re-querying MediaKeys might be the best solution, then. We would have to test it thoroughly on all supported platforms, though, so we need to come up with a good automated test for this. |
Since the discussion was quite long, and many options were discussed, here's a summary for @vaage or whoever else winds up implementing what we discussed here: When new periods are introduced, we should filter them by re-querying MediaKeys instead of using the list of supported codec strings from the first query. We expect this should work based on the spec text, but we also need to write a good automated test to ensure it works on all supported platforms. The test would be something roughly like this:
Repeat this test for:
|
I really appreciated your detailed feedback. I am glad that you are also comfortable with the re-querying solution. So, I just wanted to share a quick update. I tested this possible re-querying solution on my fork (it might be a bit messy though, but very simple) . This solution seems to be working on Chrome and Tizen. I will continue testing on other platforms to see if I can find any issue on other platforms as well. Thanks a lot. |
@SemihGk @joeyparrish , Just so you both know, I have been following along. Will follow through on @joeyparrish recommended approach. |
Thank you very much. I look forward to waiting for your fix. Meanwhile, I tested this approach and it is working on Tizen 17/18, LG 18 ( which those only support one video element at once), Chrome and Mozilla. Thus, this solution seems very safe to go. |
Maps respect insertion order, so we don't need to have the array of what order to check the key systems anymore. Issue #1567 Change-Id: I314dea1f5626bc445deec0f9b6f47037f8889085
Rather than try to build the config by key system map across multiple calls to prepareMediaKeyConfigsForVariant_, pass all the variants to it and build it in one call. Issue #1567 Change-Id: Ibb7314117895a70dc421465cfcae1695108bba7e
Before checking support for any media keys, update all the configs to remove empty capabilities. Issue #1567 Change-Id: Ia7aa5561c767b9049ff2f5eb731f6772a1598124
Closes: #1528 Closes: #1567 Closes: #4379 Closes: #5306 --------- Co-authored-by: Álvaro Velad Galván <[email protected]>
Closes: shaka-project#1528 Closes: shaka-project#1567 Closes: shaka-project#4379 Closes: shaka-project#5306 --------- Co-authored-by: Álvaro Velad Galván <[email protected]>
Have you read the FAQ and checked for duplicate open issues?:
Yes
What version of Shaka Player are you using?:
master
Can you reproduce the issue with our latest release version?:
yes
Can you reproduce the issue with the latest code from
master
?: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?:
No, you need to test specific stream.
What browser and OS are you using?:
it is reproducible on Chrome and Tizen.
What are the manifest and license server URIs?:
If you require the custom app, I am happy to share it.
What did you do?
When an encrypted live stream becomes clear, it throws 4011 UNPLAYABLE_PERIOD error.
What did you expect to happen?
It should play the stream smoothly.
What actually happened?
playback is crashed.
This issue is introduced at this commit. When a stream is initiated, all supported types are stored at this array. However, supportedTypes only includes the codec and mime combination of initial segments as it is captured at this line. When the stream becomes clear, the ad segments have different codecs which are not in the supportedTypes array. So, these ad segments are filtered at this line. Although all ad segments are playable, they are filtered out and, eventually throws 4011 error code. I also attached supportedTypes array and an ad segment screenshots.
Please let me know if the issue is still unclear for you. Happy to help on fixing this issue.
Thank you,
Semih.
An ad segment =>
supportedTypes array =>
The text was updated successfully, but these errors were encountered: