Dynamic error tracking in RTCVideoDecoder

content/renderer/media/rtc_video_decoder.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/media/rtc_video_decoder.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/logging.h"
@@ skipping 10 matching lines @@
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/webrtc/base/bind.h"
 #include "third_party/webrtc/base/refcount.h"
 #include "third_party/webrtc/video_frame.h"
 
 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
 // resources.
 static const size_t kMaxInFlightDecodes = 8;
 
 // Number of allocated shared memory segments.
 static const size_t kNumSharedMemorySegments = 16;
 
 // Maximum number of pending WebRTC buffers that are waiting for shared memory.
 static const size_t kMaxNumOfPendingBuffers = 8;
 
 RTCVideoDecoder::BufferData::BufferData(int32_t bitstream_buffer_id,
                                         uint32_t timestamp,
                                         size_t size,
                                         const gfx::Rect& visible_rect)
     : bitstream_buffer_id(bitstream_buffer_id),
       timestamp(timestamp),
       size(size),
       visible_rect(visible_rect) {}
 
 RTCVideoDecoder::BufferData::BufferData() {}
 
 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),
       next_picture_buffer_id_(0),
       state_(UNINITIALIZED),
       decode_complete_callback_(nullptr),
       num_shm_buffers_(0),
       next_bitstream_buffer_id_(0),
       reset_bitstream_buffer_id_(ID_INVALID),
       weak_factory_(this) {
@@ skipping 85 matching lines @@
 
   base::AutoLock auto_lock(lock_);
 
   if (state_ == UNINITIALIZED || !decode_complete_callback_) {
     LOG(ERROR) << "The decoder has not initialized.";
     return WEBRTC_VIDEO_CODEC_UNINITIALIZED;
   }
 
   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;
     }
     base::AutoUnlock auto_unlock(lock_);
     Release();
     return WEBRTC_VIDEO_CODEC_ERROR;
   }
 
   if (missingFrames || !inputImage._completeFrame) {
     DLOG(ERROR) << "Missing or incomplete frames.";
@@ skipping 71 matching lines @@
     }
 
     return WEBRTC_VIDEO_CODEC_OK;
   }
 
   SaveToDecodeBuffers_Locked(inputImage, std::move(shm_buffer), buffer_data);
   factories_->GetTaskRunner()->PostTask(
       FROM_HERE,
       base::Bind(&RTCVideoDecoder::RequestBufferDecode,
                  weak_factory_.GetWeakPtr()));
+  TryResetVDAErrorCounter();
   return WEBRTC_VIDEO_CODEC_OK;
 }
 
 int32_t RTCVideoDecoder::RegisterDecodeCompleteCallback(
     webrtc::DecodedImageCallback* callback) {
   DVLOG(2) << "RegisterDecodeCompleteCallback";
   DCHECK(callback);
   base::AutoLock auto_lock(lock_);
   decode_complete_callback_ = callback;
   return WEBRTC_VIDEO_CODEC_OK;
@@ skipping 209 matching lines @@
   if (!vda_)
     return;
 
   LOG(ERROR) << "VDA Error:" << error;
   UMA_HISTOGRAM_ENUMERATION("Media.RTCVideoDecoderError", error,
                             media::VideoDecodeAccelerator::ERROR_MAX + 1);
   DestroyVDA();
 
   base::AutoLock auto_lock(lock_);
   state_ = DECODE_ERROR;
-  ++num_vda_errors_;
+  IncrementVDAErrorCounter();
 }
 
 void RTCVideoDecoder::RequestBufferDecode() {
   DCheckGpuVideoAcceleratorFactoriesTaskRunnerIsCurrent();
   if (!vda_)
     return;
 
   MovePendingBuffersToDecodeBuffers();
 
   while (CanMoreDecodeWorkBeDone()) {
@@ skipping 332 matching lines @@
   DCHECK(factories_->GetTaskRunner()->BelongsToCurrentThread());
 }
 
 void RTCVideoDecoder::ClearPendingBuffers() {
   // Delete WebRTC input buffers.
   for (const auto& pending_buffer : pending_buffers_)
     delete[] pending_buffer.first._buffer;
   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