optimize: video and audio render

crates/oak-codec/src/ffmpeg.rs

@@ -250,6 +250,15 @@ impl FFmpegDecoder {
 			.and_then(|s| s.hw_device.map(crate::hwdecode::device_type_name))
 			.map(|name| name.to_string())
 	}
+
+	/// Test-only: how many `seek()` calls the open session has performed.
+	/// Contiguous audio chunks must not increase this (they continue the
+	/// decode without re-seeking); a non-contiguous chunk must.
+	#[cfg(test)]
+	pub(crate) fn audio_seek_count(&self) -> u64 {
+		let state = self.state.lock().unwrap_or_else(|e| e.into_inner());
+		state.as_ref().map(|s| s.audio_seeks).unwrap_or(0)
+	}
 }
 
 impl Default for FFmpegDecoder {
@@ -481,6 +490,10 @@ struct DecoderState {
 	eof: bool,
 	video: Option<VideoDecodeState>,
 	audio: Option<AudioDecodeState>,
+	/// Test-only counter of `seek()` calls (proves contiguous audio chunks
+	/// skip the seek).
+	#[cfg(test)]
+	audio_seeks: u64,
 }
 
 enum DecoderInner {
@@ -523,6 +536,14 @@ struct ScalingCache {
 /// Audio decode state: a resampler keyed by (rate, layout).
 struct AudioDecodeState {
 	resampler: Option<(u32, u64, AudioResampler)>,
+	/// The first sample of the next contiguous chunk: the sample index
+	/// (at the current `sample_rate`) one past the last decoded chunk.
+	/// Set on every successful `retrieve_audio_to`; cleared on every seek
+	/// (any seek resets the decoder and resampler, breaking continuity).
+	/// When the next chunk's start sample equals this, the decode can
+	/// continue without re-seeking — skipping the per-chunk decoder flush
+	/// and resampler reset that cause boundary artifacts (pops/clicks).
+	contiguous_end_sample: Option<i64>,
 }
 
 /// A swresample conversion context.
@@ -641,7 +662,10 @@ impl DecoderState {
 			None
 		};
 		let audio = if matches!(medium, MediaType::Audio) {
-			Some(AudioDecodeState { resampler: None })
+			Some(AudioDecodeState {
+				resampler: None,
+				contiguous_end_sample: None,
+			})
 		} else {
 			None
 		};
@@ -661,6 +685,8 @@ impl DecoderState {
 			eof: false,
 			video,
 			audio,
+			#[cfg(test)]
+			audio_seeks: 0,
 		})
 	}
 
@@ -784,7 +810,16 @@ impl DecoderState {
 			DecoderInner::Video(d) => d.flush(),
 			DecoderInner::Audio(d) => d.flush(),
 		}
+		#[cfg(test)]
+		{
+			self.audio_seeks += 1;
+		}
 		self.eof = false;
+		// Any seek breaks audio continuity (decoder flush + resampler
+		// reset): the next audio chunk must not skip its own seek.
+		if let Some(a) = &mut self.audio {
+			a.contiguous_end_sample = None;
+		}
 		Ok(())
 	}
 
