Dynamic error tracking in RTCVideoDecoder

content/renderer/media/rtc_video_decoder.cc

Index: content/renderer/media/rtc_video_decoder.cc
diff --git a/content/renderer/media/rtc_video_decoder.cc b/content/renderer/media/rtc_video_decoder.cc
index ffe8b9ee00a43687c3700db720350008258ad6c5..43d4b014ec0091ae2e7e4c3c25f78006d5074e4b 100644
--- a/content/renderer/media/rtc_video_decoder.cc
+++ b/content/renderer/media/rtc_video_decoder.cc
@@ -28,7 +28,8 @@ namespace content {
 const int32_t RTCVideoDecoder::ID_LAST = 0x3FFFFFFF;
 const int32_t RTCVideoDecoder::ID_HALF = 0x20000000;
 const int32_t RTCVideoDecoder::ID_INVALID = -1;
-const uint32_t kNumVDAResetsBeforeSWFallback = 5;
+const uint32_t kNumVDAErrorsBeforeSWFallback = 5;
+const uint32_t kNumDecodeBeforeResettingVDAErrorCounter = 300;
 
 // Maximum number of concurrent VDA::Decode() operations RVD will maintain.
 // Higher values allow better pipelining in the GPU, but also require more
@@ -56,7 +57,7 @@ RTCVideoDecoder::BufferData::~BufferData() {}
 
 RTCVideoDecoder::RTCVideoDecoder(webrtc::VideoCodecType type,
                                  media::GpuVideoAcceleratorFactories* factories)
-    : num_vda_errors_(0),
+    : vda_error_counter_(0),
       video_codec_type_(type),
       factories_(factories),
       decoder_texture_target_(0),
@@ -163,8 +164,8 @@ int32_t RTCVideoDecoder::Decode(
   if (state_ == DECODE_ERROR) {
     LOG(ERROR) << "Decoding error occurred.";
     // Try reseting the session |kNumVDAErrorsHandled| times.

[mcasas, 2016/04/01 16:39:09]

nit: s/the session/ the session up to/

[emircan, 2016/04/05 18:02:18]

Done.

-    if (num_vda_errors_ > kNumVDAResetsBeforeSWFallback) {
-      DLOG(ERROR) << num_vda_errors_
+    if (vda_error_counter_ > kNumVDAErrorsBeforeSWFallback) {
+      DLOG(ERROR) << vda_error_counter_
                   << " errors reported by VDA, falling back to software decode";
       return WEBRTC_VIDEO_CODEC_FALLBACK_SOFTWARE;
     }
@@ -256,6 +257,7 @@ int32_t RTCVideoDecoder::Decode(
       FROM_HERE,
       base::Bind(&RTCVideoDecoder::RequestBufferDecode,
                  weak_factory_.GetWeakPtr()));
+  TryResetVDAErrorCounter();
   return WEBRTC_VIDEO_CODEC_OK;
 }
 
@@ -485,7 +487,7 @@ void RTCVideoDecoder::NotifyError(media::VideoDecodeAccelerator::Error error) {
 
   base::AutoLock auto_lock(lock_);
   state_ = DECODE_ERROR;
-  ++num_vda_errors_;
+  IncrementVDAErrorCounter();
 }
 
 void RTCVideoDecoder::RequestBufferDecode() {
@@ -838,4 +840,23 @@ void RTCVideoDecoder::ClearPendingBuffers() {
   pending_buffers_.clear();
 }
 
+void RTCVideoDecoder::IncrementVDAErrorCounter() {

[mcasas, 2016/04/01 16:39:09]

This file adds a suffix "_Lock" to the
methods that are supposed to work
under |lock.AssserAcquired()|, consider
doing the same here for consistency.
(I'd rather see them suffixed
"ThreadSafe" but that's another CL -
actually, can you make a bug for that?)

[emircan, 2016/04/05 18:02:18]

Done.

+  DVLOG(3) << __FUNCTION__;
+  lock_.AssertAcquired();
+
+  ++vda_error_counter_;
+  num_consecutive_vda_success_since_error_ = 0;
+}
+
+void RTCVideoDecoder::TryResetVDAErrorCounter() {
+  lock_.AssertAcquired();
+
+  if (vda_error_counter_ > 0) {

[mcasas, 2016/04/01 16:39:09]

if (vda_error_counter_ == 0)
  return;
//...

[emircan, 2016/04/05 18:02:18]

Done.

+    if (++num_consecutive_vda_success_since_error_ ==
+        kNumDecodeBeforeResettingVDAErrorCounter) {
+      vda_error_counter_ = 0;
+    }
+  }
+}
+
 }  // namespace content