From 60e3f31f33b55ddaa5ef1ab365fb770f1795f309 Mon Sep 17 00:00:00 2001 From: Fitiavana Anhy Krishna Date: Tue, 28 Jul 2026 12:29:46 +0300 Subject: [PATCH] fix: reject recording when the selected display is disconnected A display picked from the content picker is captured by its `displayID`. Disconnecting the display leaves the filter intact, so recording starts normally but the stream never delivers a frame. With audio enabled the writer session is started by the first audio buffer, which satisfies the `noFramesWritten` guard, and the recording is finalized and reported as saved with audio tracks but no video track at all. Validate the selected display against the connected displays before creating the stream, and clear the stale selection so the next attempt opens the picker. Report the number of video frames from `finishWriting` so a recording that ends up without video is saved but flagged to the user instead of announced as a normal success. Closes #184 --- BetterCapture/Service/AssetWriter.swift | 8 +- BetterCapture/Service/CaptureEngine.swift | 11 +- .../Service/ContentFilterService.swift | 29 +++ .../Service/NotificationService.swift | 29 +++ .../ViewModel/RecorderViewModel.swift | 22 +- BetterCaptureTests/AssetWriterTests.swift | 189 ++++++++++++++++++ BetterCaptureTests/ErrorTests.swift | 3 +- 7 files changed, 284 insertions(+), 7 deletions(-) create mode 100644 BetterCaptureTests/AssetWriterTests.swift diff --git a/BetterCapture/Service/AssetWriter.swift b/BetterCapture/Service/AssetWriter.swift index e83c1af..c2a2801 100644 --- a/BetterCapture/Service/AssetWriter.swift +++ b/BetterCapture/Service/AssetWriter.swift @@ -289,7 +289,10 @@ final class AssetWriter: CaptureEngineSampleBufferDelegate, @unchecked Sendable // MARK: - Finalization /// Finishes writing and finalizes the output file - func finishWriting() async throws -> URL { + /// - Returns: The output URL and the number of video frames written. A count of zero + /// means the file holds audio only, which happens when the capture source + /// stopped producing frames while audio kept flowing. + func finishWriting() async throws -> (url: URL, videoFrameCount: Int) { // First critical section: validate state and mark inputs as finished let (writerToFinish, url): (AVAssetWriter, URL) @@ -351,6 +354,7 @@ final class AssetWriter: CaptureEngineSampleBufferDelegate, @unchecked Sendable logger.info( "AssetWriter finished writing \(self.frameCount) frames to: \(url.lastPathComponent)" ) + let videoFrameCount = frameCount frameCount = 0 // Clean up @@ -360,7 +364,7 @@ final class AssetWriter: CaptureEngineSampleBufferDelegate, @unchecked Sendable self.audioInput = nil self.microphoneInput = nil - return url + return (url, videoFrameCount) } } diff --git a/BetterCapture/Service/CaptureEngine.swift b/BetterCapture/Service/CaptureEngine.swift index 70d9644..642b5e5 100644 --- a/BetterCapture/Service/CaptureEngine.swift +++ b/BetterCapture/Service/CaptureEngine.swift @@ -126,6 +126,12 @@ final class CaptureEngine: NSObject { } } + // A display selected before it was disconnected still yields a usable filter, but the + // stream never delivers frames. Fail fast instead of recording without a video track. + guard await contentFilterService.isSelectedDisplayConnected(filter) else { + throw CaptureError.selectedDisplayDisconnected + } + // Apply content filter settings (wallpaper, dock, menu bar) logger.info("Applying content filter settings...") let filteredContent = try await contentFilterService.applySettings(to: filter, settings: settings) @@ -366,12 +372,13 @@ extension CaptureEngine: SCStreamOutput { // MARK: - Errors -enum CaptureError: LocalizedError { +enum CaptureError: LocalizedError, Equatable { case noContentFilterSelected case failedToCreateStream case captureAlreadyRunning case screenRecordingPermissionDenied case microphonePermissionDenied + case selectedDisplayDisconnected var errorDescription: String? { switch self { @@ -385,6 +392,8 @@ enum CaptureError: LocalizedError { return "Screen recording permission is required. Please grant permission in System Settings → Privacy & Security → Screen Recording." case .microphonePermissionDenied: return "Microphone permission is required. Please grant permission in System Settings → Privacy & Security → Microphone." + case .selectedDisplayDisconnected: + return "The selected display is no longer connected. Please select the content to capture again." } } } diff --git a/BetterCapture/Service/ContentFilterService.swift b/BetterCapture/Service/ContentFilterService.swift index 21bf14c..3e76b24 100644 --- a/BetterCapture/Service/ContentFilterService.swift +++ b/BetterCapture/Service/ContentFilterService.swift @@ -52,6 +52,35 @@ final class ContentFilterService { } } + /// Checks whether the display targeted by a filter is still connected + /// + /// A display picked from the content picker is captured by its `displayID`. Disconnecting + /// the display leaves the filter intact but makes it produce no frames, so it has to be + /// validated against the currently connected displays before capture starts. Window and + /// application filters are not bound to a display and always pass. + /// - Parameter filter: The filter to validate + /// - Returns: true if the filter can still be captured + func isSelectedDisplayConnected(_ filter: SCContentFilter) async -> Bool { + guard filter.style == .display, + let displayID = filter.includedDisplays.first?.displayID else { + return true + } + + guard let content = try? await SCShareableContent.current else { + // Nothing to validate against - let the capture attempt surface the real failure + logger.warning("Could not read shareable content, skipping display validation") + return true + } + + let isConnected = content.displays.contains { $0.displayID == displayID } + + if !isConnected { + logger.warning("Selected display \(displayID) is no longer connected") + } + + return isConnected + } + /// Applies user settings to a content filter for display capture /// - Parameters: /// - filter: The original filter from the content picker diff --git a/BetterCapture/Service/NotificationService.swift b/BetterCapture/Service/NotificationService.swift index 1deba14..539a382 100644 --- a/BetterCapture/Service/NotificationService.swift +++ b/BetterCapture/Service/NotificationService.swift @@ -126,6 +126,35 @@ final class NotificationService: NSObject { } } + /// Sends a notification for a recording that was saved without any video frames + /// - Parameter fileURL: The URL of the saved recording file + func sendRecordingMissingVideoNotification(fileURL: URL) { + let content = UNMutableNotificationContent() + content.title = "Recording Saved Without Video" + content.body = "No video was captured. Only audio was saved to \(fileURL.lastPathComponent)" + content.sound = .default + content.categoryIdentifier = NotificationIdentifier.categoryRecordingSaved + + // Store the folder URL for opening when notification is clicked + let folderURL = fileURL.deletingLastPathComponent() + content.userInfo = [UserInfoKey.folderURL: folderURL.path()] + + let request = UNNotificationRequest( + identifier: UUID().uuidString, + content: content, + trigger: nil + ) + + Task { + do { + try await UNUserNotificationCenter.current().add(request) + logger.info("Recording missing video notification sent") + } catch { + logger.error("Failed to send missing video notification: \(error.localizedDescription)") + } + } + } + /// Sends a notification for a failed recording /// - Parameter error: The error that caused the recording to fail func sendRecordingFailedNotification(error: Error) { diff --git a/BetterCapture/ViewModel/RecorderViewModel.swift b/BetterCapture/ViewModel/RecorderViewModel.swift index 519553d..b0d9a5f 100644 --- a/BetterCapture/ViewModel/RecorderViewModel.swift +++ b/BetterCapture/ViewModel/RecorderViewModel.swift @@ -285,6 +285,16 @@ final class RecorderViewModel { cameraSession.stop() selectionBorderFrame.dismiss() settings.stopAccessingOutputDirectory() + + // The selection points at a display that is gone, so drop it. The next recording + // attempt then opens the picker instead of failing the same way again. lastError + // has no UI representation, so the reason has to be surfaced as a notification. + if error as? CaptureError == .selectedDisplayDisconnected { + await resetAreaSelection() + captureEngine.clearSelection() + notificationService.sendRecordingFailedNotification(error: error) + } + logger.error("Failed to start recording: \(error.localizedDescription)") } } @@ -304,7 +314,7 @@ final class RecorderViewModel { isPresenterOverlayActive = false // Finalize file - let outputURL = try await assetWriter.finishWriting() + let (outputURL, videoFrameCount) = try await assetWriter.finishWriting() state = .idle recordingDuration = 0 @@ -314,8 +324,14 @@ final class RecorderViewModel { // Brief delay to ensure screen sharing mode has fully stopped before sending notification try? await Task.sleep(for: .milliseconds(100)) - // Send notification - notificationService.sendRecordingSavedNotification(fileURL: outputURL) + // Send notification. The file is kept either way - an audio-only recording is + // still worth more than a deleted one - but the user has to be told about it. + if videoFrameCount == 0 { + logger.error("Recording contains no video frames: \(outputURL.lastPathComponent)") + notificationService.sendRecordingMissingVideoNotification(fileURL: outputURL) + } else { + notificationService.sendRecordingSavedNotification(fileURL: outputURL) + } settings.stopAccessingOutputDirectory() diff --git a/BetterCaptureTests/AssetWriterTests.swift b/BetterCaptureTests/AssetWriterTests.swift new file mode 100644 index 0000000..f73ad2a --- /dev/null +++ b/BetterCaptureTests/AssetWriterTests.swift @@ -0,0 +1,189 @@ +// +// AssetWriterTests.swift +// BetterCaptureTests +// +// Created by Krishna Ramaroson on 28.07.26. +// + +import AVFoundation +import ScreenCaptureKit +import Testing +@testable import BetterCapture + +/// Tests for the video frame count AssetWriter reports when finishing a recording. +/// +/// A capture source that stops delivering frames - a disconnected display, for example - +/// while audio keeps flowing produces a file with audio tracks and no video track. The +/// frame count is what lets the caller detect that case. +@MainActor +struct AssetWriterTests { + + private let videoSize = CGSize(width: 640, height: 480) + + // MARK: - Tests + + @Test func audioOnlyRecordingReportsZeroVideoFrames() async throws { + let settings = makeStore() + settings.captureSystemAudio = true + + let assetWriter = AssetWriter() + try assetWriter.setup(url: makeOutputURL(), settings: settings, videoSize: videoSize) + try assetWriter.startWriting() + + // Audio flows for the whole session, no video sample ever arrives + for index in 0..<10 { + let presentationTime = CMTime(value: CMTimeValue(index * 1024), timescale: 48000) + assetWriter.appendAudioSample(try makeSilentAudioSampleBuffer(at: presentationTime)) + } + + let result = try await assetWriter.finishWriting() + defer { try? FileManager.default.removeItem(at: result.url) } + + #expect(result.videoFrameCount == 0) + + // The recording is kept - audio is still worth saving - and it holds no video track + #expect(FileManager.default.fileExists(atPath: result.url.path())) + let videoTracks = try await AVURLAsset(url: result.url).loadTracks(withMediaType: .video) + #expect(videoTracks.isEmpty) + } + + @Test func recordingWithVideoReportsFramesWritten() async throws { + let settings = makeStore() + + let assetWriter = AssetWriter() + try assetWriter.setup(url: makeOutputURL(), settings: settings, videoSize: videoSize) + try assetWriter.startWriting() + + for index in 0..<5 { + let presentationTime = CMTime(value: CMTimeValue(index), timescale: 60) + assetWriter.appendVideoSample(try makeVideoSampleBuffer(at: presentationTime)) + } + + let result = try await assetWriter.finishWriting() + defer { try? FileManager.default.removeItem(at: result.url) } + + #expect(result.videoFrameCount == 5) + } + + @Test func recordingWithoutAnySampleThrows() async throws { + let settings = makeStore() + + let assetWriter = AssetWriter() + try assetWriter.setup(url: makeOutputURL(), settings: settings, videoSize: videoSize) + try assetWriter.startWriting() + + await #expect(throws: AssetWriterError.self) { + try await assetWriter.finishWriting() + } + } + + // MARK: - Helpers + + /// Creates a SettingsStore backed by a fresh, empty UserDefaults suite. + private func makeStore() -> SettingsStore { + let suiteName = "com.sattlerjoshua.BetterCaptureTests.\(UUID().uuidString)" + let defaults = UserDefaults(suiteName: suiteName)! + return SettingsStore(defaults: defaults) + } + + private func makeOutputURL() -> URL { + FileManager.default.temporaryDirectory.appending(path: "\(UUID().uuidString).mov") + } + + /// Creates a buffer of silent 48 kHz stereo audio. + private func makeSilentAudioSampleBuffer(at presentationTime: CMTime) throws -> CMSampleBuffer { + let frameCount: AVAudioFrameCount = 1024 + + let format = try #require( + AVAudioFormat(commonFormat: .pcmFormatInt16, sampleRate: 48000, channels: 2, interleaved: true) + ) + let pcmBuffer = try #require(AVAudioPCMBuffer(pcmFormat: format, frameCapacity: frameCount)) + pcmBuffer.frameLength = frameCount + + var timing = CMSampleTimingInfo( + duration: CMTime(value: 1, timescale: 48000), + presentationTimeStamp: presentationTime, + decodeTimeStamp: .invalid + ) + + var sampleBuffer: CMSampleBuffer? + let createStatus = CMSampleBufferCreate( + allocator: kCFAllocatorDefault, + dataBuffer: nil, + dataReady: false, + makeDataReadyCallback: nil, + refcon: nil, + formatDescription: format.formatDescription, + sampleCount: CMItemCount(frameCount), + sampleTimingEntryCount: 1, + sampleTimingArray: &timing, + sampleSizeEntryCount: 0, + sampleSizeArray: nil, + sampleBufferOut: &sampleBuffer + ) + #expect(createStatus == noErr) + + let buffer = try #require(sampleBuffer) + let attachStatus = CMSampleBufferSetDataBufferFromAudioBufferList( + buffer, + blockBufferAllocator: kCFAllocatorDefault, + blockBufferMemoryAllocator: kCFAllocatorDefault, + flags: 0, + bufferList: pcmBuffer.mutableAudioBufferList + ) + #expect(attachStatus == noErr) + + return buffer + } + + /// Creates an empty BGRA video frame marked complete, as ScreenCaptureKit would deliver it. + private func makeVideoSampleBuffer(at presentationTime: CMTime) throws -> CMSampleBuffer { + var pixelBuffer: CVPixelBuffer? + let pixelBufferStatus = CVPixelBufferCreate( + kCFAllocatorDefault, + Int(videoSize.width), + Int(videoSize.height), + kCVPixelFormatType_32BGRA, + [kCVPixelBufferIOSurfacePropertiesKey as String: [:] as CFDictionary] as CFDictionary, + &pixelBuffer + ) + #expect(pixelBufferStatus == kCVReturnSuccess) + let imageBuffer = try #require(pixelBuffer) + + var formatDescription: CMFormatDescription? + let formatStatus = CMVideoFormatDescriptionCreateForImageBuffer( + allocator: kCFAllocatorDefault, + imageBuffer: imageBuffer, + formatDescriptionOut: &formatDescription + ) + #expect(formatStatus == noErr) + + var timing = CMSampleTimingInfo( + duration: CMTime(value: 1, timescale: 60), + presentationTimeStamp: presentationTime, + decodeTimeStamp: .invalid + ) + + let videoFormat = try #require(formatDescription) + + var sampleBuffer: CMSampleBuffer? + let createStatus = CMSampleBufferCreateReadyWithImageBuffer( + allocator: kCFAllocatorDefault, + imageBuffer: imageBuffer, + formatDescription: videoFormat, + sampleTiming: &timing, + sampleBufferOut: &sampleBuffer + ) + #expect(createStatus == noErr) + let buffer = try #require(sampleBuffer) + + // appendVideoSample only accepts frames the capture engine marked complete + let attachments = try #require( + CMSampleBufferGetSampleAttachmentsArray(buffer, createIfNecessary: true) as? [NSMutableDictionary] + ) + let attachment = try #require(attachments.first) + attachment[SCStreamFrameInfo.status.rawValue] = SCFrameStatus.complete.rawValue + + return buffer + } +} diff --git a/BetterCaptureTests/ErrorTests.swift b/BetterCaptureTests/ErrorTests.swift index 44633e4..9b1ca97 100644 --- a/BetterCaptureTests/ErrorTests.swift +++ b/BetterCaptureTests/ErrorTests.swift @@ -48,7 +48,8 @@ struct ErrorTests { .failedToCreateStream, .captureAlreadyRunning, .screenRecordingPermissionDenied, - .microphonePermissionDenied + .microphonePermissionDenied, + .selectedDisplayDisconnected ] for error in cases {