From 5cb233e1c9496132fdcfe5244c2b741f4f8df4d9 Mon Sep 17 00:00:00 2001 From: Psychotoxical <171614930+Psychotoxical@users.noreply.github.com> Date: Sun, 26 Apr 2026 17:44:32 +0200 Subject: [PATCH] fix(analysis): dedupe concurrent seed_from_bytes for the same track_id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six independent paths reach analysis_cache::seed_from_bytes for the same track_id during a cache-miss play under LUFS: - track_download_task legacy stream capture-complete - ranged_download_task HTTP buffer full - audio_preload bytes-ready - psysonic-local file-read complete - spawn_analysis_seed_from_in_memory_bytes (gapless reuse / preloaded / stream cache) - analysis_enqueue_seed_from_url (frontend backfill triggered by refresh:miss while a playback path is mid-seed) Whenever a track is streamed for the first time with loudness on, two of these fire in parallel and Symphonia + EBU R128 decode the same buffer twice — ~30 s of duplicate CPU per track, plus a redundant SQLite write of an identical row. Reproduced on multiple cache-miss tracks (KjP… ranged + backfill, mPdz… preloaded bytes + backfill). Coordinate at the funnel: - New SeedFromBytesOutcome::SkippedConcurrent variant. - SEEDS_IN_FLIGHT static (Mutex>) tracks track_ids whose analysis is currently running. First caller into seed_from_bytes wins the slot via HashSet::insert; later callers return SkippedConcurrent immediately and let the winner publish the analysis:waveform-updated event. - SeedInFlightGuard (RAII) releases the slot on every exit path including panics, so a crashing analysis cannot leak the marker. - enqueue_analysis_seed in lib.rs distinguishes the SkippedConcurrent log from the genuine "seed result" log so the frontend backfill case (when it arrives second) doesn't read like a backfill failure. Existing call-site match arms in audio.rs already use Ok(_) => {} for non-Upserted outcomes — no changes needed there. enqueue_analysis_seed in lib.rs already gates the waveform-updated emit on outcome == Upserted, so the new variant is silent there too. Co-Authored-By: Claude Opus 4.7 (1M context) --- src-tauri/src/analysis_cache.rs | 53 ++++++++++++++++++++++++++++++++- src-tauri/src/lib.rs | 24 +++++++++++---- 2 files changed, 70 insertions(+), 7 deletions(-) diff --git a/src-tauri/src/analysis_cache.rs b/src-tauri/src/analysis_cache.rs index 8d1d2a2c..5907a119 100644 --- a/src-tauri/src/analysis_cache.rs +++ b/src-tauri/src/analysis_cache.rs @@ -1,6 +1,7 @@ +use std::collections::HashSet; use std::path::PathBuf; use std::io::Cursor; -use std::sync::Mutex; +use std::sync::{Mutex, OnceLock}; use std::time::{Instant, SystemTime, UNIX_EPOCH}; use ebur128::{EbuR128, Mode as Ebur128Mode}; @@ -324,6 +325,39 @@ pub enum SeedFromBytesOutcome { SkippedWaveformCacheHit, /// `AnalysisCache` was not registered on the app handle. SkippedNoAnalysisCache, + /// Another seed for the same `track_id` is already running on a different + /// thread; this caller bails out so the winner's result is used. + SkippedConcurrent, +} + +/// Track-ids whose `seed_from_bytes` is currently running. There are six +/// independent producers (legacy stream capture, ranged stream completion, +/// audio_preload, psysonic-local file read, in-memory play paths, and the +/// frontend-triggered `analysis_enqueue_seed_from_url` backfill) that can +/// fire for the same id concurrently when LUFS is on. Without this guard +/// the same MP3 gets fully decoded by Symphonia + EBU R128 twice — ~30 s +/// of wasted CPU per cache-miss track. First caller wins; the rest return +/// `SkippedConcurrent` immediately. +static SEEDS_IN_FLIGHT: OnceLock>> = OnceLock::new(); + +fn seeds_in_flight() -> &'static Mutex> { + SEEDS_IN_FLIGHT.get_or_init(|| Mutex::new(HashSet::new())) +} + +/// RAII helper: removes the in-flight marker on drop. Survives panics and +/// any early `?`-returns inside `seed_from_bytes`. +struct SeedInFlightGuard { + track_id: String, +} + +impl Drop for SeedInFlightGuard { + fn drop(&mut self) { + if let Some(set) = SEEDS_IN_FLIGHT.get() { + if let Ok(mut g) = set.lock() { + g.remove(&self.track_id); + } + } + } } pub fn seed_from_bytes( @@ -331,6 +365,23 @@ pub fn seed_from_bytes( track_id: &str, bytes: &[u8], ) -> Result { + // Reserve the slot for this track_id. Atomic under the mutex: insert() + // returns false if the id was already present. + let track_key = track_id.to_string(); + let claimed = { + let mut guard = seeds_in_flight().lock().map_err(|_| "seeds-in-flight lock poisoned".to_string())?; + guard.insert(track_key.clone()) + }; + if !claimed { + crate::app_deprintln!( + "[analysis] full-track analysis skip track_id={} reason=concurrent_seed_in_flight bytes={}", + track_id, + bytes.len() + ); + return Ok(SeedFromBytesOutcome::SkippedConcurrent); + } + let _flight_guard = SeedInFlightGuard { track_id: track_key }; + let started = Instant::now(); let Some(cache) = app.try_state::() else { crate::app_deprintln!( diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 92b74a1d..69d1fe34 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -1406,12 +1406,24 @@ fn enqueue_analysis_seed(app: &tauri::AppHandle, track_id: &str, bytes: &[u8]) - .try_state::() .and_then(|cache| cache.get_latest_loudness_for_track(track_id).ok().flatten()) .is_some(); - crate::app_deprintln!( - "[analysis] seed result track_id={} bytes={} has_loudness={}", - track_id, - bytes.len(), - has_loudness - ); + // SkippedConcurrent gets logged with the actual outcome so the line doesn't + // read "seed result has_loudness=false" when in fact another path is mid-seed + // and will publish the row in seconds. + if outcome == analysis_cache::SeedFromBytesOutcome::SkippedConcurrent { + crate::app_deprintln!( + "[analysis] seed deferred to in-flight peer track_id={} bytes={} has_loudness_now={}", + track_id, + bytes.len(), + has_loudness + ); + } else { + crate::app_deprintln!( + "[analysis] seed result track_id={} bytes={} has_loudness={}", + track_id, + bytes.len(), + has_loudness + ); + } Ok(has_loudness) }