w3c / encrypted-media

Encrypted Media Extensions
https://w3c.github.io/encrypted-media/
Other
180 stars 80 forks source link

Fix #336: Allow implementations to finish rendering frames before firing waitingforkey #349

Closed ddorwin closed 7 years ago

ddorwin commented 7 years ago

This is an alternate fix for #336. It uses a variable to control whether the Wait for Key algorithm is run rather than trying to describe the condition as the previous PR #346 did.

jdsmith3000 commented 7 years ago

There's a fine difference between decryption requires usable key and waiting for key, and it feels like the former fills one of the reasons the latter was originally created. It has a finer meaning though, indicating that decryption can't proceed, but not that the pipeline is stalled. The existing waiting for key now only gets set once the pipeline can't proceed. I agree the names don't make this very clear.

I wonder if we want names that directly state decryption stalled waiting for key and playback stalled waiting for key.

I like how this fix gets rid of the algorithm "continue" step.

ddorwin commented 7 years ago

I'm fine with those names, though I worry that "stalled" has a different meaning the HTML spec.

@mwatson2, @cpearce, @foolip: Any comments on the naming or PR overall?

jdsmith3000 commented 7 years ago

Strictly speaking, only the decryption part is waiting for a key. The pipeline is waiting for decrypted media.

foolip commented 7 years ago

I don't see "stalled" in the changes, but in HTML that's about the network, fired when no data has been delivered for 3 seconds IIRC.

ddorwin commented 7 years ago

@foolip, "stalled" was in the suggested names in Jerry's comment. I used "blocked": decryption blocked waiting for key and playback blocked waiting for key.

jdsmith3000 commented 7 years ago

"blocked" works for me.

ddorwin commented 7 years ago

If there are no other comments, I'll merge this later today. We need to resolve the underlying issue, and I think we can address any remaining comments as separate issues or PRs.