fix(macos): recover capture after the display sleeps

src/platform/macos/av_video.h

@@ -88,4 +88,16 @@ typedef bool (^FrameCallbackBlock)(CMSampleBufferRef);
  */
 - (dispatch_semaphore_t)capture:(FrameCallbackBlock)frameCallback;
 
+/**
+ * @brief Abandon a capture that has not ended on its own.
+ *
+ * The frame callback normally tears its own capture down by returning false. When the
+ * caller gives up on a capture that stopped delivering frames, such as after the display
+ * slept, this performs that teardown on its behalf.
+ *
+ * @param signal Semaphore previously returned by capture:.
+ * @note This method waits for any in-flight frame callback to finish before returning.
+ */
+- (void)stopCapture:(dispatch_semaphore_t)signal;
+
 @end

src/platform/macos/av_video.m

@@ -5,6 +5,21 @@
 // local includes
 #import "av_video.h"
 
+/**
+ * @brief Private capture lifecycle helpers for AVVideo.
+ */
+@interface AVVideo ()
+
+/**
+ * @brief Tear down one capture after its callback queue has been serialized.
+ *
+ * @param connection Capture connection to tear down.
+ * @param signalCompletion Whether to wake the thread waiting for normal capture completion.
+ */
+- (void)finishCapture:(AVCaptureConnection *)connection signalCompletion:(BOOL)signalCompletion;
+
+@end
+
 @implementation AVVideo
 
 - (id)initWithDisplay:(CGDirectDisplayID)displayID frameRate:(int)frameRate {
@@ -93,22 +108,74 @@ - (dispatch_semaphore_t)capture:(FrameCallbackBlock)frameCallback {
   }
 }
 
