Skip to content

Commit 43597ba

Browse files
itaybreitaybre
authored
fix: Ensure video writer inputs are marked as finished on cancellation in SentryVideoFrameProcessor (#5608)
* fix: Ensure video writer inputs are marked as finished on cancellation in SentryVideoFrameProcessor * Add tests * Update changelog
1 parent c585b1e commit 43597ba

3 files changed

Lines changed: 41 additions & 3 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@
66

77
- Extract video processing to a new class (#5604)
88

9+
### Fixes
10+
11+
- Fix video replay crashes due to video writer inputs not marked as finished on cancellation (#5608)
12+
913
## 8.53.2
1014

1115
### Fixes

‎Sources/Swift/Integrations/SessionReplay/SentryVideoFrameProcessor.swift‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ class SentryVideoFrameProcessor {
4949
SentrySDKLog.debug("[Session Replay] Video writer input is ready, status: \(videoWriter.status)")
5050
guard videoWriter.status == .writing else {
5151
SentrySDKLog.error("[Session Replay] Video writer is not writing anymore, cancelling the writing session, reason: \(videoWriter.error?.localizedDescription ?? "Unknown error")")
52+
videoWriter.inputs.forEach { $0.markAsFinished() }
5253
videoWriter.cancelWriting()
5354
return onCompletion(.failure(videoWriter.error ?? SentryOnDemandReplayError.errorRenderingVideo))
5455
}
@@ -80,6 +81,7 @@ class SentryVideoFrameProcessor {
8081
).timeValue
8182
guard currentPixelBuffer.append(image: image, presentationTime: presentTime) else {
8283
SentrySDKLog.error("[Session Replay] Failed to append image to pixel buffer, cancelling the writing session, reason: \(String(describing: videoWriter.error))")
84+
videoWriter.inputs.forEach { $0.markAsFinished() }
8385
videoWriter.cancelWriting()
8486
return onCompletion(.failure(videoWriter.error ?? SentryOnDemandReplayError.errorRenderingVideo))
8587
}

‎Tests/SentryTests/Integrations/SessionReplay/SentryVideoFrameProcessorTests.swift‎

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ class SentryVideoFrameProcessorTests: XCTestCase {
2828
var errorOverride: Error?
2929
var cancelWritingCalled = false
3030
var finishWritingCalled = false
31-
var markAsFinishedCalled = false
31+
var trackedInputs: [AVAssetWriterInput] = []
3232

3333
override var status: AVAssetWriter.Status {
3434
return statusOverride
@@ -48,7 +48,11 @@ class SentryVideoFrameProcessorTests: XCTestCase {
4848
}
4949

5050
override var inputs: [AVAssetWriterInput] {
51-
return []
51+
return trackedInputs
52+
}
53+
54+
override func add(_ input: AVAssetWriterInput) {
55+
trackedInputs.append(input)
5256
}
5357
}
5458

@@ -77,6 +81,7 @@ class SentryVideoFrameProcessorTests: XCTestCase {
7781
var requestMediaDataWhenReadyCalled = false
7882
var requestMediaDataWhenReadyQueue: DispatchQueue?
7983
var requestMediaDataWhenReadyBlock: (() -> Void)?
84+
var markAsFinishedInvocations = Invocations<Void>()
8085

8186
override var isReadyForMoreMediaData: Bool {
8287
return isReadyForMoreMediaDataOverride
@@ -89,7 +94,7 @@ class SentryVideoFrameProcessorTests: XCTestCase {
8994
}
9095

9196
override func markAsFinished() {
92-
// No-op for testing
97+
markAsFinishedInvocations.record(Void())
9398
}
9499
}
95100

@@ -207,12 +212,14 @@ class SentryVideoFrameProcessorTests: XCTestCase {
207212
func testProcessFrames_WhenVideoWriterNotWriting_ShouldCancelWriting() {
208213
let sut = fixture.getSut()
209214
let videoWriterInput = TestAVAssetWriterInput(mediaType: .video, outputSettings: nil)
215+
fixture.videoWriter.add(videoWriterInput)
210216
fixture.videoWriter.statusOverride = .failed
211217
let completionInvocations = Invocations<Result<SentryRenderVideoResult, any Error>>()
212218

213219
sut.processFrames(videoWriterInput: videoWriterInput) { completionInvocations.record($0) }
214220

215221
XCTAssertTrue(fixture.videoWriter.cancelWritingCalled)
222+
XCTAssertEqual(videoWriterInput.markAsFinishedInvocations.count, 1)
216223
XCTAssertEqual(completionInvocations.count, 1)
217224

218225
let result = completionInvocations.invocations.first
@@ -229,6 +236,7 @@ class SentryVideoFrameProcessorTests: XCTestCase {
229236
func testProcessFrames_WhenNoMoreFrames_ShouldFinishVideo() {
230237
let sut = fixture.getSut()
231238
let videoWriterInput = TestAVAssetWriterInput(mediaType: .video, outputSettings: nil)
239+
fixture.videoWriter.add(videoWriterInput)
232240
let completionInvocations = Invocations<Result<SentryRenderVideoResult, any Error>>()
233241

234242
// Process all frames
@@ -240,11 +248,13 @@ class SentryVideoFrameProcessorTests: XCTestCase {
240248
sut.processFrames(videoWriterInput: videoWriterInput) { completionInvocations.record($0) }
241249

242250
XCTAssertTrue(fixture.videoWriter.finishWritingCalled)
251+
XCTAssertEqual(videoWriterInput.markAsFinishedInvocations.count, 1)
243252
XCTAssertEqual(completionInvocations.count, 1)
244253
}
245254

246255
func testProcessFrames_WhenImageSizeChanges_ShouldFinishVideo() {
247256
let videoWriterInput = TestAVAssetWriterInput(mediaType: .video, outputSettings: nil)
257+
fixture.videoWriter.add(videoWriterInput)
248258
let completionInvocations = Invocations<Result<SentryRenderVideoResult, any Error>>()
249259
let sutWithDifferentSize = SentryVideoFrameProcessor(
250260
videoFrames: fixture.videoFrames,
@@ -261,19 +271,22 @@ class SentryVideoFrameProcessorTests: XCTestCase {
261271
sutWithDifferentSize.processFrames(videoWriterInput: videoWriterInput) { completionInvocations.record($0) }
262272

263273
XCTAssertTrue(fixture.videoWriter.finishWritingCalled)
274+
XCTAssertEqual(videoWriterInput.markAsFinishedInvocations.count, 1)
264275
XCTAssertEqual(completionInvocations.count, 1)
265276
}
266277

267278
func testProcessFrames_WhenPixelBufferAppendFails_ShouldCancelWriting() {
268279
let sut = fixture.getSut()
269280
let videoWriterInput = TestAVAssetWriterInput(mediaType: .video, outputSettings: nil)
281+
fixture.videoWriter.add(videoWriterInput)
270282
fixture.currentPixelBuffer.appendShouldReturn = false
271283
let completionInvocations = Invocations<Result<SentryRenderVideoResult, any Error>>()
272284

273285
// Process all frames
274286
sut.processFrames(videoWriterInput: videoWriterInput) { completionInvocations.record($0) }
275287

276288
XCTAssertTrue(fixture.videoWriter.cancelWritingCalled)
289+
XCTAssertEqual(videoWriterInput.markAsFinishedInvocations.count, 1)
277290
XCTAssertEqual(completionInvocations.count, 1)
278291

279292
let result = completionInvocations.invocations.first
@@ -319,6 +332,8 @@ class SentryVideoFrameProcessorTests: XCTestCase {
319332

320333
func testFinishVideo_WhenWriterCompleted_ShouldReturnVideoInfo() {
321334
let sut = fixture.getSut()
335+
let videoWriterInput = TestAVAssetWriterInput(mediaType: .video, outputSettings: nil)
336+
fixture.videoWriter.add(videoWriterInput)
322337
fixture.videoWriter.statusOverride = .completed
323338
let completionInvocations = Invocations<Result<SentryRenderVideoResult, any Error>>()
324339

@@ -333,6 +348,7 @@ class SentryVideoFrameProcessorTests: XCTestCase {
333348
completionInvocations.record(result)
334349
})
335350

351+
XCTAssertEqual(videoWriterInput.markAsFinishedInvocations.count, 1)
336352
XCTAssertEqual(completionInvocations.count, 1)
337353

338354
let result = completionInvocations.invocations.first
@@ -356,13 +372,16 @@ class SentryVideoFrameProcessorTests: XCTestCase {
356372

357373
func testFinishVideo_WhenWriterCancelled_ShouldReturnNilVideoInfo() {
358374
let sut = fixture.getSut()
375+
let videoWriterInput = TestAVAssetWriterInput(mediaType: .video, outputSettings: nil)
376+
fixture.videoWriter.add(videoWriterInput)
359377
fixture.videoWriter.statusOverride = .cancelled
360378
let completionInvocations = Invocations<Result<SentryRenderVideoResult, any Error>>()
361379

362380
sut.finishVideo(frameIndex: 2, onCompletion: { result in
363381
completionInvocations.record(result)
364382
})
365383

384+
XCTAssertEqual(videoWriterInput.markAsFinishedInvocations.count, 1)
366385
XCTAssertEqual(completionInvocations.count, 1)
367386

368387
let result = completionInvocations.invocations.first
@@ -380,6 +399,8 @@ class SentryVideoFrameProcessorTests: XCTestCase {
380399

381400
func testFinishVideo_WhenWriterFailed_ShouldReturnError() {
382401
let sut = fixture.getSut()
402+
let videoWriterInput = TestAVAssetWriterInput(mediaType: .video, outputSettings: nil)
403+
fixture.videoWriter.add(videoWriterInput)
383404
fixture.videoWriter.statusOverride = .failed
384405
fixture.videoWriter.errorOverride = SentryOnDemandReplayError.errorRenderingVideo
385406
let completionInvocations = Invocations<Result<SentryRenderVideoResult, any Error>>()
@@ -388,6 +409,7 @@ class SentryVideoFrameProcessorTests: XCTestCase {
388409
completionInvocations.record(result)
389410
})
390411

412+
XCTAssertEqual(videoWriterInput.markAsFinishedInvocations.count, 1)
391413
XCTAssertEqual(completionInvocations.count, 1)
392414

393415
let result = completionInvocations.invocations.first
@@ -404,6 +426,8 @@ class SentryVideoFrameProcessorTests: XCTestCase {
404426
func testFinishVideo_WhenSelfIsDeallocated_ShouldReturnNilVideoInfo() {
405427
let delayedVideoWriter = try! DelayedTestAVAssetWriter(url: fixture.outputFileURL, fileType: .mp4)
406428
delayedVideoWriter.statusOverride = .completed
429+
let videoWriterInput = TestAVAssetWriterInput(mediaType: .video, outputSettings: nil)
430+
delayedVideoWriter.add(videoWriterInput)
407431

408432
// Create a weak reference to track deallocation
409433
weak var weakSut: SentryVideoFrameProcessor?
@@ -439,6 +463,7 @@ class SentryVideoFrameProcessorTests: XCTestCase {
439463
delayedVideoWriter.executeCompletion()
440464

441465
// Verify the completion was called with nil video info
466+
XCTAssertEqual(videoWriterInput.markAsFinishedInvocations.count, 1)
442467
XCTAssertEqual(completionInvocations.count, 1)
443468

444469
let result = completionInvocations.invocations.first
@@ -456,6 +481,8 @@ class SentryVideoFrameProcessorTests: XCTestCase {
456481

457482
func testFinishVideo_WhenOutputFileDoesNotExist_ShouldReturnError() {
458483
let sut = fixture.getSut()
484+
let videoWriterInput = TestAVAssetWriterInput(mediaType: .video, outputSettings: nil)
485+
fixture.videoWriter.add(videoWriterInput)
459486
fixture.videoWriter.statusOverride = .completed
460487
let completionInvocations = Invocations<Result<SentryRenderVideoResult, any Error>>()
461488

@@ -469,6 +496,7 @@ class SentryVideoFrameProcessorTests: XCTestCase {
469496
completionInvocations.record(result)
470497
})
471498

499+
XCTAssertEqual(videoWriterInput.markAsFinishedInvocations.count, 1)
472500
XCTAssertEqual(completionInvocations.count, 1)
473501

474502
let result = completionInvocations.invocations.first
@@ -556,16 +584,19 @@ class SentryVideoFrameProcessorTests: XCTestCase {
556584
)
557585

558586
let videoWriterInput = TestAVAssetWriterInput(mediaType: .video, outputSettings: nil)
587+
fixture.videoWriter.add(videoWriterInput)
559588
sut.processFrames(videoWriterInput: videoWriterInput) { completionInvocations.record($0) }
560589

561590
XCTAssertTrue(fixture.videoWriter.finishWritingCalled)
591+
XCTAssertEqual(videoWriterInput.markAsFinishedInvocations.count, 1)
562592
XCTAssertEqual(completionInvocations.count, 1)
563593
XCTAssertEqual(sut.frameIndex, 0)
564594
}
565595

566596
func testProcessFrames_WithLargeFrameIndex_ShouldFinishImmediately() {
567597
let sut = fixture.getSut()
568598
let videoWriterInput = TestAVAssetWriterInput(mediaType: .video, outputSettings: nil)
599+
fixture.videoWriter.add(videoWriterInput)
569600
let completionInvocations = Invocations<Result<SentryRenderVideoResult, any Error>>()
570601

571602
// Set frame index beyond available frames
@@ -574,6 +605,7 @@ class SentryVideoFrameProcessorTests: XCTestCase {
574605
sut.processFrames(videoWriterInput: videoWriterInput) { completionInvocations.record($0) }
575606

576607
XCTAssertTrue(fixture.videoWriter.finishWritingCalled)
608+
XCTAssertEqual(videoWriterInput.markAsFinishedInvocations.count, 1)
577609
XCTAssertEqual(completionInvocations.count, 1)
578610
XCTAssertEqual(sut.frameIndex, 10)
579611
}

0 commit comments

Comments
 (0)