Skip to content

Commit a451f83

Browse files
committed
Fix AVAssetReader unwraps and add session notes
1 parent 3f25623 commit a451f83

File tree

3 files changed

+65
-4
lines changed

3 files changed

+65
-4
lines changed

crates/editor/PLAYBACK-FINDINGS.md

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,37 @@ The CPU RGBA→NV12 conversion was taking 15-25ms per frame for 3024x1964 resolu
324324

325325
---
326326

327+
### Session 2026-02-15 (Performance Check + AVAssetReader Fix)
328+
329+
**Goal**: Run playback benchmarks, fix panics in decoder fallback path
330+
331+
**What was done**:
332+
1. Ran full playback validation on MP4 and fragmented recordings
333+
2. Identified AVAssetReader panicking with `unwrap()` on directory paths (fragmented recordings)
334+
3. Fixed by replacing `unwrap()` with proper error propagation
335+
336+
**Changes Made**:
337+
- `crates/video-decode/src/avassetreader.rs`: Replaced `ffmpeg::format::input(&path).unwrap()` and `.ok_or(...).unwrap()` with `map_err()?` and `ok_or_else()?` for clean error propagation instead of panics
338+
339+
**Results** (MP4 Mode):
340+
- ✅ Decoder: AVAssetReader (hardware), display init=114-123ms, camera init=25-33ms
341+
- ✅ Playback: 637-640 fps effective, avg=1.6ms, p95=5.0ms, p99=6.3ms
342+
- ✅ Camera sync: 0ms drift (perfect)
343+
- ✅ Mic sync: 88-100ms (borderline on this run, normally 77-88ms)
344+
- 🟡 System audio: 193-205ms (known issue, inherited from recording)
345+
346+
**Results** (Fragmented Mode):
347+
- ✅ Decoder: FFmpeg (hardware) with VideoToolbox, display init=100-110ms, camera init=7ms
348+
- ✅ Playback: 153-173 fps effective, avg=5.8-6.5ms, p95=9.0-12.4ms
349+
- ✅ Camera sync: 0ms drift (perfect)
350+
- ✅ Mic sync: 10-23ms (excellent)
351+
- ✅ AVAssetReader now cleanly falls back to FFmpeg without panicking
352+
- 🟡 System audio: 85-116ms (borderline, known issue)
353+
354+
**Stopping point**: All playback metrics healthy. AVAssetReader panic fixed. No further action needed.
355+
356+
---
357+
327358
## References
328359

329360
- `PLAYBACK-BENCHMARKS.md` - Raw performance test data (auto-updated by test runner)

crates/recording/FINDINGS.md

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,32 @@ System Audio ────┘ ├─► MP4 (macos.rs) ─
415415

416416
---
417417

418+
### Session 2026-02-15 (Performance Check + AVAssetReader Fix)
419+
420+
**Goal**: Run recording and playback benchmarks, fix any issues
421+
422+
**What was done**:
423+
1. Ran MP4 baseline benchmarks (cold + warm runs)
424+
2. Ran fragmented baseline benchmark
425+
3. Ran playback benchmark on resulting recordings
426+
4. Fixed AVAssetReader panic on directory paths (fragmented recordings)
427+
428+
**Changes Made**:
429+
- `crates/video-decode/src/avassetreader.rs`: Replaced two `unwrap()` calls with proper error propagation via `?` and `map_err`. Previously panicked when given a directory path (fragmented recordings); now returns clean error that triggers graceful FFmpeg fallback.
430+
431+
**Results**:
432+
- ✅ MP4: 29.2fps, 10.4-10.7ms jitter, 2.7% dropped, 0ms A/V sync, 81-94ms mic timing
433+
- ✅ Fragmented: 29.5-29.6fps, 4.6-5.9ms jitter, 1.3% dropped, 0ms A/V sync, 1-4ms mic timing
434+
- ✅ Playback MP4: 637fps effective, 1.6ms avg, 5.0ms p95, 0ms camera drift
435+
- ✅ Playback Fragmented: 153fps effective, 6.5ms avg, 12.4ms p95, 0ms camera drift
436+
- ✅ AVAssetReader no longer panics on directory paths
437+
- 🟡 System audio: 120-246ms (known lower-priority issue)
438+
- 🟡 MP4 dropped frames at 2.7% (single 160ms spike from encoder warmup, not actionable)
439+
440+
**Stopping point**: All major metrics pass. AVAssetReader panic fixed. System audio timing remains as documented known issue.
441+
442+
---
443+
418444
## References
419445

420446
- `BENCHMARKS.md` - Raw performance test data (auto-updated by test runner)

crates/video-decode/src/avassetreader.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -237,13 +237,13 @@ impl AVAssetReaderDecoder {
237237
keyframe_index: Option<Arc<KeyframeIndex>>,
238238
) -> Result<Self, String> {
239239
let (pixel_format, width, height) = {
240-
let input = ffmpeg::format::input(&path).unwrap();
240+
let input = ffmpeg::format::input(&path)
241+
.map_err(|e| format!("Failed to open video input '{}': {e}", path.display()))?;
241242

242243
let input_stream = input
243244
.streams()
244245
.best(ffmpeg::media::Type::Video)
245-
.ok_or("Could not find a video stream")
246-
.unwrap();
246+
.ok_or_else(|| format!("No video stream in '{}'", path.display()))?;
247247

248248
let decoder = avcodec::Context::from_parameters(input_stream.parameters())
249249
.map_err(|e| format!("decoder context / {e}"))?
@@ -338,7 +338,11 @@ impl AVAssetReaderDecoder {
338338
height: u32,
339339
) -> Result<(R<av::AssetReaderTrackOutput>, R<av::AssetReader>), String> {
340340
let asset = av::UrlAsset::with_url(
341-
&ns::Url::with_fs_path_str(path.to_str().unwrap(), false),
341+
&ns::Url::with_fs_path_str(
342+
path.to_str()
343+
.ok_or_else(|| format!("Invalid UTF-8 in path: {path:?}"))?,
344+
false,
345+
),
342346
None,
343347
)
344348
.ok_or_else(|| format!("UrlAsset::with_url{{{path:?}}}"))?;

0 commit comments

Comments
 (0)