mirror of
https://github.com/Psychotoxical/psysonic.git
synced 2026-07-22 07:15:47 +00:00
fix(analysis): dedupe concurrent seed_from_bytes for the same track_id
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<HashSet<String>>) 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) <noreply@anthropic.com>
This commit is contained in:
committed by
Maxim Isaev
parent
2d222e2691
commit
5cb233e1c9
@@ -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<Mutex<HashSet<String>>> = OnceLock::new();
|
||||
|
||||
fn seeds_in_flight() -> &'static Mutex<HashSet<String>> {
|
||||
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<SeedFromBytesOutcome, String> {
|
||||
// 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::<AnalysisCache>() else {
|
||||
crate::app_deprintln!(
|
||||
|
||||
+18
-6
@@ -1406,12 +1406,24 @@ fn enqueue_analysis_seed(app: &tauri::AppHandle, track_id: &str, bytes: &[u8]) -
|
||||
.try_state::<analysis_cache::AnalysisCache>()
|
||||
.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)
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user