diff --git a/src/platform/macos/av_video.h b/src/platform/macos/av_video.h index 8832f32649e..9f5eb621e2c 100644 --- a/src/platform/macos/av_video.h +++ b/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 diff --git a/src/platform/macos/av_video.m b/src/platform/macos/av_video.m index 5ad931c5a63..a644d2e302e 100644 --- a/src/platform/macos/av_video.m +++ b/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]; } } } diff --git a/src/platform/macos/display.mm b/src/platform/macos/display.mm index d5fe3d024b2..83d22d2e422 100644 --- a/src/platform/macos/display.mm +++ b/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; } diff --git a/tests/unit/platform/macos/test_av_video.mm b/tests/unit/platform/macos/test_av_video.mm new file mode 100644 index 00000000000..6fdba7d242c --- /dev/null +++ b/tests/unit/platform/macos/test_av_video.mm @@ -0,0 +1,197 @@ +/** + * @file tests/unit/platform/macos/test_av_video.mm + * @brief Unit tests for serialized macOS video capture teardown. + */ + +// Only compile these tests on macOS +#ifdef __APPLE__ + + #include "../../../tests_common.h" + + #import + #import + +/** + * @brief Minimal capture session double that records lifecycle calls. + */ +@interface FakeCaptureSession: NSObject + +@property (atomic, assign) NSUInteger startRunningCount; ///< Number of startRunning calls. +@property (atomic, assign) NSUInteger stopRunningCount; ///< Number of stopRunning calls. +@property (atomic, assign) NSUInteger removeOutputCount; ///< Number of removeOutput calls. + +@end + +@implementation FakeCaptureSession + +- (void)startRunning { + self.startRunningCount++; +} + +- (void)stopRunning { + self.stopRunningCount++; +} + +- (void)removeOutput:(AVCaptureOutput *)output { + (void) output; + self.removeOutputCount++; +} + +@end + +/** + * @brief Fixture that installs a synthetic capture without accessing display hardware. + */ +class AVVideoTest: public PlatformTestSuite { +protected: + AVVideo *video {}; ///< Video capture object under test. + FakeCaptureSession *session {}; ///< Session double used by the capture object. + AVCaptureConnection *connection {}; ///< Synthetic map key representing a capture connection. + AVCaptureVideoDataOutput *video_output {}; ///< Video output that owns the callback queue. + dispatch_queue_t callback_queue {}; ///< Serial queue used for frame callbacks. + dispatch_semaphore_t capture_signal {}; ///< Semaphore returned to the capture wait loop. + CMSampleBufferRef sample_buffer {}; ///< Empty sample buffer passed to the capture callback. + + void SetUp() override { + video = [[AVVideo alloc] init]; + session = [[FakeCaptureSession alloc] init]; + connection = (AVCaptureConnection *) [[NSObject alloc] init]; + video_output = [[AVCaptureVideoDataOutput alloc] init]; + callback_queue = dispatch_queue_create("testAVVideoCallbackQueue", DISPATCH_QUEUE_SERIAL); + capture_signal = dispatch_semaphore_create(0); + ASSERT_EQ(CMSampleBufferCreate(kCFAllocatorDefault, nullptr, true, nullptr, nullptr, nullptr, 0, 0, nullptr, 0, nullptr, &sample_buffer), noErr); + ASSERT_NE(sample_buffer, nullptr); + + video.session = (AVCaptureSession *) session; + video.videoOutputs = [[NSMapTable alloc] init]; + video.captureCallbacks = [[NSMapTable alloc] init]; + video.captureSignals = [[NSMapTable alloc] init]; + + [video_output setSampleBufferDelegate:video queue:callback_queue]; + } + + void TearDown() override { + [video_output setSampleBufferDelegate:nil queue:nil]; + [video release]; + [connection release]; + [video_output release]; + [session release]; + dispatch_release(callback_queue); + dispatch_release(capture_signal); + if (sample_buffer != nullptr) { + CFRelease(sample_buffer); + } + } + + /** + * @brief Register a callback and its capture resources in the object under test. + * + * @param callback Frame callback to register. + */ + void register_capture(FrameCallbackBlock callback) { + [video.videoOutputs setObject:video_output forKey:connection]; + [video.captureCallbacks setObject:callback forKey:connection]; + [video.captureSignals setObject:capture_signal forKey:connection]; + } +}; + +/** + * @test Verify forced teardown waits for an in-flight callback and only tears down once. + */ +TEST_F(AVVideoTest, StopCaptureWaitsForInFlightCallback) { + dispatch_semaphore_t callback_entered = dispatch_semaphore_create(0); + dispatch_semaphore_t release_callback = dispatch_semaphore_create(0); + dispatch_semaphore_t stop_started = dispatch_semaphore_create(0); + dispatch_semaphore_t stop_finished = dispatch_semaphore_create(0); + + register_capture(^bool(CMSampleBufferRef sample_buffer) { + (void) sample_buffer; + dispatch_semaphore_signal(callback_entered); + dispatch_semaphore_wait(release_callback, DISPATCH_TIME_FOREVER); + return false; + }); + + dispatch_async(callback_queue, ^{ + [video captureOutput:video_output didOutputSampleBuffer:sample_buffer fromConnection:connection]; + }); + ASSERT_EQ(dispatch_semaphore_wait(callback_entered, dispatch_time(DISPATCH_TIME_NOW, NSEC_PER_SEC)), 0); + + dispatch_async(dispatch_get_global_queue(QOS_CLASS_USER_INITIATED, 0), ^{ + dispatch_semaphore_signal(stop_started); + [video stopCapture:capture_signal]; + dispatch_semaphore_signal(stop_finished); + }); + ASSERT_EQ(dispatch_semaphore_wait(stop_started, dispatch_time(DISPATCH_TIME_NOW, NSEC_PER_SEC)), 0); + EXPECT_NE(dispatch_semaphore_wait(stop_finished, dispatch_time(DISPATCH_TIME_NOW, 50 * NSEC_PER_MSEC)), 0); + + dispatch_semaphore_signal(release_callback); + ASSERT_EQ(dispatch_semaphore_wait(stop_finished, dispatch_time(DISPATCH_TIME_NOW, NSEC_PER_SEC)), 0); + + EXPECT_EQ(dispatch_semaphore_wait(capture_signal, DISPATCH_TIME_NOW), 0); + EXPECT_EQ(video.videoOutputs.count, 0); + EXPECT_EQ(video.captureCallbacks.count, 0); + EXPECT_EQ(video.captureSignals.count, 0); + EXPECT_EQ(session.stopRunningCount, 1); + EXPECT_EQ(session.removeOutputCount, 1); + EXPECT_EQ(session.startRunningCount, 1); + + dispatch_release(callback_entered); + dispatch_release(release_callback); + dispatch_release(stop_started); + dispatch_release(stop_finished); +} + +/** + * @test Verify forced teardown is idempotent and does not report normal callback completion. + */ +TEST_F(AVVideoTest, StopCaptureWithoutCallbackIsIdempotent) { + __block bool callback_invoked = false; + register_capture(^bool(CMSampleBufferRef sample_buffer) { + (void) sample_buffer; + callback_invoked = true; + return false; + }); + + [video_output setSampleBufferDelegate:nil queue:nil]; + [video stopCapture:capture_signal]; + [video stopCapture:capture_signal]; + [video captureOutput:video_output didOutputSampleBuffer:sample_buffer fromConnection:connection]; + + EXPECT_FALSE(callback_invoked); + EXPECT_NE(dispatch_semaphore_wait(capture_signal, DISPATCH_TIME_NOW), 0); + EXPECT_EQ(video.videoOutputs.count, 0); + EXPECT_EQ(video.captureCallbacks.count, 0); + EXPECT_EQ(video.captureSignals.count, 0); + EXPECT_EQ(session.stopRunningCount, 1); + EXPECT_EQ(session.removeOutputCount, 1); + EXPECT_EQ(session.startRunningCount, 1); +} + +/** + * @test Verify a callback that accepts a frame leaves the capture active. + */ +TEST_F(AVVideoTest, CaptureOutputKeepsCaptureActive) { + __block bool callback_invoked = false; + register_capture(^bool(CMSampleBufferRef sample_buffer) { + (void) sample_buffer; + callback_invoked = true; + return true; + }); + + dispatch_sync(callback_queue, ^{ + [video captureOutput:video_output didOutputSampleBuffer:sample_buffer fromConnection:connection]; + }); + + EXPECT_TRUE(callback_invoked); + EXPECT_NE(dispatch_semaphore_wait(capture_signal, DISPATCH_TIME_NOW), 0); + EXPECT_EQ(video.videoOutputs.count, 1); + EXPECT_EQ(video.captureCallbacks.count, 1); + EXPECT_EQ(video.captureSignals.count, 1); + EXPECT_EQ(session.stopRunningCount, 0); + EXPECT_EQ(session.removeOutputCount, 0); + EXPECT_EQ(session.startRunningCount, 0); + + [video stopCapture:capture_signal]; +} + +#endif // __APPLE__