fix(cache): reuse masters obtained at the requested quality

Record the successful request separately from the preferred quality and the
returned format. Preserve explicit fallback limits across playback and
prefetch, retain provenance when spilling to disk, and read old sidecars
conservatively without inventing acquisition history.

Cover available CD/96 kHz masters, fallback and authentication boundaries,
file identity and sidecar versions. The regression fails with the previous
predicate; 299 core tests pass. Playback smoke confirms CD-quality reuse
from disk after acquisition at the maximum requested tier.

cx
This commit is contained in:
vicrodh committed 2026-09-14 18:07:08 -06:00
1 parent da7c4899ea
commit 180f96d378
6 files changed
+246 -26

No files matched your search

+34 -4
View File
@@ -187,7 +187,14 @@ impl PlaybackCache {
let mut bytes = Vec::new();
fs::File::open(self.quality_path(track_id)).ok()?.take(4096).read_to_end(&mut bytes).ok()?;
let record: QualityRecord = serde_json::from_slice(&bytes).ok()?;
(record.version == 1 && record.file == identity).then_some(record.quality)
if record.file != identity { return None; }
match record.version {
// v1 did not record the successful request. Never infer it
// from the preference or from a response's available format.
1 => crate::CacheQuality::from_resolved(record.quality.requested, record.quality.resolved.id()),
2 => Some(record.quality),
_ => None,
}
})();
Some((file, quality))
}
@@ -201,7 +208,7 @@ impl PlaybackCache {
let published = (|| -> Option<()> {
let quality = quality?;
let file = fs::File::open(self.track_path(track_id)).ok()?;
let record = QualityRecord { version: 1, file: FileIdentity::read(&file)?, quality };
let record = QualityRecord { version: 2, file: FileIdentity::read(&file)?, quality };
let bytes = serde_json::to_vec(&record).ok()?;
let mut pending = tempfile::NamedTempFile::new_in(&self.cache_dir).ok()?;
pending.write_all(&bytes).ok()?;
@@ -513,7 +520,7 @@ mod disk_reader_tests {
use qbz_models::Quality;
let temp = tempfile::tempdir().unwrap();
let cache = PlaybackCache::with_path(temp.path().into(), 1024).unwrap();
let quality = crate::CacheQuality::from_resolved(Quality::UltraHiRes, 27);
let quality = crate::CacheQuality::from_acquisition(Quality::UltraHiRes, Quality::UltraHiRes, 7);
assert!(cache.insert_with_quality(7, b"first", quality));
let record = fs::read(cache.quality_path(7)).unwrap();
assert_eq!(cache.open_with_quality(7).unwrap().1, quality);
@@ -535,7 +542,7 @@ mod disk_reader_tests {
let temp = tempfile::tempdir().unwrap();
let disk = std::sync::Arc::new(PlaybackCache::with_path(temp.path().into(), 1024).unwrap());
let memory = crate::AudioCache::with_playback_cache(5, disk.clone());
let quality = crate::CacheQuality::from_resolved(qbz_models::Quality::UltraHiRes, 27);
let quality = crate::CacheQuality::from_acquisition(qbz_models::Quality::UltraHiRes, qbz_models::Quality::UltraHiRes, 6);
memory.insert_with_quality(1, b"first".to_vec(), quality);
assert_eq!(memory.get(1).unwrap().quality, quality);
memory.insert(2, b"other".to_vec());
@@ -544,6 +551,29 @@ mod disk_reader_tests {
assert_eq!(disk.get(1).unwrap(), b"first");
}
#[test]
fn successful_request_sidecar_version_controls_trust() {
use qbz_models::Quality;
let temp = tempfile::tempdir().unwrap();
let cache = PlaybackCache::with_path(temp.path().into(), 1024).unwrap();
let quality = crate::CacheQuality::from_acquisition(Quality::UltraHiRes, Quality::UltraHiRes, 6);
assert!(cache.insert_with_quality(7, b"complete audio", quality));
let path = cache.quality_path(7);
let mut record: serde_json::Value = serde_json::from_slice(&fs::read(&path).unwrap()).unwrap();
assert_eq!(record["version"], 2);
assert!(cache.open_with_quality(7).unwrap().1.unwrap().satisfies(Quality::UltraHiRes));
// Even a stray new field in an old record cannot grant new trust.
record["version"] = 1.into();
fs::write(&path, serde_json::to_vec(&record).unwrap()).unwrap();
let old = cache.open_with_quality(7).unwrap().1.unwrap();
assert!(!old.satisfies(Quality::UltraHiRes));
assert!(old.satisfies(Quality::Lossless));
record["version"] = 99.into();
fs::write(&path, serde_json::to_vec(&record).unwrap()).unwrap();
assert_eq!(cache.open_with_quality(7).unwrap().1, None);
assert_eq!(cache.get(7).unwrap(), b"complete audio");
}
#[test]
fn startup_enforces_reduced_budget_and_preserves_unowned_files() {
let temp = tempfile::tempdir().unwrap();
+72 -1
View File
@@ -6,6 +6,10 @@ use serde::{Deserialize, Serialize};
pub struct CacheQuality {
pub requested: Quality,
pub resolved: Quality,
/// The request that actually succeeded, after any explicit fallback.
/// Absent in older records: the response format alone cannot recover it.
#[serde(default, skip_serializing_if = "Option::is_none")]
successful_request: Option<Quality>,
}
impl CacheQuality {
@@ -13,11 +17,34 @@ impl CacheQuality {
Some(Self {
requested,
resolved: Quality::from_id(format_id)?,
successful_request: None,
})
}
pub fn from_acquisition(
requested: Quality,
successful_request: Quality,
format_id: u32,
) -> Option<Self> {
if successful_request > requested {
return None;
}
Some(Self {
successful_request: Some(successful_request),
..Self::from_resolved(requested, format_id)?
})
}
pub fn satisfies(self, requested: Quality) -> bool {
self.requested >= requested && self.resolved >= requested
if self.requested < requested || (requested != Quality::Mp3 && self.resolved == Quality::Mp3) {
return false;
}
match self.successful_request {
// A successful highest-tier request can return a CD or 96 kHz
// master. Only an explicit lower-tier request limits reuse.
Some(actual) => actual <= self.requested && actual >= requested,
None => self.resolved >= requested,
}
}
}
@@ -25,6 +52,50 @@ impl CacheQuality {
mod tests {
use super::*;
#[test]
fn successful_request_reuses_available_cd_and_96khz_masters() {
for response_format in [6, 7, 27] {
let acquired = CacheQuality::from_acquisition(
Quality::UltraHiRes, Quality::UltraHiRes, response_format,
).unwrap();
for target in Quality::fallback_order() {
assert!(acquired.satisfies(*target));
}
}
}
#[test]
fn successful_request_preserves_explicit_fallback_and_lossy_limits() {
for actual in Quality::fallback_order() {
let acquired = CacheQuality::from_acquisition(
Quality::UltraHiRes, *actual, actual.id(),
).unwrap();
for target in Quality::fallback_order() {
assert_eq!(acquired.satisfies(*target), actual >= target);
}
}
// Even an unexpected lossy response to a lossless request must not
// qualify as the best lossless master.
let lossy = CacheQuality::from_acquisition(Quality::UltraHiRes, Quality::UltraHiRes, 5).unwrap();
assert!(!lossy.satisfies(Quality::Lossless));
assert!(lossy.satisfies(Quality::Mp3));
assert!(CacheQuality::from_acquisition(Quality::HiRes, Quality::UltraHiRes, 7).is_none());
assert!(CacheQuality::from_acquisition(Quality::UltraHiRes, Quality::UltraHiRes, 999).is_none());
}
#[test]
fn successful_request_missing_or_inconsistent_remains_conservative() {
let old: CacheQuality = serde_json::from_str(
r#"{"requested":"UltraHiRes","resolved":"HiRes"}"#,
).unwrap();
assert!(!old.satisfies(Quality::UltraHiRes));
assert!(old.satisfies(Quality::HiRes));
let inconsistent: CacheQuality = serde_json::from_str(
r#"{"requested":"HiRes","resolved":"HiRes","successful_request":"UltraHiRes"}"#,
).unwrap();
assert!(!inconsistent.satisfies(Quality::HiRes));
}
#[test]
fn acquisition_ceiling_distinguishes_best_master_from_lower_tier_copy() {
let best = CacheQuality::from_resolved(Quality::UltraHiRes, 27).unwrap();
+36 -19
View File
@@ -5690,7 +5690,7 @@ impl Player {
let content_key = cmaf_info.content_key;
let flac_header = cmaf_info.flac_header;
let n_segments = cmaf_info.n_segments;
let cache_quality = qbz_cache::CacheQuality::from_resolved(quality, cmaf_info.format_id);
let cache_quality = qbz_cache::CacheQuality::from_acquisition(quality, quality, cmaf_info.format_id);
let cache = self.audio_cache.clone();
let feeder_state = self.state.clone();
let (cancel_tx, cancel_rx) = tokio::sync::watch::channel(false);
@@ -5754,8 +5754,8 @@ impl Player {
// Legacy fallback: get the stream URL
log::info!("Player: Getting stream URL...");
let stream_url = client
.get_stream_url_with_fallback(track_id, quality)
let (stream_url, successful_request) = client
.get_stream_url_with_acquisition(track_id, quality)
.await
.map_err(|_| {
log::error!("Player: Failed to get stream URL (details omitted)");
@@ -5774,7 +5774,7 @@ impl Player {
let (source, writer) = self.download_buffered(&stream_url.url).await?;
if !self.is_current_play(gen) { return Ok(()); }
if !skip_cache {
Self::persist_stream(self.audio_cache.clone(), writer, track_id, qbz_cache::CacheQuality::from_resolved(quality, stream_url.format_id)).await?;
Self::persist_stream(self.audio_cache.clone(), writer, track_id, qbz_cache::CacheQuality::from_acquisition(quality, successful_request, stream_url.format_id)).await?;
}
if !self.is_current_play(gen) { return Ok(()); }
let (meta, duration) = disk_playback::source_metadata(&source)?;
@@ -5809,11 +5809,11 @@ impl Player {
Ok(info) => info,
Err(error) => {
log::warn!("[GAPLESS-STREAM] CMAF setup failed: {error}; trying buffered legacy source");
let url = client.get_stream_url_with_fallback(track_id, quality).await.map_err(|e| e.to_string())?;
let (url, successful_request) = client.get_stream_url_with_acquisition(track_id, quality).await.map_err(|e| e.to_string())?;
let (source, writer) = self.download_buffered(&url.url).await?;
if !self.is_current_play(gen) { return Err("gapless legacy fetch superseded".into()); }
let skip_cache = self.audio_settings.lock().map(|s| s.streaming_only).unwrap_or(false);
if !skip_cache { Self::persist_stream(self.audio_cache.clone(), writer, track_id, qbz_cache::CacheQuality::from_resolved(quality, url.format_id)).await?; }
if !skip_cache { Self::persist_stream(self.audio_cache.clone(), writer, track_id, qbz_cache::CacheQuality::from_acquisition(quality, successful_request, url.format_id)).await?; }
if !self.is_current_play(gen) { return Err("gapless legacy fetch superseded".into()); }
let (meta, duration_secs) = disk_playback::source_metadata(&source)?;
return self.tx.send(AudioCommand::PlayNextStreaming {
@@ -5877,7 +5877,7 @@ impl Player {
let content_key = cmaf_info.content_key;
let flac_header = cmaf_info.flac_header;
let n_segments = cmaf_info.n_segments;
let cache_quality = qbz_cache::CacheQuality::from_resolved(quality, cmaf_info.format_id);
let cache_quality = qbz_cache::CacheQuality::from_acquisition(quality, quality, cmaf_info.format_id);
let cache = self.audio_cache.clone();
let skip_cache = self.audio_settings.lock().map(|settings| settings.streaming_only).unwrap_or(false);
tokio::spawn(async move {
@@ -6015,7 +6015,7 @@ impl Player {
let (_cancel_tx, cancel_rx) = tokio::sync::watch::channel(false);
Self::cmaf_stream_segments(&info.url_template, info.n_segments, info.content_key,
info.flac_header, writer, track_id, self.audio_cache.clone(),
qbz_cache::CacheQuality::from_resolved(quality, info.format_id), false, size, cancel_rx).await?;
qbz_cache::CacheQuality::from_acquisition(quality, quality, info.format_id), false, size, cancel_rx).await?;
Ok::<(), String>(())
}.await;
let result = match cmaf_result {
@@ -6023,9 +6023,9 @@ impl Player {
Err(error) => {
log::warn!("[PREFETCH] CMAF failed for {track_id}: {error}; trying legacy");
async {
let url = client.get_stream_url_with_fallback(track_id, quality).await.map_err(|e| e.to_string())?;
let (url, successful_request) = client.get_stream_url_with_acquisition(track_id, quality).await.map_err(|e| e.to_string())?;
let (_source, writer) = self.download_buffered(&url.url).await?;
Self::persist_stream(self.audio_cache.clone(), writer, track_id, qbz_cache::CacheQuality::from_resolved(quality, url.format_id)).await
Self::persist_stream(self.audio_cache.clone(), writer, track_id, qbz_cache::CacheQuality::from_acquisition(quality, successful_request, url.format_id)).await
}.await
}
};
@@ -6196,12 +6196,12 @@ impl Player {
self.ensure_loudness_row(client, track_id)
);
let downloaded = match downloaded {
Ok((data, resolved)) => Some((data, qbz_cache::CacheQuality::from_resolved(quality, resolved.format_id))),
Ok((data, resolved)) => Some((data, qbz_cache::CacheQuality::from_acquisition(quality, quality, resolved.format_id))),
Err(e) => {
log::warn!("[GAPLESS] CMAF failed for track {track_id}: {e}, trying legacy");
match client.get_stream_url_with_fallback(track_id, quality).await {
Ok(stream_url) => match self.download_audio(&stream_url.url).await {
Ok(data) => Some((data, qbz_cache::CacheQuality::from_resolved(quality, stream_url.format_id))),
match client.get_stream_url_with_acquisition(track_id, quality).await {
Ok((stream_url, successful_request)) => match self.download_audio(&stream_url.url).await {
Ok(data) => Some((data, qbz_cache::CacheQuality::from_acquisition(quality, successful_request, stream_url.format_id))),
Err(e) => {
log::warn!("[GAPLESS] Legacy download failed for {track_id}: {e}");
None
@@ -6272,7 +6272,7 @@ impl Player {
let content_key = cmaf_info.content_key;
let flac_header = cmaf_info.flac_header;
let n_segments = cmaf_info.n_segments;
let cache_quality = qbz_cache::CacheQuality::from_resolved(quality, cmaf_info.format_id);
let cache_quality = qbz_cache::CacheQuality::from_acquisition(quality, quality, cmaf_info.format_id);
let cache = self.audio_cache.clone();
tokio::spawn(async move {
match Self::cmaf_stream_segments(
@@ -6361,7 +6361,7 @@ impl Player {
q.bit_depth
);
// Warm L1 so a subsequent local replay skips the network.
self.audio_cache.insert_with_quality(track_id, bytes.clone(), qbz_cache::CacheQuality::from_resolved(quality, q.format_id));
self.audio_cache.insert_with_quality(track_id, bytes.clone(), qbz_cache::CacheQuality::from_acquisition(quality, quality, q.format_id));
return Some(ExternalStreamAsset {
bytes,
content_type: "audio/flac".to_string(),
@@ -6377,8 +6377,8 @@ impl Player {
// Fallback: legacy stream URL + plain HTTP download. Quality and MIME
// come from the resolved StreamUrl (which carries the granted tier).
match client.get_stream_url_with_fallback(track_id, quality).await {
Ok(stream_url) => {
match client.get_stream_url_with_acquisition(track_id, quality).await {
Ok((stream_url, successful_request)) => {
let content_type =
external_content_type(&stream_url.mime_type, stream_url.format_id);
let q = StreamQualityInfo::from_raw(
@@ -6394,7 +6394,7 @@ impl Player {
q.format_id,
content_type
);
self.audio_cache.insert_with_quality(track_id, bytes.clone(), qbz_cache::CacheQuality::from_resolved(quality, q.format_id));
self.audio_cache.insert_with_quality(track_id, bytes.clone(), qbz_cache::CacheQuality::from_acquisition(quality, successful_request, q.format_id));
Some(ExternalStreamAsset {
bytes,
content_type,
@@ -7508,6 +7508,23 @@ mod tests {
assert!(cached_quality_below_requested(b"corrupt", Quality::UltraHiRes, highest));
}
#[test]
fn successful_request_reuses_lower_rate_master_but_not_explicit_fallback() {
for (rate, depth, format) in [(96_000, 24, 7), (44_100, 16, 6)] {
let meta = AudioMetadata { sample_rate: rate, channels: 2, bit_depth: Some(depth),
codec: symphonia::core::codecs::CODEC_TYPE_FLAC };
let acquired = qbz_cache::CacheQuality::from_acquisition(Quality::UltraHiRes, Quality::UltraHiRes, format);
assert!(!cached_metadata_below_requested(&meta, Quality::UltraHiRes, acquired));
let fallback = qbz_cache::CacheQuality::from_acquisition(Quality::UltraHiRes, Quality::from_id(format).unwrap(), format);
assert!(cached_metadata_below_requested(&meta, Quality::UltraHiRes, fallback));
let old = qbz_cache::CacheQuality::from_resolved(Quality::UltraHiRes, format);
assert!(cached_metadata_below_requested(&meta, Quality::UltraHiRes, old));
assert!(cached_quality_below_requested(b"corrupt", Quality::UltraHiRes, acquired));
let lossy = AudioMetadata { codec: symphonia::core::codecs::CODEC_TYPE_MP3, ..meta };
assert!(cached_metadata_below_requested(&lossy, Quality::UltraHiRes, acquired));
}
}
#[test]
fn buffering_transport_intent_is_scoped_to_current_play() {
let state = SharedState::new();
+93 -2
View File
@@ -2815,6 +2815,32 @@ impl QobuzClient {
track_id: u64,
preferred: Quality,
) -> Result<StreamUrl> {
self.get_stream_url_with_acquisition(track_id, preferred)
.await
.map(|(url, _)| url)
}
/// Also return the quality of the successful request. The response's
/// format_id describes the available file, not whether fallback occurred.
pub async fn get_stream_url_with_acquisition(
&self,
track_id: u64,
preferred: Quality,
) -> Result<(StreamUrl, Quality)> {
Self::resolve_stream_url_with_fallback(track_id, preferred, |quality| {
self.get_stream_url(track_id, quality)
}).await
}
async fn resolve_stream_url_with_fallback<F, Fut>(
track_id: u64,
preferred: Quality,
mut fetch: F,
) -> Result<(StreamUrl, Quality)>
where
F: FnMut(Quality) -> Fut,
Fut: std::future::Future<Output = Result<StreamUrl>>,
{
log::info!(
"Getting stream URL with fallback for track {}, preferred quality: {:?}",
track_id,
@@ -2827,13 +2853,13 @@ impl QobuzClient {
for quality in &qualities[start_idx..] {
log::info!("Trying quality: {:?}", quality);
match self.get_stream_url(track_id, *quality).await {
match fetch(*quality).await {
Ok(url) if !url.has_restrictions() => {
log::info!(
"Got stream URL for requested quality format_id={}",
quality.id()
);
return Ok(url);
return Ok((url, *quality));
}
Ok(_) => {
log::info!("Quality {:?} has restrictions, trying next", quality);
@@ -3579,6 +3605,71 @@ impl Default for QobuzClient {
mod tests {
use super::*;
fn available_master(format_id: u32) -> StreamUrl {
StreamUrl {
url: "https://cdn.example.test/audio".into(), format_id,
mime_type: "audio/flac".into(),
sampling_rate: if format_id == 6 { 44.1 } else { 96.0 },
bit_depth: Some(if format_id == 6 { 16 } else { 24 }),
track_id: 7, restrictions: Vec::new(), sample: false,
}
}
#[tokio::test]
async fn successful_request_keeps_available_format_without_extra_attempts() {
for format_id in [6, 7] {
let mut attempts = Vec::new();
let (url, actual) = QobuzClient::resolve_stream_url_with_fallback(
7, Quality::UltraHiRes, |quality| {
attempts.push(quality);
std::future::ready(Ok(available_master(format_id)))
},
).await.unwrap();
assert_eq!(attempts, vec![Quality::UltraHiRes]);
assert_eq!(actual, Quality::UltraHiRes);
assert_eq!(url.format_id, format_id);
}
}
#[tokio::test]
async fn successful_request_reports_actual_fallback_not_initial_preference() {
let mut attempts = Vec::new();
let (url, actual) = QobuzClient::resolve_stream_url_with_fallback(
7, Quality::UltraHiRes, |quality| {
attempts.push(quality);
let mut url = available_master(7);
if quality == Quality::UltraHiRes {
url.restrictions.push(qbz_models::StreamRestriction {
code: "FormatRestrictedByFormatAvailability".into(),
});
}
std::future::ready(Ok(url))
},
).await.unwrap();
assert_eq!(attempts, vec![Quality::UltraHiRes, Quality::HiRes]);
assert_eq!(actual, Quality::HiRes);
assert_eq!(url.format_id, 7);
}
#[tokio::test]
async fn successful_request_aborts_auth_failure_without_lower_quality_requests() {
for error in [ApiError::Forbidden("fixture".into()),
ApiError::ForbiddenCircuitOpen(30),
ApiError::AuthenticationError("fixture".into()), ApiError::InvalidAppSecret] {
let expected = std::mem::discriminant(&error);
let mut error = Some(error);
let mut attempts = Vec::new();
let result = QobuzClient::resolve_stream_url_with_fallback(
7, Quality::UltraHiRes, |quality| {
attempts.push(quality);
std::future::ready(Err(error.take().expect("must abort after first failure")))
},
).await;
assert_eq!(std::mem::discriminant(&result.unwrap_err()), expected);
assert_eq!(attempts, vec![Quality::UltraHiRes]);
}
}
#[test]
fn remote_body_diagnostic_never_echoes_payload() {
let marker = "jwt=secret&request_sig=signed-value";