From 1added5666cd23622de46b7f9a5a9a34b2500e2a Mon Sep 17 00:00:00 2001 From: Markus Handell Date: Fri, 10 Jul 2020 12:56:40 +0000 Subject: [PATCH] Revert "Migrate modules/desktop_capture and modules/video_capture to webrtc::Mutex." This reverts commit 44dd3d743517fe85212ba4f68bda1e78c2e6d7ec. Reason for revert: crbug.com/1104081 Original change's description: > Migrate modules/desktop_capture and modules/video_capture to webrtc::Mutex. > > Bug: webrtc:11567 > Change-Id: I7bfca17f91bf44151148f863480ce77804d53a04 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/178805 > Commit-Queue: Markus Handell > Reviewed-by: Tommi > Cr-Commit-Position: refs/heads/master@{#31681} TBR=tommi@webrtc.org,handellm@webrtc.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: webrtc:11567 Change-Id: I4ee39947ba206522bce611341caef84ddb538068 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/179080 Reviewed-by: Markus Handell Commit-Queue: Markus Handell Cr-Commit-Position: refs/heads/master@{#31702} --- modules/desktop_capture/BUILD.gn | 1 - .../mac/desktop_configuration_monitor.cc | 6 +++--- .../mac/desktop_configuration_monitor.h | 4 ++-- .../win/dxgi_duplicator_controller.cc | 16 ++++++++-------- .../win/dxgi_duplicator_controller.h | 4 ++-- modules/video_capture/BUILD.gn | 3 --- .../video_capture/linux/video_capture_linux.cc | 8 ++++---- .../video_capture/linux/video_capture_linux.h | 6 +++--- .../test/video_capture_unittest.cc | 18 +++++++++--------- modules/video_capture/video_capture_impl.cc | 8 ++++---- modules/video_capture/video_capture_impl.h | 4 ++-- .../video_capture/windows/video_capture_ds.cc | 4 ++-- 12 files changed, 39 insertions(+), 43 deletions(-) diff --git a/modules/desktop_capture/BUILD.gn b/modules/desktop_capture/BUILD.gn index eb26f5c7bb..c9314368c5 100644 --- a/modules/desktop_capture/BUILD.gn +++ b/modules/desktop_capture/BUILD.gn @@ -480,7 +480,6 @@ rtc_library("desktop_capture_generic") { "../../api:scoped_refptr", "../../rtc_base", # TODO(kjellander): Cleanup in bugs.webrtc.org/3806. "../../rtc_base:checks", - "../../rtc_base/synchronization:mutex", "../../rtc_base/synchronization:rw_lock_wrapper", "../../rtc_base/system:arch", "../../rtc_base/system:rtc_export", diff --git a/modules/desktop_capture/mac/desktop_configuration_monitor.cc b/modules/desktop_capture/mac/desktop_configuration_monitor.cc index 048a679ecc..e2225cd4a9 100644 --- a/modules/desktop_capture/mac/desktop_configuration_monitor.cc +++ b/modules/desktop_capture/mac/desktop_configuration_monitor.cc @@ -21,7 +21,7 @@ DesktopConfigurationMonitor::DesktopConfigurationMonitor() { DesktopConfigurationMonitor::DisplaysReconfiguredCallback, this); if (err != kCGErrorSuccess) RTC_LOG(LS_ERROR) << "CGDisplayRegisterReconfigurationCallback " << err; - MutexLock lock(&desktop_configuration_lock_); + rtc::CritScope cs(&desktop_configuration_lock_); desktop_configuration_ = MacDesktopConfiguration::GetCurrent( MacDesktopConfiguration::TopLeftOrigin); } @@ -34,7 +34,7 @@ DesktopConfigurationMonitor::~DesktopConfigurationMonitor() { } MacDesktopConfiguration DesktopConfigurationMonitor::desktop_configuration() { - MutexLock lock(&desktop_configuration_lock_); + rtc::CritScope crit(&desktop_configuration_lock_); return desktop_configuration_; } @@ -64,7 +64,7 @@ void DesktopConfigurationMonitor::DisplaysReconfigured( reconfiguring_displays_.erase(display); if (reconfiguring_displays_.empty()) { - MutexLock lock(&desktop_configuration_lock_); + rtc::CritScope cs(&desktop_configuration_lock_); desktop_configuration_ = MacDesktopConfiguration::GetCurrent( MacDesktopConfiguration::TopLeftOrigin); } diff --git a/modules/desktop_capture/mac/desktop_configuration_monitor.h b/modules/desktop_capture/mac/desktop_configuration_monitor.h index 46a66d1d4c..1ed4c6bbcf 100644 --- a/modules/desktop_capture/mac/desktop_configuration_monitor.h +++ b/modules/desktop_capture/mac/desktop_configuration_monitor.h @@ -19,7 +19,7 @@ #include "api/ref_counted_base.h" #include "modules/desktop_capture/mac/desktop_configuration.h" #include "rtc_base/constructor_magic.h" -#include "rtc_base/synchronization/mutex.h" +#include "rtc_base/critical_section.h" namespace webrtc { @@ -41,7 +41,7 @@ class DesktopConfigurationMonitor : public rtc::RefCountedBase { void DisplaysReconfigured(CGDirectDisplayID display, CGDisplayChangeSummaryFlags flags); - Mutex desktop_configuration_lock_; + rtc::CriticalSection desktop_configuration_lock_; MacDesktopConfiguration desktop_configuration_ RTC_GUARDED_BY(&desktop_configuration_lock_); std::set reconfiguring_displays_; diff --git a/modules/desktop_capture/win/dxgi_duplicator_controller.cc b/modules/desktop_capture/win/dxgi_duplicator_controller.cc index 1334703a6e..bdf495837e 100644 --- a/modules/desktop_capture/win/dxgi_duplicator_controller.cc +++ b/modules/desktop_capture/win/dxgi_duplicator_controller.cc @@ -85,14 +85,14 @@ void DxgiDuplicatorController::Release() { } bool DxgiDuplicatorController::IsSupported() { - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); return Initialize(); } bool DxgiDuplicatorController::RetrieveD3dInfo(D3dInfo* info) { bool result = false; { - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); result = Initialize(); *info = d3d_info_; } @@ -116,7 +116,7 @@ DxgiDuplicatorController::Result DxgiDuplicatorController::DuplicateMonitor( } DesktopVector DxgiDuplicatorController::dpi() { - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); if (Initialize()) { return dpi_; } @@ -124,7 +124,7 @@ DesktopVector DxgiDuplicatorController::dpi() { } int DxgiDuplicatorController::ScreenCount() { - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); if (Initialize()) { return ScreenCountUnlocked(); } @@ -133,7 +133,7 @@ int DxgiDuplicatorController::ScreenCount() { bool DxgiDuplicatorController::GetDeviceNames( std::vector* output) { - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); if (Initialize()) { GetDeviceNamesUnlocked(output); return true; @@ -145,7 +145,7 @@ DxgiDuplicatorController::Result DxgiDuplicatorController::DoDuplicate( DxgiFrame* frame, int monitor_id) { RTC_DCHECK(frame); - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); // The dxgi components and APIs do not update the screen resolution without // a reinitialization. So we use the GetDC() function to retrieve the screen @@ -198,12 +198,12 @@ DxgiDuplicatorController::Result DxgiDuplicatorController::DoDuplicate( } void DxgiDuplicatorController::Unload() { - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); Deinitialize(); } void DxgiDuplicatorController::Unregister(const Context* const context) { - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); if (ContextExpired(context)) { // The Context has not been setup after a recent initialization, so it // should not been registered in duplicators. diff --git a/modules/desktop_capture/win/dxgi_duplicator_controller.h b/modules/desktop_capture/win/dxgi_duplicator_controller.h index 31da8c7f17..a24e9781b3 100644 --- a/modules/desktop_capture/win/dxgi_duplicator_controller.h +++ b/modules/desktop_capture/win/dxgi_duplicator_controller.h @@ -25,7 +25,7 @@ #include "modules/desktop_capture/win/dxgi_adapter_duplicator.h" #include "modules/desktop_capture/win/dxgi_context.h" #include "modules/desktop_capture/win/dxgi_frame.h" -#include "rtc_base/synchronization/mutex.h" +#include "rtc_base/critical_section.h" namespace webrtc { @@ -219,7 +219,7 @@ class DxgiDuplicatorController { std::atomic_int refcount_; // This lock must be locked whenever accessing any of the following objects. - Mutex lock_; + rtc::CriticalSection lock_; // A self-incremented integer to compare with the one in Context. It ensures // a Context instance is always initialized after DxgiDuplicatorController. diff --git a/modules/video_capture/BUILD.gn b/modules/video_capture/BUILD.gn index b4e8372620..f73472617c 100644 --- a/modules/video_capture/BUILD.gn +++ b/modules/video_capture/BUILD.gn @@ -36,7 +36,6 @@ rtc_library("video_capture_module") { "../../media:rtc_media_base", "../../rtc_base:rtc_base_approved", "../../rtc_base:stringutils", - "../../rtc_base/synchronization:mutex", "../../rtc_base/synchronization:rw_lock_wrapper", "../../system_wrappers", "//third_party/libyuv", @@ -52,7 +51,6 @@ if (!build_with_chromium) { "../../api:scoped_refptr", "../../rtc_base:checks", "../../rtc_base:rtc_base_approved", - "../../rtc_base/synchronization:mutex", "../../system_wrappers", ] @@ -131,7 +129,6 @@ if (!build_with_chromium) { "../../api/video:video_rtp_headers", "../../common_video", "../../rtc_base:rtc_base_approved", - "../../rtc_base/synchronization:mutex", "../../system_wrappers", "../../test:frame_utils", "../../test:test_main", diff --git a/modules/video_capture/linux/video_capture_linux.cc b/modules/video_capture/linux/video_capture_linux.cc index 504565f512..30865235b6 100644 --- a/modules/video_capture/linux/video_capture_linux.cc +++ b/modules/video_capture/linux/video_capture_linux.cc @@ -115,7 +115,7 @@ int32_t VideoCaptureModuleV4L2::StartCapture( } } - MutexLock lock(&capture_lock_); + rtc::CritScope cs(&_captureCritSect); // first open /dev/video device char device[20]; sprintf(device, "/dev/video%d", (int)_deviceId); @@ -264,7 +264,7 @@ int32_t VideoCaptureModuleV4L2::StartCapture( int32_t VideoCaptureModuleV4L2::StopCapture() { if (_captureThread) { { - MutexLock lock(&capture_lock_); + rtc::CritScope cs(&_captureCritSect); quit_ = true; } // Make sure the capture thread stop stop using the critsect. @@ -272,7 +272,7 @@ int32_t VideoCaptureModuleV4L2::StopCapture() { _captureThread.reset(); } - MutexLock lock(&capture_lock_); + rtc::CritScope cs(&_captureCritSect); if (_captureStarted) { _captureStarted = false; @@ -387,7 +387,7 @@ bool VideoCaptureModuleV4L2::CaptureProcess() { } { - MutexLock lock(&capture_lock_); + rtc::CritScope cs(&_captureCritSect); if (quit_) { return false; diff --git a/modules/video_capture/linux/video_capture_linux.h b/modules/video_capture/linux/video_capture_linux.h index ddb5d5ba87..ac9409e23a 100644 --- a/modules/video_capture/linux/video_capture_linux.h +++ b/modules/video_capture/linux/video_capture_linux.h @@ -18,8 +18,8 @@ #include "modules/video_capture/video_capture_defines.h" #include "modules/video_capture/video_capture_impl.h" +#include "rtc_base/critical_section.h" #include "rtc_base/platform_thread.h" -#include "rtc_base/synchronization/mutex.h" namespace webrtc { namespace videocapturemodule { @@ -43,8 +43,8 @@ class VideoCaptureModuleV4L2 : public VideoCaptureImpl { // TODO(pbos): Stop using unique_ptr and resetting the thread. std::unique_ptr _captureThread; - Mutex capture_lock_; - bool quit_ RTC_GUARDED_BY(capture_lock_); + rtc::CriticalSection _captureCritSect; + bool quit_ RTC_GUARDED_BY(_captureCritSect); int32_t _deviceId; int32_t _deviceFd; diff --git a/modules/video_capture/test/video_capture_unittest.cc b/modules/video_capture/test/video_capture_unittest.cc index 1a0cf2d5da..be443e0820 100644 --- a/modules/video_capture/test/video_capture_unittest.cc +++ b/modules/video_capture/test/video_capture_unittest.cc @@ -23,7 +23,7 @@ #include "common_video/libyuv/include/webrtc_libyuv.h" #include "modules/utility/include/process_thread.h" #include "modules/video_capture/video_capture_factory.h" -#include "rtc_base/synchronization/mutex.h" +#include "rtc_base/critical_section.h" #include "rtc_base/time_utils.h" #include "system_wrappers/include/sleep.h" #include "test/frame_utils.h" @@ -74,7 +74,7 @@ class TestVideoCaptureCallback } void OnFrame(const webrtc::VideoFrame& videoFrame) override { - webrtc::MutexLock lock(&capture_lock_); + rtc::CritScope cs(&capture_cs_); int height = videoFrame.height(); int width = videoFrame.width(); #if defined(WEBRTC_ANDROID) && WEBRTC_ANDROID @@ -106,38 +106,38 @@ class TestVideoCaptureCallback } void SetExpectedCapability(VideoCaptureCapability capability) { - webrtc::MutexLock lock(&capture_lock_); + rtc::CritScope cs(&capture_cs_); capability_ = capability; incoming_frames_ = 0; last_render_time_ms_ = 0; } int incoming_frames() { - webrtc::MutexLock lock(&capture_lock_); + rtc::CritScope cs(&capture_cs_); return incoming_frames_; } int timing_warnings() { - webrtc::MutexLock lock(&capture_lock_); + rtc::CritScope cs(&capture_cs_); return timing_warnings_; } VideoCaptureCapability capability() { - webrtc::MutexLock lock(&capture_lock_); + rtc::CritScope cs(&capture_cs_); return capability_; } bool CompareLastFrame(const webrtc::VideoFrame& frame) { - webrtc::MutexLock lock(&capture_lock_); + rtc::CritScope cs(&capture_cs_); return webrtc::test::FrameBufsEqual(last_frame_, frame.video_frame_buffer()); } void SetExpectedCaptureRotation(webrtc::VideoRotation rotation) { - webrtc::MutexLock lock(&capture_lock_); + rtc::CritScope cs(&capture_cs_); rotate_frame_ = rotation; } private: - webrtc::Mutex capture_lock_; + rtc::CriticalSection capture_cs_; VideoCaptureCapability capability_; int64_t last_render_time_ms_; int incoming_frames_; diff --git a/modules/video_capture/video_capture_impl.cc b/modules/video_capture/video_capture_impl.cc index 6619d15924..9d53a91157 100644 --- a/modules/video_capture/video_capture_impl.cc +++ b/modules/video_capture/video_capture_impl.cc @@ -96,12 +96,12 @@ VideoCaptureImpl::~VideoCaptureImpl() { void VideoCaptureImpl::RegisterCaptureDataCallback( rtc::VideoSinkInterface* dataCallBack) { - MutexLock lock(&api_lock_); + rtc::CritScope cs(&_apiCs); _dataCallBack = dataCallBack; } void VideoCaptureImpl::DeRegisterCaptureDataCallback() { - MutexLock lock(&api_lock_); + rtc::CritScope cs(&_apiCs); _dataCallBack = NULL; } int32_t VideoCaptureImpl::DeliverCapturedFrame(VideoFrame& captureFrame) { @@ -118,7 +118,7 @@ int32_t VideoCaptureImpl::IncomingFrame(uint8_t* videoFrame, size_t videoFrameLength, const VideoCaptureCapability& frameInfo, int64_t captureTime /*=0*/) { - MutexLock lock(&api_lock_); + rtc::CritScope cs(&_apiCs); const int32_t width = frameInfo.width; const int32_t height = frameInfo.height; @@ -223,7 +223,7 @@ int32_t VideoCaptureImpl::CaptureSettings( } int32_t VideoCaptureImpl::SetCaptureRotation(VideoRotation rotation) { - MutexLock lock(&api_lock_); + rtc::CritScope cs(&_apiCs); _rotateFrame = rotation; return 0; } diff --git a/modules/video_capture/video_capture_impl.h b/modules/video_capture/video_capture_impl.h index cbc99b76c1..197bfd387c 100644 --- a/modules/video_capture/video_capture_impl.h +++ b/modules/video_capture/video_capture_impl.h @@ -25,7 +25,7 @@ #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/synchronization/mutex.h" +#include "rtc_base/critical_section.h" namespace webrtc { @@ -78,7 +78,7 @@ class VideoCaptureImpl : public VideoCaptureModule { ~VideoCaptureImpl() override; char* _deviceUniqueId; // current Device unique name; - Mutex api_lock_; + rtc::CriticalSection _apiCs; VideoCaptureCapability _requestedCapability; // Should be set by platform // dependent code in // StartCapture. diff --git a/modules/video_capture/windows/video_capture_ds.cc b/modules/video_capture/windows/video_capture_ds.cc index 6dca74750c..615a1b56ea 100644 --- a/modules/video_capture/windows/video_capture_ds.cc +++ b/modules/video_capture/windows/video_capture_ds.cc @@ -130,7 +130,7 @@ int32_t VideoCaptureDS::Init(const char* deviceUniqueIdUTF8) { } int32_t VideoCaptureDS::StartCapture(const VideoCaptureCapability& capability) { - MutexLock lock(&api_lock_); + rtc::CritScope cs(&_apiCs); if (capability != _requestedCapability) { DisconnectGraph(); @@ -148,7 +148,7 @@ int32_t VideoCaptureDS::StartCapture(const VideoCaptureCapability& capability) { } int32_t VideoCaptureDS::StopCapture() { - MutexLock lock(&api_lock_); + rtc::CritScope cs(&_apiCs); HRESULT hr = _mediaControl->Pause(); if (FAILED(hr)) {