From 52fec7d3e9c8503d5c7455398784a874a1fed90f Mon Sep 17 00:00:00 2001 From: Jan Grulich Date: Sun, 11 Feb 2024 11:26:01 +0100 Subject: [PATCH] Video capture V4L2: fix wrong usage of capture race checker This RaceChecker is intended to be used on API thread only when we are not capturing, however, since StartCapture() can be called while already capturing, we have to avoid using it to guard members that do not meet this expectations. Use API checker for _captureStarted instead and move the capture race checker down where we can be sure that capturing is not happening. Bug: webrtc:15181 Change-Id: I52f74b893f2c36c3ce0facd053b003fa497101b0 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/338040 Reviewed-by: Ilya Nikolaevskiy Commit-Queue: Jan Grulich Cr-Commit-Position: refs/heads/main@{#41714} --- .../video_capture/linux/video_capture_v4l2.cc | 19 ++++++++++++++----- .../video_capture/linux/video_capture_v4l2.h | 3 ++- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/modules/video_capture/linux/video_capture_v4l2.cc b/modules/video_capture/linux/video_capture_v4l2.cc index 7a70c2ff88..377c1ec878 100644 --- a/modules/video_capture/linux/video_capture_v4l2.cc +++ b/modules/video_capture/linux/video_capture_v4l2.cc @@ -112,7 +112,6 @@ VideoCaptureModuleV4L2::~VideoCaptureModuleV4L2() { int32_t VideoCaptureModuleV4L2::StartCapture( const VideoCaptureCapability& capability) { RTC_DCHECK_RUN_ON(&api_checker_); - RTC_CHECK_RUNS_SERIALIZED(&capture_checker_); if (_captureStarted) { if (capability == _requestedCapability) { @@ -122,6 +121,13 @@ int32_t VideoCaptureModuleV4L2::StartCapture( } } + // We don't want members above to be guarded by capture_checker_ as + // it's meant to be for members that are accessed on the API thread + // only when we are not capturing. The code above can be called many + // times while sharing instance of VideoCaptureV4L2 between websites + // and therefore it would not follow the requirements of this checker. + RTC_CHECK_RUNS_SERIALIZED(&capture_checker_); + // Set a baseline of configured parameters. It is updated here during // configuration, then read from the capture thread. configured_capability_ = capability; @@ -283,6 +289,7 @@ int32_t VideoCaptureModuleV4L2::StartCapture( _requestedCapability = capability; _captureStarted = true; + _streaming = true; // start capture thread; if (_captureThread.empty()) { @@ -310,10 +317,12 @@ int32_t VideoCaptureModuleV4L2::StopCapture() { _captureThread.Finalize(); } + _captureStarted = false; + RTC_CHECK_RUNS_SERIALIZED(&capture_checker_); MutexLock lock(&capture_lock_); - if (_captureStarted) { - _captureStarted = false; + if (_streaming) { + _streaming = false; DeAllocateVideoBuffers(); close(_deviceFd); @@ -397,7 +406,7 @@ bool VideoCaptureModuleV4L2::DeAllocateVideoBuffers() { } bool VideoCaptureModuleV4L2::CaptureStarted() { - RTC_CHECK_RUNS_SERIALIZED(&capture_checker_); + RTC_DCHECK_RUN_ON(&api_checker_); return _captureStarted; } @@ -434,7 +443,7 @@ bool VideoCaptureModuleV4L2::CaptureProcess() { return true; } - if (_captureStarted) { + if (_streaming) { struct v4l2_buffer buf; memset(&buf, 0, sizeof(struct v4l2_buffer)); buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE; diff --git a/modules/video_capture/linux/video_capture_v4l2.h b/modules/video_capture/linux/video_capture_v4l2.h index 0191e41876..9bc4ce8402 100644 --- a/modules/video_capture/linux/video_capture_v4l2.h +++ b/modules/video_capture/linux/video_capture_v4l2.h @@ -50,7 +50,8 @@ class VideoCaptureModuleV4L2 : public VideoCaptureImpl { int32_t _buffersAllocatedByDevice RTC_GUARDED_BY(capture_lock_); VideoCaptureCapability configured_capability_ RTC_GUARDED_BY(capture_checker_); - bool _captureStarted RTC_GUARDED_BY(capture_checker_); + bool _streaming RTC_GUARDED_BY(capture_checker_); + bool _captureStarted RTC_GUARDED_BY(api_checker_); struct Buffer { void* start; size_t length;