Bug 1817724 - In VideoCaptureAvFoundation use MediaEvent to handle frames. r=webrtc-reviewers,ng

Hypothetically if the frame callback is doing something leading to crashes, this
patch should at least affect those symptoms, giving us more clues to where the
problem lies.

This patch changes away from calling the CamerasParent callback directly on the
frame queue, to notifying a MediaEvent owned by the VideoCaptureAdapter. The
frames are then handled on the video capture thread.

Differential Revision: https://phabricator.services.mozilla.com/D171962

dom/media/systemservices/objc_video_capture/video_capture_avfoundation.h

@@ -11,17 +11,25 @@
 
 #include "api/scoped_refptr.h"
 #include "api/sequence_checker.h"
+#include "MediaEventSource.h"
 #include "modules/video_capture/video_capture_impl.h"
 #include "mozilla/Maybe.h"
 #include "PerformanceRecorder.h"
 
-@class VideoCaptureAdapter;
+@interface VideoCaptureAdapter : NSObject <RTCVideoCapturerDelegate> {
+ @public
+  mozilla::MediaEventProducerExc<__strong RTCVideoFrame* _Nullable> frameEvent;
+}
+@end
 
 namespace webrtc::videocapturemodule {
 
 /**
  * VideoCaptureImpl implementation of the libwebrtc ios/mac sdk camera backend.
- * Single threaded except for OnFrame() that happens on a platform callback thread.
+ * Single threaded API. Callbacks run on the API thread.
+ *
+ * Internally callbacks come on a dedicated dispatch queue. We bounce them via a MediaEvent to the
+ * API thread.
  */
 class VideoCaptureAvFoundation : public VideoCaptureImpl {
  public:
@@ -34,19 +42,16 @@ class VideoCaptureAvFoundation : public VideoCaptureImpl {
 
   // Starts capturing synchronously. Idempotent. If an existing capture is live and another
   // capability is requested we'll restart the underlying backend with the new capability.
-  int32_t StartCapture(const VideoCaptureCapability& aCapability) MOZ_EXCLUDES(api_lock_) override;
+  int32_t StartCapture(const VideoCaptureCapability& aCapability) override;
   // Stops capturing synchronously. Idempotent.
-  int32_t StopCapture() MOZ_EXCLUDES(api_lock_) override;
-  bool CaptureStarted() MOZ_EXCLUDES(api_lock_) override;
+  int32_t StopCapture() override;
+  bool CaptureStarted() override;
   int32_t CaptureSettings(VideoCaptureCapability& aSettings) override;
 
-  // Callback. This can be called on any thread.
-  int32_t OnFrame(__strong RTCVideoFrame* _Nonnull aFrame) MOZ_EXCLUDES(api_lock_);
-
-  void SetTrackingId(uint32_t aTrackingIdProcId) MOZ_EXCLUDES(api_lock_) override;
+  // Callback on the mChecker thread.
+  void OnFrame(__strong RTCVideoFrame* _Nonnull aFrame);
 
-  // Registers the current thread with the profiler if not already registered.
-  void MaybeRegisterCallbackThread();
+  void SetTrackingId(uint32_t aTrackingIdProcId) override;
 
  private:
   // Control thread checker.
@@ -54,26 +59,21 @@ class VideoCaptureAvFoundation : public VideoCaptureImpl {
   AVCaptureDevice* _Nonnull const mDevice RTC_GUARDED_BY(mChecker);
   VideoCaptureAdapter* _Nonnull const mAdapter RTC_GUARDED_BY(mChecker);
   RTCCameraVideoCapturer* _Nonnull const mCapturer RTC_GUARDED_BY(mChecker);
-  // If capture has started, this is the capability it was started for. Written on the mChecker
-  // thread only.
-  mozilla::Maybe<VideoCaptureCapability> mCapability MOZ_GUARDED_BY(api_lock_);
+  // If capture has started, this is the capability it was started for.
+  mozilla::Maybe<VideoCaptureCapability> mCapability RTC_GUARDED_BY(mChecker);
   // The image type that mCapability maps to. Set in lockstep with mCapability.
-  mozilla::Maybe<mozilla::CaptureStage::ImageType> mImageType MOZ_GUARDED_BY(api_lock_);
-  // Id string uniquely identifying this capture source. Written on the mChecker thread only.
-  mozilla::Maybe<mozilla::TrackingId> mTrackingId MOZ_GUARDED_BY(api_lock_);
-  // Adds frame specific markers to the profiler while mTrackingId is set. Callback thread only.
-  mozilla::PerformanceRecorderMulti<mozilla::CaptureStage> mCaptureRecorder;
-  mozilla::PerformanceRecorderMulti<mozilla::CopyVideoStage> mConversionRecorder;
-  std::atomic<ProfilerThreadId> mCallbackThreadId;
+  mozilla::Maybe<mozilla::CaptureStage::ImageType> mImageType RTC_GUARDED_BY(mChecker);
+  // Id string uniquely identifying this capture source.
+  mozilla::Maybe<mozilla::TrackingId> mTrackingId RTC_GUARDED_BY(mChecker);
+  // Handle for the mAdapter frame event. mChecker thread only.
+  mozilla::MediaEventListener mFrameListener RTC_GUARDED_BY(mChecker);
+  // Adds frame specific markers to the profiler while mTrackingId is set.
+  mozilla::PerformanceRecorderMulti<mozilla::CaptureStage> mCaptureRecorder
+      RTC_GUARDED_BY(mChecker);
+  mozilla::PerformanceRecorderMulti<mozilla::CopyVideoStage> mConversionRecorder
+      RTC_GUARDED_BY(mChecker);
 };
 
 }  // namespace webrtc::videocapturemodule
 
-@interface VideoCaptureAdapter : NSObject <RTCVideoCapturerDelegate> {
-  webrtc::Mutex _mutex;
-  webrtc::videocapturemodule::VideoCaptureAvFoundation* _Nullable _capturer RTC_GUARDED_BY(_mutex);
-}
-- (void)setCapturer:(webrtc::videocapturemodule::VideoCaptureAvFoundation* _Nullable)capturer;
-@end
-
 #endif

dom/media/systemservices/objc_video_capture/video_capture_avfoundation.mm

@@ -73,20 +73,9 @@ AVCaptureDeviceFormat* _Nullable FindFormat(AVCaptureDevice* _Nonnull aDevice,
 }  // namespace
 
 @implementation VideoCaptureAdapter
-- (void)setCapturer:(webrtc::videocapturemodule::VideoCaptureAvFoundation* _Nullable)capturer {
-  webrtc::MutexLock lock(&_mutex);
-  _capturer = capturer;
-}
-
 - (void)capturer:(RTCVideoCapturer* _Nonnull)capturer
     didCaptureVideoFrame:(RTCVideoFrame* _Nonnull)frame {
-  rtc::scoped_refptr<webrtc::videocapturemodule::VideoCaptureAvFoundation> cap;
-  {
-    webrtc::MutexLock lock(&_mutex);
-    cap = rtc::scoped_refptr(_capturer);
-  }
-  if (!cap) return;
-  cap->OnFrame(frame);
+  frameEvent.Notify(frame);
 }
 @end
 
@@ -94,8 +83,7 @@ namespace webrtc::videocapturemodule {
 VideoCaptureAvFoundation::VideoCaptureAvFoundation(AVCaptureDevice* _Nonnull aDevice)
     : mDevice(aDevice),
       mAdapter([[VideoCaptureAdapter alloc] init]),
-      mCapturer([[RTC_OBJC_TYPE(RTCCameraVideoCapturer) alloc] initWithDelegate:mAdapter]),
-      mCallbackThreadId() {
+      mCapturer([[RTC_OBJC_TYPE(RTCCameraVideoCapturer) alloc] initWithDelegate:mAdapter]) {
   const char* uniqueId = [[aDevice uniqueID] UTF8String];
   size_t len = strlen(uniqueId);
   _deviceUniqueId = new (std::nothrow) char[len + 1];
@@ -150,7 +138,8 @@ int32_t VideoCaptureAvFoundation::StartCapture(const VideoCaptureCapability& aCa
     }
   }
 
-  [mAdapter setCapturer:this];
+  mFrameListener = mAdapter->frameEvent.Connect(GetCurrentSerialEventTarget(), this,
+                                                &VideoCaptureAvFoundation::OnFrame);
 
   {
     Monitor monitor("VideoCaptureAVFoundation::StartCapture");
@@ -237,14 +226,13 @@ int32_t VideoCaptureAvFoundation::StopCapture() {
     monitor.Wait();
   }
 
-  [mAdapter setCapturer:nil];
+  mFrameListener.Disconnect();
 
   return 0;
 }
 
 bool VideoCaptureAvFoundation::CaptureStarted() {
   RTC_DCHECK_RUN_ON(&mChecker);
-  MutexLock lock(&api_lock_);
   return mCapability.isSome();
 }
 
@@ -253,9 +241,9 @@ int32_t VideoCaptureAvFoundation::CaptureSettings(VideoCaptureCapability& aSetti
   return -1;
 }
 
-int32_t VideoCaptureAvFoundation::OnFrame(__strong RTCVideoFrame* _Nonnull aFrame) {
-  MaybeRegisterCallbackThread();
-  if (MutexLock lock(&api_lock_); MOZ_LIKELY(mTrackingId)) {
+void VideoCaptureAvFoundation::OnFrame(__strong RTCVideoFrame* _Nonnull aFrame) {
+  RTC_DCHECK_RUN_ON(&mChecker);
+  if (MOZ_LIKELY(mTrackingId)) {
     mCaptureRecorder.Start(0, "VideoCaptureAVFoundation"_ns, *mTrackingId, aFrame.width,
                            aFrame.height, mImageType.valueOr(CaptureStage::ImageType::Unknown));
     if (mCapability && mCapability->videoType != webrtc::VideoType::kI420) {
@@ -277,14 +265,12 @@ int32_t VideoCaptureAvFoundation::OnFrame(__strong RTCVideoFrame* _Nonnull aFram
                    .build();
 
   MutexLock lock(&api_lock_);
-  int32_t rv = DeliverCapturedFrame(frame);
+  DeliverCapturedFrame(frame);
   mCaptureRecorder.Record(0);
-  return rv;
 }
 
 void VideoCaptureAvFoundation::SetTrackingId(uint32_t aTrackingIdProcId) {
   RTC_DCHECK_RUN_ON(&mChecker);
-  MutexLock lock(&api_lock_);
   if (NS_WARN_IF(mTrackingId.isSome())) {
     // This capture instance must be shared across multiple camera requests. For now ignore other
     // requests than the first.
@@ -292,13 +278,4 @@ void VideoCaptureAvFoundation::SetTrackingId(uint32_t aTrackingIdProcId) {
   }
   mTrackingId.emplace(TrackingId::Source::Camera, aTrackingIdProcId);
 }
-
-void VideoCaptureAvFoundation::MaybeRegisterCallbackThread() {
-  ProfilerThreadId id = profiler_current_thread_id();
-  if (MOZ_LIKELY(id == mCallbackThreadId)) {
-    return;
-  }
-  mCallbackThreadId = id;
-  CallbackThreadRegistry::Get()->Register(mCallbackThreadId, "VideoCaptureAVFoundationCallback");
-}
 }  // namespace webrtc::videocapturemodule