From 8776ebe82d100c1bed877007c2945ff7c3ed4305 Mon Sep 17 00:00:00 2001 From: vicrodh <103399283+vicrodh@users.noreply.github.com> Date: Sat, 1 Aug 2026 21:36:07 -0600 Subject: [PATCH] fix(qt): carry the real track data into every queue, and shuffle for real MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defect classes the owner found by playing music, both of them the port diverging from a reference that gets it right. QUEUE TRACK FIDELITY. The reference has ONE builder, make_queue_track (playback.rs:2426), and it takes the raw Track, so bit_depth and sample_rate come straight off the API model. This port forked that into ten builders and five of them mapped from a DISPLAY ROW instead — a row that carries qualityTier / qualityDetail / qualityLabel, the already-formatted strings, and no raw numbers. Those five hardcoded bit_depth: None, sample_rate: None: playlist_qt, artist_qt, label_qt, library_qt and playback_qt's feed_queue_track. The consequence reached the user as a broken NPB AudioStamp: the track edge seeds quality_state from those same numbers, so starting playback from a playlist, an artist page, a label page or a Library-feed row left TRACK_MAX at zero and the badge could not render a complete bit-depth / sample-rate line. Starting from an album was perfect, which is what made it look intermittent — the album path was one of the five that had always built from the raw Track. The rows now carry the numbers alongside the strings, because the producer derives the strings FROM those numbers and was discarding them; parsing "24-bit / 96 kHz" back into integers would have been a second source of truth. The same builders were dropping artwork_url, which is why the queue sidebar had no covers for playlist-sourced or local rows. Also fixed here, found while tracing: local_playlist_qt::play shipped context: None, so set_queue_stamped fell through to derive_context, which answers None for a many-album playlist. The song card then used its per-track album fallback and pointed at a local album_group_key instead of the playlist. Pre-existing for the header Play, but a guard added last round routed the row click through the same path, which turned it into a regression there. SHUFFLE. set_shuffle(true) only randomises what comes NEXT, so four entry points raised the mode and then played the list's #1 track: the album header, the playlist header, the local-playlist header and every local play_rows call. The one track the user actually hears on pressing Shuffle was deterministic. All four now mix their own list with the xorshift Fisher-Yates the port already uses in play_track_list_in and start at the top of the mixed order. The mode stays raised so playback continues shuffled past the list. The owner traced the flag-and-play-from-the-top shape to a very early Tauri experiment; it is not the behaviour anywhere. Not addressed, going to a separate pass: a Qobuz playlist carrying local or Plex sidecar rows. The reference routes those through the local snapshot (playback.rs:3488 is_mixed); this port has no sidecar concept on playlist_qt at all, so such a row is still typed as qobuz. Same defect class, one layer up. cc --- crates/qbz-qt/qml/shell/QueuePanel.qml | 39 ++++-- crates/qbz-qt/qml/views/ArtistView.qml | 12 +- crates/qbz-qt/src/album_qt.rs | 20 +++ crates/qbz-qt/src/artist_qt.rs | 13 +- crates/qbz-qt/src/foryou_qt.rs | 3 + crates/qbz-qt/src/label_qt.rs | 13 +- crates/qbz-qt/src/library_qt.rs | 31 ++++- crates/qbz-qt/src/local_albums.rs | 8 +- crates/qbz-qt/src/local_bulk.rs | 11 ++ crates/qbz-qt/src/local_playback.rs | 165 ++++++++++++++++++++++--- crates/qbz-qt/src/local_playlist_qt.rs | 66 ++++++++-- crates/qbz-qt/src/local_tree.rs | 41 +++++- crates/qbz-qt/src/main.rs | 24 ++++ crates/qbz-qt/src/playback_qt.rs | 68 ++++++++-- crates/qbz-qt/src/playlist_qt.rs | 45 ++++++- 15 files changed, 502 insertions(+), 57 deletions(-) diff --git a/crates/qbz-qt/qml/shell/QueuePanel.qml b/crates/qbz-qt/qml/shell/QueuePanel.qml index fe4e08df5..3c94a7b04 100644 --- a/crates/qbz-qt/qml/shell/QueuePanel.qml +++ b/crates/qbz-qt/qml/shell/QueuePanel.qml @@ -54,13 +54,11 @@ Rectangle { if (!visible) return QbzQueue.queuePanelOpened() - // AND re-dispatch the covers. `onDocChanged` alone is not enough: the - // panel is `visible`-gated rather than mounted on demand, and a queue - // that was published while it was hidden leaves the covers never - // requested — measured live, a local track's now-playing thumbnail - // stayed blank with NO artwork dispatch after the publish. The - // publish dedup means `queuePanelOpened` above often changes nothing, - // so it cannot be relied on to trigger the doc change either. + // AND re-dispatch the covers: the publish dedup means + // `queuePanelOpened` above often changes nothing, so it cannot be + // relied on to trigger the doc change either. (This handler runs + // OUTSIDE the docChanged storm, which is why it used to be the only + // dispatch that asked for the right urls — see `dispatchCovers`.) dispatchCovers() } @@ -163,12 +161,33 @@ Rectangle { } Component.onCompleted: dispatchCovers() onDocChanged: dispatchCovers() + // Collect the covers of the document CURRENTLY on the property and ask the + // shared pipeline for them. + // + // It re-derives current/upcoming/history from `root.doc` instead of reading + // the `currentRow` / `upcoming` / `historyRows` bindings, and that is the + // whole fix: called from `onDocChanged`, those three derived properties are + // still holding the PREVIOUS document. Measured on this Qt build (three + // publishes, one per event-loop turn): reading the derived properties in + // the handler dispatched [], then A's urls on B's publish, then B's on C's + // — always exactly one publish behind, so the row on screen NEVER had its + // cover requested. Re-reading `root.doc` in the same handler dispatched + // A, B, C in step. + // + // It is also why the defect looked source-dependent: an ALBUM queue has one + // cover for every row, so asking one publish late asks for the same url and + // is indistinguishable from correct. A PLAYLIST queue (a different album per + // row) and a mixed LOCAL queue ask for the previous row's cover forever. function dispatchCovers() { + var d = root.doc + var cur = d.current || null + var up = d.upcoming || [] + var hist = d.history || [] var urls = [] - if (currentRow && currentRow.artUrl) urls.push(currentRow.artUrl) + if (cur && cur.artUrl) urls.push(cur.artUrl) var i - for (i = 0; i < upcoming.length; i++) if (upcoming[i].artUrl) urls.push(upcoming[i].artUrl) - for (i = 0; i < historyRows.length; i++) if (historyRows[i].artUrl) urls.push(historyRows[i].artUrl) + for (i = 0; i < up.length; i++) if (up[i].artUrl) urls.push(up[i].artUrl) + for (i = 0; i < hist.length; i++) if (hist[i].artUrl) urls.push(hist[i].artUrl) if (urls.length > 0) { QbzShell.sidebarArtworkWindow(JSON.stringify(urls)) root.artPulse = true diff --git a/crates/qbz-qt/qml/views/ArtistView.qml b/crates/qbz-qt/qml/views/ArtistView.qml index 0ac272190..1ecd89f0d 100644 --- a/crates/qbz-qt/qml/views/ArtistView.qml +++ b/crates/qbz-qt/qml/views/ArtistView.qml @@ -476,7 +476,17 @@ Rectangle { for (i = 0; i < rawPlaylists.length; i++) if (rawPlaylists[i].artUrl) urls.push(rawPlaylists[i].artUrl) // Magazine story thumbnails ride the same pipeline (arc-cdn URLs). - for (i = 0; i < stories.length; i++) if (stories[i].artUrl) urls.push(stories[i].artUrl) + // Off `artist.stories`, NOT the derived `stories` property (:110): this + // function is called from `onArtistChanged`, and a derived binding still + // holds the PREVIOUS document inside its source's change handler on this + // Qt build (measured — the same race QueuePanel.dispatchCovers and + // AlbumView's `onHeaderChanged` exist for). The stories land in a LATER + // publish than the page itself, so a stale read asked for the previous + // pass's thumbnails and the story cards stayed empty frames whenever the + // stories pass was the last publish. + var rawStories = artist.stories || [] + for (i = 0; i < rawStories.length; i++) + if (rawStories[i].artUrl) urls.push(rawStories[i].artUrl) dispatchArtwork(urls) } diff --git a/crates/qbz-qt/src/album_qt.rs b/crates/qbz-qt/src/album_qt.rs index 8e030ef9e..ac49f74c4 100644 --- a/crates/qbz-qt/src/album_qt.rs +++ b/crates/qbz-qt/src/album_qt.rs @@ -60,6 +60,24 @@ pub struct TrackRow { pub quality_tier: String, #[serde(rename = "qualityDetail")] pub quality_detail: String, + /// RAW catalog max bit depth / sample rate (kHz) — the SAME two numbers + /// `quality_detail` above is derived from. + /// + /// THE CONTRACT (reference: `crates/qbz/src/playback.rs:2426` + /// `make_queue_track`, which fills `bit_depth: track.maximum_bit_depth` / + /// `sample_rate: track.maximum_sampling_rate` straight off the API model): + /// a queue track must carry the numbers, never only the formatted string. + /// The row builders here map a DISPLAY row into a `QueueTrack` + /// (`artist_qt::track_row_to_queue`, `label_qt::track_row_to_queue`), so + /// without these fields they physically could not fill them and hardcoded + /// `None` — which zeroes `quality_state`'s `TRACK_MAX_*` seed and leaves + /// the NPB AudioStamp with a tier and no detail line. Re-parsing + /// `quality_detail` back into numbers would be a second source of truth + /// (and lossy); the producer has the numbers in hand, so it passes them. + #[serde(rename = "bitDepth", skip_serializing_if = "Option::is_none")] + pub bit_depth: Option, + #[serde(rename = "sampleRate", skip_serializing_if = "Option::is_none")] + pub sample_rate: Option, pub explicit: bool, pub disc: u32, /// Work-section header text ("" = none; album view only). @@ -464,6 +482,8 @@ fn map_track(track: &Track) -> TrackRow { track.maximum_bit_depth, track.maximum_sampling_rate, ), + bit_depth: track.maximum_bit_depth, + sample_rate: track.maximum_sampling_rate, explicit: track.parental_warning, disc: track.media_number.unwrap_or(1), work_header: work, diff --git a/crates/qbz-qt/src/artist_qt.rs b/crates/qbz-qt/src/artist_qt.rs index e38aa5b0d..558ac802c 100644 --- a/crates/qbz-qt/src/artist_qt.rs +++ b/crates/qbz-qt/src/artist_qt.rs @@ -325,6 +325,11 @@ fn map_track(index: usize, track: PageArtistTrack) -> TrackRow { duration: mmss(track.duration.unwrap_or(0)), quality_tier: home_qt::quality_tier_from_depth(bit_depth).to_string(), quality_detail: home_qt::quality_detail_from_parts(bit_depth, sample_rate), + // The RAW numbers ride with the row (see `TrackRow::bit_depth`) so + // `track_row_to_queue` can fill the queue entry the way the reference's + // `make_top_track_queue` (playback.rs:3188) fills it off `audio_info`. + bit_depth, + sample_rate, explicit: track.parental_warning.unwrap_or(false), disc: 1, work_header: String::new(), @@ -593,9 +598,13 @@ fn track_row_to_queue(row: &TrackRow) -> QueueTrack { } else { Some(row.artwork_url.clone()) }, + // playback.rs `make_top_track_queue` (:3188): the CATALOG max travels + // with the queue track. `None` here zeroed `quality_state`'s + // `TRACK_MAX_*` seed, so the NPB AudioStamp drew a bare tier with no + // "24-bit / 96 kHz" line on every artist Popular-Tracks play. hires: row.quality_tier == "hires", - bit_depth: None, - sample_rate: None, + bit_depth: row.bit_depth, + sample_rate: row.sample_rate, is_local: false, album_id: if row.album_id.is_empty() { None diff --git a/crates/qbz-qt/src/foryou_qt.rs b/crates/qbz-qt/src/foryou_qt.rs index 946d7c918..b4f1ab8f5 100644 --- a/crates/qbz-qt/src/foryou_qt.rs +++ b/crates/qbz-qt/src/foryou_qt.rs @@ -987,6 +987,9 @@ fn to_row(track: &Track) -> TrackRow { duration: mmss(track.duration), quality_tier: crate::home_qt::quality_tier_from_depth(bit_depth).to_string(), quality_detail: crate::home_qt::quality_detail_from_parts(bit_depth, sample_rate), + // See `TrackRow::bit_depth` — the raw numbers ride with the row. + bit_depth, + sample_rate, explicit: track.parental_warning, artwork_url: track .album diff --git a/crates/qbz-qt/src/label_qt.rs b/crates/qbz-qt/src/label_qt.rs index 04fc1cd33..374a64c01 100644 --- a/crates/qbz-qt/src/label_qt.rs +++ b/crates/qbz-qt/src/label_qt.rs @@ -1281,6 +1281,11 @@ fn parse_top_track(index: usize, raw: &Value) -> TrackRow { duration: mmss(duration as u32), quality_tier: crate::home_qt::quality_tier_from_depth(bit_depth).to_string(), quality_detail: crate::home_qt::quality_detail_from_parts(bit_depth, sample_rate), + // Same contract as the album / artist rows (`TrackRow::bit_depth`): + // the RAW numbers ride with the row so `track_row_to_queue` can fill + // the queue entry instead of hardcoding `None`. + bit_depth, + sample_rate, explicit: raw .get("parental_warning") .and_then(|v| v.as_bool()) @@ -1399,9 +1404,13 @@ fn track_row_to_queue(row: &TrackRow) -> QueueTrack { } else { Some(row.artwork_url.clone()) }, + // playback.rs `make_queue_track` (:2426): the CATALOG max travels with + // the queue track. `None` here zeroed `quality_state`'s `TRACK_MAX_*` + // seed, so a label Popular-Tracks play drew a bare tier on the NPB + // AudioStamp with no "24-bit / 96 kHz" line. hires: row.quality_tier == "hires", - bit_depth: None, - sample_rate: None, + bit_depth: row.bit_depth, + sample_rate: row.sample_rate, is_local: false, album_id: if row.album_id.is_empty() { None diff --git a/crates/qbz-qt/src/library_qt.rs b/crates/qbz-qt/src/library_qt.rs index 9f47eeef9..ae789cc05 100644 --- a/crates/qbz-qt/src/library_qt.rs +++ b/crates/qbz-qt/src/library_qt.rs @@ -70,6 +70,19 @@ pub struct FeedItem { pub quality_tier: String, #[serde(rename = "qualityDetail")] pub quality_detail: String, + /// RAW catalog max bit depth / sample rate (kHz) — the SAME two numbers + /// `quality_detail` above is derived from. + /// + /// THE CONTRACT (reference: `crates/qbz/src/playback.rs:2426` + /// `make_queue_track`): a queue track carries the NUMBERS. Both feed -> + /// queue builders (`feed_track_to_queue` below and + /// `playback_qt::feed_queue_track`) map THIS row into a `QueueTrack`, so + /// without the fields they hardcoded `None` and every Library-feed track + /// play left the NPB AudioStamp with a tier and no detail line. + #[serde(rename = "bitDepth", skip_serializing_if = "Option::is_none")] + pub bit_depth: Option, + #[serde(rename = "sampleRate", skip_serializing_if = "Option::is_none")] + pub sample_rate: Option, #[serde(rename = "isFavorite")] pub is_favorite: bool, pub genre: String, @@ -338,6 +351,10 @@ fn map_track(track: Track) -> FeedItem { track.maximum_bit_depth, track.maximum_sampling_rate, ), + // See `FeedItem::bit_depth` — the raw catalog numbers ride with the + // row so the two feed -> queue builders can hand them to the queue. + bit_depth: track.maximum_bit_depth, + sample_rate: track.maximum_sampling_rate, explicit: track.parental_warning, image_url: artwork_url, is_favorite: true, @@ -805,6 +822,12 @@ async fn fetch_purchases( album_id: aid, image_url: img, quality_tier: tier.into(), + // See `FeedItem::bit_depth` — a purchased TRACK row is a queue + // source too (`feed_track_to_queue` / `playback_qt:: + // feed_queue_track` look this row up by id), so it carries the + // raw catalog numbers like every other track producer. + bit_depth: t.maximum_bit_depth, + sample_rate: t.maximum_sampling_rate, id: t.id.to_string(), title: t.title, ..Default::default() @@ -1301,9 +1324,13 @@ pub(crate) fn feed_track_to_queue(item: &FeedItem) -> Option { } else { Some(item.image_url.clone()) }, + // playback.rs `make_queue_track` (:2426): the CATALOG max travels with + // the queue track. `None` here zeroed `quality_state`'s `TRACK_MAX_*` + // seed, so a Library track play drew a bare tier on the NPB AudioStamp + // with no "24-bit / 96 kHz" line. hires: item.quality_tier == "hires", - bit_depth: None, - sample_rate: None, + bit_depth: item.bit_depth, + sample_rate: item.sample_rate, is_local: false, album_id: if item.album_id.is_empty() { None diff --git a/crates/qbz-qt/src/local_albums.rs b/crates/qbz-qt/src/local_albums.rs index 7e94f12ce..72179df02 100644 --- a/crates/qbz-qt/src/local_albums.rs +++ b/crates/qbz-qt/src/local_albums.rs @@ -339,10 +339,16 @@ pub fn fetch_album_tracks_blocking(id: &str) -> Vec { /// `local_album_actions`, which is also what the version picker and the /// per-disc menu read back. pub fn load_album_detail_blocking(id: &str) -> Option { - let tracks = fetch_album_tracks_blocking(id); + let mut tracks = fetch_album_tracks_blocking(id); if tracks.is_empty() { return None; } + // Backfill covers from cover.jpg/folder.jpg on disk (the DB may not have an + // artwork_path even when a cover sits in the folder) — the reference does + // exactly this here, `local_library.rs:1826`. It is what makes the detail + // rows AND everything that later reads `detail_raw` (the per-disc menu, the + // bulk bar, `find_track_blocking`) carry the cover the folder has. + crate::local_playback::fill_missing_covers(&mut tracks); crate::local_album_actions::open_versions(id, tracks) } diff --git a/crates/qbz-qt/src/local_bulk.rs b/crates/qbz-qt/src/local_bulk.rs index 10618eb9a..1cc70312e 100644 --- a/crates/qbz-qt/src/local_bulk.rs +++ b/crates/qbz-qt/src/local_bulk.rs @@ -257,6 +257,17 @@ async fn apply(rows: Vec, action: &str) -> bool { /// (Slint local_bulk.rs:261-269 is likewise unhooked). async fn enqueue_rows(rows: Vec, mode: &str) { let runtime = crate::app(); + // Same folder-cover backfill the single-row path runs + // (`local_playback::enqueue`): a bulk-queued row must reach the queue with + // the cover its folder has, not a blank thumbnail. Blocking fs, so it goes + // through spawn_blocking like every other caller. + let rows = tokio::task::spawn_blocking(move || { + let mut rows = rows; + crate::local_playback::fill_missing_covers(&mut rows); + rows + }) + .await + .unwrap_or_default(); let queue: Vec = rows.iter().map(local_queue_track).collect(); log::info!("[qbz-qt] local bulk {mode}: {} track(s)", queue.len()); match mode { diff --git a/crates/qbz-qt/src/local_playback.rs b/crates/qbz-qt/src/local_playback.rs index 1db4e6def..ff6dc16c0 100644 --- a/crates/qbz-qt/src/local_playback.rs +++ b/crates/qbz-qt/src/local_playback.rs @@ -24,6 +24,112 @@ use crate::local_state::{state, with_db}; type Runtime = Arc>; +// --------------------------------------------------------------------------- +// On-disk cover backfill (playback.rs `fill_missing_covers` + +// local_library.rs `find_folder_cover`, both 1:1) +// --------------------------------------------------------------------------- + +/// The reference's robust on-disk cover lookup for one folder +/// (`local_library.rs:3110 find_folder_cover`): a known stem first, then a +/// file named after the folder, then any image at all. Stems and extensions +/// are the reference's list verbatim — do not prune it, the order IS the +/// preference. +pub(crate) fn find_folder_cover(folder: &Path) -> Option { + const STEMS: &[&str] = &[ + "cover", + "folder", + "front", + "art", + "album", + "albumart", + "albumartsmall", + "thumb", + "artwork", + "scan", + "booklet", + "title", + ]; + const EXTS: &[&str] = &["jpg", "jpeg", "png", "webp", "bmp", "gif", "tif", "tiff"]; + let is_img = |p: &Path| { + p.extension() + .and_then(|e| e.to_str()) + .map(|e| EXTS.iter().any(|x| x.eq_ignore_ascii_case(e))) + .unwrap_or(false) + }; + let mut entries: Vec = std::fs::read_dir(folder) + .ok()? + .flatten() + .map(|e| e.path()) + .filter(|p| p.is_file() && is_img(p)) + .collect(); + if entries.is_empty() { + return None; + } + entries.sort(); + let stem_lower = |p: &Path| { + p.file_stem() + .and_then(|s| s.to_str()) + .map(|s| s.to_lowercase()) + .unwrap_or_default() + }; + let folder_name = folder + .file_name() + .and_then(|s| s.to_str()) + .unwrap_or("") + .to_lowercase(); + let by_stem = entries + .iter() + .find(|p| STEMS.contains(&stem_lower(p).as_str())); + let by_name = entries + .iter() + .find(|p| !folder_name.is_empty() && stem_lower(p) == folder_name); + by_stem + .or(by_name) + .cloned() + .or_else(|| entries.into_iter().next()) + .map(|p| p.to_string_lossy().into_owned()) +} + +/// Fill `artwork_path` for the rows that lack one, from a cover image sitting +/// in the track's folder — the port of `playback.rs:1490 fill_missing_covers`, +/// which the reference runs on EVERY local play/enqueue path before it maps +/// rows to `QueueTrack`s (playback.rs:1094/1322/1349/1382, local_library.rs:1826, +/// main.rs:10636, offline_favorites.rs:128). This port had none of it, so a +/// library row whose `artwork_path` is NULL — the offline cache writes +/// `cover.jpg` next to the file without always backfilling the index, and a +/// ripped folder often carries the art only on disk — reached the queue with +/// `artwork_url: None` and the panel had nothing to request. +/// +/// BLOCKING (one `read_dir` per distinct folder, memoized), so every caller +/// runs it inside `spawn_blocking`: a NAS-backed library would otherwise stall +/// a tokio worker. +pub(crate) fn fill_missing_covers(tracks: &mut [LocalTrack]) { + use std::collections::HashMap; + let mut memo: HashMap> = HashMap::new(); + for t in tracks.iter_mut() { + if t.artwork_path.as_deref().is_some_and(|s| !s.is_empty()) { + continue; + } + let p = Path::new(&t.file_path); + let folder = if p.is_dir() { + p.to_path_buf() + } else { + match p.parent() { + Some(d) => d.to_path_buf(), + None => continue, + } + }; + let key = folder.to_string_lossy().into_owned(); + let cover = memo + .entry(key) + .or_insert_with(|| find_folder_cover(&folder)) + .clone(); + if cover.is_some() { + t.artwork_path = cover; + } + } +} + // --------------------------------------------------------------------------- // Queue mapping (playback.rs `local_queue_track` 1:1, all three sources) // --------------------------------------------------------------------------- @@ -273,13 +379,35 @@ async fn play_rows(runtime: &Runtime, tracks: Vec, start: usize, shu if tracks.is_empty() { return; } - let queue: Vec = tracks.iter().map(local_queue_track).collect(); - let start = start.min(queue.len() - 1); - let first = queue[start].clone(); - if shuffle { + // Folder-cover backfill BEFORE the mapping (the reference runs it at every + // local play site: playback.rs:1094 / :1322 / :1349 / :1382). This is the + // funnel for album / folder / folder-track / Tracks-tab play, so one call + // here covers all four. Blocking fs, hence spawn_blocking. + let tracks = tokio::task::spawn_blocking(move || { + let mut tracks = tracks; + fill_missing_covers(&mut tracks); + tracks + }) + .await + .unwrap_or_default(); + if tracks.is_empty() { + return; + } + let mut queue: Vec = tracks.iter().map(local_queue_track).collect(); + // Shuffle reorders THIS list and starts at the top of the mixed order. The + // mode alone only randomises what comes next, so the first track the user + // hears was the folder's #1 every time — owner ruling 2026-08-01, every + // shuffle must be genuinely random. The caller's anchor is meaningless once + // the order is mixed, so it is dropped (`play_track_list_in` does the same). + let start = if shuffle { runtime.core().set_shuffle(true).await; crate::now_playing::set_shuffle(true); - } + crate::playback_qt::xorshift_shuffle(&mut queue); + 0 + } else { + start.min(queue.len() - 1) + }; + let first = queue[start].clone(); // Through the SHARED seam (not `core().set_queue`) so the origin is derived // from the queue: one album -> ("album", group_key), a mixed folder queue -> // nothing, and the per-track album fallback lands it on the same view. @@ -412,15 +540,24 @@ fn find_track_blocking(row_id: i64) -> Option { pub async fn enqueue(runtime: &Runtime, kind: String, id: String, mode: String) { let k = kind.clone(); let ident = id.clone(); - let tracks: Vec = tokio::task::spawn_blocking(move || match k.as_str() { - "track" => ident - .parse::() - .ok() - .and_then(find_track_blocking) - .into_iter() - .collect(), - "folder" => with_db(|db| db.list_folder_tracks_recursive(&ident, false)).unwrap_or_default(), - _ => fetch_album_tracks_blocking(&ident), + let tracks: Vec = tokio::task::spawn_blocking(move || { + let mut rows: Vec = match k.as_str() { + "track" => ident + .parse::() + .ok() + .and_then(find_track_blocking) + .into_iter() + .collect(), + "folder" => { + with_db(|db| db.list_folder_tracks_recursive(&ident, false)).unwrap_or_default() + } + _ => fetch_album_tracks_blocking(&ident), + }; + // Same backfill the play funnel does — an enqueued row must carry the + // same cover it would have carried had it been played + // (reference: main.rs:10636 fills before `enqueue_local_tracks`). + fill_missing_covers(&mut rows); + rows }) .await .unwrap_or_default(); diff --git a/crates/qbz-qt/src/local_playlist_qt.rs b/crates/qbz-qt/src/local_playlist_qt.rs index f3adc206a..129616077 100644 --- a/crates/qbz-qt/src/local_playlist_qt.rs +++ b/crates/qbz-qt/src/local_playlist_qt.rs @@ -530,6 +530,18 @@ fn row_to_display(item: &RowItem) -> (PlaylistTrackRow, Option) { t.bit_depth, Some(t.sample_rate), ), + // The raw numbers, normalized to kHz exactly like + // `local_playback::local_queue_track` (:155-159) does — a local + // row reports Hz (44100), the catalog reports kHz (44.1), and + // `quality_state::rate_to_hz` must not multiply a kHz value + // twice. Kept in sync with the row's own `quality_detail` + // above so nothing downstream has to re-parse the string. + bit_depth: t.bit_depth, + sample_rate: Some(if t.sample_rate >= 1000.0 { + t.sample_rate / 1000.0 + } else { + t.sample_rate + }), art_url: t.artwork_path.clone().unwrap_or_default(), // `TrackRow.qml` renders `artPath`, never `artUrl` — the Qobuz // arm fills it from the download cache, and a local row's @@ -818,19 +830,40 @@ pub async fn load(runtime: &Runtime, playlist_id: &str) -> bool { /// cleared for every non-offline-only playlist, or a stamp would leak into the /// next thing the user plays. pub async fn play(runtime: &Runtime, start_row_id: &str) { - let (queue, offline_only) = { + play_in(runtime, start_row_id, false).await +} + +/// Header Shuffle for a LOCAL playlist: the same path, with the list mixed and +/// the anchor dropped. +/// +/// Raising the shuffle MODE and playing from the top is NOT a shuffle — the +/// mode only randomises what comes NEXT, so the first track was the playlist's +/// #1 every time. Owner ruling 2026-08-01: every shuffle must be genuinely +/// random. Same shape as `playlist_qt::play_shuffled` and +/// `playback_qt::play_track_list_in`. +pub async fn play_shuffled(runtime: &Runtime) { + play_in(runtime, "", true).await +} + +async fn play_in(runtime: &Runtime, start_row_id: &str, shuffle: bool) { + let (queue, offline_only, playlist_id) = { let Ok(q) = CURRENT_QUEUE.lock() else { return }; - let offline_only = CURRENT_META + let meta = CURRENT_META .lock() .ok() - .and_then(|m| m.as_ref().map(|(_, o)| *o)) - .unwrap_or(false); - (q.clone(), offline_only) + .and_then(|m| m.clone()); + let offline_only = meta.as_ref().map(|(_, o)| *o).unwrap_or(false); + let playlist_id = meta.map(|(id, _)| id).unwrap_or_default(); + (q.clone(), offline_only, playlist_id) }; if queue.is_empty() { return; } - let start = if start_row_id.is_empty() { + let mut queue = queue; + let start = if shuffle { + crate::playback_qt::xorshift_shuffle(&mut queue); + 0 + } else if start_row_id.is_empty() { 0 } else { queue @@ -840,7 +873,26 @@ pub async fn play(runtime: &Runtime, start_row_id: &str) { }; runtime.core().set_queue_offline_only(offline_only); let first_id = queue[start].id; - crate::playback_qt::set_queue_stamped(runtime, queue, Some(start), None).await; + // The "playing from" origin is ("playlist", ) — the reference + // stamps it on BOTH local play paths (`local_playlist.rs:1578` play_all / + // Shuffle and `:1599` play_from_visible), with the explicit note that "the + // now-playing context stays ('playlist', id)" for a mixed detail because + // anything Qobuz-bound re-resolves membership and excludes the sidecar rows + // by construction (playback.rs:3478-3488). + // + // Passing `None` here was the port's gap: `set_queue_stamped` then falls to + // `derive_context`, which returns None for a many-album playlist, so + // `refresh_now_playing` used its per-track ALBUM fallback and the song card + // drew the album glyph pointing at a local `album_group_key` instead of the + // playlist. It also made the row click REGRESS once main.rs started routing + // it here — `playlist_qt::play_track` had been stamping `open_context()`. + crate::playback_qt::set_queue_stamped( + runtime, + queue, + Some(start), + crate::playback_qt::PlayContext::playlist(&playlist_id), + ) + .await; crate::playback_qt::play_queue_track_public(runtime, first_id).await; } diff --git a/crates/qbz-qt/src/local_tree.rs b/crates/qbz-qt/src/local_tree.rs index 8cd4f9bce..9beb3ad9b 100644 --- a/crates/qbz-qt/src/local_tree.rs +++ b/crates/qbz-qt/src/local_tree.rs @@ -21,6 +21,43 @@ use serde::Serialize; use crate::local_rows::{basename, folder_key, map_track, FolderDetail, SubfolderRow, TrackRow, TreeNode}; use crate::local_state::{state, with_art, with_db}; +/// `list_folder_children` + the reference's on-disk cover fallback for the +/// Folder entries (`local_library.rs:2947-2966`): the index carries an +/// `artwork` only when a track under the folder had embedded art, so a folder +/// whose cover sits on disk as `cover.jpg` / `folder.jpg` / `.jpg` drew +/// a blank subcard here while the Slint drew the cover. Same +/// `find_folder_cover` the queue backfill uses, so the tree card and the queue +/// thumbnail can never disagree. +/// +/// BLOCKING (one `read_dir` per child folder that has no indexed artwork) — +/// both callers are `*_blocking` and run inside `spawn_blocking`, which is +/// where the reference puts it too ("Off-thread, so the fs scan is fine here"). +fn folder_children_with_covers(path: &str) -> Vec { + with_db(|db| db.list_folder_children(path, false)) + .unwrap_or_default() + .into_iter() + .map(|e| match e { + FolderTreeEntry::Folder { + path, + segment, + track_count_under, + artwork, + } => { + let artwork = artwork.filter(|a| !a.is_empty()).or_else(|| { + crate::local_playback::find_folder_cover(std::path::Path::new(&path)) + }); + FolderTreeEntry::Folder { + path, + segment, + track_count_under, + artwork, + } + } + other => other, + }) + .collect() +} + fn entry_to_node( e: &FolderTreeEntry, depth: i32, @@ -109,7 +146,7 @@ pub fn tree_collapse(path: &str) -> Vec { /// Expand: fetch ONE child level and splice it in (never a recursive /// preload). pub fn tree_expand_blocking(path: &str) -> Vec { - let children = with_db(|db| db.list_folder_children(path, false)).unwrap_or_default(); + let children = folder_children_with_covers(path); let nodes = state(|s| s.tree.iter().position(|n| n.path == path).map(|pos| (pos, s.tree[pos].depth))); let Some((pos, depth)) = nodes else { return state(|s| s.tree.clone()); @@ -354,7 +391,7 @@ pub fn tree_select_all_blocking() { /// The right pane of tree mode: subfolders (cover cards) + the folder's /// DIRECT tracks + the recursive count shown next to the name. pub fn load_folder_detail_blocking(path: &str) -> FolderDetail { - let children = with_db(|db| db.list_folder_children(path, false)).unwrap_or_default(); + let children = folder_children_with_covers(path); let tracks = with_db(|db| db.list_folder_tracks(path, false)).unwrap_or_default(); let count = with_db(|db| db.count_folder_tracks_recursive(path, false)).unwrap_or(0); let (subfolders, rows) = with_art(|art| { diff --git a/crates/qbz-qt/src/main.rs b/crates/qbz-qt/src/main.rs index 818654c5f..fb6fecf98 100644 --- a/crates/qbz-qt/src/main.rs +++ b/crates/qbz-qt/src/main.rs @@ -1242,6 +1242,17 @@ pub(crate) fn playlist_play_all() { pub(crate) fn playlist_shuffle() { let runtime = app(); spawn(async move { + // The local guard `playlist_play_all` carries. The MODE is raised so + // what follows this list stays shuffled, but the list itself is mixed + // by `play_shuffled` — the flag alone would start on the playlist's #1 + // track every time (owner ruling 2026-08-01: every shuffle must be + // genuinely random). + if local_playlist_qt::open_id().is_some() { + runtime.core().set_shuffle(true).await; + now_playing::set_shuffle(true); + local_playlist_qt::play_shuffled(&runtime).await; + return; + } if let Err(e) = playlist_qt::play_shuffled(&runtime).await { log::error!("[qbz-qt] playlist shuffle failed: {e}"); } @@ -1279,6 +1290,19 @@ pub(crate) fn playlist_delete() { pub(crate) fn playlist_play_track(track_id: String) { let runtime = app(); spawn(async move { + // Same guard as `playlist_play_all` above, and for the same reason: a + // LOCAL detail plays from its OWN resolved snapshot. Without it a row + // click went through `playlist_qt::play_track` -> `current_queue()` -> + // `row_to_queue`, which types EVERY row as Qobuz (`is_local: false`, + // `source: "qobuz"`) — so a local file was queued as a catalog track + // and `governed` came out true on the quality seed. `local_playlist_qt + // ::play` matches on the same display id the rows carry + // (`row_to_display` sets `id: queue.id.to_string()`), Qobuz rows + // included, so the whole mixed detail is served by one path. + if local_playlist_qt::open_id().is_some() { + local_playlist_qt::play(&runtime, &track_id).await; + return; + } if let Err(e) = playlist_qt::play_track(&runtime, &track_id).await { log::error!("[qbz-qt] playlist row play failed: {e}"); } diff --git a/crates/qbz-qt/src/playback_qt.rs b/crates/qbz-qt/src/playback_qt.rs index 9e07fed88..62c742e3f 100644 --- a/crates/qbz-qt/src/playback_qt.rs +++ b/crates/qbz-qt/src/playback_qt.rs @@ -838,7 +838,13 @@ fn xorshift_shuffle_seeded(items: &mut [T], seed: u64) { } /// `xorshift_shuffle_seeded` with the reference's wall-clock seed. -fn xorshift_shuffle(items: &mut [T]) { +/// +/// `pub(crate)` so every Shuffle entry point can reorder its OWN list instead +/// of only raising the global shuffle MODE. Owner ruling 2026-08-01: "todos los +/// shuffles deben ser totalmente random"; a path that raises the flag and then +/// plays the list's #1 track is not shuffled, it is the original order with a +/// toggle lit — an artifact of a very early Tauri experiment. +pub(crate) fn xorshift_shuffle(items: &mut [T]) { let seed = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .map(|d| d.as_nanos() as u64) @@ -1110,15 +1116,31 @@ pub async fn enqueue_album_track( Ok(()) } -/// Shuffle-play an album (AlbumView header Shuffle): enable shuffle, then -/// play — the core queue owns the shuffled order. +/// Shuffle-play an album (AlbumView header Shuffle): mix the album's own list +/// and start at the top of the mixed order. +/// +/// The MODE alone was not a shuffle. `set_shuffle(true)` only randomises what +/// comes NEXT, so `play_album` still started on track 1 — pressing Shuffle on +/// an album played its opener every single time. Owner ruling 2026-08-01: +/// every shuffle must be genuinely random; the flag-and-play-from-the-top +/// shape is an artifact of a very early Tauri experiment. The mode is still +/// raised so playback stays shuffled past this album. +/// +/// A LOCAL album keeps going through the local path, which owns its own queue +/// building and does the same mix inside `local_playback::play_rows`. pub async fn play_album_shuffled( runtime: &Arc>, album_id: &str, ) -> Result<(), String> { runtime.core().set_shuffle(true).await; crate::now_playing::set_shuffle(true); - play_album(runtime, album_id).await + if is_local_album(album_id) { + crate::local_playback::play_album(runtime, album_id.to_string(), None, true).await; + return Ok(()); + } + let mut tracks = fetch_album_queue(runtime, album_id).await?; + xorshift_shuffle(&mut tracks); + play_track_list_in(runtime, tracks, 0, false, PlayContext::album(album_id)).await } /// Build the queue meta for a Library-feed track (shared by @@ -1151,9 +1173,14 @@ fn feed_queue_track(track_id: u64) -> Result { } else { Some(item.image_url.clone()) }, + // playback.rs `make_queue_track` (:2426): the CATALOG max travels with + // the queue track. `None` here zeroed `quality_state`'s `TRACK_MAX_*` + // seed, so a Library-feed track play drew a bare tier on the NPB + // AudioStamp with no "24-bit / 96 kHz" line. `library_qt::FeedItem` + // carries the raw numbers alongside the formatted `quality_detail`. hires: item.quality_tier == "hires", - bit_depth: None, - sample_rate: None, + bit_depth: item.bit_depth, + sample_rate: item.sample_rate, is_local: false, album_id: if item.album_id.is_empty() { None @@ -1759,19 +1786,38 @@ pub(crate) async fn refresh_now_playing(runtime: &Arc }); } -/// Quality tier + exact label from a queue track's bit depth / sample rate -/// (kHz float, catalog), matching the discover card badge format. +/// Quality tier + exact label from a queue track's bit depth / sample rate. +/// +/// 1:1 with `playback.rs:2025-2042` (`refresh_now_playing_meta`), which this +/// had forked from in three ways, each of them visible on the NPB AudioStamp: +/// +/// - **DSD.** `bit_depth == Some(1)` marks a DSD stream (1-bit, `sample_rate` +/// = the DSD bit rate). The reference tiers it "hires" and labels it +/// through `dsd_multiple_label` ("DSD64"); the forked version tiered it +/// "cd" and printed "1-bit / 2822.4 kHz". +/// - **Hz vs kHz.** `quality_state::detail` normalizes either unit (the +/// `>= 1000` guard) exactly like the track rows do, so the stamp and the +/// row can never disagree. The forked `format!` printed a raw Hz value as +/// "44100 kHz". +/// - **The empty-detail gate.** The reference blanks the line only when the +/// TIER is unknown or BOTH params are missing; the fork blanked it whenever +/// either one was missing, so a track with a known rate and no depth lost +/// its whole detail line instead of taking the formatter's depth default. fn quality_badge(track: &QueueTrack) -> (String, String) { let tier = match track.bit_depth { + Some(1) => "hires", Some(d) if d >= 24 => "hires", Some(_) => "cd", None if track.hires => "hires", None => "", } .to_string(); - let label = match (track.bit_depth, track.sample_rate) { - (Some(bd), Some(sr)) => format!("{}-bit / {} kHz", bd, crate::home_qt::format_rate(sr)), - _ => String::new(), + let label = if tier.is_empty() || (track.bit_depth.is_none() && track.sample_rate.is_none()) { + String::new() + } else if track.bit_depth == Some(1) { + crate::quality_state::dsd_multiple_label(track.sample_rate) + } else { + crate::quality_state::detail(track.bit_depth, track.sample_rate) }; (tier, label) } diff --git a/crates/qbz-qt/src/playlist_qt.rs b/crates/qbz-qt/src/playlist_qt.rs index fb9d45249..184730a0f 100644 --- a/crates/qbz-qt/src/playlist_qt.rs +++ b/crates/qbz-qt/src/playlist_qt.rs @@ -80,6 +80,22 @@ pub struct PlaylistTrackRow { pub quality_detail: String, #[serde(rename = "qualityLabel")] pub quality_label: String, + /// RAW catalog max bit depth / sample rate (kHz) — the SAME two numbers + /// `quality_detail` / `quality_label` above are derived from. + /// + /// THE CONTRACT (reference: `crates/qbz/src/playback.rs:2426` + /// `make_queue_track`): a queue track carries the NUMBERS, never only the + /// formatted string. `row_to_queue` below maps this display row into a + /// `QueueTrack`; without these fields it could not fill them and hardcoded + /// `None`, which zeroes `quality_state`'s `TRACK_MAX_*` seed — the NPB + /// AudioStamp then shows a tier with NO "24-bit / 96 kHz" line for every + /// playlist-sourced play. Re-parsing the formatted string back into + /// numbers would be lossy and a second source of truth; the producer has + /// them in hand, so it passes them. + #[serde(rename = "bitDepth", skip_serializing_if = "Option::is_none")] + pub bit_depth: Option, + #[serde(rename = "sampleRate", skip_serializing_if = "Option::is_none")] + pub sample_rate: Option, pub explicit: bool, #[serde(rename = "artUrl")] pub art_url: String, @@ -490,6 +506,10 @@ pub(crate) fn map_track(track: &Track) -> PlaylistTrackRow { track.maximum_sampling_rate, ), quality_label: quality_label(track.maximum_bit_depth, track.maximum_sampling_rate), + // See `PlaylistTrackRow::bit_depth` — the raw catalog numbers ride + // with the row so `row_to_queue` can hand them to the queue. + bit_depth: track.maximum_bit_depth, + sample_rate: track.maximum_sampling_rate, explicit: track.parental_warning, art_url: album .and_then(|a| a.image.best().cloned()) @@ -518,9 +538,14 @@ fn row_to_queue(row: &PlaylistTrackRow) -> QueueTrack { } else { Some(row.art_url.clone()) }, + // playback.rs `make_queue_track` (:2426): the CATALOG max travels with + // the queue track. `None` here zeroed `quality_state`'s `TRACK_MAX_*` + // seed, so a playlist play drew a bare tier on the NPB AudioStamp with + // no "24-bit / 96 kHz" line — perfect from an album, blank from a + // playlist. The row carries the numbers now (see the struct). hires: row.quality_tier == "hires", - bit_depth: None, - sample_rate: None, + bit_depth: row.bit_depth, + sample_rate: row.sample_rate, is_local: false, album_id: if row.album_id.is_empty() { None @@ -1662,12 +1687,22 @@ pub async fn play_all(runtime: &Arc>) -> Result<(), S play_queue_at(runtime, tracks, 0, open_context()).await } -/// Header Shuffle: shuffle on, then play (the core queue owns the order — -/// play_album_shuffled parity). +/// Header Shuffle: reorder THIS list and play the top of the shuffled order. +/// +/// The flag alone is not a shuffle. Raising `set_shuffle(true)` and calling +/// `play_all` starts on the playlist's #1 track every single time — the core's +/// mode only randomises what comes NEXT — so the one track the user actually +/// hears when they press Shuffle was deterministic. Owner ruling 2026-08-01: +/// every shuffle must be genuinely random. Same shape as +/// `playback_qt::play_track_list_in`'s shuffle arm and as the reference's +/// `play_album_shuffled` (playback.rs:3945-3957): mix the Vec, start at 0. +/// The MODE is still raised, so continuing past this list stays shuffled. pub async fn play_shuffled(runtime: &Arc>) -> Result<(), String> { runtime.core().set_shuffle(true).await; crate::now_playing::set_shuffle(true); - play_all(runtime).await + let mut tracks = current_queue(); + crate::playback_qt::xorshift_shuffle(&mut tracks); + play_queue_at(runtime, tracks, 0, open_context()).await } /// Row play: the playlist as the queue, starting at this track.