commit 8f1903e3cff7ca9169c3704e73420dd24c34bb08
author Andreas Pehrson <apehrson@mozilla.com> Wed May 17 12:03:17 2023
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Jun 14 13:49:09 2023
tree fded2b2cea457ca16ce19b11496363f944d582e7
parent b7af6b963bedfa9015d015f4b08d9b1e7766bcda

In VideoCaptureImpl lock api_lock_ when accessing apply_rotation_

A comment says SetApplyRotation could deadlock if grabbing the lock, but
it does not make any calls under the lock so that is impossible, unless
the caller of SetApplyRotation involves a second lock that the callback
tries to grab, in which case it appears to be a problem of the caller.

Bug: webrtc:15181
Change-Id: Ie16cb01ffb84e9118dd5c87863c29bd107a6c94e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/305646
Commit-Queue: Stefan Holmer <stefan@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40279}

modules/video_capture/video_capture_impl.cc

diff --git a/modules/video_capture/video_capture_impl.cc b/modules/video_capture/video_capture_impl.cc
index ef1300f..3864ffa 100644
--- a/modules/video_capture/video_capture_impl.cc
+++ b/modules/video_capture/video_capture_impl.cc
@@ -167,10 +167,7 @@
   int target_width = width;
   int target_height = abs(height);
 
-  // SetApplyRotation doesn't take any lock. Make a local copy here.
-  bool apply_rotation = apply_rotation_;
-
-  if (apply_rotation) {
+  if (apply_rotation_) {
     // Rotating resolution when for 90/270 degree rotations.
     if (_rotateFrame == kVideoRotation_90 ||
         _rotateFrame == kVideoRotation_270) {
@@ -186,7 +183,7 @@
       target_width, target_height, stride_y, stride_uv, stride_uv);
 
   libyuv::RotationMode rotation_mode = libyuv::kRotate0;
-  if (apply_rotation) {
+  if (apply_rotation_) {
     switch (_rotateFrame) {
       case kVideoRotation_0:
         rotation_mode = libyuv::kRotate0;
@@ -221,7 +218,7 @@
           .set_video_frame_buffer(buffer)
           .set_timestamp_rtp(0)
           .set_timestamp_ms(rtc::TimeMillis())
-          .set_rotation(!apply_rotation ? _rotateFrame : kVideoRotation_0)
+          .set_rotation(!apply_rotation_ ? _rotateFrame : kVideoRotation_0)
           .build();
   captureFrame.set_ntp_time_ms(captureTime);
 
@@ -256,14 +253,13 @@
 }
 
 bool VideoCaptureImpl::SetApplyRotation(bool enable) {
-  // We can't take any lock here as it'll cause deadlock with IncomingFrame.
-
-  // The effect of this is the last caller wins.
+  MutexLock lock(&api_lock_);
   apply_rotation_ = enable;
   return true;
 }
 
 bool VideoCaptureImpl::GetApplyRotation() {
+  MutexLock lock(&api_lock_);
   return apply_rotation_;
 }