From 83eaa2e720e5a26e67f1f52df670e441aabc7b54 Mon Sep 17 00:00:00 2001 From: Baptiste Parmantier Date: Wed, 12 Aug 2026 17:40:05 +0200 Subject: [PATCH] fix(audio): offset the analysis lookup by the track's placement An audio track with start: 73 played the file from its beginning at t=73 in the video, while a waveform or audio_spectrum at t=73 drew the file's content *at 73 seconds*. Picture and sound disagreed by exactly `start`. Two independent time bases: encode/audio.rs places the track at track.start * TARGET_SAMPLE_RATE and copies the file from its own sample 0, while the analysis indexes the file from its own frame 0 and the painters call amplitude_at(ctx.time) with scenario time. Carry `start`/`end` in AudioAnalysis and apply them in one place, `track_time`, which every accessor now goes through. The consumers are unchanged: the cache is keyed by path and the painters never see the AudioTrack, so the knowledge belongs with the data rather than at three call sites. Outside [start, end) the accessors return 0, so a visualisation goes flat exactly when the track is silent instead of drawing an envelope nobody hears. The cache fingerprint gains the placement: the analysis content depends only on the file, but the lookup no longer does, so an entry computed for one placement must not be reused for another. --- .../src/engine/renderer/audio_analysis.rs | 39 +++++++++++ .../rustmotion/src/encode/audio_analysis.rs | 24 +++++-- crates/rustmotion/src/tests.rs | 65 +++++++++++++++++++ 3 files changed, 124 insertions(+), 4 deletions(-) diff --git a/crates/rustmotion-core/src/engine/renderer/audio_analysis.rs b/crates/rustmotion-core/src/engine/renderer/audio_analysis.rs index 5d402c46..94657d28 100644 --- a/crates/rustmotion-core/src/engine/renderer/audio_analysis.rs +++ b/crates/rustmotion-core/src/engine/renderer/audio_analysis.rs @@ -10,11 +10,41 @@ pub struct AudioAnalysis { pub amplitude: Vec, /// 16 log-spaced frequency bands (20 Hz–16 kHz) per frame, normalized 0..1. pub bands: Vec<[f32; 16]>, + /// Where the track sits on the scenario timeline (`AudioTrack::start`). + /// + /// The analysis indexes the *file* from its own sample 0, while every + /// consumer asks in *scenario* time. Without this the two only line up when + /// `start == 0`: a track placed at 73 s played from its beginning while the + /// waveform drew the file's content at 73 s. Held here rather than at the + /// call sites because the cache is keyed by path and the painters never see + /// the `AudioTrack`. + pub start: f64, + /// `AudioTrack::end`, past which the track is cut and the visualisation + /// must go flat rather than keep drawing an envelope nobody hears. + pub end: Option, } impl AudioAnalysis { + /// Scenario time → offset into the file, or `None` when the track is not + /// playing at that moment. Every lookup below goes through this, so the + /// placement is applied in exactly one place. + fn track_time(&self, time: f64) -> Option { + if time < self.start { + return None; + } + if let Some(end) = self.end { + if time >= end { + return None; + } + } + Some(time - self.start) + } + /// Get the amplitude at a given time (seconds), clamped to valid range. pub fn amplitude_at(&self, time: f64) -> f32 { + let Some(time) = self.track_time(time) else { + return 0.0; + }; let idx = (time * self.frame_rate as f64) as usize; self.amplitude .get(idx.min(self.amplitude.len().saturating_sub(1))) @@ -24,6 +54,9 @@ impl AudioAnalysis { /// Get a specific band [0..16) value at a given time. pub fn band_at(&self, time: f64, band: u8) -> f32 { + let Some(time) = self.track_time(time) else { + return 0.0; + }; let idx = (time * self.frame_rate as f64) as usize; let idx = idx.min(self.bands.len().saturating_sub(1)); self.bands @@ -37,6 +70,9 @@ impl AudioAnalysis { if smoothing_frames == 0 || self.amplitude.is_empty() { return self.amplitude_at(time); } + let Some(time) = self.track_time(time) else { + return 0.0; + }; let end_idx = (time * self.frame_rate as f64) as usize; let end_idx = end_idx.min(self.amplitude.len().saturating_sub(1)); let start_idx = end_idx.saturating_sub(smoothing_frames as usize); @@ -52,6 +88,9 @@ impl AudioAnalysis { if smoothing_frames == 0 || self.bands.is_empty() { return self.band_at(time, band); } + let Some(time) = self.track_time(time) else { + return 0.0; + }; let end_idx = (time * self.frame_rate as f64) as usize; let end_idx = end_idx.min(self.bands.len().saturating_sub(1)); let start_idx = end_idx.saturating_sub(smoothing_frames as usize); diff --git a/crates/rustmotion/src/encode/audio_analysis.rs b/crates/rustmotion/src/encode/audio_analysis.rs index 8dabef13..fa68e3a2 100644 --- a/crates/rustmotion/src/encode/audio_analysis.rs +++ b/crates/rustmotion/src/encode/audio_analysis.rs @@ -31,7 +31,10 @@ impl std::fmt::Display for AudioAnalysisFailure { /// up that way), so without this a track whose *content* changed under a stable /// path — the normal case when someone re-exports a mix while the studio is /// open — would keep serving the old envelope forever. -type SourceFingerprint = (u64, u128, u32); +/// Also carries the track's placement: the analysis content depends only +/// on the file, but the *lookup* now applies `start`/`end`, so an entry +/// computed for one placement must not be reused for another. +type SourceFingerprint = (u64, u128, u32, u64, u64); static FINGERPRINTS: OnceLock>> = OnceLock::new(); @@ -42,7 +45,12 @@ fn fingerprints() -> &'static Mutex> { /// `None` when the file cannot be stat'ed — treated as "changed", so the next /// analysis attempt runs and reports a real decode error instead of silently /// reusing a stale entry. -fn source_fingerprint(src: &str, fps: u32) -> Option { +fn source_fingerprint( + src: &str, + fps: u32, + start: f64, + end: Option, +) -> Option { let meta = std::fs::metadata(src).ok()?; let mtime = meta .modified() @@ -50,7 +58,13 @@ fn source_fingerprint(src: &str, fps: u32) -> Option { .duration_since(std::time::UNIX_EPOCH) .ok()? .as_nanos(); - Some((meta.len(), mtime, fps)) + Some(( + meta.len(), + mtime, + fps, + start.to_bits(), + end.unwrap_or(f64::INFINITY).to_bits(), + )) } /// Build the 16 log-spaced band frequency boundaries (Hz) from 20..16000. @@ -93,7 +107,7 @@ pub fn analyze_scenario_audio(scenario: &ResolvedScenario) -> Vec Vec 0.5, + "0.2 s after `start` is 0.2 s into the file — inside the sine" + ); + assert!( + analysis.amplitude_at(6.2) < 0.1, + "1.2 s after `start` is 1.2 s into the file — inside the silence" + ); + assert_eq!( + analysis.amplitude_at(6.6), + 0.0, + "past `end` the track is cut, so the visualisation must go flat \ + rather than keep drawing an envelope nobody hears" + ); + + // The smoothed accessors take the same path, or a bound component + // would disagree with the waveform next to it. + assert_eq!(analysis.amplitude_smoothed(4.9, 3), 0.0); + assert_eq!(analysis.band_at(4.9, 4), 0.0); + assert_eq!(analysis.band_smoothed(4.9, 4, 3), 0.0); + } + /// A track that cannot be decoded must be *reported*, not swallowed: /// silence here leaves `waveform`/`audio_spectrum` on their flat fallback /// with nothing anywhere saying why. @@ -2164,6 +2223,8 @@ mod audio_tests { frame_rate: 30, amplitude: vec![1.0; 30], bands, + start: 0.0, + end: None, }), ); @@ -2220,6 +2281,8 @@ mod audio_tests { frame_rate: 30, amplitude, bands: vec![[0.0f32; 16]; 60], + start: 0.0, + end: None, }), ); @@ -2256,6 +2319,8 @@ mod audio_tests { frame_rate: 30, amplitude, bands: vec![[0.0f32; 16]; 90], + start: 0.0, + end: None, }), );