commit d0a71ba1ae69f73025e7381fa7e5593b3a0f7833
author tommi <tommi@webrtc.org> Tue Mar 14 04:16:20 2017 -0700
committer Commit bot <commit-bot@chromium.org> Tue Mar 14 11:16:20 2017 +0000
tree d846203992c69b891b971bbdfc5e61eec628b819
parent 4262f1f892ea4fed87cc3eb940954039a2aa261d

Updates to VCMDecodedFrameCallback, VideoReceiver and a few related classes/tests.

* The _receiveCallback member of VCMDecodedFrameCallback does actually not require locking now that the threading model is slightly clearer. Documentation and checks have been added.
* UserReceiveCallback() never returns null and must always be called on the decoder thread. Checks have been added and the two test suites that were failing to set this callback, have been fixed and a new mock class added.  (looks like sakal@ may have hit some issues with flaky tests there).
* Changed VcmPayloadSink to use move semantics which I suspect was the intention at the time the code was written (when we didn't have move semantics).
* Added thread checker to a couple of classes and started adding thread checks for known behavior.  There's more to be  done there.
* Remove the |_decoder| member variable in VideoReceiver. It is not needed and as it could be used, left us open to a race.
* TODOs added for places where we can reduce locking. I suspect that we can get away with not needing a lock around _codecDataBase in VideoReceiver once we've got a clear picture of the threading model and ensured that all adhere to it.

BUG=webrtc:7328

Review-Url: https://codereview.webrtc.org/2744013002
Cr-Commit-Position: refs/heads/master@{#17226}

webrtc/modules/video_coding/video_coding_impl.cc

diff --git a/webrtc/modules/video_coding/video_coding_impl.cc b/webrtc/modules/video_coding/video_coding_impl.cc
index c53a687..8718d1e 100644
--- a/webrtc/modules/video_coding/video_coding_impl.cc
+++ b/webrtc/modules/video_coding/video_coding_impl.cc
@@ -14,6 +14,7 @@
 #include <utility>
 
 #include "webrtc/base/criticalsection.h"
+#include "webrtc/base/thread_checker.h"
 #include "webrtc/common_types.h"
 #include "webrtc/common_video/include/video_bitrate_allocator.h"
 #include "webrtc/common_video/libyuv/include/webrtc_libyuv.h"
@@ -98,8 +99,8 @@
   int64_t TimeUntilNextProcess() override {
     int64_t sender_time = sender_.TimeUntilNextProcess();
     int64_t receiver_time = receiver_.TimeUntilNextProcess();
-    assert(sender_time >= 0);
-    assert(receiver_time >= 0);
+    RTC_DCHECK_GE(sender_time, 0);
+    RTC_DCHECK_GE(receiver_time, 0);
     return VCM_MIN(sender_time, receiver_time);
   }
 
@@ -190,6 +191,7 @@
 
   int32_t RegisterReceiveCallback(
       VCMReceiveCallback* receiveCallback) override {
+    RTC_DCHECK(construction_thread_.CalledOnValidThread());
     return receiver_.RegisterReceiveCallback(receiveCallback);
   }
 
@@ -210,6 +212,7 @@
 
   int32_t RegisterPacketRequestCallback(
       VCMPacketRequestCallback* callback) override {
+    RTC_DCHECK(construction_thread_.CalledOnValidThread());
     return receiver_.RegisterPacketRequestCallback(callback);
   }
 
@@ -277,6 +280,7 @@
   void TriggerDecoderShutdown() override { receiver_.TriggerDecoderShutdown(); }
 
  private:
+  rtc::ThreadChecker construction_thread_;
   EncodedImageCallbackWrapper post_encode_callback_;
   vcm::VideoSender sender_;
   std::unique_ptr<VideoBitrateAllocator> rate_allocator_;
@@ -316,8 +320,8 @@
     EventFactory* event_factory,
     NackSender* nack_sender,
     KeyFrameRequestSender* keyframe_request_sender) {
-  assert(clock);
-  assert(event_factory);
+  RTC_DCHECK(clock);
+  RTC_DCHECK(event_factory);
   return new VideoCodingModuleImpl(clock, event_factory, nack_sender,
                                    keyframe_request_sender, nullptr);
 }