@@ -1137,88 +1172,71 @@ impl DecoderState {
 
 		dest.fill(0.0);
 
-		// Seek to just before the range start.
-		let start_ts =
-			oak_rational(self.stream_time_base).time_to_timestamp(Rational::from_double(start_sec));
-		self.seek(start_ts)?;
-
-		// Take the cached resampler out (or create one) so the decode loop
-		// below can borrow `self` freely; it is put back before returning.
-		let src_layout = channel_layout_from_mask(self.input_channel_layout_mask);
-		let mut resampler = match self.audio.as_mut().expect("audio session").resampler.take() {
-			Some((rate, layout, rs)) if rate == sample_rate as u32 && layout == channel_layout => {
-				rs
-			}
-			_ => AudioResampler::get(
-				self.input_sample_format,
-				src_layout,
-				self.input_sample_rate,
-				Sample::F32(SampleType::Packed),
-				dst_layout,
-				sample_rate as u32,
-			)?,
-		};
-		let stream_time_base = self.stream_time_base;
-
-		let mut next_sample: Option<i64> = None;
-		let dest_frames = (dest.len() / dst_channels) as i64;
-
-		while let Some(frame) = self.next_frame()? {
-			let DecodedFrame::Audio(audio) = frame else {
-				continue;
-			};
-			let converted = resample_to_interleaved_f32(&mut resampler, &audio)?;
-			if converted.is_empty() {
-				continue;
-			}
-			let chunk_samples = (converted.len() / dst_channels) as i64;
-			let frame_start = match audio.pts() {
-				Some(pts) => {
-					let secs = oak_rational(stream_time_base)
-						.timestamp_to_time(pts)
-						.to_f64();
-					(secs * sample_rate as f64).round() as i64
-				}
-				None => next_sample.unwrap_or(start_sample),
-			};
-			let offset = frame_start - start_sample;
-			if offset < dest_frames && offset + chunk_samples > 0 {
-				let copy_start = offset.max(0) as usize;
-				let copy_end = (offset + chunk_samples).min(dest_frames).max(0) as usize;
-				if copy_end > copy_start {
-					let src_off = (copy_start as i64 - offset) as usize * dst_channels;
-					let n = (copy_end - copy_start) * dst_channels;
-					let dst_off = copy_start * dst_channels;
-					if dst_off + n <= dest.len() {
-						dest[dst_off..dst_off + n]
-							.copy_from_slice(&converted[src_off..src_off + n]);
-					}
-				}
-			}
-			next_sample = Some(frame_start + chunk_samples);
-			if offset + chunk_samples >= dest_frames {
-				break;
-			}
+		// Continuity fast path: when this chunk's start sample equals the
+		// sample one past the previous chunk's end, the decoder and
+		// resampler are still positioned exactly there — continue without
+		// seeking. Skipping the per-chunk seek avoids the decoder flush and
+		// resampler reset whose boundary artifacts (silence gaps, phase
+		// resets) were audible as pops/clicks at chunk edges. The caller's
+		// anchored sample grid (`round(out·rate) − round(in·rate)`) makes
+		// the next chunk's start exactly the previous chunk's recorded end,
+		// so exact comparison is correct.
+		let contiguous = self
+			.audio
+			.as_ref()
+			.map(|a| a.contiguous_end_sample == Some(start_sample))
+			.unwrap_or(false);
+		if !contiguous {
+			// Seek to just before the range start.
+			let start_ts =
+				oak_rational(self.stream_time_base).time_to_timestamp(Rational::from_double(start_sec));
+			self.seek(start_ts)?;
 		}
 
-		// Put the resampler back into the cache for the next call.
-		self.audio.as_mut().expect("audio session").resampler =
-			Some((sample_rate as u32, channel_layout, resampler));
+		// The fill+decode+flush body runs in a closure so the success path
+		// can record the chunk's end sample (continuity) and an error path
+		// can clear it (the decoder/resampler position is unknown after a
+		// mid-chunk failure).
+		let read = (|| -> crate::error::Result<RetrieveAudioStatus> {
+			// Take the cached resampler out (or create one) so the decode loop
+			// below can borrow `self` freely; it is put back before returning.
+			let src_layout = channel_layout_from_mask(self.input_channel_layout_mask);
+			let mut resampler = match self.audio.as_mut().expect("audio session").resampler.take() {
+				Some((rate, layout, rs)) if rate == sample_rate as u32 && layout == channel_layout => {
+					rs
+				}
+				_ => AudioResampler::get(
+					self.input_sample_format,
+					src_layout,
+					self.input_sample_rate,
+					Sample::F32(SampleType::Packed),
+					dst_layout,
+					sample_rate as u32,
+				)?,
+			};
+			let stream_time_base = self.stream_time_base;
 
-		// Flush any samples still buffered in the resampler (rate conversion
-		// tail), appending after the last decoded sample.
-		let resampler = self
-			.audio
-			.as_mut()
-			.expect("audio session")
-			.resampler
-			.as_mut()
-			.map(|r| &mut r.2)
-			.unwrap();
-		if let Some(flush) = flush_resampler_interleaved_f32(resampler)? {
-			if !flush.is_empty() {
-				let chunk_samples = (flush.len() / dst_channels) as i64;
-				let frame_start = next_sample.unwrap_or(start_sample);
+			let mut next_sample: Option<i64> = None;
+			let dest_frames = (dest.len() / dst_channels) as i64;
+
+			while let Some(frame) = self.next_frame()? {
+				let DecodedFrame::Audio(audio) = frame else {
+					continue;
+				};
+				let converted = resample_to_interleaved_f32(&mut resampler, &audio)?;
+				if converted.is_empty() {
+					continue;
+				}
+				let chunk_samples = (converted.len() / dst_channels) as i64;
+				let frame_start = match audio.pts() {
+					Some(pts) => {
+						let secs = oak_rational(stream_time_base)
+							.timestamp_to_time(pts)
+							.to_f64();
+						(secs * sample_rate as f64).round() as i64
+					}
+					None => next_sample.unwrap_or(start_sample),
+				};
 				let offset = frame_start - start_sample;
 				if offset < dest_frames && offset + chunk_samples > 0 {
 					let copy_start = offset.max(0) as usize;
@@ -1229,14 +1247,70 @@ impl DecoderState {
 						let dst_off = copy_start * dst_channels;
 						if dst_off + n <= dest.len() {
 							dest[dst_off..dst_off + n]
-								.copy_from_slice(&flush[src_off..src_off + n]);
+								.copy_from_slice(&converted[src_off..src_off + n]);
+						}
+					}
+				}
+				next_sample = Some(frame_start + chunk_samples);
+				if offset + chunk_samples >= dest_frames {
+					break;
+				}
+			}
+
+			// Put the resampler back into the cache for the next call.
+			self.audio.as_mut().expect("audio session").resampler =
+				Some((sample_rate as u32, channel_layout, resampler));
+
+			// Flush any samples still buffered in the resampler (rate conversion
+			// tail), appending after the last decoded sample.
+			let resampler = self
+				.audio
+				.as_mut()
+				.expect("audio session")
+				.resampler
+				.as_mut()
+				.map(|r| &mut r.2)
+				.unwrap();
+			if let Some(flush) = flush_resampler_interleaved_f32(resampler)? {
+				if !flush.is_empty() {
+					let chunk_samples = (flush.len() / dst_channels) as i64;
+					let frame_start = next_sample.unwrap_or(start_sample);
+					let offset = frame_start - start_sample;
+					if offset < dest_frames && offset + chunk_samples > 0 {
+						let copy_start = offset.max(0) as usize;
+						let copy_end = (offset + chunk_samples).min(dest_frames).max(0) as usize;
+						if copy_end > copy_start {
+							let src_off = (copy_start as i64 - offset) as usize * dst_channels;
+							let n = (copy_end - copy_start) * dst_channels;
+							let dst_off = copy_start * dst_channels;
+							if dst_off + n <= dest.len() {
+								dest[dst_off..dst_off + n]
+									.copy_from_slice(&flush[src_off..src_off + n]);
+							}
 						}
 					}
 				}
 			}
-		}
 
-		Ok(RetrieveAudioStatus::Success)
+			// Record the end sample: the next chunk starting exactly here
+			// continues without a seek.
+			self.audio.as_mut().expect("audio session").contiguous_end_sample =
+				Some(start_sample + dest_frames);
+
+			Ok(RetrieveAudioStatus::Success)
+		})();
+
+		match read {
+			Ok(status) => Ok(status),
+			Err(e) => {
+				// A failed chunk leaves the decoder/resampler position
+				// unknown: force a fresh seek on the next chunk.
+				if let Some(a) = &mut self.audio {
+					a.contiguous_end_sample = None;
+				}
+				Err(e)
+			}
+		}
 	}
 
 	/// Conform the open stream's audio into per-channel PCM files, mirroring

crates/oak-codec/src/hwdecode.rs

@@ -57,17 +57,61 @@ pub static HW_TRANSFERS: std::sync::atomic::AtomicU64 = std::sync::atomic::Atomi
 /// 0 = force software). Default ON by user mandate.
 pub const CONFIG_KEY_HARDWARE_DECODING: &str = "HardwareDecoding";
 
+/// Process-wide negative cache of failed device types (a `static`, not a
+/// `const` — a const array is inlined per use site, so writes from
+/// `mark_device_unavailable` would never be visible to `device_unavailable`).
+/// Creating a hardware device context (`av_hwdevice_ctx_create`) is
+/// expensive when the driver is missing and emits FFmpeg's
+/// `[VAAPI @ ...] Failed to initialise VAAPI connection` spam every time —
+/// on a box without libva that is once per decoder open, per process, with
+/// the full log line. The cache marks a device type unavailable after its
+/// first failed creation, so every later open skips it (and its log noise)
+/// entirely. The slot count is generous: `AVHWDeviceType` values are small
+/// non-negative enumerants (< 32), 64 covers them all.
+static UNAVAILABLE_DEVICES: [std::sync::atomic::AtomicBool; 64] =
+    [const { std::sync::atomic::AtomicBool::new(false) }; 64];
+
+/// Test-only counter of `open_hw_accel` device-context creation attempts
+/// (incremented at the top of the function, before any FFmpeg call). Lets
+/// tests prove the negative cache short-circuits before FFmpeg is
+/// involved.
+#[cfg(test)]
+static CREATE_ATTEMPTS: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0);
+
+/// Whether `device_type` is known unavailable — a device-context
+/// creation failed once earlier in this process.
+pub fn device_unavailable(device_type: sys::AVHWDeviceType) -> bool {
+    UNAVAILABLE_DEVICES
+        .get(device_type as usize)
+        .map(|f| f.load(std::sync::atomic::Ordering::Relaxed))
+        .unwrap_or(false)
+}
+
+/// Record that `device_type`'s device context failed to create (sticky
+/// for the process; the caller falls back to the next candidate).
+pub fn mark_device_unavailable(device_type: sys::AVHWDeviceType) {
+    if let Some(f) = UNAVAILABLE_DEVICES.get(device_type as usize) {
+        f.store(true, std::sync::atomic::Ordering::Relaxed);
+    }
+}
+
 /// Whether hardware decoding is preferred (the config switch). Default
 /// ON by user mandate; only an explicit `"false"` turns it off (the
 /// string accessor, same convention as the app's config helpers — the
-/// store's typed `get_bool` only parses pre-typed Bool entries).
+/// store's typed `get_bool` only parses pre-typed Bool entries). The
+/// `OAK_HWACCEL` environment variable overrides both: `0` force-disables
+/// hardware decode entirely (a diagnostic escape hatch on machines where
+/// probing the device is slow or noisy).
 pub fn hardware_decoding_enabled() -> bool {
-	match oak_common::configstore::ConfigStore::instance()
-		.get(None, CONFIG_KEY_HARDWARE_DECODING)
-	{
-		Ok(value) => value != "false",
-		Err(_) => true,
-	}
+    if let Ok(v) = std::env::var("OAK_HWACCEL") {
+        return v != "0";
+    }
+    match oak_common::configstore::ConfigStore::instance()
+        .get(None, CONFIG_KEY_HARDWARE_DECODING)
+    {
+        Ok(value) => value != "false",
+        Err(_) => true,
+    }
 }
 
 /// The hardware device types to try, most preferred first. On a machine
@@ -116,6 +160,18 @@ pub fn open_hw_accel(
 	codec: ffmpeg::Codec,
 	device_type: sys::AVHWDeviceType,
 ) -> Option<(ffmpeg::codec::decoder::Opened, sys::AVHWDeviceType)> {
+	// Negative cache: a device type whose context creation failed once
+	// (no driver, headless box) is never tried again — creation is slow
+	// and logs `[VAAPI @ ...] Failed to initialise VAAPI connection` per
+	// attempt. Checked before any FFmpeg call so marked types cost
+	// nothing.
+	if device_unavailable(device_type) {
+		return None;
+	}
+	#[cfg(test)]
+	{
+		CREATE_ATTEMPTS.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
+	}
 	let mut context = ffmpeg::codec::Context::from_parameters(params.clone()).ok()?;
 	let mut device: *mut sys::AVBufferRef = std::ptr::null_mut();
 	// SAFETY: `device` is a valid out-pointer; on success it owns the
@@ -130,6 +186,9 @@ pub fn open_hw_accel(
 		)
 	};
 	if rc < 0 || device.is_null() {
+		// The device is unavailable (missing driver / no hardware): mark
+		// it so later opens skip the attempt and its log noise.
+		mark_device_unavailable(device_type);
 		return None;
 	}
 	// SAFETY: `hw_device_ctx` takes ownership of the reference; the codec
@@ -241,4 +300,47 @@ mod tests {
 		);
 		assert!(opened.is_some(), "VideoToolbox hwaccel must open for H.264");
 	}