+- (void)stopCapture:(dispatch_semaphore_t)signal {
+  AVCaptureConnection *target = nil;
+  AVCaptureVideoDataOutput *videoOutput = nil;
+
+  @synchronized(self) {
+    for (AVCaptureConnection *connection in self.captureSignals) {
+      if ([self.captureSignals objectForKey:connection] == signal) {
+        target = [connection retain];
+        videoOutput = [[self.videoOutputs objectForKey:connection] retain];
+        break;
+      }
+    }
+  }
+
+  if (target == nil) {
+    return;
+  }
+
+  // The callback is invoked on a serial queue. Running teardown on that same queue waits for
+  // an in-flight callback and orders this teardown before any callback that has not started.
+  dispatch_queue_t callbackQueue = [videoOutput sampleBufferCallbackQueue];
+  if (callbackQueue != nil) {
+    dispatch_sync(callbackQueue, ^{
+      [self finishCapture:target signalCompletion:NO];
+    });
+  } else {
+    [self finishCapture:target signalCompletion:NO];
+  }
+
+  [videoOutput release];
+  [target release];
+}
+
+- (void)finishCapture:(AVCaptureConnection *)connection signalCompletion:(BOOL)signalCompletion {
+  @synchronized(self) {
+    AVCaptureVideoDataOutput *videoOutput = [self.videoOutputs objectForKey:connection];
+    dispatch_semaphore_t signal = [self.captureSignals objectForKey:connection];
+    if (videoOutput == nil || signal == nil) {
+      return;
+    }
+
+    // Claim this capture before stopping the session so queued callbacks become no-ops.
+    [self.captureCallbacks removeObjectForKey:connection];
+    [self.session stopRunning];
+    [self.session removeOutput:videoOutput];
+    [self.videoOutputs removeObjectForKey:connection];
+    if (signalCompletion) {
+      dispatch_semaphore_signal(signal);
+    }
+    [self.captureSignals removeObjectForKey:connection];
+    [self.session startRunning];
+  }
+}
+
 - (void)captureOutput:(AVCaptureOutput *)captureOutput
   didOutputSampleBuffer:(CMSampleBufferRef)sampleBuffer
          fromConnection:(AVCaptureConnection *)connection {
-  FrameCallbackBlock callback = [self.captureCallbacks objectForKey:connection];
+  FrameCallbackBlock callback = nil;
+  @synchronized(self) {
+    callback = [[self.captureCallbacks objectForKey:connection] copy];
+  }
 
   if (callback != nil) {
-    if (!callback(sampleBuffer)) {
-      @synchronized(self) {
-        [self.session stopRunning];
-        [self.captureCallbacks removeObjectForKey:connection];
-        [self.session removeOutput:[self.videoOutputs objectForKey:connection]];
-        [self.videoOutputs removeObjectForKey:connection];
-        dispatch_semaphore_signal([self.captureSignals objectForKey:connection]);
-        [self.captureSignals removeObjectForKey:connection];
-        [self.session startRunning];
-      }
+    const bool shouldStopCapture = !callback(sampleBuffer);
+    [callback release];
+
+    if (shouldStopCapture) {
+      [self finishCapture:connection signalCompletion:YES];
     }
   }
 }

src/platform/macos/display.mm

@@ -52,6 +52,31 @@ OSType videotoolbox_pixel_format(const video::config_t &config) {
       const auto colorspace {video::colorspace_from_client_config(config, false)};
       return colorspace.bit_depth == 10 ? kCVPixelFormatType_420YpCbCr10BiPlanarVideoRange : kCVPixelFormatType_420YpCbCr8BiPlanarVideoRange;
     }
+
+    /**
+     * @brief How often the capture loop wakes up to check on the display while capturing.
+     *
+     * The capture semaphore is only signalled when capture ends, so the loop needs its own
+     * cadence to notice a display that slept and woke.
+     */
+    constexpr auto capture_poll_interval {250ms};
+
+    /**
+     * @brief How long dummy_img() waits for a single frame before giving up.
+     *
+     * Without a bound this blocks forever when the display is asleep during encoder probing.
+     */
+    constexpr auto dummy_img_timeout {5s};
+
+    /**
+     * @brief Convert a duration to an absolute dispatch timeout.
+     *
+     * @param duration How far in the future the timeout should fire.
+     * @return Dispatch time suitable for dispatch_semaphore_wait().
+     */
+    dispatch_time_t dispatch_timeout_from_now(std::chrono::nanoseconds duration) {
+      return dispatch_time(DISPATCH_TIME_NOW, duration.count());
+    }
   }  // namespace
 
   /**
@@ -105,8 +130,25 @@ capture_e capture(const push_captured_image_cb_t &push_captured_image_cb, const
         return true;
       }];
 
-      // FIXME: We should time out if an image isn't returned for a while
-      dispatch_semaphore_wait(signal, DISPATCH_TIME_FOREVER);
+      // The semaphore is only signalled once capture ends, so waiting on it forever used to
+      // park this thread for the lifetime of the process: a sleeping display stops
+      // AVCaptureSession delivering sample buffers, and the session does not resume when the
+      // display wakes again. Poll instead, so a display that slept and woke can be reported
+      // to the caller as a display that needs rebuilding.
+      bool display_slept {false};
+      while (dispatch_semaphore_wait(signal, dispatch_timeout_from_now(capture_poll_interval)) != 0) {
+        if (CGDisplayIsAsleep(display_id)) {
+          display_slept = true;
+        } else if (display_slept) {
+          BOOST_LOG(info) << "Display ["sv << display_id << "] woke from sleep, reinitializing capture"sv;
+
+          // Tear the capture down before returning, so that no callback outlives this call
+          // and the output does not survive into our destructor still owned by the session.
+          [av_capture stopCapture:signal];
+
+          return capture_e::reinit;
+        }
+      }
 
       return capture_e::ok;
     }
@@ -184,7 +226,16 @@ int dummy_img(img_t *img) override {
         return false;
       }];
 
-      dispatch_semaphore_wait(signal, DISPATCH_TIME_FOREVER);
+      // Unlike capture(), this callback stops after a single frame, so the semaphore really
+      // does mean "one image arrived". Bound the wait anyway: with the display asleep no
+      // frame is ever delivered, and encoder probing would hang here forever.
+      if (dispatch_semaphore_wait(signal, dispatch_timeout_from_now(dummy_img_timeout)) != 0) {
+        BOOST_LOG(error) << "Timed out waiting for a frame from display ["sv << display_id << "], is it asleep?"sv;
+
+        [av_capture stopCapture:signal];
+
+        return 1;
+      }
 
       return 0;
     }