1214469 - [EME] gmp-clearkey doesn't account for mNumInputTasks when flushing

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P2)

Description

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

From VideoDecoder.cpp:

void
VideoDecoder::DecodeTask(GMPVideoEncodedFrame* aInput)
{
  CK_LOGD("VideoDecoder::DecodeTask");
  AutoReleaseVideoFrame ensureFrameReleased(aInput);
  HRESULT hr;

  if (mIsFlushing) {
    CK_LOGD("VideoDecoder::DecodeTask rejecting frame: flushing.");
    return;
  }

  {
    AutoLock lock(mMutex);
    mNumInputTasks--;
    assert(mNumInputTasks >= 0);
  }
  // ...

The problem here is that if mIsFlushing it true, we won't decrement mNumInputTasks, and that means we won't call the input exhausted callback further down when we need to.

I bet this is the source of our timeouts.

Comment 1

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

Attachment: Patch: Ensure gmp-clearkey accounts for mNumInputTasks when flushing.

Ensure we'll decrement mNumInputTasks once for every DecodeTask.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=88e189600d93

Comment 2

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

If this fixes our tests on Windows, we should re-enable them, and uplift.

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/942fff0fb140

Comment 4

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

https://hg.mozilla.org/mozilla-central/rev/942fff0fb140

Comment 5

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

Comment on attachment 8673441 [details] [diff] [review]
Patch: Ensure gmp-clearkey accounts for mNumInputTasks when flushing.

Approval Request Comment
[Feature/regressing bug #]: EME
[User impact if declined]: This fixes intermittent timeouts in EME mochitests. This only affects clearkey EME, not Adobe EME.
[Describe test coverage new/current, TreeHerder]: Lots of EME mochitest, which are greener with this patch!
[Risks and why]: Low.
[String/UUID change made/needed]: None.

Comment 6

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8673441 [details] [diff] [review]
Patch: Ensure gmp-clearkey accounts for mNumInputTasks when flushing.

Improve the testsuite, taking it.
Should be in 42 beta 7

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ddb52d0e20cb

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/b09a797a251b