+
+	/// A device type marked unavailable is short-circuited before any
+	/// FFmpeg call: `open_hw_accel` returns None without attempting device
+	/// creation (which is what used to log
+	/// `[VAAPI @ ...] Failed to initialise VAAPI connection` every time on
+	/// boxes without the driver).
+	#[test]
+	fn negative_cache_skips_marked_device() {
+		// VDPAU is not a candidate on any supported platform, so marking
+		// it cannot disturb the platform tests in this process (e.g. the
+		// macOS VideoToolbox test above).
+		let dev = sys::AVHWDeviceType::AV_HWDEVICE_TYPE_VDPAU;
+		mark_device_unavailable(dev);
+		assert!(device_unavailable(dev));
+
+		let input = ffmpeg::format::input(&"../oak-app/tests/demo.mp4".to_string())
+			.expect("open demo.mp4");
+		let fstream = input.stream(0).expect("stream 0");
+		let params = fstream.parameters();
+		let codec = ffmpeg::decoder::find(params.id()).expect("software h264 codec");
+
+		CREATE_ATTEMPTS.store(0, std::sync::atomic::Ordering::Relaxed);
+		let opened = open_hw_accel(&params, codec, dev);
+		assert!(opened.is_none(), "marked device must not open");
+		assert_eq!(
+			CREATE_ATTEMPTS.load(std::sync::atomic::Ordering::Relaxed),
+			0,
+			"marked device must be skipped before any creation attempt"
+		);
+
+		// Restore so the (non-candidate) type stays clean for other tests.
+		UNAVAILABLE_DEVICES[dev as usize].store(false, std::sync::atomic::Ordering::Relaxed);
+	}
+
+	/// A device type never tried before is not cached as unavailable
+	/// (only a failed creation marks it). DXVA2 is not a candidate on any
+	/// supported platform (Windows uses D3D11VA/CUDA), so it is never
+	/// touched by other tests in this process.
+	#[test]
+	fn unmarked_device_type_is_not_unavailable() {
+		let dev = sys::AVHWDeviceType::AV_HWDEVICE_TYPE_DXVA2;
+		assert!(!device_unavailable(dev));
+	}
 }

