Consider non-keyframes for RTCVideoEncoder error counter

Consider non-keyframes for RTCVideoEncoder error counter

This CL addresses the bug below. Earlier on, we experimented and
decided that we would fallback to SW decoded after 50 consecutive
HW decode errors. However, after an error in keyframe, non-keyframes
would be dropped and not added to the error counter. As a result,
we would wait for 50 keyframe errors before SW fallback, which
takes a long time. This CL fixes the issues resulting from this.

BUG=616968
TEST=Tried the problematic Cisco stream referred in bug.
First 3-5 seconds, we don't get a video(due to HW errors) but works
afterwards(falling back to SW).

Committed: https://crrev.com/371076858624f9f2619703fa7ef2e1909a8ee733
Cr-Commit-Position: refs/heads/master@{#403353}

[emircan, 2016-06-23 22:50:23]

Description was changed from

==========
webex-fix

BUG=
==========

to

==========
Consider non-keyframes for RTCVideoEncoder error counter

This CL addresses the bug below. Earlier on, we experimented and 
decided that we would fallback to SW decoded after 50 consecutive 
HW decode errors. However, after an error in keyframe, non-keyframes 
would be dropped and not added to the error counter. As a result, 
we would wait for 50 keyframe errors before SW fallback, which 
takes a long time. This CL fixes the issues resulting from this.

BUG=616968
==========

[emircan, 2016-06-23 22:50:23]

emircan@chromium.org changed reviewers:
+ mcasas@chromium.org

[emircan, 2016-06-23 22:52:25]

PTAL.

[emircan, 2016-06-29 02:20:15]

Detailed explanation: This CL tries to address the bug we found while debugging
Webex problems.
- Currently, we wait for 50 consecutive key frame errors from HW decoder to
fallback to SW encoder. This is a long time.
- Instead, we can wait for 50 errors combined (keyframe and non-keyframe) to
fallback.

[mcasas, 2016-06-30 18:07:26]

LGTM in form, please add some TEST=
line detailing how this is verified in the
field if it makes sense.

https://codereview.chromium.org/2093823002/diff/1/content/renderer/media/rtc_...
File content/renderer/media/rtc_video_decoder.cc (right):

https://codereview.chromium.org/2093823002/diff/1/content/renderer/media/rtc_...
content/renderer/media/rtc_video_decoder.cc:239: DVLOG(1) << "The first frame
should be a key frame. Drop this.";
This DVLOG() goes with l.243 now.

https://codereview.chromium.org/2093823002/diff/1/content/renderer/media/rtc_...
content/renderer/media/rtc_video_decoder.cc:241: ++vda_error_counter_;
I'd write this instead:

// If we're are in an error condition, increase the counter
vda_error_counter_ += vda_error_counter_ ? 1 : 0;

WDYT?

https://codereview.chromium.org/2093823002/diff/1/content/renderer/media/rtc_...
File content/renderer/media/rtc_video_decoder_unittest.cc (right):

https://codereview.chromium.org/2093823002/diff/1/content/renderer/media/rtc_...
content/renderer/media/rtc_video_decoder_unittest.cc:285: 
Remove empty line. Better off: write a test
description :) something like:
 
// Tests/Verifies that |rtc_encoder_| drops incoming frames and its error
// counter is increased when in an error condition.

[emircan, 2016-06-30 20:07:17]

Description was changed from

==========
Consider non-keyframes for RTCVideoEncoder error counter

This CL addresses the bug below. Earlier on, we experimented and 
decided that we would fallback to SW decoded after 50 consecutive 
HW decode errors. However, after an error in keyframe, non-keyframes 
would be dropped and not added to the error counter. As a result, 
we would wait for 50 keyframe errors before SW fallback, which 
takes a long time. This CL fixes the issues resulting from this.

BUG=616968
==========

to

==========
Consider non-keyframes for RTCVideoEncoder error counter

This CL addresses the bug below. Earlier on, we experimented and 
decided that we would fallback to SW decoded after 50 consecutive 
HW decode errors. However, after an error in keyframe, non-keyframes 
would be dropped and not added to the error counter. As a result, 
we would wait for 50 keyframe errors before SW fallback, which 
takes a long time. This CL fixes the issues resulting from this.

BUG=616968
TEST=Tried the problematic Cisco stream referred in bug.
First 3-5 seconds, we don't get a video(due to HW errors) but works
afterwards(falling back to SW).
==========

[emircan, 2016-06-30 20:39:17]

