Fix dropped demuxer buffers for fallback decoder

Fix dropped demuxer buffers for fallback decoder

An edge case was missed due to a typo in a DCHECK, during the original
fallback decoder submission.

Currently, the following scenario can happen:
- DecoderStream requests buffers from the demuxer
  (STATE_PENDING_DEMUXER_READ).
- |decoder_| returns a DECODE_ERROR before a frame was ouptutted,
  initiating a decoder fallback (STATE_REINITIALIZING_DECODER).
- OnDecoderSelected() successfully completes (STATE_NORMAL).
- OnBufferReady() is called back (the typo in the DCHECK meant we never
  considered when we enter the function in STATE_NORMAL). We return at
  line 601 because the state is not STATE_PENDING_DEMUXER_READ, and drop
  the buffer.

This CL fixes the typo, and changes so we only drop the buffer whilst in
STATE_ERROR, and also handles the case where we had saved buffers in the
fallback buffer queue.

BUG=615141
TEST= Setup GpuVideoDecoder to fail 10% and refreshed the page for
several minutes. Hard to test the end to end though...

Committed: https://crrev.com/4849043c3e003ef57a431552f9a97cf159debd06
Cr-Commit-Position: refs/heads/master@{#396371}

[tguilbert, 2016-05-26 18:52:50]

tguilbert@chromium.org changed reviewers:
+ dalecurtis@google.com, sandersd@chromium.org

[tguilbert, 2016-05-26 18:52:51]

PTAL! :)

[DaleCurtis, 2016-05-26 18:55:53]

dalecurtis@chromium.org changed reviewers:
+ dalecurtis@chromium.org

[DaleCurtis, 2016-05-26 18:55:54]

lgtm nice catch! Is a unit test too had to write here?

[sandersd (OOO until July 31), 2016-05-26 18:56:47]

lgtm

https://codereview.chromium.org/2012293002/diff/20001/media/filters/decoder_s...
File media/filters/decoder_stream.cc (right):

https://codereview.chromium.org/2012293002/diff/20001/media/filters/decoder_s...
media/filters/decoder_stream.cc:341: // |pending_buffers_| has alreadby been
copied to |fallback_buffers_| and we
already

[tguilbert, 2016-05-26 21:58:26]

On 2016/05/26 18:55:54, DaleCurtis wrote:
> lgtm nice catch! Is a unit test too had to write here?

I started writing one and then stopped. It is non-trivial, due to the fact that
we immediately call ReadFromDemuxerStream() on successful re-init. We would need
to add test hooks into production code for the sole purpose of this test.

[tguilbert, 2016-05-26 22:01:27]

Thank you!

https://codereview.chromium.org/2012293002/diff/20001/media/filters/decoder_s...
File media/filters/decoder_stream.cc (right):

https://codereview.chromium.org/2012293002/diff/20001/media/filters/decoder_s...
media/filters/decoder_stream.cc:341: // |pending_buffers_| has alreadby been
copied to |fallback_buffers_| and we
On 2016/05/26 18:56:47, sandersd wrote:
> already

Done.

[tguilbert, 2016-05-26 22:01:38]

The CQ bit was checked by tguilbert@chromium.org

[tguilbert, 2016-05-26 22:01:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
dalecurtis@chromium.org, sandersd@chromium.org
Link to the patchset: https://codereview.chromium.org/2012293002/#ps40001
(title: "Typo in comments")

[commit-bot: I haz the power, 2016-05-26 22:02:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2012293002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2012293002/40001

[commit-bot: I haz the power, 2016-05-27 03:11:26]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-05-27 03:12:35]

Description was changed from

==========
Fix dropped demuxer buffers for fallback decoder

An edge case was missed due to a typo in a DCHECK, during the original
fallback decoder submission.

Currently, the following scenario can happen:
- DecoderStream requests buffers from the demuxer
  (STATE_PENDING_DEMUXER_READ).
- |decoder_| returns a DECODE_ERROR before a frame was ouptutted,
  initiating a decoder fallback (STATE_REINITIALIZING_DECODER).
- OnDecoderSelected() successfully completes (STATE_NORMAL).
- OnBufferReady() is called back (the typo in the DCHECK meant we never
  considered when we enter the function in STATE_NORMAL). We return at
  line 601 because the state is not STATE_PENDING_DEMUXER_READ, and drop
  the buffer.

This CL fixes the typo, and changes so we only drop the buffer whilst in
STATE_ERROR, and also handles the case where we had saved buffers in the
fallback buffer queue.

BUG=615141
TEST= Setup GpuVideoDecoder to fail 10% and refreshed the page for
several minutes. Hard to test the end to end though...
==========

to

==========
Fix dropped demuxer buffers for fallback decoder

An edge case was missed due to a typo in a DCHECK, during the original
fallback decoder submission.

Currently, the following scenario can happen:
- DecoderStream requests buffers from the demuxer
  (STATE_PENDING_DEMUXER_READ).
- |decoder_| returns a DECODE_ERROR before a frame was ouptutted,
  initiating a decoder fallback (STATE_REINITIALIZING_DECODER).
- OnDecoderSelected() successfully completes (STATE_NORMAL).
- OnBufferReady() is called back (the typo in the DCHECK meant we never
  considered when we enter the function in STATE_NORMAL). We return at
  line 601 because the state is not STATE_PENDING_DEMUXER_READ, and drop
  the buffer.

This CL fixes the typo, and changes so we only drop the buffer whilst in
STATE_ERROR, and also handles the case where we had saved buffers in the
fallback buffer queue.

BUG=615141
TEST= Setup GpuVideoDecoder to fail 10% and refreshed the page for
several minutes. Hard to test the end to end though...

Committed: https://crrev.com/4849043c3e003ef57a431552f9a97cf159debd06
Cr-Commit-Position: refs/heads/master@{#396371}
==========

[commit-bot: I haz the power, 2016-05-27 03:12:36]

Patchset 3 (id:??) landed as
https://crrev.com/4849043c3e003ef57a431552f9a97cf159debd06
Cr-Commit-Position: refs/heads/master@{#396371}