Start documenting the threading model

webrtc/modules/video_coding/generic_decoder.cc

Index: webrtc/modules/video_coding/generic_decoder.cc
diff --git a/webrtc/modules/video_coding/generic_decoder.cc b/webrtc/modules/video_coding/generic_decoder.cc
index 34500c69e2a39f9b69b0e2488318ce75ee0e370f..619cb8b886141f9c30404b4abf772271237dd9ee 100644
--- a/webrtc/modules/video_coding/generic_decoder.cc
+++ b/webrtc/modules/video_coding/generic_decoder.cc
@@ -8,11 +8,13 @@
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
+#include "webrtc/modules/video_coding/generic_decoder.h"
+
 #include "webrtc/base/checks.h"
 #include "webrtc/base/logging.h"
+#include "webrtc/base/platform_thread.h"
 #include "webrtc/base/trace_event.h"
 #include "webrtc/modules/video_coding/include/video_coding.h"
-#include "webrtc/modules/video_coding/generic_decoder.h"
 #include "webrtc/modules/video_coding/internal_defines.h"
 #include "webrtc/system_wrappers/include/clock.h"
 
@@ -20,25 +22,34 @@ namespace webrtc {
 
 VCMDecodedFrameCallback::VCMDecodedFrameCallback(VCMTiming* timing,
                                                  Clock* clock)
-    : _critSect(CriticalSectionWrapper::CreateCriticalSection()),
-      _clock(clock),
+    : _clock(clock),
       _receiveCallback(NULL),
       _timing(timing),
       _timestampMap(kDecoderFrameMemoryLength),
-      _lastReceivedPictureID(0) {}
+      _lastReceivedPictureID(0) {
+  decoder_thread_checker_.DetachFromThread();
+}
 
 VCMDecodedFrameCallback::~VCMDecodedFrameCallback() {
-  delete _critSect;
+  RTC_DCHECK(construction_thread_checker_.CalledOnValidThread());
 }
 
 void VCMDecodedFrameCallback::SetUserReceiveCallback(
     VCMReceiveCallback* receiveCallback) {
-  CriticalSectionScoped cs(_critSect);
+  RTC_DCHECK(construction_thread_checker_.CalledOnValidThread());
+  // TODO(tommi): Can we require that this be called before decoding starts
+  // or after it has been stopped?  If so, we don't need a lock.
+  rtc::CritScope cs(&_critSect);
   _receiveCallback = receiveCallback;
 }
 
 VCMReceiveCallback* VCMDecodedFrameCallback::UserReceiveCallback() {
-  CriticalSectionScoped cs(_critSect);
+  RTC_DCHECK(decoder_thread_checker_.CalledOnValidThread());
+  rtc::CritScope cs(&_critSect);
+  // BUG: Returning a pointer that requires a lock to get.
+  // TODO(tommi): If we're always called on the decoder thread and that
+  // modifying this value is not allowed while the thread is running,
+  // we can remove the lock and allow returning this pointer.
   return _receiveCallback;
 }
 
@@ -58,6 +69,8 @@ int32_t VCMDecodedFrameCallback::Decoded(VideoFrame& decodedImage,
 void VCMDecodedFrameCallback::Decoded(VideoFrame& decodedImage,
                                       rtc::Optional<int32_t> decode_time_ms,
                                       rtc::Optional<uint8_t> qp) {
+  RTC_DCHECK(decoder_thread_checker_.CalledOnValidThread());
+
   TRACE_EVENT_INSTANT1("webrtc", "VCMDecodedFrameCallback::Decoded",
                        "timestamp", decodedImage.timestamp());
   // TODO(holmer): We should improve this so that we can handle multiple
@@ -65,7 +78,7 @@ void VCMDecodedFrameCallback::Decoded(VideoFrame& decodedImage,
   VCMFrameInformation* frameInfo;
   VCMReceiveCallback* callback;
   {
-    CriticalSectionScoped cs(_critSect);
+    rtc::CritScope cs(&_critSect);
     frameInfo = _timestampMap.Pop(decodedImage.timestamp());
     callback = _receiveCallback;
   }
@@ -98,7 +111,8 @@ void VCMDecodedFrameCallback::Decoded(VideoFrame& decodedImage,
 
 int32_t VCMDecodedFrameCallback::ReceivedDecodedReferenceFrame(
     const uint64_t pictureId) {
-  CriticalSectionScoped cs(_critSect);
+  RTC_DCHECK(decoder_thread_checker_.CalledOnValidThread());
+  rtc::CritScope cs(&_critSect);
   if (_receiveCallback != NULL) {
     return _receiveCallback->ReceivedDecodedReferenceFrame(pictureId);
   }
@@ -107,29 +121,38 @@ int32_t VCMDecodedFrameCallback::ReceivedDecodedReferenceFrame(
 
 int32_t VCMDecodedFrameCallback::ReceivedDecodedFrame(
     const uint64_t pictureId) {
+  RTC_DCHECK(decoder_thread_checker_.CalledOnValidThread());
   _lastReceivedPictureID = pictureId;
   return 0;
 }
 
 uint64_t VCMDecodedFrameCallback::LastReceivedPictureID() const {
+  RTC_DCHECK(decoder_thread_checker_.CalledOnValidThread());
   return _lastReceivedPictureID;
 }
 
 void VCMDecodedFrameCallback::OnDecoderImplementationName(
     const char* implementation_name) {
-  CriticalSectionScoped cs(_critSect);
+  RTC_DCHECK(decoder_thread_checker_.CalledOnValidThread());
+  rtc::CritScope cs(&_critSect);
   if (_receiveCallback)
     _receiveCallback->OnDecoderImplementationName(implementation_name);
 }
 
 void VCMDecodedFrameCallback::Map(uint32_t timestamp,
                                   VCMFrameInformation* frameInfo) {
-  CriticalSectionScoped cs(_critSect);
+  RTC_DCHECK(decoder_thread_checker_.CalledOnValidThread());
+  // TODO(tommi): Looks like _timestampMap is only used on the decoder thread
+  // now?
+  rtc::CritScope cs(&_critSect);
   _timestampMap.Add(timestamp, frameInfo);
 }
 
 int32_t VCMDecodedFrameCallback::Pop(uint32_t timestamp) {
-  CriticalSectionScoped cs(_critSect);
+  RTC_DCHECK(decoder_thread_checker_.CalledOnValidThread());
+  // TODO(tommi): Looks like _timestampMap is only used on the decoder thread
+  // now?
+  rtc::CritScope cs(&_critSect);
   if (_timestampMap.Pop(timestamp) == NULL) {
     return VCM_GENERAL_ERROR;
   }
@@ -145,10 +168,19 @@ VCMGenericDecoder::VCMGenericDecoder(VideoDecoder* decoder, bool isExternal)
       _isExternal(isExternal),
       _keyFrameDecoded(false) {}
 
-VCMGenericDecoder::~VCMGenericDecoder() {}
+VCMGenericDecoder::~VCMGenericDecoder() {
+  // Can be deleted outside of the decoder thread (which is the construction
+  // thread).  Bug?
+  // See CallPerfTest.PlaysOutAudioAndVideoInSyncWithVideoNtpDrift
+
+  // In |modules_unittests|, the object can be created on the construction
+  // thread.
+  // RTC_DCHECK(!decoder_thread_checker_.CalledOnValidThread());
+}
 
 int32_t VCMGenericDecoder::InitDecode(const VideoCodec* settings,
                                       int32_t numberOfCores) {
+  RTC_DCHECK(decoder_thread_checker_.CalledOnValidThread());
   TRACE_EVENT0("webrtc", "VCMGenericDecoder::InitDecode");
   _codecType = settings->codecType;
 
@@ -156,49 +188,68 @@ int32_t VCMGenericDecoder::InitDecode(const VideoCodec* settings,
 }
 
 int32_t VCMGenericDecoder::Decode(const VCMEncodedFrame& frame, int64_t nowMs) {
-    TRACE_EVENT1("webrtc", "VCMGenericDecoder::Decode", "timestamp",
-                 frame.EncodedImage()._timeStamp);
-    _frameInfos[_nextFrameInfoIdx].decodeStartTimeMs = nowMs;
-    _frameInfos[_nextFrameInfoIdx].renderTimeMs = frame.RenderTimeMs();
-    _frameInfos[_nextFrameInfoIdx].rotation = frame.rotation();
-    _callback->Map(frame.TimeStamp(), &_frameInfos[_nextFrameInfoIdx]);
-
-    _nextFrameInfoIdx = (_nextFrameInfoIdx + 1) % kDecoderFrameMemoryLength;
-    const RTPFragmentationHeader dummy_header;
-    int32_t ret = _decoder->Decode(frame.EncodedImage(), frame.MissingFrame(),
-                                   &dummy_header,
-                                   frame.CodecSpecific(), frame.RenderTimeMs());
-
-    _callback->OnDecoderImplementationName(_decoder->ImplementationName());
-    if (ret < WEBRTC_VIDEO_CODEC_OK) {
-        LOG(LS_WARNING) << "Failed to decode frame with timestamp "
-                        << frame.TimeStamp() << ", error code: " << ret;
-        _callback->Pop(frame.TimeStamp());
-        return ret;
-    } else if (ret == WEBRTC_VIDEO_CODEC_NO_OUTPUT ||
-               ret == WEBRTC_VIDEO_CODEC_REQUEST_SLI) {
-        // No output
-        _callback->Pop(frame.TimeStamp());
-    }
+  RTC_DCHECK(decoder_thread_checker_.CalledOnValidThread());
+  TRACE_EVENT1("webrtc", "VCMGenericDecoder::Decode", "timestamp",
+               frame.EncodedImage()._timeStamp);
+  _frameInfos[_nextFrameInfoIdx].decodeStartTimeMs = nowMs;
+  _frameInfos[_nextFrameInfoIdx].renderTimeMs = frame.RenderTimeMs();
+  _frameInfos[_nextFrameInfoIdx].rotation = frame.rotation();
+  _callback->Map(frame.TimeStamp(), &_frameInfos[_nextFrameInfoIdx]);
+
+  _nextFrameInfoIdx = (_nextFrameInfoIdx + 1) % kDecoderFrameMemoryLength;
+  const RTPFragmentationHeader dummy_header;
+  int32_t ret = _decoder->Decode(frame.EncodedImage(), frame.MissingFrame(),
+                                 &dummy_header, frame.CodecSpecific(),
+                                 frame.RenderTimeMs());
+
+  _callback->OnDecoderImplementationName(_decoder->ImplementationName());
+  if (ret < WEBRTC_VIDEO_CODEC_OK) {
+    LOG(LS_WARNING) << "Failed to decode frame with timestamp "
+                    << frame.TimeStamp() << ", error code: " << ret;
+    _callback->Pop(frame.TimeStamp());
     return ret;
+  }
+
+  if (ret == WEBRTC_VIDEO_CODEC_NO_OUTPUT ||
+      ret == WEBRTC_VIDEO_CODEC_REQUEST_SLI) {
+    // No output
+    _callback->Pop(frame.TimeStamp());
+  }
+
+  return ret;
 }
 
 int32_t VCMGenericDecoder::Release() {
+  // Looks like this is called outside of the decoder thread.
+  // See CallPerfTest.PlaysOutAudioAndVideoInSyncWithVideoNtpDrift.
+  // TODO(tommi): This is called after the decoder thread has been stopped.
+  // Can we DCHECK that somehow?
+  // RTC_DCHECK(decoder_thread_checker_.CalledOnValidThread());
   return _decoder->Release();
 }
 
 int32_t VCMGenericDecoder::RegisterDecodeCompleteCallback(
     VCMDecodedFrameCallback* callback) {
+  RTC_DCHECK(decoder_thread_checker_.CalledOnValidThread());
   _callback = callback;
   return _decoder->RegisterDecodeCompleteCallback(callback);
 }
 
 bool VCMGenericDecoder::External() const {
+  // Called outside of the decoder thread.
+  // See CallPerfTest.PlaysOutAudioAndVideoInSyncWithVideoNtpDrift
+  // RTC_DCHECK(decoder_thread_checker_.CalledOnValidThread());
   return _isExternal;
 }
 
 bool VCMGenericDecoder::PrefersLateDecoding() const {
+  RTC_DCHECK(decoder_thread_checker_.CalledOnValidThread());
   return _decoder->PrefersLateDecoding();
 }
 
+void VCMGenericDecoder::PollDecodedFrames() {
+  RTC_DCHECK(decoder_thread_checker_.CalledOnValidThread());
+  _decoder->PollDecodedFrames();
+}
+
 }  // namespace webrtc