diff --git a/crates/media/src/pool.rs b/crates/media/src/pool.rs index 5ce5730f..db3e19a4 100644 --- a/crates/media/src/pool.rs +++ b/crates/media/src/pool.rs @@ -53,8 +53,10 @@ struct Entry { } /// Conformed audio files written by builds before this version are ignored (and written again): -/// version 1 files of sources longer than about 12:36 at 48 kHz were silent from there on (#172). -const CONFORM_VERSION: u32 = 2; +/// version 1 files of sources longer than about 12:36 at 48 kHz were silent from there on (#172); +/// version 2 files were written silent where the source failed to decode, and kept the footage +/// silent from then on (#324). +const CONFORM_VERSION: u32 = 3; /// Footage longer than this (24 hours) is read from its decoder rather than conformed: its /// duration comes from the file's header, and a damaged one claiming days of audio had the @@ -200,6 +202,10 @@ struct Inner { /// Conformed files being written right now. #[cfg_attr(target_arch = "wasm32", allow(dead_code))] conforming: Mutex>, + /// Conformed files that failed to write (the source didn't decode) this session: their + /// audio is read from the decoder instead of trying again on every read. + #[cfg_attr(target_arch = "wasm32", allow(dead_code))] + conform_failed: Mutex>, } /// Decodes footage for the renderer: stills, image sequences and movies (FilmCraft codecs), plus @@ -266,6 +272,7 @@ impl MediaPool { opened: AtomicU64::new(0), conform: Mutex::default(), conforming: Mutex::default(), + conform_failed: Mutex::default(), }), } } @@ -397,18 +404,23 @@ impl MediaPool { /// thread (once). #[cfg(not(target_arch = "wasm32"))] fn conform(&self, footage: &Footage, file: std::path::PathBuf, frames: usize, rate: u32) { - if !lock(&self.inner.conforming).insert(file.clone()) { + if lock(&self.inner.conform_failed).contains(&file) || !lock(&self.inner.conforming).insert(file.clone()) { return; } let (pool, f) = (self.clone(), footage.clone()); let spawned = std::thread::Builder::new().name("ec-conform-audio".into()).spawn(move || { let tmp = file.with_extension("tmp"); - let ok = file.parent().is_some_and(|d| std::fs::create_dir_all(d).is_ok()) - && pool.write_conformed(&f, &tmp, frames, rate).is_ok() - && std::fs::rename(&tmp, &file).is_ok(); - if !ok { + let written = match file.parent().map(std::fs::create_dir_all) { + Some(Ok(())) => pool.write_conformed(&f, &tmp, frames, rate).and_then(|()| std::fs::rename(&tmp, &file)), + Some(Err(e)) => Err(e), + None => Err(std::io::Error::other("no folder")), + }; + // A failed decode leaves no file: one written silent would keep the footage silent + // in every later session (#324). + if let Err(e) = written { let _ = std::fs::remove_file(&tmp); - log::warn!("media: could not write conformed audio {}", file.display()); + log::warn!("media: could not write conformed audio {} of {}: {e}", file.display(), f.path); + lock(&pool.inner.conform_failed).insert(file.clone()); } lock(&pool.inner.conforming).remove(&file); }); @@ -419,6 +431,7 @@ impl MediaPool { /// Write `frames` sample frames of `footage`'s audio at `rate` to `path` (raw interleaved /// stereo `f32`) a chunk at a time: the whole track was held in memory before it was written. + /// Fails when any chunk doesn't decode. #[cfg(not(target_arch = "wasm32"))] fn write_conformed(&self, footage: &Footage, path: &std::path::Path, frames: usize, rate: u32) -> std::io::Result<()> { use std::io::Write; @@ -426,7 +439,7 @@ impl MediaPool { let mut at = 0usize; while at < frames { let n = (frames - at).min(1 << 16); - for v in self.decode_audio(footage, sample_time(at, rate), n, rate) { + for v in self.try_decode_audio(footage, sample_time(at, rate), n, rate).map_err(std::io::Error::other)? { out.write_all(&v.to_le_bytes())?; } at += n; @@ -470,19 +483,27 @@ impl MediaPool { /// applied the edit list's priming offset twice and ramped the start of every read: AAC /// footage not at the mix rate stuttered in previews and exports (#274). fn decode_audio(&self, footage: &Footage, start: Tick, frames: usize, rate: u32) -> Vec { + self.try_decode_audio(footage, start, frames, rate).unwrap_or_else(|e| { + log::warn!("media: audio of {}: {e}", footage.path); + vec![0.0; frames * 2] + }) + } + + /// [`MediaPool::decode_audio`], failing when the source can't be opened or read. + fn try_decode_audio(&self, footage: &Footage, start: Tick, frames: usize, rate: u32) -> std::result::Result, String> { let path: Arc = footage.path.as_str().into(); - let Some(src) = self.inner.source(&path) else { return vec![0.0; frames * 2] }; + let src = self.inner.source(&path).ok_or("the file can't be opened")?; let s0 = start.to_units_floor(rate as i64); let native = src.info().audio.as_ref().map_or(rate, |a| a.sample_rate); if native == 0 || rate == 0 || native == rate { - return read_stereo(&src, &path, s0, frames, rate); + return read_stereo(&src, s0, frames, rate); } // Output sample `n` sits at source position n × native / rate. let (native_i, rate_i) = (i128::from(native), i128::from(rate)); let first = (i128::from(s0) * native_i).div_euclid(rate_i); let last = ((i128::from(s0) + frames as i128) * native_i).div_euclid(rate_i) + 1; - let (Ok(first64), Ok(count)) = (i64::try_from(first), usize::try_from(last - first + 1)) else { return vec![0.0; frames * 2] }; - let buf = read_stereo(&src, &path, first64, count, native); + let (Ok(first64), Ok(count)) = (i64::try_from(first), usize::try_from(last - first + 1)) else { return Ok(vec![0.0; frames * 2]) }; + let buf = read_stereo(&src, first64, count, native)?; let mut out = Vec::with_capacity(frames * 2); for k in 0..frames as i128 { let pos = (i128::from(s0) + k) * native_i; @@ -494,7 +515,7 @@ impl MediaPool { out.push(a + (b - a) * f); } } - out + Ok(out) } /// A thumbnail of `footage` (its first frame) fitting in `max_side` × `max_side`, aspect kept. @@ -800,27 +821,21 @@ impl FootageSource for MediaPool { /// `frames` stereo sample frames (interleaved) of `src` from sample `s0` at `rate` Hz, which /// FilmCraft serves as decoded when it is the source's own rate. Mono is duplicated to both -/// channels; silence before the start and where decoding fails. -fn read_stereo(src: &SharedSource, path: &str, s0: i64, frames: usize, rate: u32) -> Vec { +/// channels; silence before the start. An error where decoding fails. +fn read_stereo(src: &SharedSource, s0: i64, frames: usize, rate: u32) -> std::result::Result, String> { let mut out = vec![0.0; frames * 2]; let skip = usize::try_from(s0.saturating_neg()).unwrap_or(0); if skip >= frames { - return out; + return Ok(out); } - let buf = match src.audio(s0.max(0), frames - skip, rate) { - Ok(b) => b, - Err(e) => { - log::warn!("media: audio of {path}: {e}"); - return out; - } - }; - let (Some(l), Some(r)) = (buf.channels.first(), buf.channels.get(1).or(buf.channels.first())) else { return out }; - let Some(dst) = skip.checked_mul(2).and_then(|i| out.get_mut(i..)) else { return out }; + let buf = src.audio(s0.max(0), frames - skip, rate).map_err(|e| e.to_string())?; + let (Some(l), Some(r)) = (buf.channels.first(), buf.channels.get(1).or(buf.channels.first())) else { return Ok(out) }; + let Some(dst) = skip.checked_mul(2).and_then(|i| out.get_mut(i..)) else { return Ok(out) }; for ([o0, o1], (a, b)) in dst.as_chunks_mut::<2>().0.iter_mut().zip(l.iter().zip(r)) { *o0 = *a; *o1 = *b; } - out + Ok(out) } fn lock(m: &Mutex) -> std::sync::MutexGuard<'_, T> { @@ -880,6 +895,43 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + /// #324: audio that fails to decode isn't conformed. A conformed file of silence was written + /// instead, and as it is found by the file's path, size and date the footage stayed silent + /// in every later session (a renamed copy played). + #[test] + fn audio_that_fails_to_decode_is_not_conformed() { + let dir = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../../target/test-conform-fail"); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("song.mp3"); + std::fs::write(&path, b"ID3, not really").unwrap(); + let footage = Footage { + path: path.to_string_lossy().into(), + kind: FootageKind::Audio, + duration: Tick::from_seconds_f64(2.0), + has_audio: true, + ..Default::default() + }; + let pool = MediaPool::new(); + let conformed = dir.join("conformed"); + pool.set_conform_folder(Some(conformed.clone())); + let file = pool.conformed_path(&footage.path, 48_000).unwrap(); + assert!(pool.audio_samples(&footage, Tick::ZERO, 1024, 48_000).iter().all(|v| *v == 0.0)); + for _ in 0..500 { + if lock(&pool.inner.conforming).is_empty() { + break; + } + std::thread::sleep(std::time::Duration::from_millis(10)); + } + assert!(lock(&pool.inner.conforming).is_empty(), "the conform thread finished"); + assert!(!file.exists() && !file.with_extension("tmp").exists(), "no conformed file was left"); + assert!(lock(&pool.inner.conform_failed).contains(&file)); + // Later reads go to the decoder without trying again. + pool.audio_samples(&footage, Tick::ZERO, 1024, 48_000); + assert!(lock(&pool.inner.conforming).is_empty()); + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn frame_index_loops_and_clamps() { let r = FrameRate::FPS_30;