https://codereview.chromium.org/2093823002/diff/1/content/renderer/media/rtc_...
File content/renderer/media/rtc_video_decoder.cc (right):

https://codereview.chromium.org/2093823002/diff/1/content/renderer/media/rtc_...
content/renderer/media/rtc_video_decoder.cc:239: DVLOG(1) << "The first frame
should be a key frame. Drop this.";
On 2016/06/30 18:07:25, mcasas wrote:
> This DVLOG() goes with l.243 now.

Done.

https://codereview.chromium.org/2093823002/diff/1/content/renderer/media/rtc_...
content/renderer/media/rtc_video_decoder.cc:241: ++vda_error_counter_;
On 2016/06/30 18:07:25, mcasas wrote:
> I'd write this instead:
> 
> // If we're are in an error condition, increase the counter
> vda_error_counter_ += vda_error_counter_ ? 1 : 0;
> 
> WDYT?

Done.

[emircan, 2016-06-30 21:21:21]

Patchset #2 (id:20001) has been deleted

[emircan, 2016-06-30 22:41:20]

Patchset #2 (id:40001) has been deleted

[emircan, 2016-06-30 23:31:10]

The CQ bit was checked by emircan@chromium.org

[emircan, 2016-06-30 23:31:10]

The patchset sent to the CQ was uploaded after l-g-t-m from mcasas@chromium.org
Link to the patchset: https://codereview.chromium.org/2093823002/#ps60001
(title: "mcasas@ comments.")

[commit-bot: I haz the power, 2016-06-30 23:32:06]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-30 23:55:31]

Description was changed from

==========
Consider non-keyframes for RTCVideoEncoder error counter

This CL addresses the bug below. Earlier on, we experimented and 
decided that we would fallback to SW decoded after 50 consecutive 
HW decode errors. However, after an error in keyframe, non-keyframes 
would be dropped and not added to the error counter. As a result, 
we would wait for 50 keyframe errors before SW fallback, which 
takes a long time. This CL fixes the issues resulting from this.

BUG=616968
TEST=Tried the problematic Cisco stream referred in bug.
First 3-5 seconds, we don't get a video(due to HW errors) but works
afterwards(falling back to SW).
==========

to

==========
Consider non-keyframes for RTCVideoEncoder error counter

This CL addresses the bug below. Earlier on, we experimented and 
decided that we would fallback to SW decoded after 50 consecutive 
HW decode errors. However, after an error in keyframe, non-keyframes 
would be dropped and not added to the error counter. As a result, 
we would wait for 50 keyframe errors before SW fallback, which 
takes a long time. This CL fixes the issues resulting from this.

BUG=616968
TEST=Tried the problematic Cisco stream referred in bug.
First 3-5 seconds, we don't get a video(due to HW errors) but works
afterwards(falling back to SW).
==========

[commit-bot: I haz the power, 2016-06-30 23:55:32]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2016-06-30 23:55:47]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-06-30 23:57:08]

Description was changed from

==========
Consider non-keyframes for RTCVideoEncoder error counter

This CL addresses the bug below. Earlier on, we experimented and 
decided that we would fallback to SW decoded after 50 consecutive 
HW decode errors. However, after an error in keyframe, non-keyframes 
would be dropped and not added to the error counter. As a result, 
we would wait for 50 keyframe errors before SW fallback, which 
takes a long time. This CL fixes the issues resulting from this.

BUG=616968
TEST=Tried the problematic Cisco stream referred in bug.
First 3-5 seconds, we don't get a video(due to HW errors) but works
afterwards(falling back to SW).
==========

to

==========
Consider non-keyframes for RTCVideoEncoder error counter

This CL addresses the bug below. Earlier on, we experimented and
decided that we would fallback to SW decoded after 50 consecutive
HW decode errors. However, after an error in keyframe, non-keyframes
would be dropped and not added to the error counter. As a result,
we would wait for 50 keyframe errors before SW fallback, which
takes a long time. This CL fixes the issues resulting from this.

BUG=616968
TEST=Tried the problematic Cisco stream referred in bug.
First 3-5 seconds, we don't get a video(due to HW errors) but works
afterwards(falling back to SW).

Committed: https://crrev.com/371076858624f9f2619703fa7ef2e1909a8ee733
Cr-Commit-Position: refs/heads/master@{#403353}
==========

[commit-bot: I haz the power, 2016-06-30 23:57:09]

Patchset 2 (id:??) landed as
https://crrev.com/371076858624f9f2619703fa7ef2e1909a8ee733
Cr-Commit-Position: refs/heads/master@{#403353}