From fa3cbc64cac8bad2f8bee395c46adb7a7f19532e Mon Sep 17 00:00:00 2001 From: KeelrLeek <156356974+KeelrLeek@users.noreply.github.com> Date: Tue, 28 Jul 2026 13:00:09 -0500 Subject: [PATCH] loudness: a prefix may lower a track's volume but never raise it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A track that opens on a lone guitar and swells into a full chorus played that chorus far too loud. The head of an unmeasured stream was being treated as a measurement of the whole song: at ten decoded seconds the analyzer's running integrated value became the gain for the entire play, and a quiet opening under-reads, so the estimator asked to turn the track UP and the song then arrived at its real level already amplified. `PRELIM_MAX_BOOST_DB = 3.0` bounded that mistake without preventing it, and the three decibels land exactly on the loudest part of the song. Measured on the 69 complete tracks in the stream cache, with the app's own meter and its own gain policy, comparing what the 10 s head asks for against what the finished track asks for: prefix error in applied gain (+ = plays too loud) 1s min -4.6 p50 +7.7 p95 +15.8 max +17.0 too loud: 61/69 3s min -0.9 p50 +5.2 p95 +15.7 max +17.0 too loud: 57/69 10s min -1.5 p50 +2.9 p95 +14.6 max +17.0 too loud: 57/69 The head is wrong in one direction and wrong by a lot. So it may only ever cut: `PRELIM_MAX_BOOST_DB` is 0.0, and a boost has to be earned by a measurement that reached the end of the media. **Cuts-only rather than the flat unity the design called for.** The design doc's stated first-release policy is that an unknown partial stream stays at 0 dB, with cuts-only left as a later experiment. Measured against this corpus, unity is the worse of the two in the direction being complained about — first-play loudness relative to the −14 LUFS target: unity (doc's policy) p50 +5.6 p95 +8.5 LOUDEST +10.6 >3dB over: 57/69 cuts-only p50 +2.7 p95 +6.7 LOUDEST +8.0 >3dB over: 31/69 old policy, capped +3 p50 +2.7 p95 +9.5 LOUDEST +11.0 >3dB over: 32/69 verified (later plays) p50 +0.0 p95 +0.0 LOUDEST +0.0 >3dB over: 0/69 Unity would hand back the cut that loud-opening tracks already get today: "Spread Eagle Cross the Block" reads −3.4 LUFS over its head and −3.4 over the whole track, and is cut 10.6 dB — under unity its first play would be 10.6 dB louder than it is now, which is the reported bug wearing a different hat. The tail risk that argues for unity is a loud opening over a quiet body, where the head over-cuts; on this corpus that case tops out at 1.5 dB (My Bloody Valentine, "Only Shallow": head −11.3, track −12.8) and the existing −15 dB floor bounds it. The corpus is one library and contains no fortissimo opening over a pianissimo body, so it bounds the error it can see, not the error that exists. Revisit if a cut ever sounds wrong. **Only a whole programme may be cached.** `analyze_loudness` returned the same `Option<(f64, f32)>` whether it reached the end of the media or the bytes simply stopped, and `on_track_started` stored it either way — with a comment saying it was valid "whatever happened to this one". It is not: `GrowingFile::read` maps an aborted transfer to `Ok(0)`, a graceful EOF, so skipping a song mid-download recorded whatever its intro measured, filed under the full declared remote size, and every later play boosted the whole track off it by up to `MAX_GAIN_DB` — the same bug as above, but permanent and three times larger. It now returns `Analysis::Complete | Incomplete`, where completeness is decided by asking the transfer how it ended (`StreamProgress::terminal`) rather than by asking a reader that cannot tell, and only `Complete` reaches the cache, the preload boundary gain, or a preview clip's gain. That path was reachable but had not fired. Re-measuring every cached track against its stored value found no disagreement, and of the ten interrupted transfers still sitting in the stream cache, none had left a loudness entry at all — the analyzer normally wins its race with the download and either finishes properly or is cancelled before the abort reaches it. So the existing cache is not quarantined and not wiped: of 141 entries only 6 ask for any boost above 0.5 dB, four of those are present on disk and verified correct by a fresh scan, and the other two ask +1.4 and +1.0 dB. Discarding 141 good measurements to protect against a hazard that measurement says is not there would cost more than it saves, and any record that is wrong is overwritten by the next complete play. **The audible start is now bounded from the play request.** Both existing windows are measured from the first measurement, and a track opening below the −70 LUFS gate never produces one, so silence or a long fade-in held playback until the analyzer found audio loud enough to measure — on a slow stream, a wait with no upper bound. `PRELIM_HARD_DEADLINE` (2.5 s) releases the start regardless, at the best cut the evidence so far justifies. This costs nothing: the player blocks on the growing file anyway, so releasing early hands it the start rather than making it wait. Verified against a 223 MB FLAC whose first bytes had not arrived at 2.2 s. **Stop now cancels the measurement window.** Found while verifying the deadline: `player_control("stop")` never touched the loudness generation, so a track stopped during its window could be handed its deferred start a moment later and begin playing after the user had stopped it. Pre-existing, but the deadline makes it reliably reachable, so `on_playback_stopped` orphans the analyzer and the held start. Not done, and deliberately: true peak and the −1.0 dBTP ceiling (the meter measures sample peak, so `PEAK_CEILING` is still 0.98 sample), the versioned measurement cache with real asset identity, ReplayGain/R128 metadata, the output limiter, track-bound gapless gain, and queue-ahead analysis. Those are later phases and none of them is needed to stop the over-boost. Verified live, driving the running app: quiet intro, first play head −27.4 LUFS → 0.0 dB (was +3.0 dB) quiet intro, second play track −6.0 LUFS → −8.01 dB from the cache loud intro, first play head −3.4 LUFS → −10.64 dB cut still applied Soulseek stream, cold head −18.8 LUFS → 0.0 dB, track measured −9.0 and cached for the next play without touching the gain in force stop mid-window no start behind the stop, no cache entry written The debug overlay now names what the gain was decided from — "whole-track measurement (cache)", "prefix (cut only)", "unity (nothing measured yet)" — because the difference is invisible in the numbers alone, which is how this went unnoticed. Settings no longer claims "boosts never clip", which nothing in the output path enforces; it describes the actual guarantee instead. The module header's claims that the gain glides mid-track and that previews are not normalized were both false and are gone. 150 Rust tests and 85 JS tests pass. --- crates/player/src/lib.rs | 4 +- crates/player/src/loudness.rs | 286 ++++++++++++++++++++++++++--- crates/player/src/source.rs | 33 ++++ src-tauri/src/commands.rs | 8 +- src-tauri/src/loudness.rs | 266 +++++++++++++++++++++------ src-tauri/src/preview.rs | 7 +- src/components/DebugOverlay.svelte | 5 +- src/views/Settings.svelte | 2 +- 8 files changed, 523 insertions(+), 88 deletions(-) diff --git a/crates/player/src/lib.rs b/crates/player/src/lib.rs index 22f4c2b..c90fa25 100644 --- a/crates/player/src/lib.rs +++ b/crates/player/src/lib.rs @@ -10,8 +10,8 @@ mod loudness; mod source; mod waveform; -pub use loudness::{analyze_loudness, AnalysisSource, LoudnessMeter}; -pub use source::{GrowingFile, StreamProgress}; +pub use loudness::{analyze_loudness, Analysis, AnalysisSource, Incomplete, LoudnessMeter}; +pub use source::{GrowingFile, StreamProgress, Terminal}; pub use waveform::compute_waveform; use std::collections::VecDeque; diff --git a/crates/player/src/loudness.rs b/crates/player/src/loudness.rs index 2a6957a..cff1b67 100644 --- a/crates/player/src/loudness.rs +++ b/crates/player/src/loudness.rs @@ -219,7 +219,7 @@ use symphonia::core::io::{MediaSourceStream, ReadOnlySource}; use symphonia::core::meta::MetadataOptions; use symphonia::core::probe::Hint; -use crate::source::{GrowingFile, StreamProgress}; +use crate::source::{GrowingFile, StreamProgress, Terminal}; /// What the loudness analyzer reads. `Growing` is the streaming case: the /// same blocking growing-file reader playback uses, so analysis proceeds at @@ -239,19 +239,67 @@ pub enum AnalysisSource { /// heard enough to commit a pre-playback gain. const PUBLISH_EVERY_SECS: f64 = 1.0; +/// How an analysis ended. +/// +/// The distinction is the whole point: integrated loudness is a property of +/// a COMPLETE programme, and the numbers a half-decoded track produces look +/// exactly like real ones. Only [`Analysis::Complete`] may be recorded as a +/// track's measurement. +pub enum Analysis { + /// Decoded from the first sample to the natural end of the media. + Complete { lufs: f64, peak: f32 }, + /// Ended before the media did — whatever was measured describes a + /// prefix, not the track. + Incomplete(Incomplete), +} + +/// Why an analysis stopped short. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Incomplete { + /// A newer track superseded this one. + Cancelled, + /// The stream was stopped or switched away from. + Aborted, + /// The transfer died mid-file. + TransferFailed, + /// The bytes stopped arriving without the transfer reporting either + /// completion or failure. + Truncated, + /// Unreadable container, no supported track, or a decoder that gave up. + DecodeFailed, + /// Never enough audio above the gate to produce a measurement. + TooShort, +} + +/// Why the decode loop stopped, before the transfer gets a say. +enum Exit { + /// Ran out of packets. For a growing file this may be a real end of + /// media OR an abort wearing its clothes — the transfer decides. + Ended, + Failed(Incomplete), +} + /// Decode `source` from the top and measure it. `on_update` receives -/// (partial LUFS, peak, seconds analyzed) roughly every 5 s of audio; -/// returns the final (integrated LUFS, peak) at EOF, or None on decode -/// failure / cancellation / too little audio. Blocking — run on a worker -/// thread. An aborted stream unblocks the reader and lands here as EOF. +/// (partial LUFS, peak, seconds analyzed) roughly every second of audio, for +/// a caller that wants to act before the end; the RETURN value is the only +/// thing that says whether the measurement covers the whole track. +/// +/// Blocking — run on a worker thread. pub fn analyze_loudness( source: AnalysisSource, cancelled: &dyn Fn() -> bool, on_update: &mut dyn FnMut(f64, f32, f64), -) -> Option<(f64, f32)> { +) -> Analysis { + use Analysis::Incomplete as Inc; + + // Kept so a growing stream can be asked, once the packets stop, whether + // that was the end of the file or the end of the transfer. + let mut transfer: Option> = None; let (mss, path) = match source { AnalysisSource::Local(path) => { - let file = std::fs::File::open(&path).ok()?; + let Ok(file) = std::fs::File::open(&path) else { + return Inc(Incomplete::DecodeFailed); + }; ( MediaSourceStream::new(Box::new(file), Default::default()), path, @@ -262,7 +310,10 @@ pub fn analyze_loudness( total, progress, } => { - let growing = GrowingFile::open(&path, total, progress).ok()?; + transfer = Some(progress.clone()); + let Ok(growing) = GrowingFile::open(&path, total, progress) else { + return Inc(Incomplete::DecodeFailed); + }; ( MediaSourceStream::new(Box::new(ReadOnlySource::new(growing)), Default::default()), path, @@ -285,40 +336,53 @@ pub fn analyze_loudness( hint.with_extension(ext); } - let probed = symphonia::default::get_probe() - .format( - &hint, - mss, - &FormatOptions::default(), - &MetadataOptions::default(), - ) - .ok()?; + let Ok(probed) = symphonia::default::get_probe().format( + &hint, + mss, + &FormatOptions::default(), + &MetadataOptions::default(), + ) else { + return Inc(Incomplete::DecodeFailed); + }; let mut format = probed.format; - let track = format.default_track()?; + let Some(track) = format.default_track() else { + return Inc(Incomplete::DecodeFailed); + }; let track_id = track.id; - let mut decoder = symphonia::default::get_codecs() + let Ok(mut decoder) = symphonia::default::get_codecs() .make(&track.codec_params, &DecoderOptions::default()) - .ok()?; + else { + return Inc(Incomplete::DecodeFailed); + }; let mut sample_buf: Option> = None; let mut meter: Option = None; let mut last_published = 0.0f64; - loop { + let exit = loop { if cancelled() { - return None; + return Inc(Incomplete::Cancelled); } let packet = match format.next_packet() { Ok(p) => p, - Err(_) => break, // EOF (or aborted stream) → finish with what we have + // Out of packets. Symphonia signals a clean end of media as an + // unexpected-EOF io error; anything else is the file or the + // demuxer failing, which is not an end of programme. + Err(symphonia::core::errors::Error::IoError(e)) + if e.kind() == std::io::ErrorKind::UnexpectedEof => + { + break Exit::Ended + } + Err(_) => break Exit::Failed(Incomplete::DecodeFailed), }; if packet.track_id() != track_id { continue; } let decoded = match decoder.decode(&packet) { Ok(d) => d, + // A corrupt packet is recoverable; the standard says skip it. Err(symphonia::core::errors::Error::DecodeError(_)) => continue, - Err(_) => break, + Err(_) => break Exit::Failed(Incomplete::DecodeFailed), }; let spec = *decoded.spec(); let channels = spec.channels.count().max(1); @@ -341,15 +405,38 @@ pub fn analyze_loudness( } last_published = secs; } - } + }; - let m = meter?; - m.integrated_lufs().map(|l| (l, m.sample_peak())) + // Running out of packets only means the whole track was seen if the + // BYTES also ran out for the right reason. An aborted growing file + // returns a graceful EOF, so without this the head of a stream the user + // skipped away from would be indistinguishable from a finished track. + match (exit, transfer.as_deref().map(StreamProgress::terminal)) { + // A transfer that stopped explains the exit whichever way the + // decoder happened to experience it — an abort arrives as a clean + // end of file, a failure as an unreadable one. + (_, Some(Terminal::Aborted)) => Inc(Incomplete::Aborted), + (_, Some(Terminal::Failed)) => Inc(Incomplete::TransferFailed), + (Exit::Failed(why), _) => Inc(why), + // Out of packets while bytes were still expected: the reader gave up + // early, so this is a prefix even though nothing reported an error. + (Exit::Ended, Some(Terminal::Running)) => Inc(Incomplete::Truncated), + (Exit::Ended, None | Some(Terminal::Done)) => { + match meter.as_ref().and_then(LoudnessMeter::integrated_lufs) { + Some(lufs) => Analysis::Complete { + lufs, + peak: meter.expect("measured").sample_peak(), + }, + None => Inc(Incomplete::TooShort), + } + } + } } #[cfg(test)] mod tests { use super::*; + use std::time::Duration; fn sine(freq: f64, fs: u32, secs: f64, amplitude: f64, channels: usize) -> Vec { let frames = (fs as f64 * secs) as usize; @@ -415,4 +502,151 @@ mod tests { m.push(&sine(997.0, 48_000, 0.2, 0.5, 2)); assert!(m.integrated_lufs().is_none()); } + + // ------------------------------------------------- completion ---- + + /// A 16-bit PCM WAV of `secs` seconds, so the analyzer has a real + /// container to decode rather than a synthetic packet stream. + fn wav(secs: f64, amplitude: f64) -> Vec { + let (fs, ch) = (48_000u32, 1usize); + let pcm = sine(997.0, fs, secs, amplitude, ch); + let data: Vec = pcm + .iter() + .flat_map(|s| ((s * 32767.0) as i16).to_le_bytes()) + .collect(); + let mut out = Vec::with_capacity(44 + data.len()); + out.extend(b"RIFF"); + out.extend(((36 + data.len()) as u32).to_le_bytes()); + out.extend(b"WAVEfmt "); + out.extend(16u32.to_le_bytes()); + out.extend(1u16.to_le_bytes()); // PCM + out.extend((ch as u16).to_le_bytes()); + out.extend(fs.to_le_bytes()); + out.extend((fs * ch as u32 * 2).to_le_bytes()); // byte rate + out.extend(((ch * 2) as u16).to_le_bytes()); // block align + out.extend(16u16.to_le_bytes()); // bits + out.extend(b"data"); + out.extend((data.len() as u32).to_le_bytes()); + out.extend(data); + out + } + + fn fixture(name: &str, bytes: &[u8]) -> PathBuf { + let dir = std::env::temp_dir().join("ssplus-player-test"); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join(name); + std::fs::write(&path, bytes).unwrap(); + path + } + + /// A transfer that finished is a whole-track measurement. + #[test] + fn a_finished_stream_measures_complete() { + let bytes = wav(5.0, 0.5); + let path = fixture("complete.wav", &bytes); + let progress = StreamProgress::new(); + progress.update(bytes.len() as u64, true, None); + let got = analyze_loudness( + AnalysisSource::Growing { + path, + total: bytes.len() as u64, + progress, + }, + &|| false, + &mut |_, _, _| {}, + ); + assert!(matches!(got, Analysis::Complete { .. }), "expected Complete"); + } + + /// The cache-poisoning path: a stream stopped during its opening reads + /// as a graceful EOF, and must NOT come back as a track measurement. + /// Before this, skipping a song mid-download recorded whatever its intro + /// measured under the full declared file size, and every later play + /// boosted the whole track off it. + #[test] + fn an_aborted_stream_is_not_a_measurement() { + let bytes = wav(30.0, 0.5); + // Only the first fifth has arrived — plenty for a gated reading. + let head = bytes.len() / 5; + let path = fixture("aborted.wav", &bytes); + let progress = StreamProgress::new(); + progress.update(head as u64, false, None); + + let killer = { + let progress = progress.clone(); + std::thread::spawn(move || { + std::thread::sleep(Duration::from_millis(300)); + progress.abort(); + }) + }; + let mut measured_a_prefix = false; + let got = analyze_loudness( + AnalysisSource::Growing { + path, + total: bytes.len() as u64, + progress, + }, + &|| false, + &mut |_, _, _| measured_a_prefix = true, + ); + killer.join().unwrap(); + + // The point is precisely that there WAS a usable-looking number. + assert!(measured_a_prefix, "fixture never produced a partial reading"); + assert!( + matches!(got, Analysis::Incomplete(Incomplete::Aborted)), + "an aborted stream must not report a whole-track measurement" + ); + } + + /// A transfer that died mid-file is likewise not a measurement. + #[test] + fn a_failed_transfer_is_not_a_measurement() { + let bytes = wav(30.0, 0.5); + let head = bytes.len() / 5; + let path = fixture("failed.wav", &bytes); + let progress = StreamProgress::new(); + progress.update(head as u64, false, None); + let killer = { + let progress = progress.clone(); + std::thread::spawn(move || { + std::thread::sleep(Duration::from_millis(300)); + progress.update(head as u64, false, Some("peer went away".into())); + }) + }; + let got = analyze_loudness( + AnalysisSource::Growing { + path, + total: bytes.len() as u64, + progress, + }, + &|| false, + &mut |_, _, _| {}, + ); + killer.join().unwrap(); + assert!( + matches!(got, Analysis::Incomplete(Incomplete::TransferFailed)), + "a dead transfer must not report a whole-track measurement" + ); + } + + /// A complete local file is the ordinary case and must still measure. + #[test] + fn a_local_file_measures_complete() { + let path = fixture("local.wav", &wav(5.0, 0.5)); + let got = analyze_loudness(AnalysisSource::Local(path), &|| false, &mut |_, _, _| {}); + let Analysis::Complete { lufs, .. } = got else { + panic!("expected Complete"); + }; + // -6 dBFS sine, mono: ~-9 LUFS. + assert!((lufs - (-9.0)).abs() < 0.5, "got {lufs}"); + } + + /// Cancellation stops the work and reports why. + #[test] + fn cancellation_reports_itself() { + let path = fixture("cancel.wav", &wav(5.0, 0.5)); + let got = analyze_loudness(AnalysisSource::Local(path), &|| true, &mut |_, _, _| {}); + assert!(matches!(got, Analysis::Incomplete(Incomplete::Cancelled))); + } } diff --git a/crates/player/src/source.rs b/crates/player/src/source.rs index 78cce46..f5188b6 100644 --- a/crates/player/src/source.rs +++ b/crates/player/src/source.rs @@ -19,6 +19,22 @@ struct ProgressState { aborted: bool, } +/// How a transfer ended. A reader cannot tell these apart on its own — +/// [`GrowingFile::read`] reports an abort as a graceful end-of-file so +/// playback stops cleanly — so anything that cares whether it saw the WHOLE +/// file (loudness measurement) has to ask the transfer itself. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Terminal { + /// Still arriving. + Running, + /// Every byte arrived. An end-of-file after this one is the real end. + Done, + /// Stopped on purpose — the track was switched or playback stopped. + Aborted, + /// The transfer died before it finished. + Failed, +} + /// Shared write-progress of an in-flight download. The downloader calls /// [`update`](Self::update); the audio thread's reads wait on it. #[derive(Debug, Default)] @@ -48,6 +64,23 @@ impl StreamProgress { (st.received, st.done) } + /// How this transfer ended, if it has. `Done` wins over `Aborted`: the + /// player aborts the stream when a track ends normally, and by then + /// every byte has already arrived — a reader that hit EOF read the + /// whole file, so the abort says nothing about coverage. + pub fn terminal(&self) -> Terminal { + let st = self.state.lock().unwrap(); + if st.done { + Terminal::Done + } else if st.aborted { + Terminal::Aborted + } else if st.failed.is_some() { + Terminal::Failed + } else { + Terminal::Running + } + } + /// Unblock any waiting reads permanently (track switched / stopped). pub fn abort(&self) { let mut st = self.state.lock().unwrap(); diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index 1e1d351..c3fa121 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -4100,6 +4100,7 @@ pub async fn waveform( /// Player transport: action ∈ pause | resume | stop, or seek/volume with value. #[tauri::command] pub fn player_control( + app: tauri::AppHandle, players: State<'_, PlayerState>, action: String, value: Option, @@ -4108,7 +4109,12 @@ pub fn player_control( match action.as_str() { "pause" => players.player.pause(), "resume" => players.player.resume(), - "stop" => players.player.stop(), + "stop" => { + // Before the player, so no analyzer is still holding a start + // that could fire behind the stop. + crate::loudness::on_playback_stopped(&app); + players.player.stop() + } "seek" => players.player.seek(value.unwrap_or(0.0)), "scratch" => players .player diff --git a/src-tauri/src/loudness.rs b/src-tauri/src/loudness.rs index 4a87d10..6601ce2 100644 --- a/src-tauri/src/loudness.rs +++ b/src-tauri/src/loudness.rs @@ -4,14 +4,22 @@ //! reference), so track-to-track volume stops being all over the place. //! //! Streaming-first: an unmeasured track is analyzed DURING its first play, -//! reading the same growing partial file the player streams from — analysis -//! runs at download speed, so the true integrated loudness usually lands -//! within seconds and the gain glides there (the player slew-limits it). -//! Measured tracks live in `loudness-cache.json`; their exact gain is -//! applied before the first sample on every later play. +//! reading the same growing partial file the player streams from, and the +//! finished measurement lands in `loudness-cache.json` for every later play, +//! where the exact gain is applied before the first sample. +//! +//! The rule that shapes everything here is that a PREFIX IS NOT THE TRACK. +//! What the opening seconds measure is only ever evidence about the opening +//! seconds, so the head of an unmeasured track may lower the volume but +//! never raise it, and a measurement that did not reach the end of the media +//! is never written to the cache. Both are one-directional protections +//! against the same mistake: turning audio up on the strength of a quiet +//! passage, and meeting the loud one still turned up. //! //! Toggleable via config `volume_normalization` (default on). Preview clips -//! are deliberately NOT normalized — Deezer masters them consistently. +//! are normalized too, from a complete measurement of the downloaded clip +//! (see `preview.rs`) — they are short and arrive whole, so they never need +//! the provisional path. use std::collections::HashMap; use std::path::{Path, PathBuf}; @@ -21,7 +29,7 @@ use std::sync::{Arc, Mutex}; use serde::{Deserialize, Serialize}; use tauri::Manager; -use player::{analyze_loudness, AnalysisSource, StreamProgress}; +use player::{analyze_loudness, Analysis, AnalysisSource, StreamProgress}; use crate::playback::PlayerState; @@ -59,6 +67,10 @@ pub struct CurrentTrack { pub cached: bool, /// The analyzer is still measuring this play. pub analyzing: bool, + /// What the applied gain was actually decided from — the difference + /// between a whole-track measurement and a prefix is invisible in the + /// numbers alone, which is how the over-boost went unnoticed. + pub source: &'static str, } pub struct LoudnessState { @@ -155,6 +167,18 @@ pub fn set_enabled(app: &tauri::AppHandle, enabled: bool) { } } +/// Playback stopped: orphan this track's analyzer and, with it, the held +/// start that the measurement window is still sitting on. +/// +/// Stop used to leave the loudness generation alone, so a track stopped +/// during its measurement window could be handed its deferred `play` a +/// moment later and start playing again after the user had stopped it. +pub fn on_playback_stopped(app: &tauri::AppHandle) { + let state = app.state::(); + state.generation.fetch_add(1, Ordering::Relaxed); + *state.current.lock().expect("loudness current lock") = None; +} + /// Pre-playback measurement window: the audible start is HELD until this /// much audio is analyzed (at download/disk speed — normally well under a /// second of wall time)… @@ -164,10 +188,41 @@ const PRELIM_MIN_SECS: f64 = 10.0; /// BEFORE the first audible sample and NEVER changes mid-track (user /// decision 2026-07-23 — the earlier mid-play glide was audible and bad). const PRELIM_WALL_BUDGET: std::time::Duration = std::time::Duration::from_millis(800); -/// A head estimate can under-read a track that opens quietly, so -/// preliminary BOOSTS are capped harder than cached ones; cuts are trusted -/// (a loud head is reliably loud). -const PRELIM_MAX_BOOST_DB: f64 = 3.0; +/// …and no later than this after the play was REQUESTED, whatever the +/// analyzer has to say. +/// +/// Both windows above are measured from the first measurement, and a track +/// that opens below the −70 LUFS gate never produces one — silence, a long +/// fade-in, a pre-gap. Those held the audible start until the analyzer found +/// audio loud enough to measure, which on a slow stream is a wait with no +/// upper bound. Releasing early costs nothing that matters: the player +/// blocks on the growing file anyway, so this hands it the start rather than +/// making it wait, and the gain it starts with is the best cut the evidence +/// so far justifies. +const PRELIM_HARD_DEADLINE: std::time::Duration = std::time::Duration::from_millis(2500); +/// A prefix may only ever CUT, never boost. +/// +/// The head of a track is not evidence about the rest of it, and the error is +/// one-directional: a quiet opening under-reads the track, so the estimator +/// asks to turn it UP, and then the song arrives at its real level already +/// amplified. A track that opens on a lone guitar and swells into a full +/// chorus was being boosted off the intro and playing the chorus far too loud. +/// The old +3 dB ceiling bounded that mistake without preventing it — 3 dB is +/// a doubling of power, and it lands exactly on the loudest part of the song. +/// +/// A ceiling of zero rather than a flat unity gain, because the error is not +/// symmetric. Measured over 69 real cached tracks, the 10 s head asks for too +/// much gain on 57 of them (median +2.9 dB, worst +17.0) and too little on 2, +/// by at most 1.5 dB. So a prefix is a bad witness for turning a track UP and +/// a serviceable one for turning it down, and refusing its cuts would be a +/// regression in the direction being complained about: a track whose opening +/// is already at −3.4 LUFS gets −10.6 dB today, and unity would hand it back +/// every one of those decibels on its first play. +/// +/// A real boost still happens — it just has to be earned by a whole-track +/// measurement, which arrives from the cache, from a preload, or from this +/// track's own analyzer in time for the NEXT play. +const PRELIM_MAX_BOOST_DB: f64 = 0.0; fn prelim_gain_db(m: &Measured) -> f64 { gain_db(m).min(PRELIM_MAX_BOOST_DB) @@ -193,12 +248,61 @@ fn store_measurement(app: &tauri::AppHandle, key: &str, lufs: f64, peak: f32, si save_cache(app, &state); } +/// The held start of an unmeasured track, and the best evidence anyone has +/// found for it so far. Two things race to open it — the analyzer, once it +/// has heard enough, and the startup deadline — so both go through +/// [`Gate::release`] and exactly one of them wins. +struct Gate { + play: Option>, + /// Most recent prefix measurement, or None if nothing has gated yet. + heard: Option, +} + +impl Gate { + /// Start playback at the gain the evidence so far justifies — which for + /// a prefix is a cut or nothing. Returns the committed gain if this call + /// was the one that opened the gate, and None if it had already opened. + fn release(gate: &Mutex, app: &tauri::AppHandle) -> Option { + // The closure is taken under the lock and called outside it: `play` + // reaches into the player, and nothing here should hold a lock that + // the audio side might want. + let (play, heard) = { + let mut g = gate.lock().expect("loudness gate lock"); + match g.play.take() { + Some(play) => (play, g.heard), + None => return None, // someone else already started it + } + }; + let db = heard.as_ref().map(prelim_gain_db).unwrap_or(0.0); + play(); + app.state::() + .player + .set_norm_gain_db(db as f32, true); + *app.state::() + .current + .lock() + .expect("loudness current lock") = Some(CurrentTrack { + lufs: heard.map(|m| m.lufs), + peak: heard.map(|m| m.peak).unwrap_or(0.0), + gain_db: db, + cached: false, + analyzing: true, + source: if heard.is_some() { + "prefix (cut only)" + } else { + "unity (nothing measured yet)" + }, + }); + Some(db) + } +} + /// The hook every main-player track start funnels through. `play` is the /// DEFERRED start of playback: a cache hit or disabled normalization fires /// it immediately; an unmeasured track holds it for the preliminary /// measurement window so the gain is right from the very first sample. -/// The analyzer then keeps running silently to EOF — only to fill the -/// cache, never to touch the volume of the play in progress. +/// The analyzer then keeps running silently to the end of the media — only +/// to fill the cache, never to touch the volume of the play in progress. pub fn on_track_started( app: &tauri::AppHandle, cache_path: &Path, @@ -235,6 +339,7 @@ pub fn on_track_started( gain_db: g, cached: true, analyzing: false, + source: "whole-track measurement (cache)", }); return; } @@ -245,10 +350,33 @@ pub fn on_track_started( gain_db: 0.0, cached: false, analyzing: true, + source: "measuring", }); // Unmeasured → the analyzer races ahead of playback (download/disk // speed) and RELEASES the held play once the preliminary window is met. + let gate = Arc::new(Mutex::new(Gate { + play: Some(play), + heard: None, + })); + + // The other end of the race: the audible start is never held longer than + // the deadline, however little the analyzer has managed to hear. + { + let app = app.clone(); + let gate = gate.clone(); + let gen_ctr = state.generation.clone(); + std::thread::Builder::new() + .name("loudness-deadline".into()) + .spawn(move || { + std::thread::sleep(PRELIM_HARD_DEADLINE); + if gen_ctr.load(Ordering::Relaxed) == generation { + Gate::release(&gate, &app); + } + }) + .expect("spawn loudness deadline"); + } + let app = app.clone(); let gen_ctr = state.generation.clone(); std::thread::Builder::new() @@ -263,19 +391,15 @@ pub fn on_track_started( None => AnalysisSource::Local(PathBuf::from(&key)), }; let cancelled = || gen_ctr.load(Ordering::Relaxed) != generation; - let mut play_slot = Some(play); let mut applied_db: Option = None; - let mut first_measure: Option = None; { let app = &app; let cancelled = &cancelled; + let gate = &gate; + let mut first_measure: Option = None; let mut on_update = |lufs: f64, peak: f32, secs: f64| { - if cancelled() || play_slot.is_none() { - return; // started already: NO further volume touches - } - let first = *first_measure.get_or_insert_with(std::time::Instant::now); - if secs < PRELIM_MIN_SECS && first.elapsed() < PRELIM_WALL_BUDGET { - return; // keep listening a moment longer + if cancelled() { + return; } let m = Measured { lufs, @@ -283,36 +407,39 @@ pub fn on_track_started( size: 0, at_epoch_ms: 0, }; - let g = prelim_gain_db(&m); - (play_slot.take().expect("gate fires once"))(); - app.state::() - .player - .set_norm_gain_db(g as f32, true); - applied_db = Some(g); - *app.state::() - .current - .lock() - .expect("loudness current lock") = Some(CurrentTrack { - lufs: Some(lufs), - peak, - gain_db: g, - cached: false, - analyzing: true, - }); + { + let mut g = gate.lock().expect("loudness gate lock"); + if g.play.is_none() { + return; // started already: NO further volume touches + } + g.heard = Some(m); + } + let first = *first_measure.get_or_insert_with(std::time::Instant::now); + if secs < PRELIM_MIN_SECS && first.elapsed() < PRELIM_WALL_BUDGET { + return; // keep listening a moment longer + } + applied_db = Gate::release(gate, app).or(applied_db); }; - let done = analyze_loudness(source, cancelled, &mut on_update); + let outcome = analyze_loudness(source, cancelled, &mut on_update); // Gate never fired (clip shorter than a gating block, decode // failure, dead transfer): start at unity — unless a newer // track superseded this one, then never start it at all. - if let Some(play) = play_slot.take() { - if !cancelled() { - play(); - } + if !cancelled() { + applied_db = Gate::release(gate, app).or(applied_db); } - let Some((lufs, peak)) = done else { return }; - // The full measurement is valid for every future play of - // this file, whatever happened to this one. + + // ONLY a measurement that reached the end of the media + // describes the track. Anything else describes however much + // of the opening happened to arrive, and caching it would + // hand a later play a boost earned by a quiet intro — the + // provisional cap above, but permanent and up to MAX_GAIN_DB. + let Analysis::Complete { lufs, peak } = outcome else { + if let Analysis::Incomplete(why) = outcome { + log::debug!("loudness: no measurement for {key}: {why:?}"); + } + return; + }; store_measurement(app, &key, lufs, peak, size); if !cancelled() { *app.state::() @@ -324,6 +451,9 @@ pub fn on_track_started( gain_db: applied_db.unwrap_or(0.0), cached: false, analyzing: false, + // The gain in force is still whatever the prefix + // justified; this measurement is for the next play. + source: "prefix (cut only) · measured for next play", }); } } @@ -358,12 +488,16 @@ pub fn on_preload_ready(app: &tauri::AppHandle, cache_path: &Path) { std::thread::Builder::new() .name("loudness-preload".into()) .spawn(move || { - let done = analyze_loudness( + // A preload is a finished file on disk, so this is a whole-track + // measurement or nothing — a truncated or corrupt download must + // not arm a boundary gain, let alone be cached as the track's. + let Analysis::Complete { lufs, peak } = analyze_loudness( AnalysisSource::Local(PathBuf::from(&key)), &|| false, &mut |_, _, _| {}, - ); - let Some((lufs, peak)) = done else { return }; + ) else { + return; + }; store_measurement(&app, &key, lufs, peak, 0); // Only hand the boundary gain over if no new track started // since — a stale preload must not color a different queue. @@ -424,11 +558,39 @@ mod tests { assert_eq!(gain_db(&m(20.0, 1.0)), MIN_GAIN_DB); } + /// The reported bug, as an assertion: a track that opens quietly and + /// swells asks the prefix policy for a large boost, and must get none. #[test] - fn preliminary_boosts_are_capped_but_cuts_pass_through() { - // A quiet head would ask +6 dB; the head estimate only gets +3. - assert!((prelim_gain_db(&m(-20.0, 0.3)) - PRELIM_MAX_BOOST_DB).abs() < 1e-9); - // Cuts are trusted at full strength — a loud head is reliably loud. + fn a_quiet_intro_never_earns_a_boost() { + // Measured off "01 - Ain't Got Nobody": the first 10 s read −27.4 + // LUFS against a whole-track −6.0, the worst head in the corpus. + assert_eq!(prelim_gain_db(&m(-27.4, 0.31)), 0.0); + // And the merely-quiet openings either side of it. + assert_eq!(prelim_gain_db(&m(-20.0, 0.3)), 0.0); + assert_eq!(prelim_gain_db(&m(-14.1, 0.5)), 0.0); + } + + /// The other half of the rule: a prefix may still turn a track DOWN, so + /// the first play of a brickwalled master is not left at full level. + #[test] + fn a_loud_intro_still_earns_its_cut() { assert!((prelim_gain_db(&m(-8.0, 1.0)) - (-6.0)).abs() < 1e-9); + // "03 Spread Eagle Cross the Block" — head and track both −3.4. + assert!((prelim_gain_db(&m(-3.4, 1.0)) - (-10.6)).abs() < 1e-9); + // A cut is bounded by the same floor a full measurement gets. + assert_eq!(prelim_gain_db(&m(20.0, 1.0)), MIN_GAIN_DB); + } + + /// Whatever the prefix policy allows, it can never exceed what the same + /// numbers would get from a whole-track measurement. + #[test] + fn a_prefix_is_never_louder_than_the_full_measurement() { + for lufs in [-40.0, -27.4, -20.0, -14.0, -8.0, -3.4, 0.0, 20.0] { + for peak in [0.01f32, 0.3, 0.9, 1.0] { + let (pre, full) = (prelim_gain_db(&m(lufs, peak)), gain_db(&m(lufs, peak))); + assert!(pre <= full + 1e-9, "{lufs} LUFS peak {peak}: {pre} > {full}"); + assert!(pre <= 0.0 + 1e-9, "{lufs} LUFS peak {peak}: boosted {pre}"); + } + } } } diff --git a/src-tauri/src/preview.rs b/src-tauri/src/preview.rs index fd748d3..dc2a1c5 100644 --- a/src-tauri/src/preview.rs +++ b/src-tauri/src/preview.rs @@ -415,10 +415,13 @@ fn measure_clip_gain(app: &tauri::AppHandle, path: &std::path::Path) -> f32 { &|| false, &mut |_, _, _| {}, ) { - Some((lufs, peak)) => { + player::Analysis::Complete { lufs, peak } => { 10f64.powf(crate::loudness::normalization_gain_db(lufs, peak) / 20.0) as f32 } - None => 1.0, + // A clip that did not decode all the way through is measured over an + // unknown fraction of itself; play it as mastered rather than at a + // gain derived from part of it. + player::Analysis::Incomplete(_) => 1.0, } } diff --git a/src/components/DebugOverlay.svelte b/src/components/DebugOverlay.svelte index 43b6285..d7c4574 100644 --- a/src/components/DebugOverlay.svelte +++ b/src/components/DebugOverlay.svelte @@ -133,10 +133,7 @@ {:else}on · idle{/if} {#if stats.normalizationEnabled && loud} -
- peak {loud.peak.toFixed(2)} - · {loud.cached ? "cache hit" : loud.analyzing ? "analyzing" : "measured this play"} -
+
peak {loud.peak.toFixed(2)} · {loud.source}
{/if} {#if previewUi.open}
diff --git a/src/views/Settings.svelte b/src/views/Settings.svelte index 6c91b7c..7e9b893 100644 --- a/src/views/Settings.svelte +++ b/src/views/Settings.svelte @@ -235,7 +235,7 @@
Normalize volume
-
Plays every track at roughly the same loudness (−14 LUFS, like Spotify), so quiet and loud masters stop needing the volume knob. Streams are measured during their first play; boosts never clip.
+
Plays every track at roughly the same loudness (−14 LUFS, like Spotify), so quiet and loud masters stop needing the volume knob. A track is measured while it first streams and matched exactly from then on; until that finishes SoulStream will turn it down but never up, so a quiet intro can't make the chorus too loud.