Editor playback optimization

[richiemcilroy]

Optimise playback performance by implementing a comprehensive benchmarking infrastructure, tuning decoder prefetch, batching GPU command submissions, pipelining GPU readbacks, and improving audio synchronization.

This PR addresses key bottlenecks such as the double GPU round-trip for frames, conservative prefetching, and synchronous GPU readbacks, aiming for smoother 60fps playback, faster scrubbing, and tighter audio-video sync across macOS and Windows.

---

 

Greptile Overview

Greptile Summary

Comprehensive performance optimization focused on eliminating GPU bottlenecks and improving playback smoothness through pipelined operations and smarter prefetching.

Key optimizations:

• Pipelined GPU readbacks with triple buffering eliminate synchronous waits between frames
• Batched GPU command submissions reduce overhead by combining YUV conversion and rendering into single command buffers
• Increased prefetch buffer (60→90 frames) and parallel decode tasks (4→6) with ramped startup (8 tasks initially)
• LRU frame cache with eviction strategy prevents memory bloat during seeks
• Reduced decoder timeouts (200ms→100ms) and separate initial seek timeout (10s vs 2s)
• NV12 output format option bypasses double GPU round-trip for WebSocket streaming
• Comprehensive benchmarking infrastructure added for measuring pipeline performance

Architecture improvements:

• Shared YUV converter pipelines with bind group caching reduce GPU object creation overhead
• Encoder-based layer preparation allows batching GPU work instead of immediate submission
• prepare_with_encoder variants across display and camera layers enable command buffering
• Frame cache eviction when seeking prevents unbounded memory growth
• Statistics logging every 2s tracks cache hits, prefetch hits, and sync decodes

Confidence Score: 4/5

• Safe to merge with minor observations on timeout tuning and buffer sizing
• Well-structured performance optimizations with clear separation of concerns. The pipelined GPU operations and batched command submission are sound architectural improvements. Timeout reductions and increased buffer sizes are reasonable tuning choices. One concern is the reduced GPU timeout (30s→10s) may be aggressive for slower systems, but the retry logic provides fallback. The NV12 conversion path is properly validated with dimension checks.
• Pay attention to crates/editor/src/playback.rs timeout tuning and crates/rendering/src/frame_pipeline.rs GPU buffer timeout reduction

Important Files Changed

Filename	Overview
crates/editor/src/playback.rs	Optimized playback prefetching with increased buffer sizes (90 frames), parallel decode tasks (6-8), reduced timeouts, and added frame cache eviction strategy
crates/rendering/src/frame_pipeline.rs	Added pipelined GPU readbacks with triple buffering, RGBA-to-NV12 converter, reduced GPU timeout from 30s to 10s, and improved async buffer mapping
crates/rendering/src/yuv_converter.rs	New YUV converter with shared pipelines, bind group caching, encoder batching support for both NV12 and YUV420P formats
crates/rendering/src/lib.rs	Added NV12 rendering path, pipeline flushing for final frames, encoder-based layer preparation to batch GPU commands
crates/rendering/src/layers/display.rs	Added prepare_with_encoder variant to batch YUV conversion with other GPU work instead of submitting separately

Flowchart

flowchart TD
    A[Playback Start] --> B[Prefetch Task Spawned]
    A --> C[Main Playback Loop]
    
    B --> D{Frames Decoded < 15?}
    D -->|Yes| E[Use 8 Parallel Tasks]
    D -->|No| F[Use 6 Parallel Tasks]
    
    E --> G[Decode Frames Ahead]
    F --> G
    
    G --> H[Send to Prefetch Buffer]
    H --> I[LRU Frame Cache]
    
    C --> J{Frame in Cache?}
    J -->|Yes| K[Cache Hit - Use Cached]
    J -->|No| L{Frame in Prefetch Buffer?}
    
    L -->|Yes| M[Prefetch Hit - Use Frame]
    L -->|No| N{Frame In-Flight?}
    
    N -->|Yes| O[Wait 100ms for Frame]
    N -->|No| P[Sync Decode with Timeout]
    
    K --> Q[Render Pipeline]
    M --> Q
    O --> Q
    P --> Q
    
    Q --> R[Prepare Layers with Encoder]
    R --> S[YUV to RGBA Conversion]
    S --> T[Batch GPU Commands]
    T --> U{Output Format?}
    
    U -->|RGBA| V[Pipelined RGBA Readback]
    U -->|NV12| W[RGBA to NV12 Conversion]
    
    W --> X[Pipelined NV12 Readback]
    
    V --> Y[Triple Buffer Rotation]
    X --> Y
    
    Y --> Z[Return Previous Frame]
    Z --> AA[WebSocket Send]
    
    AA --> AB{Frames Behind?}
    AB -->|Yes > 6| AC[Skip Frames]
    AB -->|No| C
    AC --> AD[Evict Cache]
    AD --> C

