From 71f8d0897064149eb48ba0af893288a58bf1656f Mon Sep 17 00:00:00 2001 From: vicrodh <103399283+vicrodh@users.noreply.github.com> Date: Thu, 20 Aug 2026 21:14:08 -0600 Subject: [PATCH] refactor(qt): hoist the quality badge into one classifier and teach it DSD MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `myqbz_builder_qt.rs` carried its own copy of the reference's `quality::badge` plus three helpers, and its own note called hoisting them out a named follow-up that "must not be done in a domain commit". This is that commit: they move VERBATIM into `quality_qt.rs`. The hoist is not tidying. Local Library never had those helpers, so it read a DSD row's nominal 1-bit depth and 2 822 400 Hz bit rate literally: the CD badge, and "1-bit / 2822.4 kHz" where MyQBZ printed DSD64 for the same file. Six surfaces disagreed with a seventh about the same track. `local_rows` gains a `detail_of` to pair with `tier_of`, and all SIX call sites are routed through the pair — the pairing is what stops the two halves of one badge from drifting again. `tier_of` keeps its extra "max" tier on purpose: `LocalLibraryView.qml:435` filters hi-res on that word and `QualityBadge.qml:66` folds it back to "hires". Measured before/after on the same album: CD + "1-bit / 2822.4 kHz" -> hi-res + "DSD64", on the grid, the rows, the album header and the playlists. cc --- crates/qbz-qt/src/library_qt.rs | 4 +- crates/qbz-qt/src/local_album_actions.rs | 5 +- crates/qbz-qt/src/local_playlist_qt.rs | 5 +- crates/qbz-qt/src/local_rows.rs | 26 ++++- crates/qbz-qt/src/main.rs | 1 + crates/qbz-qt/src/myqbz_builder_qt.rs | 65 ++---------- crates/qbz-qt/src/quality_qt.rs | 124 +++++++++++++++++++++++ 7 files changed, 163 insertions(+), 67 deletions(-) create mode 100644 crates/qbz-qt/src/quality_qt.rs diff --git a/crates/qbz-qt/src/library_qt.rs b/crates/qbz-qt/src/library_qt.rs index b57021efb..6ee17ea8b 100644 --- a/crates/qbz-qt/src/library_qt.rs +++ b/crates/qbz-qt/src/library_qt.rs @@ -924,7 +924,7 @@ fn all_local_feed_blocking() -> Vec { artist: a.artist, image_url: a.artwork_path.unwrap_or_default(), quality_tier: tier_of(&a.format, a.bit_depth, a.sample_rate).into(), - quality_detail: home_qt::quality_detail_from_parts(a.bit_depth, Some(a.sample_rate)), + quality_detail: crate::local_rows::detail_of(&a.format, a.bit_depth, a.sample_rate), bit_depth: a.bit_depth, sample_rate: Some(khz(a.sample_rate)), is_favorite: hearted("album", &a.id), @@ -1048,7 +1048,7 @@ fn all_local_feed_blocking() -> Vec { album_id: t.album_group_key, image_url: t.artwork_path.unwrap_or_default(), quality_tier: tier_of(&t.format, t.bit_depth, t.sample_rate).into(), - quality_detail: home_qt::quality_detail_from_parts(t.bit_depth, Some(t.sample_rate)), + quality_detail: crate::local_rows::detail_of(&t.format, t.bit_depth, t.sample_rate), bit_depth: t.bit_depth, sample_rate: Some(khz(t.sample_rate)), is_favorite: hearted("track", &id), diff --git a/crates/qbz-qt/src/local_album_actions.rs b/crates/qbz-qt/src/local_album_actions.rs index 4da2d79c7..306b63bf0 100644 --- a/crates/qbz-qt/src/local_album_actions.rs +++ b/crates/qbz-qt/src/local_album_actions.rs @@ -245,9 +245,10 @@ fn album_header(id: &str, tracks: &[LocalTrack]) -> AlbumRow { track_count: tracks.len() as u32, duration: total_duration(tracks.iter().map(|t| t.duration_secs).sum()), quality_tier: tier_of(&best.format, best.bit_depth, best.sample_rate).into(), - quality_detail: crate::home_qt::quality_detail_from_parts( + quality_detail: crate::local_rows::detail_of( + &best.format, best.bit_depth, - Some(best.sample_rate), + best.sample_rate, ), format: best.format.to_string(), art_key: album_key(id), diff --git a/crates/qbz-qt/src/local_playlist_qt.rs b/crates/qbz-qt/src/local_playlist_qt.rs index 68752a970..b76b737e5 100644 --- a/crates/qbz-qt/src/local_playlist_qt.rs +++ b/crates/qbz-qt/src/local_playlist_qt.rs @@ -650,9 +650,10 @@ pub(crate) fn row_to_display(item: &RowItem) -> (PlaylistTrackRow, Option String { /// Tier for the shared QualityBadge, mirroring its own derivation order /// (MP3 first, then max / hires / cd) so a local card and a Qobuz card can /// never disagree. +/// +/// DSD is answered before the depth match for the reason `quality_qt::badge` +/// spells out: a DSD row arrives as depth 1 / rate 2 822 400, so `Some(_)` +/// caught it and every DSD file in the library wore the CD mark. The extra +/// `"max"` tier below is NOT folded into `hires` here — `LocalLibraryView +/// .qml:435` filters hi-res on the word and `QualityBadge.qml:66` folds it. pub fn tier_of(format: &AudioFormat, bit_depth: Option, sample_rate_hz: f64) -> &'static str { if matches!(format, AudioFormat::Mp3) { return "mp3"; } + if matches!(format, AudioFormat::Dsd) { + return "hires"; + } let khz = if sample_rate_hz >= 1000.0 { sample_rate_hz / 1000.0 } else { @@ -214,6 +223,19 @@ pub fn tier_of(format: &AudioFormat, bit_depth: Option, sample_rate_hz: f64 } } +/// The detail line that rides under `tier_of`'s tier. Its ONLY job beyond +/// `home_qt::quality_detail_from_parts` is the DSD arm: that helper takes no +/// format, so it read a DSD row's nominal 1-bit depth and 2 822 400 Hz bit +/// rate literally and printed "1-bit / 2822.4 kHz" where the rest of the app +/// (and MyQBZ, via `quality_qt::badge`) prints `DSD64`. Every caller of +/// `tier_of` pairs with this so the two halves of one badge cannot disagree. +pub fn detail_of(format: &AudioFormat, bit_depth: Option, sample_rate_hz: f64) -> String { + if matches!(format, AudioFormat::Dsd) { + return crate::quality_qt::dsd_multiple_label(Some(sample_rate_hz)); + } + crate::home_qt::quality_detail_from_parts(bit_depth, Some(sample_rate_hz)) +} + pub fn basename(path: &str) -> String { std::path::Path::new(path) .file_name() @@ -328,7 +350,7 @@ pub fn map_album(a: LocalAlbum, art: &mut HashMap) - .unwrap_or(0); AlbumRow { quality_tier: tier_of(&a.format, a.bit_depth, a.sample_rate).into(), - quality_detail: crate::home_qt::quality_detail_from_parts(a.bit_depth, Some(a.sample_rate)), + quality_detail: detail_of(&a.format, a.bit_depth, a.sample_rate), format: a.format.to_string(), year: a.year.map(|y| y.to_string()).unwrap_or_default(), duration: total_duration(a.total_duration_secs), @@ -363,7 +385,7 @@ pub fn map_track(t: &LocalTrack, art: &mut HashMap) disc: t.disc_number.unwrap_or(1), duration: mmss(t.duration_secs), quality_tier: tier_of(&t.format, t.bit_depth, t.sample_rate).into(), - quality_detail: crate::home_qt::quality_detail_from_parts(t.bit_depth, Some(t.sample_rate)), + quality_detail: detail_of(&t.format, t.bit_depth, t.sample_rate), format: t.format.to_string(), year: t.year.map(|y| y.to_string()).unwrap_or_default(), art_key: key, diff --git a/crates/qbz-qt/src/main.rs b/crates/qbz-qt/src/main.rs index e84afc8a2..a533d3a61 100644 --- a/crates/qbz-qt/src/main.rs +++ b/crates/qbz-qt/src/main.rs @@ -242,6 +242,7 @@ mod cast_qt; mod label_qt; mod now_playing; mod output_labels; +mod quality_qt; mod quality_state; mod offline_fwd; // Offline cache (downloads tier): state activation on login + the action diff --git a/crates/qbz-qt/src/myqbz_builder_qt.rs b/crates/qbz-qt/src/myqbz_builder_qt.rs index 5a4767ac3..97e848edf 100644 --- a/crates/qbz-qt/src/myqbz_builder_qt.rs +++ b/crates/qbz-qt/src/myqbz_builder_qt.rs @@ -347,65 +347,12 @@ pub(crate) fn quality_score( // ─────────────────────────── quality badge (tier) ────────────────────────── // -// The reference calls `crate::quality::badge(format, bit, rate)`. This port has -// no format-string-aware tier helper: `local_rows::tier_of` takes an -// `AudioFormat` enum (and adds a "max" tier `QualityBadgeFull.qml:30` does not -// know), and `home_qt::quality_tier_from_depth` ignores the container entirely. -// So the tier arm below is a local 1:1 of `qbz/src/quality.rs:63,90` and the -// DETAIL comes from the shared `home_qt::quality_detail_from_parts` (which the -// spec pins as byte-identical to `quality.rs fn detail`). -// HANDOFF: hoisting this into a shared `quality_qt.rs` is a named follow-up — -// it must not be done in a domain commit. - -fn is_lossless_format(format: &str) -> bool { - matches!( - format.trim().to_ascii_lowercase().as_str(), - "flac" | "wav" | "wave" | "aiff" | "aif" | "alac" | "ape" | "dsd" | "dsf" | "dff" - ) -} - -fn is_dsd_format(format: &str) -> bool { - matches!( - format.trim().to_ascii_lowercase().as_str(), - "dsd" | "dsf" | "dff" - ) -} - -/// "DSD64" / "DSD128" / … from the DSD bit rate (Hz or kHz). -fn dsd_multiple_label(sample_rate: Option) -> String { - let hz = match sample_rate { - Some(r) if r >= 1_000_000.0 => r, - Some(r) if r >= 1_000.0 => r * 1000.0, - _ => return "DSD".to_string(), - }; - format!("DSD{}", ((hz / 2_822_400.0).round() as u32).saturating_mul(64)) -} - -/// `(tier, detail)` for `controls/QualityBadgeFull.qml` — tiers are exactly -/// `hires | cd | mp3 | lossless | ""` (`QualityBadgeFull.qml:30`). -fn badge(format: &str, bit_depth: Option, sample_rate: Option) -> (String, String) { - if is_dsd_format(format) { - return ("hires".to_string(), dsd_multiple_label(sample_rate)); - } - let tier = if format.trim().eq_ignore_ascii_case("mp3") { - "mp3" - } else { - match bit_depth { - Some(b) if b >= 24 => "hires", - Some(_) => "cd", - None if is_lossless_format(format) => "lossless", - None => "", - } - }; - match tier { - "" | "mp3" => (tier.to_string(), String::new()), - "lossless" => (tier.to_string(), format.trim().to_uppercase()), - _ => ( - tier.to_string(), - crate::home_qt::quality_detail_from_parts(bit_depth, sample_rate), - ), - } -} +// The reference calls `crate::quality::badge(format, bit, rate)`. So does this +// now: the four helpers that used to live inline here were hoisted VERBATIM +// into `crate::quality_qt` — the named follow-up this block's own note asked +// for, done in its own commit as that note required. Local Library badged DSD +// as CD for want of exactly these. +use crate::quality_qt::badge; // ──────────────────────────── buildGroups ───────────────────────────────── diff --git a/crates/qbz-qt/src/quality_qt.rs b/crates/qbz-qt/src/quality_qt.rs new file mode 100644 index 000000000..a5b1229d4 --- /dev/null +++ b/crates/qbz-qt/src/quality_qt.rs @@ -0,0 +1,124 @@ +//! The ONE quality-badge classifier for this frontend — `(tier, detail)` from +//! a container/codec name plus a (possibly unknown) bit depth and rate. +//! +//! Port of `crates/qbz/src/quality.rs` (`is_lossless_format`, `is_dsd_format`, +//! `dsd_multiple_label`, `tier`, `badge`). It lived inline in +//! `myqbz_builder_qt.rs`, whose own note called hoisting it here a named +//! follow-up that "must not be done in a domain commit" — this is that commit. +//! +//! Why one classifier matters: the earlier per-surface duplication is what +//! produced "96 kHz" on one surface and "96000 kHz" on another, and — until +//! this file existed — a DSD track badged as CD on every Local Library +//! surface while MyQBZ badged the same file `DSD64`. +//! +//! Tiers are exactly the five names the QML marks draw: `hires | cd | mp3 | +//! lossless | ""` (`controls/QualityBadgeFull.qml:28`). `local_rows::tier_of` +//! keeps its own extra `"max"` tier on purpose — `LocalLibraryView.qml:435` +//! filters hi-res on it and `QualityBadge.qml:66` folds it back to `hires`. + +/// Lossless container/codec formats — the file IS lossless even when its exact +/// bit depth / sample rate isn't known yet (e.g. an un-hydrated Plex track). +pub(crate) fn is_lossless_format(format: &str) -> bool { + matches!( + format.trim().to_ascii_lowercase().as_str(), + "flac" | "wav" | "wave" | "aiff" | "aif" | "alac" | "ape" | "dsd" | "dsf" | "dff" + ) +} + +/// DSD container/format names. `AudioFormat::Dsd` prints `"DSD"` +/// (`qbz-library/src/models.rs:33`); the extensions cover the string-typed +/// callers. +pub(crate) fn is_dsd_format(format: &str) -> bool { + matches!( + format.trim().to_ascii_lowercase().as_str(), + "dsd" | "dsf" | "dff" + ) +} + +/// "DSD64" / "DSD128" / … from the DSD bit rate. Accepts Hz (2 822 400) or +/// kHz (2 822.4) — some surfaces normalize the stored rate to kHz upstream. +pub(crate) fn dsd_multiple_label(sample_rate: Option) -> String { + let hz = match sample_rate { + Some(r) if r >= 1_000_000.0 => r, + Some(r) if r >= 1_000.0 => r * 1000.0, + _ => return "DSD".to_string(), + }; + format!("DSD{}", ((hz / 2_822_400.0).round() as u32).saturating_mul(64)) +} + +/// `(tier, detail)` for `controls/QualityBadgeFull.qml`. +/// +/// DSD is answered FIRST and never reaches the depth match: its nominal depth +/// is 1 bit and its `sample_rate` is the DSD bit rate, so the generic detail +/// would read "1-bit / 2822.4 kHz" and the generic tier would read `"cd"`. +pub(crate) fn badge( + format: &str, + bit_depth: Option, + sample_rate: Option, +) -> (String, String) { + if is_dsd_format(format) { + return ("hires".to_string(), dsd_multiple_label(sample_rate)); + } + let tier = if format.trim().eq_ignore_ascii_case("mp3") { + "mp3" + } else { + match bit_depth { + Some(b) if b >= 24 => "hires", + Some(_) => "cd", + None if is_lossless_format(format) => "lossless", + None => "", + } + }; + match tier { + "" | "mp3" => (tier.to_string(), String::new()), + "lossless" => (tier.to_string(), format.trim().to_uppercase()), + _ => ( + tier.to_string(), + crate::home_qt::quality_detail_from_parts(bit_depth, sample_rate), + ), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn dsd_is_hires_and_labelled_by_its_multiple() { + // The bug this module exists for: `AudioFormat::Dsd` reaches the badge + // as depth 1 / rate 2 822 400, which the generic arms read as CD. + assert_eq!( + badge("DSD", Some(1), Some(2_822_400.0)), + ("hires".to_string(), "DSD64".to_string()) + ); + assert_eq!( + badge("dsf", Some(1), Some(5_644_800.0)), + ("hires".to_string(), "DSD128".to_string()) + ); + } + + #[test] + fn a_rate_already_in_khz_is_not_re_divided() { + assert_eq!(dsd_multiple_label(Some(2_822.4)), "DSD64"); + assert_eq!(dsd_multiple_label(None), "DSD"); + } + + #[test] + fn the_non_dsd_arms_are_unchanged() { + assert_eq!(badge("MP3", Some(16), Some(44_100.0)).0, "mp3"); + assert_eq!( + badge("FLAC", Some(24), Some(96_000.0)), + ("hires".to_string(), "24-bit / 96 kHz".to_string()) + ); + assert_eq!( + badge("FLAC", Some(16), Some(44_100.0)), + ("cd".to_string(), "16-bit / 44.1 kHz".to_string()) + ); + // Un-hydrated: depth unknown but the container is lossless. + assert_eq!( + badge("FLAC", None, None), + ("lossless".to_string(), "FLAC".to_string()) + ); + assert_eq!(badge("Unknown", None, None), (String::new(), String::new())); + } +}