fix(macos): drop video frames whose timestamp does not advance

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
+	}
+}

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

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))
+	}
+}