diff --git a/electron/native/screencapturekit/Sources/OpenScreenCaptureCore/VideoTimestampGate.swift b/electron/native/screencapturekit/Sources/OpenScreenCaptureCore/VideoTimestampGate.swift new file mode 100644 index 000000000..d02445668 --- /dev/null +++ b/electron/native/screencapturekit/Sources/OpenScreenCaptureCore/VideoTimestampGate.swift @@ -0,0 +1,75 @@ +import CoreMedia + +/// Keeps the video track's presentation times strictly increasing, which is the one thing +/// `AVAssetWriter` will not forgive. +/// +/// # What a non-increasing timestamp costs +/// +/// The writer validates every appended sample's timing in MediaToolbox's +/// `MediaSampleTimingGenerator`, and a frame whose presentation time is not after the previous +/// one — equal, earlier, or earlier than the session start — fails it with an OSStatus of +/// -16364. That fails the whole writer, not the sample. It also does not fail the append that +/// carried the bad frame: that one answers `true`, and it is the NEXT append that returns +/// `false` with `AVFoundationErrorDomain -11800` wrapping -16364. Every frame after it is +/// dropped while ScreenCaptureKit carries on delivering, so the take ends at the last complete +/// fragment and the rest is lost. Measured with the helper's exact writer settings, fragmented +/// and not; it is not a fragmentation defect like -16341. +/// +/// # Where one comes from +/// +/// Pause/resume. The helper measures a pause on the host clock and shifts every later sample +/// back by it, but a ScreenCaptureKit frame's presentation time is not the instant it was +/// delivered: it runs a few milliseconds ahead of the host clock, by an amount that varies from +/// frame to frame (median 4.8 ms, spread about 20 ms). Subtracting the exact pause length lands +/// the first frame after a resume behind the last one before it when that lead drops, between +/// the two frames, by more than the delivery gaps on either side of the pause add up to. Over +/// 200 real pause/resume cycles at 1080p60 on an M1 (macOS 26.5), 6 resumes did, by 0.2 to +/// 1.9 ms; 7 did at 4K60. Every one was captured after resume, so gating frames on when they +/// were captured would not have caught them. +/// +/// The gate refuses frames until time moves past the last one the writer received, however many +/// that takes. Every overlap measured was under 2 ms, so in practice that has been a single frame +/// at 60 fps. Audio is not involved — its track is clocked by `AudioTrackMixer`, and the writer +/// accepts audio that steps backwards. +/// +/// The gate compares in the samples' own time base. Two frames closer together than the track's +/// 1/600 s media timescale are accepted and re-spaced by the writer, so there is no rounding to +/// guard against here. +public struct VideoTimestampGate { + public enum Verdict: Equatable { + case admit + /// The timestamp is not a number the writer can place. + case invalid + /// Not after the last frame handed to the writer. + case notAfterPrevious(previous: CMTime) + } + + /// The presentation time of the last frame actually handed to the writer. + public private(set) var lastRecorded: CMTime? + /// Frames refused so far, for the diagnostic at the end of a take. + public private(set) var rejectedCount = 0 + + public init() {} + + /// Decides whether a frame may be appended. Counts a refusal, records nothing else: a frame + /// that passes but is then not appended (the input was not ready) must not move the gate. + public mutating func check(_ presentationTime: CMTime) -> Verdict { + guard presentationTime.isValid, presentationTime.isNumeric else { + rejectedCount += 1 + return .invalid + } + if let lastRecorded, CMTimeCompare(presentationTime, lastRecorded) <= 0 { + rejectedCount += 1 + return .notAfterPrevious(previous: lastRecorded) + } + return .admit + } + + /// Call with the frame's presentation time just before handing it to `append`. + /// + /// Before, not after, and whatever `append` answers: the writer judges a frame as soon as it + /// receives it, so a frame it was given is the one the next frame has to follow. + public mutating func record(_ presentationTime: CMTime) { + lastRecorded = presentationTime + } +} diff --git a/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift b/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift index 00706a9f9..9d549c724 100644 --- a/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift +++ b/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift @@ -134,6 +134,8 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { private var isPaused = false private var pauseStartedAt: CMTime? private var totalPausedDuration = CMTime.zero + /// Sample queue only. See `VideoTimestampGate` for the failure it exists to prevent. + private var videoTimestampGate = VideoTimestampGate() private var nativeMicrophoneEnabled = false private var outputWidth = 1920 private var outputHeight = 1080 @@ -290,6 +292,19 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { return } let presentationTime = CMSampleBufferGetPresentationTimeStamp(sampleBuffer) + // A frame that does not move the timeline forward fails the whole writer one append + // later (-16364), so it is dropped here instead. After a resume that is one frame the + // user never sees; without the check it was the end of the recording. + switch videoTimestampGate.check(presentationTime) { + case .admit: + break + case .invalid: + reportRefusedVideoFrame(presentationTime, previous: nil, pauseOffset: pauseState.offset) + return + case .notAfterPrevious(let previous): + reportRefusedVideoFrame(presentationTime, previous: previous, pauseOffset: pauseState.offset) + return + } if !didStartWriting { writer.startWriting() writer.startSession(atSourceTime: presentationTime) @@ -299,6 +314,7 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { } if videoInput.isReadyForMoreMediaData { + videoTimestampGate.record(presentationTime) let appended = videoInput.append(sampleBuffer) if appended, !didEmitRecordingStarted { didEmitRecordingStarted = true @@ -343,6 +359,24 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { ) } + /// Once per take, with the numbers that tell the cause apart. A refusal with a non-zero pause + /// offset, a few milliseconds behind the previous frame, is the pause shift measured in + /// `VideoTimestampGate`. One with no pause offset at all is a source handing over time that + /// goes backwards, which nothing here has observed yet and is worth a report. + private func reportRefusedVideoFrame(_ presentationTime: CMTime, previous: CMTime?, pauseOffset: CMTime) { + guard videoTimestampGate.rejectedCount == 1 else { + return + } + emit([ + "event": "warning", + "code": "video-frame-timestamp-refused", + "message": "Dropped a video frame whose timestamp did not advance.", + "presentationTimeSeconds": presentationTime.isNumeric ? CMTimeGetSeconds(presentationTime) : -1, + "previousSeconds": previous.map { CMTimeGetSeconds($0) } ?? -1, + "pauseOffsetSeconds": pauseOffset.isNumeric ? CMTimeGetSeconds(pauseOffset) : 0, + ]) + } + private func ensureRequestedPermissions() throws { if !CGPreflightScreenCaptureAccess() { let granted = CGRequestScreenCaptureAccess() @@ -637,6 +671,16 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { } } + let refusedVideoFrames = sampleQueue.sync { videoTimestampGate.rejectedCount } + if refusedVideoFrames > 1 { + emit([ + "event": "warning", + "code": "video-frame-timestamps-refused", + "message": "Dropped \(refusedVideoFrames) video frames whose timestamps did not advance.", + "count": refusedVideoFrames, + ]) + } + videoInput?.markAsFinished() audioInput?.markAsFinished() @@ -700,13 +744,16 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { arrayToFill: &timing, entriesNeededOut: nil ) + // Both failures drop the sample rather than pass it on unshifted. An unshifted frame sits + // a whole pause ahead of the timeline, and every correctly shifted frame after it would + // then fall behind it — refused by the timestamp gate for as long as the pause lasted. if timingStatus != noErr { emit([ "event": "warning", "code": "sample-retime-failed", "message": "Unable to read sample timing info: \(timingStatus).", ]) - return sampleBuffer + return nil } for index in timing.indices { @@ -735,7 +782,7 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { "code": "sample-retime-failed", "message": "Unable to copy sample timing info: \(copyStatus).", ]) - return sampleBuffer + return nil } return retimedBuffer diff --git a/electron/native/screencapturekit/Tests/OpenScreenCaptureCoreTests/VideoTimestampGateTests.swift b/electron/native/screencapturekit/Tests/OpenScreenCaptureCoreTests/VideoTimestampGateTests.swift new file mode 100644 index 000000000..e5d480160 --- /dev/null +++ b/electron/native/screencapturekit/Tests/OpenScreenCaptureCoreTests/VideoTimestampGateTests.swift @@ -0,0 +1,95 @@ +import CoreMedia +import XCTest + +import OpenScreenCaptureCore + +/// Does the writer only ever see video timestamps that move forward? +/// +/// A single frame that does not is -16364, and it ends the recording: see `VideoTimestampGate`. +final class VideoTimestampGateTests: XCTestCase { + private func seconds(_ value: Double) -> CMTime { + CMTime(seconds: value, preferredTimescale: 1_000_000_000) + } + + func testFirstFrameIsAdmitted() { + var gate = VideoTimestampGate() + XCTAssertEqual(gate.check(seconds(2419.6)), .admit) + } + + func testIncreasingFramesAreAdmitted() { + var gate = VideoTimestampGate() + for index in 0..<120 { + let time = seconds(10 + Double(index) / 60) + XCTAssertEqual(gate.check(time), .admit) + gate.record(time) + } + XCTAssertEqual(gate.rejectedCount, 0) + } + + /// The pause/resume case as it was measured on real ScreenCaptureKit frames: the first frame + /// after resume, shifted back by the pause, landed 1.78 ms before the last frame appended + /// before it. + func testFrameShiftedBehindThePreviousOneByAResumeIsRefused() { + var gate = VideoTimestampGate() + let lastBeforePause = seconds(2419.6246833760001) + gate.record(lastBeforePause) + + let firstAfterResume = CMTimeSubtract(seconds(2420.5588043749999), seconds(0.93589941600000004)) + XCTAssertEqual(gate.check(firstAfterResume), .notAfterPrevious(previous: lastBeforePause)) + + // This overlap was under one frame, so the frame after it is already clear. + let next = CMTimeSubtract(seconds(2420.5588043749999 + 1.0 / 60), seconds(0.93589941600000004)) + XCTAssertEqual(gate.check(next), .admit) + XCTAssertEqual(gate.rejectedCount, 1) + } + + /// Nothing bounds the overlap to one frame, so the gate has to keep refusing until time moves + /// past the last frame the writer received, and then let the stream through again. + func testOverlapLongerThanOneFrameIsRefusedUntilTimeAdvances() { + var gate = VideoTimestampGate() + let lastBeforePause = seconds(100) + gate.record(lastBeforePause) + + let frame = 1.0 / 60 + let first = seconds(100 - 1.5 * frame) + let second = seconds(100 - 0.5 * frame) + let third = seconds(100 + 0.5 * frame) + XCTAssertEqual(gate.check(first), .notAfterPrevious(previous: lastBeforePause)) + XCTAssertEqual(gate.check(second), .notAfterPrevious(previous: lastBeforePause)) + XCTAssertEqual(gate.check(third), .admit) + XCTAssertEqual(gate.rejectedCount, 2) + } + + func testDuplicateTimestampIsRefused() { + var gate = VideoTimestampGate() + gate.record(seconds(5)) + XCTAssertEqual(gate.check(seconds(5)), .notAfterPrevious(previous: seconds(5))) + } + + /// Equal instants in different time bases are still the same instant. + func testDuplicateInAnotherTimescaleIsRefused() { + var gate = VideoTimestampGate() + gate.record(CMTime(value: 3000, timescale: 600)) + XCTAssertEqual( + gate.check(seconds(5)), + .notAfterPrevious(previous: CMTime(value: 3000, timescale: 600)) + ) + } + + func testInvalidTimestampIsRefused() { + var gate = VideoTimestampGate() + XCTAssertEqual(gate.check(.invalid), .invalid) + XCTAssertEqual(gate.check(.indefinite), .invalid) + XCTAssertEqual(gate.rejectedCount, 2) + } + + /// A frame that passed but was never appended — the input was not ready — must not raise the + /// bar for the frames after it. + func testCheckingDoesNotMoveTheGate() { + var gate = VideoTimestampGate() + gate.record(seconds(1)) + XCTAssertEqual(gate.check(seconds(3)), .admit) + XCTAssertEqual(gate.check(seconds(2)), .admit) + XCTAssertEqual(gate.lastRecorded, seconds(1)) + } +}