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.
258 lines
8.2 KiB
Rust
258 lines
8.2 KiB
Rust
//! Growing-file media source: reads a file that is still being written,
|
|
//! blocking (instead of returning EOF) when playback catches up to the
|
|
//! download's write head.
|
|
|
|
use std::io::{Read, Seek, SeekFrom};
|
|
use std::path::Path;
|
|
use std::sync::{Arc, Condvar, Mutex};
|
|
use std::time::Duration;
|
|
|
|
/// Everything rodio's Decoder needs from a source.
|
|
pub trait MediaRead: Read + Seek + Send + Sync {}
|
|
impl<T: Read + Seek + Send + Sync> MediaRead for T {}
|
|
|
|
#[derive(Debug, Default)]
|
|
struct ProgressState {
|
|
received: u64,
|
|
done: bool,
|
|
failed: Option<String>,
|
|
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)]
|
|
pub struct StreamProgress {
|
|
state: Mutex<ProgressState>,
|
|
cv: Condvar,
|
|
}
|
|
|
|
impl StreamProgress {
|
|
pub fn new() -> Arc<Self> {
|
|
Arc::new(Self::default())
|
|
}
|
|
|
|
pub fn update(&self, received: u64, done: bool, failed: Option<String>) {
|
|
let mut st = self.state.lock().unwrap();
|
|
st.received = st.received.max(received);
|
|
st.done |= done;
|
|
if st.failed.is_none() {
|
|
st.failed = failed;
|
|
}
|
|
self.cv.notify_all();
|
|
}
|
|
|
|
/// Current (received bytes, done) — diagnostics/observability readout.
|
|
pub fn received_done(&self) -> (u64, bool) {
|
|
let st = self.state.lock().unwrap();
|
|
(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();
|
|
st.aborted = true;
|
|
self.cv.notify_all();
|
|
}
|
|
|
|
/// Wait until at least `wanted` bytes exist (or terminal state).
|
|
/// Returns Ok(available bytes) or Err(reason) on failure/abort.
|
|
fn wait_for(&self, wanted: u64) -> Result<u64, String> {
|
|
let mut st = self.state.lock().unwrap();
|
|
loop {
|
|
if st.aborted {
|
|
return Err("stream aborted".into());
|
|
}
|
|
if let Some(reason) = &st.failed {
|
|
return Err(reason.clone());
|
|
}
|
|
if st.done || st.received >= wanted {
|
|
return Ok(st.received);
|
|
}
|
|
let (guard, _) = self
|
|
.cv
|
|
.wait_timeout(st, Duration::from_millis(250))
|
|
.map_err(|_| "progress lock poisoned".to_string())?;
|
|
st = guard;
|
|
}
|
|
}
|
|
|
|
fn snapshot(&self) -> (u64, bool) {
|
|
let st = self.state.lock().unwrap();
|
|
(st.received, st.done)
|
|
}
|
|
}
|
|
|
|
/// A file being written by a download, exposed as a blocking Read + Seek.
|
|
pub struct GrowingFile {
|
|
file: std::fs::File,
|
|
pos: u64,
|
|
total: u64,
|
|
progress: Arc<StreamProgress>,
|
|
}
|
|
|
|
impl GrowingFile {
|
|
pub fn open(
|
|
path: &Path,
|
|
total: u64,
|
|
progress: Arc<StreamProgress>,
|
|
) -> std::io::Result<GrowingFile> {
|
|
// The file may not exist until the remote queue clears; wait for
|
|
// the first bytes before opening.
|
|
progress.wait_for(1).map_err(std::io::Error::other)?;
|
|
let file = std::fs::File::open(path)?;
|
|
Ok(GrowingFile {
|
|
file,
|
|
pos: 0,
|
|
total,
|
|
progress,
|
|
})
|
|
}
|
|
}
|
|
|
|
impl Read for GrowingFile {
|
|
fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
|
|
let (received, done) = self.progress.snapshot();
|
|
|
|
// At the write head and still downloading: wait for more.
|
|
if self.pos >= received && !done {
|
|
match self.progress.wait_for(self.pos + 1) {
|
|
Ok(_) => {}
|
|
Err(e) if e == "stream aborted" => return Ok(0), // graceful EOF
|
|
Err(e) => return Err(std::io::Error::other(e)),
|
|
}
|
|
}
|
|
|
|
let n = self.file.read(buf)?;
|
|
if n == 0 {
|
|
// Raced the writer: data was promised but the handle hasn't
|
|
// caught up; only report EOF once the download is done.
|
|
let (_, done) = self.progress.snapshot();
|
|
if !done {
|
|
std::thread::sleep(Duration::from_millis(50));
|
|
let n2 = self.file.read(buf)?;
|
|
self.pos += n2 as u64;
|
|
return Ok(n2);
|
|
}
|
|
}
|
|
self.pos += n as u64;
|
|
Ok(n)
|
|
}
|
|
}
|
|
|
|
impl Seek for GrowingFile {
|
|
fn seek(&mut self, pos: SeekFrom) -> std::io::Result<u64> {
|
|
// Normalize End against the *final* size so seek math is stable
|
|
// even while the file is still growing.
|
|
let target = match pos {
|
|
SeekFrom::Start(o) => o,
|
|
SeekFrom::Current(d) => self.pos.saturating_add_signed(d),
|
|
SeekFrom::End(d) => self.total.saturating_add_signed(d),
|
|
};
|
|
// Block until the target region exists, so decoder probes into
|
|
// not-yet-downloaded space stall instead of erroring.
|
|
if target > 0 {
|
|
self.progress
|
|
.wait_for(target.min(self.total))
|
|
.map_err(std::io::Error::other)?;
|
|
}
|
|
self.pos = self.file.seek(SeekFrom::Start(target))?;
|
|
Ok(self.pos)
|
|
}
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use super::*;
|
|
use std::io::Write;
|
|
|
|
#[test]
|
|
fn growing_read_blocks_until_bytes_arrive() {
|
|
let dir = std::env::temp_dir().join("ssplus-player-test");
|
|
std::fs::create_dir_all(&dir).unwrap();
|
|
let path = dir.join("grow.bin");
|
|
std::fs::write(&path, b"").unwrap();
|
|
|
|
let progress = StreamProgress::new();
|
|
let writer_progress = progress.clone();
|
|
let writer_path = path.clone();
|
|
|
|
let writer = std::thread::spawn(move || {
|
|
let mut f = std::fs::OpenOptions::new()
|
|
.write(true)
|
|
.open(&writer_path)
|
|
.unwrap();
|
|
for chunk in 0..4u8 {
|
|
std::thread::sleep(Duration::from_millis(60));
|
|
f.write_all(&[chunk; 100]).unwrap();
|
|
f.flush().unwrap();
|
|
writer_progress.update((chunk as u64 + 1) * 100, chunk == 3, None);
|
|
}
|
|
});
|
|
|
|
let mut g = GrowingFile::open(&path, 400, progress).unwrap();
|
|
let mut out = Vec::new();
|
|
g.read_to_end(&mut out).unwrap();
|
|
writer.join().unwrap();
|
|
|
|
assert_eq!(out.len(), 400);
|
|
assert_eq!(out[350], 3);
|
|
}
|
|
|
|
#[test]
|
|
fn abort_unblocks_reader() {
|
|
let dir = std::env::temp_dir().join("ssplus-player-test");
|
|
std::fs::create_dir_all(&dir).unwrap();
|
|
let path = dir.join("abort.bin");
|
|
std::fs::write(&path, b"xx").unwrap();
|
|
|
|
let progress = StreamProgress::new();
|
|
progress.update(2, false, None);
|
|
let mut g = GrowingFile::open(&path, 1000, progress.clone()).unwrap();
|
|
|
|
let killer = std::thread::spawn(move || {
|
|
std::thread::sleep(Duration::from_millis(100));
|
|
progress.abort();
|
|
});
|
|
|
|
let mut out = Vec::new();
|
|
// Reads the 2 bytes, then blocks at the write head until abort.
|
|
g.read_to_end(&mut out).unwrap();
|
|
killer.join().unwrap();
|
|
assert_eq!(out.len(), 2);
|
|
}
|
|
}
|