In VideoCaptureImpl and subclasses add thread and lock annotations

Pehrsons · WebRTC LUCI CQ · commit eee10391cac9 · 2023-06-22T13:34:40.000Z
This annotates all unannotated members in VideoCaptureImpl and its subclasses with either of: - api_checker_: access on the api thread only - capture_checker_: access in callbacks/on the capture thread while capture is active, on the api thread otherwise - a Mutex if it is already protected by it Bug: webrtc:15181 Change-Id: I5084e7752a4716c29b85a9b403a88696f66d811f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/305647 Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org> Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org> Reviewed-by: Per Kjellander <perkj@webrtc.org> Cr-Commit-Position: refs/heads/main@{#40335}
diff --git a/modules/video_capture/BUILD.gn b/modules/video_capture/BUILD.gn
@@ -29,13 +29,15 @@ rtc_library("video_capture_module") {
 
   deps = [
     "../../api:scoped_refptr",
+    "../../api:sequence_checker",
     "../../api/video:video_frame",
     "../../api/video:video_rtp_headers",
     "../../common_video",
     "../../media:rtc_media_base",
     "../../rtc_base:event_tracer",
     "../../rtc_base:logging",
     "../../rtc_base:macromagic",
+    "../../rtc_base:race_checker",
     "../../rtc_base:refcount",
     "../../rtc_base:stringutils",
     "../../rtc_base:timeutils",
diff --git a/modules/video_capture/linux/video_capture_pipewire.cc b/modules/video_capture/linux/video_capture_pipewire.cc
@@ -51,10 +51,15 @@ VideoCaptureModulePipeWire::VideoCaptureModulePipeWire(
     : VideoCaptureImpl(), session_(options->pipewire_session()) {}
 
 VideoCaptureModulePipeWire::~VideoCaptureModulePipeWire() {
+  RTC_DCHECK_RUN_ON(&api_checker_);
+
   StopCapture();
 }
 
 int32_t VideoCaptureModulePipeWire::Init(const char* deviceUniqueId) {
+  RTC_CHECK_RUNS_SERIALIZED(&capture_checker_);
+  RTC_DCHECK_RUN_ON(&api_checker_);
+
   absl::optional<int> id;
   id = rtc::StringToNumber<int>(deviceUniqueId);
   if (id == absl::nullopt)
@@ -113,6 +118,9 @@ static spa_pod* BuildFormat(spa_pod_builder* builder,
 
 int32_t VideoCaptureModulePipeWire::StartCapture(
     const VideoCaptureCapability& capability) {
+  RTC_CHECK_RUNS_SERIALIZED(&capture_checker_);
+  RTC_DCHECK_RUN_ON(&api_checker_);
+
   uint8_t buffer[1024] = {};
 
   RTC_LOG(LS_VERBOSE) << "Creating new PipeWire stream for node " << node_id_;
@@ -166,6 +174,9 @@ int32_t VideoCaptureModulePipeWire::StartCapture(
 }
 
 int32_t VideoCaptureModulePipeWire::StopCapture() {
+  RTC_CHECK_RUNS_SERIALIZED(&capture_checker_);
+  RTC_DCHECK_RUN_ON(&api_checker_);
+
   PipeWireThreadLoopLock thread_loop_lock(session_->pw_main_loop_);
   if (stream_) {
     pw_stream_destroy(stream_);
@@ -177,13 +188,16 @@ int32_t VideoCaptureModulePipeWire::StopCapture() {
 }
 
 bool VideoCaptureModulePipeWire::CaptureStarted() {
+  RTC_DCHECK_RUN_ON(&api_checker_);
   MutexLock lock(&api_lock_);
 
   return started_;
 }
 
 int32_t VideoCaptureModulePipeWire::CaptureSettings(
     VideoCaptureCapability& settings) {
+  RTC_DCHECK_RUN_ON(&api_checker_);
+
   settings = _requestedCapability;
 
   return 0;
@@ -196,12 +210,15 @@ void VideoCaptureModulePipeWire::OnStreamParamChanged(
   VideoCaptureModulePipeWire* that =
       static_cast<VideoCaptureModulePipeWire*>(data);
   RTC_DCHECK(that);
+  RTC_CHECK_RUNS_SERIALIZED(&that->capture_checker_);
 
   if (format && id == SPA_PARAM_Format)
     that->OnFormatChanged(format);
 }
 
 void VideoCaptureModulePipeWire::OnFormatChanged(const struct spa_pod* format) {
+  RTC_CHECK_RUNS_SERIALIZED(&capture_checker_);
+
   uint32_t media_type, media_subtype;
 
   if (spa_format_parse(format, &media_type, &media_subtype) < 0) {
@@ -295,6 +312,7 @@ void VideoCaptureModulePipeWire::OnStreamStateChanged(
   VideoCaptureModulePipeWire* that =
       static_cast<VideoCaptureModulePipeWire*>(data);
   RTC_DCHECK(that);
+  RTC_CHECK_RUNS_SERIALIZED(&that->capture_checker_);
 
   MutexLock lock(&that->api_lock_);
   switch (state) {
@@ -319,10 +337,13 @@ void VideoCaptureModulePipeWire::OnStreamProcess(void* data) {
   VideoCaptureModulePipeWire* that =
       static_cast<VideoCaptureModulePipeWire*>(data);
   RTC_DCHECK(that);
+  RTC_CHECK_RUNS_SERIALIZED(&that->capture_checker_);
   that->ProcessBuffers();
 }
 
 void VideoCaptureModulePipeWire::ProcessBuffers() {
+  RTC_CHECK_RUNS_SERIALIZED(&capture_checker_);
+
   while (pw_buffer* buffer = pw_stream_dequeue_buffer(stream_)) {
     struct spa_meta_header* h;
     h = static_cast<struct spa_meta_header*>(
diff --git a/modules/video_capture/linux/video_capture_pipewire.h b/modules/video_capture/linux/video_capture_pipewire.h
@@ -43,13 +43,15 @@ class VideoCaptureModulePipeWire : public VideoCaptureImpl {
   void OnFormatChanged(const struct spa_pod* format);
   void ProcessBuffers();
 
-  rtc::scoped_refptr<PipeWireSession> session_;
-  int node_id_;
-  VideoCaptureCapability configured_capability_;
+  const rtc::scoped_refptr<PipeWireSession> session_
+      RTC_GUARDED_BY(capture_checker_);
+  int node_id_ RTC_GUARDED_BY(capture_checker_);
+  VideoCaptureCapability configured_capability_
+      RTC_GUARDED_BY(capture_checker_);
   bool started_ RTC_GUARDED_BY(api_lock_);
 
-  struct pw_stream* stream_;
-  struct spa_hook stream_listener_;
+  struct pw_stream* stream_ RTC_GUARDED_BY(capture_checker_);
+  struct spa_hook stream_listener_ RTC_GUARDED_BY(capture_checker_);
 };
 }  // namespace videocapturemodule
 }  // namespace webrtc
diff --git a/modules/video_capture/linux/video_capture_v4l2.cc b/modules/video_capture/linux/video_capture_v4l2.cc
@@ -58,6 +58,8 @@ VideoCaptureModuleV4L2::VideoCaptureModuleV4L2()
       _pool(NULL) {}
 
 int32_t VideoCaptureModuleV4L2::Init(const char* deviceUniqueIdUTF8) {
+  RTC_DCHECK_RUN_ON(&api_checker_);
+
   int len = strlen((const char*)deviceUniqueIdUTF8);
   _deviceUniqueId = new (std::nothrow) char[len + 1];
   if (_deviceUniqueId) {
@@ -99,13 +101,19 @@ int32_t VideoCaptureModuleV4L2::Init(const char* deviceUniqueIdUTF8) {
 }
 
 VideoCaptureModuleV4L2::~VideoCaptureModuleV4L2() {
+  RTC_DCHECK_RUN_ON(&api_checker_);
+  RTC_CHECK_RUNS_SERIALIZED(&capture_checker_);
+
   StopCapture();
   if (_deviceFd != -1)
     close(_deviceFd);
 }
 
 int32_t VideoCaptureModuleV4L2::StartCapture(
     const VideoCaptureCapability& capability) {
+  RTC_DCHECK_RUN_ON(&api_checker_);
+  RTC_CHECK_RUNS_SERIALIZED(&capture_checker_);
+
   if (_captureStarted) {
     if (capability == _requestedCapability) {
       return 0;
@@ -291,6 +299,8 @@ int32_t VideoCaptureModuleV4L2::StartCapture(
 }
 
 int32_t VideoCaptureModuleV4L2::StopCapture() {
+  RTC_DCHECK_RUN_ON(&api_checker_);
+
   if (!_captureThread.empty()) {
     {
       MutexLock lock(&capture_lock_);
@@ -300,6 +310,7 @@ int32_t VideoCaptureModuleV4L2::StopCapture() {
     _captureThread.Finalize();
   }
 
+  RTC_CHECK_RUNS_SERIALIZED(&capture_checker_);
   MutexLock lock(&capture_lock_);
   if (_captureStarted) {
     _captureStarted = false;
@@ -317,6 +328,7 @@ int32_t VideoCaptureModuleV4L2::StopCapture() {
 // critical section protected by the caller
 
 bool VideoCaptureModuleV4L2::AllocateVideoBuffers() {
+  RTC_CHECK_RUNS_SERIALIZED(&capture_checker_);
   struct v4l2_requestbuffers rbuffer;
   memset(&rbuffer, 0, sizeof(v4l2_requestbuffers));
 
@@ -367,6 +379,7 @@ bool VideoCaptureModuleV4L2::AllocateVideoBuffers() {
 }
 
 bool VideoCaptureModuleV4L2::DeAllocateVideoBuffers() {
+  RTC_CHECK_RUNS_SERIALIZED(&capture_checker_);
   // unmap buffers
   for (int i = 0; i < _buffersAllocatedByDevice; i++)
     munmap(_pool[i].start, _pool[i].length);
@@ -384,10 +397,13 @@ bool VideoCaptureModuleV4L2::DeAllocateVideoBuffers() {
 }
 
 bool VideoCaptureModuleV4L2::CaptureStarted() {
+  RTC_CHECK_RUNS_SERIALIZED(&capture_checker_);
   return _captureStarted;
 }
 
 bool VideoCaptureModuleV4L2::CaptureProcess() {
+  RTC_CHECK_RUNS_SERIALIZED(&capture_checker_);
+
   int retVal = 0;
   fd_set rSet;
   struct timeval timeout;
@@ -447,6 +463,7 @@ bool VideoCaptureModuleV4L2::CaptureProcess() {
 
 int32_t VideoCaptureModuleV4L2::CaptureSettings(
     VideoCaptureCapability& settings) {
+  RTC_DCHECK_RUN_ON(&api_checker_);
   settings = _requestedCapability;
 
   return 0;
diff --git a/modules/video_capture/linux/video_capture_v4l2.h b/modules/video_capture/linux/video_capture_v4l2.h
@@ -38,23 +38,24 @@ class VideoCaptureModuleV4L2 : public VideoCaptureImpl {
 
   static void CaptureThread(void*);
   bool CaptureProcess();
-  bool AllocateVideoBuffers();
-  bool DeAllocateVideoBuffers();
+  bool AllocateVideoBuffers() RTC_EXCLUSIVE_LOCKS_REQUIRED(capture_lock_);
+  bool DeAllocateVideoBuffers() RTC_EXCLUSIVE_LOCKS_REQUIRED(capture_lock_);
 
-  rtc::PlatformThread _captureThread;
-  Mutex capture_lock_;
+  rtc::PlatformThread _captureThread RTC_GUARDED_BY(api_checker_);
+  Mutex capture_lock_ RTC_ACQUIRED_BEFORE(api_lock_);
   bool quit_ RTC_GUARDED_BY(capture_lock_);
-  int32_t _deviceId;
-  int32_t _deviceFd;
+  int32_t _deviceId RTC_GUARDED_BY(api_checker_);
+  int32_t _deviceFd RTC_GUARDED_BY(capture_checker_);
 
-  int32_t _buffersAllocatedByDevice;
-  VideoCaptureCapability configured_capability_;
-  bool _captureStarted;
+  int32_t _buffersAllocatedByDevice RTC_GUARDED_BY(capture_lock_);
+  VideoCaptureCapability configured_capability_
+      RTC_GUARDED_BY(capture_checker_);
+  bool _captureStarted RTC_GUARDED_BY(capture_checker_);
   struct Buffer {
     void* start;
     size_t length;
   };
-  Buffer* _pool;
+  Buffer* _pool RTC_GUARDED_BY(capture_lock_);
 };
 }  // namespace videocapturemodule
 }  // namespace webrtc
diff --git a/modules/video_capture/video_capture_impl.cc b/modules/video_capture/video_capture_impl.cc
@@ -26,6 +26,7 @@ namespace webrtc {
 namespace videocapturemodule {
 
 const char* VideoCaptureImpl::CurrentDeviceName() const {
+  RTC_DCHECK_RUN_ON(&api_checker_);
   return _deviceUniqueId;
 }
 
@@ -89,6 +90,7 @@ VideoCaptureImpl::VideoCaptureImpl()
 }
 
 VideoCaptureImpl::~VideoCaptureImpl() {
+  RTC_DCHECK_RUN_ON(&api_checker_);
   DeRegisterCaptureDataCallback();
   if (_deviceUniqueId)
     delete[] _deviceUniqueId;
@@ -114,6 +116,8 @@ void VideoCaptureImpl::DeRegisterCaptureDataCallback() {
   _rawDataCallBack = NULL;
 }
 int32_t VideoCaptureImpl::DeliverCapturedFrame(VideoFrame& captureFrame) {
+  RTC_CHECK_RUNS_SERIALIZED(&capture_checker_);
+
   UpdateFrameCount();  // frame count used for local frame rate callback.
 
   if (_dataCallBack) {
@@ -127,6 +131,8 @@ void VideoCaptureImpl::DeliverRawFrame(uint8_t* videoFrame,
                                        size_t videoFrameLength,
                                        const VideoCaptureCapability& frameInfo,
                                        int64_t captureTime) {
+  RTC_CHECK_RUNS_SERIALIZED(&capture_checker_);
+
   UpdateFrameCount();
   _rawDataCallBack->OnRawFrame(videoFrame, videoFrameLength, frameInfo,
                                _rotateFrame, captureTime);
@@ -136,6 +142,7 @@ int32_t VideoCaptureImpl::IncomingFrame(uint8_t* videoFrame,
                                         size_t videoFrameLength,
                                         const VideoCaptureCapability& frameInfo,
                                         int64_t captureTime /*=0*/) {
+  RTC_CHECK_RUNS_SERIALIZED(&capture_checker_);
   MutexLock lock(&api_lock_);
 
   const int32_t width = frameInfo.width;
@@ -229,6 +236,7 @@ int32_t VideoCaptureImpl::IncomingFrame(uint8_t* videoFrame,
 
 int32_t VideoCaptureImpl::StartCapture(
     const VideoCaptureCapability& capability) {
+  RTC_DCHECK_RUN_ON(&api_checker_);
   _requestedCapability = capability;
   return -1;
 }
@@ -264,6 +272,8 @@ bool VideoCaptureImpl::GetApplyRotation() {
 }
 
 void VideoCaptureImpl::UpdateFrameCount() {
+  RTC_CHECK_RUNS_SERIALIZED(&capture_checker_);
+
   if (_incomingFrameTimesNanos[0] / rtc::kNumNanosecsPerMicrosec == 0) {
     // first no shift
   } else {
@@ -276,6 +286,8 @@ void VideoCaptureImpl::UpdateFrameCount() {
 }
 
 uint32_t VideoCaptureImpl::CalculateFrameRate(int64_t now_ns) {
+  RTC_CHECK_RUNS_SERIALIZED(&capture_checker_);
+
   int32_t num = 0;
   int32_t nrOfFrames = 0;
   for (num = 1; num < (kFrameRateCountHistorySize - 1); ++num) {
diff --git a/modules/video_capture/video_capture_impl.h b/modules/video_capture/video_capture_impl.h
@@ -19,12 +19,14 @@
 #include <stdint.h>
 
 #include "api/scoped_refptr.h"
+#include "api/sequence_checker.h"
 #include "api/video/video_frame.h"
 #include "api/video/video_rotation.h"
 #include "api/video/video_sink_interface.h"
 #include "modules/video_capture/video_capture.h"
 #include "modules/video_capture/video_capture_config.h"
 #include "modules/video_capture/video_capture_defines.h"
+#include "rtc_base/race_checker.h"
 #include "rtc_base/synchronization/mutex.h"
 #include "rtc_base/system/rtc_export.h"
 
@@ -86,36 +88,45 @@ class RTC_EXPORT VideoCaptureImpl : public VideoCaptureModule {
   VideoCaptureImpl();
   ~VideoCaptureImpl() override;
 
-  char* _deviceUniqueId;  // current Device unique name;
+  // Calls to the public API must happen on a single thread.
+  SequenceChecker api_checker_;
+  // RaceChecker for members that can be accessed on the API thread while
+  // capture is not happening, and on a callback thread otherwise.
+  rtc::RaceChecker capture_checker_;
+  // current Device unique name;
+  char* _deviceUniqueId RTC_GUARDED_BY(api_checker_);
   Mutex api_lock_;
-  VideoCaptureCapability _requestedCapability;  // Should be set by platform
-                                                // dependent code in
-                                                // StartCapture.
+  // Should be set by platform dependent code in StartCapture.
+  VideoCaptureCapability _requestedCapability RTC_GUARDED_BY(api_checker_);
+
  private:
   void UpdateFrameCount();
   uint32_t CalculateFrameRate(int64_t now_ns);
-  int32_t DeliverCapturedFrame(VideoFrame& captureFrame);
+  int32_t DeliverCapturedFrame(VideoFrame& captureFrame)
+      RTC_EXCLUSIVE_LOCKS_REQUIRED(api_lock_);
   void DeliverRawFrame(uint8_t* videoFrame,
                        size_t videoFrameLength,
                        const VideoCaptureCapability& frameInfo,
-                       int64_t captureTime);
+                       int64_t captureTime)
+      RTC_EXCLUSIVE_LOCKS_REQUIRED(api_lock_);
 
   // last time the module process function was called.
-  int64_t _lastProcessTimeNanos;
+  int64_t _lastProcessTimeNanos RTC_GUARDED_BY(capture_checker_);
   // last time the frame rate callback function was called.
-  int64_t _lastFrameRateCallbackTimeNanos;
+  int64_t _lastFrameRateCallbackTimeNanos RTC_GUARDED_BY(capture_checker_);
 
-  rtc::VideoSinkInterface<VideoFrame>* _dataCallBack;
-  RawVideoSinkInterface* _rawDataCallBack;
+  rtc::VideoSinkInterface<VideoFrame>* _dataCallBack RTC_GUARDED_BY(api_lock_);
+  RawVideoSinkInterface* _rawDataCallBack RTC_GUARDED_BY(api_lock_);
 
-  int64_t _lastProcessFrameTimeNanos;
+  int64_t _lastProcessFrameTimeNanos RTC_GUARDED_BY(capture_checker_);
   // timestamp for local captured frames
-  int64_t _incomingFrameTimesNanos[kFrameRateCountHistorySize];
-  VideoRotation _rotateFrame;  // Set if the frame should be rotated by the
-                               // capture module.
+  int64_t _incomingFrameTimesNanos[kFrameRateCountHistorySize] RTC_GUARDED_BY(
+      capture_checker_);
+  // Set if the frame should be rotated by the capture module.
+  VideoRotation _rotateFrame RTC_GUARDED_BY(api_lock_);
 
   // Indicate whether rotation should be applied before delivered externally.
-  bool apply_rotation_;
+  bool apply_rotation_ RTC_GUARDED_BY(api_lock_);
 };
 }  // namespace videocapturemodule
 }  // namespace webrtc
diff --git a/modules/video_capture/windows/video_capture_ds.cc b/modules/video_capture/windows/video_capture_ds.cc
diff --git a/modules/video_capture/windows/video_capture_ds.h b/modules/video_capture/windows/video_capture_ds.h
