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

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

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 1

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: gmp-flush.patch

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

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.

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/18f274867bcc

Comment 4

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 5

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e48acd29930d

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/312447440b3d

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/312447440b3d

Comment 8

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

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

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8649420 [details] [diff] [review]
gmp-flush.patch

Taking it for 42, Ritu will make the call for 41.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/87a060ffcaa8

Comment 11

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

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.

Comment 12

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

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.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/13570336af5f

Comment 14

[Chris Pearce [:cpearce (Not reading bugmail)]]

(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.