crates/oak-codec/src/realmedia_tests.rs

@@ -179,6 +179,85 @@ fn audio_decode_is_non_empty() {
 	assert!(peak > 0.0, "decoded audio is all silence");
 }
 
+/// Contiguous audio chunks (each chunk's start is exactly where the
+/// previous chunk ended) must continue the decode without re-seeking:
+/// the seek count stays flat, and the concatenated chunks must match a
+/// one-shot decode of the same span. A non-contiguous chunk re-seeks.
+/// This is the regression test for the boundary pops/clicks caused by the
+/// per-chunk `av_seek_frame` + decoder flush + resampler reset.
+#[test]
+fn contiguous_audio_chunks_skip_seek_and_match_oneshot() {
+	let d = FFmpegDecoder::new();
+	let s = CodecStream::with_block(demo_path().to_string_lossy().into_owned(), 1, None);
+	d.open(&s).expect("open audio stream");
+
+	// Chunk [0s, 1s): opens the session (one seek inside retrieve).
+	let mut c1 = vec![0f32; 48000 * 2];
+	d.retrieve_audio(
+		&mut c1,
+		&TimeRange::new(Rational::new(0, 1), Rational::new(1, 1)),
+		48000,
+		0x3,
+	)
+	.expect("chunk [0,1)");
+	assert!(c1.iter().any(|&v| v != 0.0), "chunk [0,1) is all silence");
+	let seeks_after_first = d.audio_seek_count();
+
+	// Chunk [1s, 2s): contiguous with the previous one — must not seek.
+	let mut c2 = vec![0f32; 48000 * 2];
+	d.retrieve_audio(
+		&mut c2,
+		&TimeRange::new(Rational::new(1, 1), Rational::new(2, 1)),
+		48000,
+		0x3,
+	)
+	.expect("chunk [1,2)");
+	assert!(
+		d.audio_seek_count() == seeks_after_first,
+		"contiguous chunk must skip the seek ({} -> {})",
+		seeks_after_first,
+		d.audio_seek_count()
+	);
+	assert!(c2.iter().any(|&v| v != 0.0), "chunk [1,2) is all silence");
+
+	// Chunk [3s, 4s): not contiguous — must seek again.
+	let mut c4 = vec![0f32; 48000 * 2];
+	d.retrieve_audio(
+		&mut c4,
+		&TimeRange::new(Rational::new(3, 1), Rational::new(4, 1)),
+		48000,
+		0x3,
+	)
+	.expect("chunk [3,4)");
+	assert!(
+		d.audio_seek_count() > seeks_after_first,
+		"non-contiguous chunk must seek"
+	);
+
+	// Concatenating [0,1)+[1,2) must equal a one-shot decode of [0,2):
+	// both read the same decoder continuously from sample 0, so the
+	// samples must match exactly (the second chunk only differs in that it
+	// skipped its seek — no flush/resampler reset was involved).
+	let mut combined = c1;
+	combined.extend_from_slice(&c2);
+	let mut oneshot = vec![0f32; 48000 * 2 * 2];
+	d.retrieve_audio(
+		&mut oneshot,
+		&TimeRange::new(Rational::new(0, 1), Rational::new(2, 1)),
+		48000,
+		0x3,
+	)
+	.expect("oneshot [0,2)");
+	let mut max_diff = 0.0f32;
+	for (a, b) in combined.iter().zip(oneshot.iter()) {
+		max_diff = max_diff.max((a - b).abs());
+	}
+	assert!(
+		max_diff < 0.01,
+		"contiguous chunks diverge from one-shot decode (max diff {max_diff})"
+	);
+}
+
 #[test]
 fn encode_h264_roundtrip_to_tmp() {
 	let out = std::env::temp_dir().join(format!("oakcodec_roundtrip_{}.mp4", std::process::id()));