Closed Bug 1195939 Opened 5 years ago Closed 5 years ago

[EME] Seeking around an encrypted video quickly causes playback to stall

Categories

(Core :: Audio/Video, defect)

41 Branch
All
Windows
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox41 --- fixed
firefox42 --- fixed
firefox43 --- fixed

People

(Reporter: eflores, Assigned: eflores)

Details

Attachments

(1 file)

STR:
1. Load http://people.mozilla.org/~cpearce/mse-clearkey/
2. Seek around quickly by dragging the time slider around, for about 20 seconds.

Expected:
Video should seek and continue playing.

Observed:
Video stalls indefinitely, sometimes with the error "Video can't be played because the file is corrupt".


We are running out of buffers at [1], because when Reset() is called in the CDM, we tell the decoder to flush but don't flush our task queue [2].


[1] https://dxr.mozilla.org/mozilla-central/rev/90d9b7c391d38ae118865bd87b5d011feee6dded/dom/media/gmp/GMPVideoDecoderParent.cpp#148
[2] https://dxr.mozilla.org/mozilla-central/rev/90d9b7c391d38ae118865bd87b5d011feee6dded/media/gmp-clearkey/0.1/VideoDecoder.cpp?offset=0#347
Comment on attachment 8649420 [details] [diff] [review]
gmp-flush.patch

Review of attachment 8649420 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/gmp-clearkey/0.1/VideoDecoder.cpp
@@ +366,4 @@
>    if (mDecoder) {
>      mDecoder->Reset();
>    }
> +  mWorkerThread->Post(WrapTaskRefCounted(this,

Add comment explaining why this is necessary.
Attachment #8649420 - Flags: review?(cpearce) → review+
This is causing Windows permafail.
https://treeherder.mozilla.org/logviewer.html#?job_id=13018414&repo=mozilla-inbound

I don't have access to a tree to backout right now, but I've closed inbound in the mean time.
Flags: needinfo?(edwin)
Flags: needinfo?(edwin)
https://hg.mozilla.org/mozilla-central/rev/312447440b3d
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Comment on attachment 8649420 [details] [diff] [review]
gmp-flush.patch

Approval Request Comment
[Feature/regressing bug #]: ClearKey CDM
[User impact if declined]: Seeking can break when viewing encrypted media
[Describe test coverage new/current, TreeHerder]: Tested by EME mochitests
[Risks and why]: Slim to none
[String/UUID change made/needed]: No
Attachment #8649420 - Flags: approval-mozilla-beta?
Attachment #8649420 - Flags: approval-mozilla-aurora?
Comment on attachment 8649420 [details] [diff] [review]
gmp-flush.patch

Taking it for 42, Ritu will make the call for 41.
Attachment #8649420 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Chris, I will be approving this fix for uplift and it will be in 41.0b6. Since this is related to EME, do you think we need additional verification to ensure there are not unexpected fall outs w.r.t Netflix/EME scenarios? Thanks.
Flags: needinfo?(cpearce)
Comment on attachment 8649420 [details] [diff] [review]
gmp-flush.patch

Approving this given that the patch is not over complicated and the automated mochi tests pass. Let's uplift to Beta41. I have also asked Chris whether this fix needs a bit more additional testing on his side.
Attachment #8649420 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Ritu Kothari (:ritu) from comment #12)
> Comment on attachment 8649420 [details] [diff] [review]
> gmp-flush.patch
> 
> Approving this given that the patch is not over complicated and the
> automated mochi tests pass. Let's uplift to Beta41. I have also asked Chris
> whether this fix needs a bit more additional testing on his side.

This only affects "ClearKey" content, and will not affect content shown on Netflix using Adobe Primetime EME. So we need to worry about breaking Netflix; this cannot affect Netflix.
Flags: needinfo?(cpearce)
Flags: qe-verify+
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.