Loading

_{Last reviewed commit: f60bbbe}

[cursor]

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
_{Learn more about Cursor Agents}

[cursor]

You have run out of free Bugbot PR reviews for this billing cycle. This will reset on March 8.

To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

[tembo]

NV12 metadata currently hardcodes y_stride = width. Since WSFrame carries stride (and callers fill it for NV12), it seems safer to encode that stride here so the receiver can handle padded NV12 rows.

[tembo]

validate_dimensions can return effective_width/effective_height that differ from the input. This function still uploads/dispatches using the original width/height, which can exceed the allocated texture size when downscaling kicks in. Consider either early-returning when effective_* != * (since we’re not resampling here) or consistently using effective_* for uploads/dispatch after ensuring the planes match.

[tembo]

Same thing for convert_yuv420p_to_encoder: effective_width/effective_height is computed but the rest of the function uses the original width/height for uploads and dispatch. If downscaling is possible here, it’d be good to make the behavior explicit (reject vs. handle) to avoid mismatched extents.

[tembo]

This expect can take the whole render down if something goes sideways; probably better to surface it as a RenderingError like the other failure paths.

Suggested change

	.expect("just submitted a conversion");
	let pending = nv12_converter
	.take_pending()
	.ok_or(RenderingError::BufferMapWaitingFailed)?;

[tembo]

This currently drops any flush error silently. Even if you don’t want to fail the whole render on a flush failure, a warn-level log here would make it much easier to spot pipeline issues.

Suggested change

	if let Some(Ok(final_frame)) = frame_renderer.flush_pipeline().await
	&& final_frame.width > 0
	&& final_frame.height > 0
	{
	sender
	.send((final_frame, frame_number.saturating_sub(1)))
	.await?;
	}
	if let Some(result) = frame_renderer.flush_pipeline().await {
	match result {
	Ok(final_frame) if final_frame.width > 0 && final_frame.height > 0 => {
	sender
	.send((final_frame, frame_number.saturating_sub(1)))
	.await?;
	}
	Err(e) => {
	tracing::warn!(error = %e, "Failed to flush pipelined readback");
	}
	_ => {}
	}
	}

[tembo]

NV12 path currently hardcodes y_stride = width and ignores WSFrame.stride. If the NV12 producer ever has padded rows, the receiver will misinterpret the buffer.

Suggested change

	fn pack_nv12_frame_ref(
	data: &[u8],
	y_stride: u32,
	width: u32,
	height: u32,
	frame_number: u32,
	target_time_ns: u64,
	) -> Vec<u8> {
	let metadata_size = 28;
	let mut output = Vec::with_capacity(data.len() + metadata_size);
	output.extend_from_slice(data);
	output.extend_from_slice(&y_stride.to_le_bytes());
	output.extend_from_slice(&height.to_le_bytes());
	output.extend_from_slice(&width.to_le_bytes());
	output.extend_from_slice(&frame_number.to_le_bytes());
	output.extend_from_slice(&target_time_ns.to_le_bytes());
	output.extend_from_slice(&NV12_FORMAT_MAGIC.to_le_bytes());
	output
	}

[tembo]

If you take the y_stride param in pack_nv12_frame_ref, this call site should pass through the stride from the frame.

Suggested change

	WSFrameFormat::Nv12 => pack_nv12_frame_ref(
	&frame.data,
	frame.width,
	frame.height,
	frame.frame_number,
	frame.target_time_ns,
	),
	WSFrameFormat::Nv12 => pack_nv12_frame_ref(
	&frame.data,
	frame.stride,
	frame.width,
	frame.height,
	frame.frame_number,
	frame.target_time_ns,
	),

[tembo]

validate_dimensions can return effective_width/effective_height smaller than the inputs, but this function still uploads/dispatches using the original width/height. If downscaling kicks in, that looks like it can write past the allocated textures. Probably want to consistently use the effective dimensions for upload extents + dispatch (or explicitly bail when downscaled).

[tembo]

Small behavior change: previously a failed previous readback was best-effort (error dropped) and rendering continued; now it hard-errors the whole pipeline via ?. If map failures can be transient on some GPUs, keeping this non-fatal might be safer.

Suggested change

	let previous_frame = if let Some(prev) = session.pipelined_readback.take_pending() {
	Some(prev.wait(device).await?)
	} else {
	None
	};
	let previous_frame = if let Some(prev) = session.pipelined_readback.take_pending() {
	prev.wait(device).await.ok()
	} else {
	None
	};