From 83ce396485e38c6b537528be0ff7d5cd1c3cff64 Mon Sep 17 00:00:00 2001 From: Ruud van Asseldonk Date: Sun, 13 Apr 2025 20:27:25 +0200 Subject: [PATCH 01/14] Begin working on thumbnail color analysis I noticed in search, in the discographies where it shows many images, they take a few milliseconds to show up, and it makes them flicker into view which is ugly. I already added a grey backdrop, but it's still visible. I wonder if it would help to select a better background color that is more aligned with the thumbnail. Then I got a bit nerd-sniped on extracting that color, and I think I have a good algorithm now. Now we need to run that and store it in the database, and then later we can serve it in the webinterface in the response json. I suspect the reason for the flicker is not loading the image, because they are cached, but either decoding or downscaling it. I suppose the browser evicts it from memory and then has to decode it again. This commit adds an analysis step to the thumbnail pipeline, but it does not do anything with that information yet. --- src/thumb_gen.rs | 173 ++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 149 insertions(+), 24 deletions(-) diff --git a/src/thumb_gen.rs b/src/thumb_gen.rs index fb49f00..b427042 100644 --- a/src/thumb_gen.rs +++ b/src/thumb_gen.rs @@ -30,15 +30,24 @@ struct GenThumb<'a> { /// The state of generating a single thumbnail. enum GenThumbState<'a> { + /// We haven't started this task, but we know the file we need to inspect. Pending { file_id: FileId, flac_filename: &'a Path, }, + /// Imagemagick is downscaling the cover art to a temporary file. Resizing { file_id: FileId, child: process::Child, out_path: PathBuf, }, + /// Imagemagick is analyzing the thumbnail to extract the dominant color. + Analyzing { + file_id: FileId, + child: process::Child, + path: PathBuf, + }, + /// Cjpegli is compressing the thumbnail. Compressing { file_id: FileId, child: process::Child, @@ -65,8 +74,11 @@ impl<'a> GenThumb<'a> { flac_filename: &'a Path, ) -> Result>> { let task = GenThumb { - album_id: album_id, - state: GenThumbState::Pending { flac_filename, file_id }, + album_id, + state: GenThumbState::Pending { + flac_filename, + file_id, + }, }; match database::select_thumbnail_exists(tx, album_id.0 as i64)? { @@ -99,7 +111,7 @@ impl<'a> GenThumb<'a> { let out_path = get_tmp_fname(album_id); - let mut convert = Command::new("magick") + let mut magick = Command::new("magick") // Give Imagemagick enough time to open the image, recent versions // are strict about it which leads to "time limit exceeded" error // from "fatal/cache.c". The unit is seconds. @@ -144,7 +156,7 @@ impl<'a> GenThumb<'a> { .map_err(|e| Error::CommandError("Failed to spawn ImageMagick.", Some(e)))?; { - let stdin = convert + let stdin = magick .stdin .as_mut() .expect("Stdin should be there, we piped it."); @@ -153,35 +165,132 @@ impl<'a> GenThumb<'a> { self.state = GenThumbState::Resizing { file_id: file_id, - child: convert, + child: magick, out_path: out_path, }; Ok(Some(self)) } - /// When in `Resizing` state, wait for that to complete, and start compressing. + fn start_analyze(mut self) -> Result> { + let (mut magick, file_id, out_path) = match self.state { + GenThumbState::Resizing { + child, + file_id, + out_path, + } => (child, file_id, out_path), + _ => panic!("Can only call start_analyze in Resizing state."), + }; + + let exit_status = magick + .wait() + .map_err(|e| Error::CommandError("ImageMagick's 'magick' failed.", Some(e)))?; + + if !exit_status.success() { + // Clean up the intermediate png file on error. + let _rm_result_ignored = std::fs::remove_file(out_path); + return Err(Error::CommandError( + "ImageMagick's 'magick' did not exit successfully.", + None, + )); + } + + // We are going to use Imagemagick to find a dominant color in the image. + // The process below was tuned by hand on a set of thumbnails so it's + // optimized to work well on a wide range of cover art thumbnails, but + // especially those that contain one larger area of a solid color. We + // want to pick that one out. + let magick = Command::new("magick") + // Apply a timeout in seconds, like in the resize step. + .args(["-limit", "time", "120"]) + .arg(&out_path) + // Downscaling and the k-means should happen in a linear color space. + .args(["-colorspace", "RGB"]) + // For the mode filter, for pixels outside the canvas, tile and + // mirror the input. We want that rather than extend, otherwise the + // edge colors would weigh relatively more. + .args(["-virtual-pixel", "mirror"]) + // We pick the sizes so we can downscale with a simple box filter + // where every pixel is the average of its four "parents". + .args(["-filter", "box"]) + // We start with 72x72, it's close enough to half our 140x140 that + // we still get a good sense of the colors, but importantly it is + // 9 * 2 * 4, so we can downscale without creating blurry edges. + .args(["-resize", "72x72"]) + // Pick out the 5 most important colors to work with from now on. + .args(["-kmeans", "5"]) + // For every 18x18 area, pick the color that occurs most in that + // area. This has the effect of "blurring", and making prominent + // colors even more prominent. This is the main trick in the process. + // Often it already eliminates one or two of the five colors. + .args(["-statistic", "Mode", "18x18"]) + // Then we downscale, but that mixes the colors again so now we can + // have more than 5, but if the source has a large-enough area of + // one color, it will be preserved exactly. + .args(["-resize", "18x18"]) + // Now we do more passes of "blurring" making prominent colors more + // prominent, restricting colors, downscaling, etc. + .args(["-statistic", "Mode", "9x9"]) + .args(["-kmeans", "3"]) + .args(["-statistic", "Mode", "9x9"]) + .args(["-statistic", "Mode", "9x9"]) + .args(["-resize", "9x9"]) + .args(["-kmeans", "3"]) + // We end up with 3 colors on a 9x9 grid. It's on purpose odd, so + // in case of two dominant colors, there will be a winner, it can't + // be 50/50. It can be 3/3/3 though. + // For the final pick, we take the mode of a 9x9 region, so the + // center pixel at (4, 4) contains the mode of the image itself. + .args(["-statistic", "Mode", "9x9"]) + .args(["-colorspace", "sRGB"]) + // Print the color of the pixel at (4, 4) in hex to stdout. + .args(["-format", "%[hex:p{4,4}]"]) + .stdout(Stdio::piped()) + .arg("info:-") + .spawn() + .map_err(|e| Error::CommandError("Failed to spawn 'magick'.", Some(e)))?; + + self.state = GenThumbState::Analyzing { + file_id, + // The input path to this step is the output of the previous step. + path: out_path, + child: magick, + }; + + Ok(self) + } + + /// When in `Analyzing` state, wait for that to complete, and start compressing. fn start_compress(mut self) -> Result> { - let (mut convert, file_id, out_path) = match self.state { - GenThumbState::Resizing { file_id, child, out_path } => (child, file_id, out_path), + let (mut magick, file_id, path) = match self.state { + GenThumbState::Analyzing { + file_id, + child, + path, + } => (child, file_id, path), _ => panic!("Can only call start_compress in Resizing state."), }; - let exit_status = convert + let exit_status = magick .wait() .map_err(|e| Error::CommandError("ImageMagick's 'magick' failed.", Some(e)))?; if !exit_status.success() { // Clean up the intermediate png file on error. - let _rm_result_ignored = std::fs::remove_file(out_path); - return Err(Error::CommandError("ImageMagick's 'magick' did not exit successfully.", None)); + let _rm_result_ignored = std::fs::remove_file(path); + return Err(Error::CommandError( + "ImageMagick's 'magick' did not exit successfully.", + None, + )); } + // TODO: Read color from stdout. + let cjpegli = Command::new("cjpegli") .arg("--distance=0.45") .arg("--progressive_level=0") // Input is the intermediate file. - .arg(&out_path) + .arg(&path) // Output to stdout. .stdout(Stdio::piped()) .arg("-") @@ -191,10 +300,10 @@ impl<'a> GenThumb<'a> { .map_err(|e| Error::CommandError("Failed to spawn 'cjpegli'.", Some(e)))?; self.state = GenThumbState::Compressing { - file_id: file_id, + file_id, child: cjpegli, // Input file for this step is the output of the previous command. - in_path: out_path, + in_path: path, }; Ok(self) @@ -214,17 +323,25 @@ impl<'a> GenThumb<'a> { file_id, flac_filename, } => self.start_resize(album_id, file_id, flac_filename), - GenThumbState::Resizing { .. } => self.start_compress().map(Some), - GenThumbState::Compressing { mut child, file_id, in_path } => { - let exit_status = child - .wait() - .map_err(|e| Error::CommandError("Thumbnail compression with 'cjpegli' failed.", Some(e)))?; + GenThumbState::Resizing { .. } => self.start_analyze().map(Some), + GenThumbState::Analyzing { .. } => self.start_compress().map(Some), + GenThumbState::Compressing { + mut child, + file_id, + in_path, + } => { + let exit_status = child.wait().map_err(|e| { + Error::CommandError("Thumbnail compression with 'cjpegli' failed.", Some(e)) + })?; // Delete the intermediate png file. std::fs::remove_file(in_path)?; if !exit_status.success() { - return Err(Error::CommandError("'cjpegli' did not exit successfully.", None)); + return Err(Error::CommandError( + "'cjpegli' did not exit successfully.", + None, + )); } let mut stdout = child @@ -236,7 +353,12 @@ impl<'a> GenThumb<'a> { { let mut tx = db.begin()?; - database::insert_album_thumbnail(&mut tx, album_id.0 as i64, file_id.0, &jpeg_bytes[..])?; + database::insert_album_thumbnail( + &mut tx, + album_id.0 as i64, + file_id.0, + &jpeg_bytes[..], + )?; tx.commit()?; } @@ -286,7 +408,8 @@ pub fn generate_thumbnails( let album_id = track_id.album_id(); if album_id != prev_album_id { let fname = index.get_filename(kv.track.filename); - if let Some(task) = GenThumb::new(&mut tx, album_id, kv.track.file_id, fname.as_ref())? { + if let Some(task) = GenThumb::new(&mut tx, album_id, kv.track.file_id, fname.as_ref())? + { pending_tasks.push(task); status.files_to_process_thumbnails += 1; @@ -314,7 +437,7 @@ pub fn generate_thumbnails( let mutex_ref = &mutex; // Start `num_cpus` worker threads. All these threads will do is block and - // wait on IO or the external process, but both `convert` and `cjpegli` + // wait on IO or the external process, but both `magick` and `cjpegli` // are CPU-bound, so this should keep the CPU busy. When thumbnailing many // albums with a cold page cache, IO to read the thumb from the file can be // a factor too, so add one additional thread to ensure we can keep the CPU @@ -350,7 +473,9 @@ pub fn generate_thumbnails( Err(Error::CommandError(msg, detail)) => { match detail { None => eprintln!("Thumbnail generation failed: {msg}"), - Some(err) => eprintln!("Thumbnail generation failed: {msg} {err:?}"), + Some(err) => { + eprintln!("Thumbnail generation failed: {msg} {err:?}") + } } // We count failing as progress, because we did look From e434f027d0fdfd215748b138f5f88b1d483cb769 Mon Sep 17 00:00:00 2001 From: Ruud van Asseldonk Date: Thu, 1 May 2025 10:34:53 +0200 Subject: [PATCH 02/14] Store thumbnail colors in database and index It happens to fit in the Album struct nicely! This is a breaking change that requires recomputing all thumbnails. --- src/build.rs | 22 +++++++++++++----- src/database.rs | 39 +++++++++++++++++++++++++++----- src/database.sql | 16 +++++++++---- src/prim.rs | 58 +++++++++++++++++++++++++++++++++++++++--------- src/thumb_gen.rs | 18 +++++++++++++-- 5 files changed, 126 insertions(+), 27 deletions(-) diff --git a/src/build.rs b/src/build.rs index 6db9a10..99d23ba 100644 --- a/src/build.rs +++ b/src/build.rs @@ -9,9 +9,12 @@ use std::collections::{BTreeMap, BTreeSet, HashMap}; use std::fmt; use std::str::FromStr; -use crate::database::{FileMetadata, Transaction, self as db}; -use crate::prim::{AlbumId, Album, AlbumArtistsRef, ArtistId, Artist, FileId, Instant, TrackId, Track, Date, Lufs, FilenameRef, StringRef}; -use crate::string_utils::{StringDeduper, normalize_words}; +use crate::database::{self as db, FileMetadata, Transaction}; +use crate::prim::{ + Album, AlbumArtistsRef, AlbumId, Artist, ArtistId, Color, Date, FileId, FilenameRef, Instant, + Lufs, StringRef, Track, TrackId, +}; +use crate::string_utils::{normalize_words, StringDeduper}; use crate::word_index::WordMeta; pub enum BuildError { @@ -801,13 +804,19 @@ impl BuildMetaIndex { // TODO: It's inefficient to query the database once per track for the // album loudness. - let track_loudness = db::select_track_loudness_lufs(tx, track_id.0 as i64)?.map(Lufs::from_f64); - let album_loudness = db::select_album_loudness_lufs(tx, album_id.0 as i64)?.map(Lufs::from_f64); + let track_loudness = + db::select_track_loudness_lufs(tx, track_id.0 as i64)?.map(Lufs::from_f64); + let album_loudness = + db::select_album_loudness_lufs(tx, album_id.0 as i64)?.map(Lufs::from_f64); + let album_color = db::select_album_color(tx, album_id.0 as i64)? + .map(|c| Color::parse(&c).expect("Database should store only valid colors")); // Insert all the album artists if no artist with the given id existed // yet. If one did exist, verify consistency. Also fill the vector of // album artists so the album can refer to this. - let album_artists_ref = self.album_artists.insert(album_artists.iter().map(|tuple| tuple.0)); + let album_artists_ref = self + .album_artists + .insert(album_artists.iter().map(|tuple| tuple.0)); for (artist_id, aa_name, aa_name_sort) in album_artists { let artist = Artist { @@ -844,6 +853,7 @@ impl BuildMetaIndex { original_release_date: release_date, first_seen: file.mtime, loudness: album_loudness, + color: album_color.unwrap_or_default(), }; let mut add_album = true; diff --git a/src/database.rs b/src/database.rs index bde0656..da6f853 100644 --- a/src/database.rs +++ b/src/database.rs @@ -333,6 +333,11 @@ pub fn ensure_schema_exists(tx: &mut Transaction) -> Result<()> { create table if not exists thumbnails ( album_id integer primary key , file_id integer not null references files (id) on delete cascade + -- We store the color as hex string, even though that's larger than just + -- storing the 24-bit integer. The data blob is kilobytes anyway, a few bytes + -- here makes little difference, and it makes the database more readable for + -- humans. + , color string not null , data blob not null ); "#; @@ -550,11 +555,11 @@ pub fn iter_file_tags<'i, 't, 'a>(tx: &'i mut Transaction<'t, 'a>, file_id: i64) Ok(result) } -pub fn insert_album_thumbnail(tx: &mut Transaction, album_id: i64, file_id: i64, data: &[u8]) -> Result<()> { +pub fn insert_album_thumbnail(tx: &mut Transaction, album_id: i64, file_id: i64, color: &str, data: &[u8]) -> Result<()> { let sql = r#" - insert into thumbnails (album_id, file_id, data) - values (:album_id, :file_id, :data) - on conflict (album_id) do update set data = :data; + insert into thumbnails (album_id, file_id, color, data) + values (:album_id, :file_id, :color, :data) + on conflict (album_id) do update set color = :color, data = :data; "#; let statement = match tx.statements.entry(sql.as_ptr()) { Occupied(entry) => entry.into_mut(), @@ -563,7 +568,8 @@ pub fn insert_album_thumbnail(tx: &mut Transaction, album_id: i64, file_id: i64, statement.reset()?; statement.bind(1, album_id)?; statement.bind(2, file_id)?; - statement.bind(3, data)?; + statement.bind(3, color)?; + statement.bind(4, data)?; let result = match statement.next()? { Row => panic!("Query 'insert_album_thumbnail' unexpectedly returned a row."), Done => (), @@ -743,6 +749,29 @@ pub fn update_listen_completed(tx: &mut Transaction, listen_id: i64, queue_id: i Ok(result) } +pub fn select_album_color(tx: &mut Transaction, album_id: i64) -> Result> { + let sql = r#" + select color from thumbnails where album_id = :album_id; + "#; + let statement = match tx.statements.entry(sql.as_ptr()) { + Occupied(entry) => entry.into_mut(), + Vacant(vacancy) => vacancy.insert(tx.connection.prepare(sql)?), + }; + statement.reset()?; + statement.bind(1, album_id)?; + let decode_row = |statement: &Statement| Ok(statement.read(0)?); + let result = match statement.next()? { + Row => Some(decode_row(statement)?), + Done => None, + }; + if result.is_some() { + if statement.next()? != Done { + panic!("Query 'select_album_color' should return at most one row."); + } + } + Ok(result) +} + pub fn select_album_loudness_lufs(tx: &mut Transaction, album_id: i64) -> Result> { let sql = r#" select bs17704_loudness_lufs from album_loudness where album_id = :album_id; diff --git a/src/database.sql b/src/database.sql index d6a15b5..e638275 100644 --- a/src/database.sql +++ b/src/database.sql @@ -143,6 +143,11 @@ create table if not exists waveforms create table if not exists thumbnails ( album_id integer primary key , file_id integer not null references files (id) on delete cascade + -- We store the color as hex string, even though that's larger than just + -- storing the 24-bit integer. The data blob is kilobytes anyway, a few bytes + -- here makes little difference, and it makes the database more readable for + -- humans. +, color string not null , data blob not null ); -- @end ensure_schema_exists @@ -217,10 +222,10 @@ order by -- we found them in the file. id asc; --- @query insert_album_thumbnail(album_id: i64, file_id: i64, data: bytes) -insert into thumbnails (album_id, file_id, data) -values (:album_id, :file_id, :data) -on conflict (album_id) do update set data = :data; +-- @query insert_album_thumbnail(album_id: i64, file_id: i64, color: str, data: bytes) +insert into thumbnails (album_id, file_id, color, data) +values (:album_id, :file_id, :color, :data) +on conflict (album_id) do update set color = :color, data = :data; -- @query insert_album_loudness(album_id: i64, file_id: i64, loudness: f64) insert into album_loudness (album_id, file_id, bs17704_loudness_lufs) @@ -287,6 +292,9 @@ where and queue_id = :queue_id and track_id = :track_id; +-- @query select_album_color(album_id: i64) ->? str +select color from thumbnails where album_id = :album_id; + -- @query select_album_loudness_lufs(album_id: i64) ->? f64 select bs17704_loudness_lufs from album_loudness where album_id = :album_id; diff --git a/src/prim.rs b/src/prim.rs index 04b8a1d..344add3 100644 --- a/src/prim.rs +++ b/src/prim.rs @@ -7,9 +7,9 @@ //! Primitive data types for the music library. +use chrono::{DateTime, Utc}; use std::fmt; use std::str::FromStr; -use chrono::{DateTime, Utc}; // Stats of my personal music library at this point: // @@ -320,19 +320,51 @@ pub struct AlbumArtistsRef { pub end: u32, } +/// An sRGB color (used to approximate thumbnails). +#[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)] +pub struct Color(u32); + +impl Color { + #[inline] + pub fn parse(src: &str) -> Option { + u32::from_str_radix(src, 16).ok().map(Color) + } +} + +impl fmt::Display for Color { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{:06x}", self.0) + } +} + +impl Default for Color { + fn default() -> Self { + // The default color for an album, when we don't have a thumbnail, is + // very light gray, which fits with the theme. + // TODO: It would be better to store an `Option` in the album + // instead of having a default. Maybe we can do a NonZeroU32 and store + // an alpha value in the color too, which makes it nonzero. + Color(0xf8f8f8) + } +} + #[repr(C)] #[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)] pub struct Album { - pub artist_ids: AlbumArtistsRef, - pub artist: StringRef, - pub title: StringRef, - pub original_release_date: Date, - pub loudness: Option, + pub artist_ids: AlbumArtistsRef, /* 8 bytes */ + pub artist: StringRef, /* 4 bytes */ + pub title: StringRef, /* 4 bytes */ + pub original_release_date: Date, /* 4 bytes */ + + /// Dominant color in the cover art. + pub color: Color, /* 4 bytes */ /// First time that we encountered this album, can be either: /// * The minimal `mtime` across the files in the album. /// * The first play of one of the tracks in the album. (TODO) - pub first_seen: Instant, + pub first_seen: Instant, /* 8 bytes */ + + pub loudness: Option, /* 2 bytes */ } #[repr(C)] @@ -405,14 +437,20 @@ mod test { fn struct_sizes_are_as_expected() { use std::mem; assert_eq!(mem::size_of::(), 24); - assert_eq!(mem::size_of::(), 32); + assert_eq!(mem::size_of::(), 40); assert_eq!(mem::size_of::(), 8); assert_eq!(mem::size_of::(), 32); assert_eq!(mem::size_of::(), 16); - assert_eq!(mem::size_of::(), mem::align_of::()); - assert_eq!(mem::size_of::(), mem::align_of::()); + assert_eq!( + mem::size_of::(), + mem::align_of::() + ); + assert_eq!( + mem::size_of::(), + mem::align_of::() + ); assert_eq!(mem::align_of::(), 8); assert_eq!(mem::align_of::(), 8); diff --git a/src/thumb_gen.rs b/src/thumb_gen.rs index b427042..01fa3a0 100644 --- a/src/thumb_gen.rs +++ b/src/thumb_gen.rs @@ -18,7 +18,7 @@ use crate::database; use crate::database::{Connection, Transaction}; use crate::database_utils; use crate::error::{Error, Result}; -use crate::prim::{AlbumId, FileId}; +use crate::prim::{AlbumId, Color, FileId}; use crate::scan::{ScanStage, Status}; use crate::{MemoryMetaIndex, MetaIndex}; @@ -50,6 +50,7 @@ enum GenThumbState<'a> { /// Cjpegli is compressing the thumbnail. Compressing { file_id: FileId, + color: Color, child: process::Child, in_path: PathBuf, }, @@ -284,7 +285,17 @@ impl<'a> GenThumb<'a> { )); } - // TODO: Read color from stdout. + let mut color_buf = String::with_capacity(8); + magick + .stdout + .expect("We piped stdout.") + .take(8) + .read_to_string(&mut color_buf)?; + + let color = Color::parse(&color_buf).ok_or(Error::CommandError( + "ImageMagick did not return a valid color.", + None, + ))?; let cjpegli = Command::new("cjpegli") .arg("--distance=0.45") @@ -301,6 +312,7 @@ impl<'a> GenThumb<'a> { self.state = GenThumbState::Compressing { file_id, + color, child: cjpegli, // Input file for this step is the output of the previous command. in_path: path, @@ -327,6 +339,7 @@ impl<'a> GenThumb<'a> { GenThumbState::Analyzing { .. } => self.start_compress().map(Some), GenThumbState::Compressing { mut child, + color, file_id, in_path, } => { @@ -357,6 +370,7 @@ impl<'a> GenThumb<'a> { &mut tx, album_id.0 as i64, file_id.0, + &color.to_string(), &jpeg_bytes[..], )?; tx.commit()?; From acdd6b724ce5c2573a146c446ee0fdc68fe36e39 Mon Sep 17 00:00:00 2001 From: Ruud van Asseldonk Date: Thu, 1 May 2025 10:40:17 +0200 Subject: [PATCH 03/14] Add observation about Magick color output format --- src/thumb_gen.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/thumb_gen.rs b/src/thumb_gen.rs index 01fa3a0..ae09f2a 100644 --- a/src/thumb_gen.rs +++ b/src/thumb_gen.rs @@ -292,6 +292,8 @@ impl<'a> GenThumb<'a> { .take(8) .read_to_string(&mut color_buf)?; + // TODO: Sometimes the color ends in ff, sometimes it does not. What + // gives? We need to normalize this. let color = Color::parse(&color_buf).ok_or(Error::CommandError( "ImageMagick did not return a valid color.", None, From 79313d7e9a630b6d9d7016aaf6adf82ce44d7ecb Mon Sep 17 00:00:00 2001 From: Ruud van Asseldonk Date: Thu, 1 May 2025 10:50:16 +0200 Subject: [PATCH 04/14] Serve thumbnail colors from the API --- src/prim.rs | 6 +++++- src/serialization.rs | 15 +++++++++++---- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/prim.rs b/src/prim.rs index 344add3..0675b18 100644 --- a/src/prim.rs +++ b/src/prim.rs @@ -333,7 +333,11 @@ impl Color { impl fmt::Display for Color { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{:06x}", self.0) + if self.0 > 0xffffff { + write!(f, "{:06x}", self.0 >> 8) + } else { + write!(f, "{:06x}", self.0) + } } } diff --git a/src/serialization.rs b/src/serialization.rs index beb8d43..afbf21c 100644 --- a/src/serialization.rs +++ b/src/serialization.rs @@ -49,12 +49,19 @@ pub fn write_brief_album_json( // to positive, it does not need a lot of precision. The trending score // is always between 0 and 1 though, it needs more digits for precision // near the end of the ranking. - r#","release_date":"{}","first_seen":"{}","discover_score":{:.2},"trending_score":{:.4},"for_now_score":{:.3}}}"#, + r#","release_date":"{}","first_seen":"{}","color":"{}""#, album.original_release_date, album.first_seen.format_iso8601(), - scores.discover, - scores.trending, - scores.for_now, + album.color, + )?; + write!( + w, + // The discover score can have large-ish magnitude and ranges from negative + // to positive, it does not need a lot of precision. The trending score + // is always between 0 and 1 though, it needs more digits for precision + // near the end of the ranking. + r#","discover_score":{:.2},"trending_score":{:.4},"for_now_score":{:.3}}}"#, + scores.discover, scores.trending, scores.for_now, )?; Ok(()) } From 8164904715554fa25e7b8c4dc773db8597c7101c Mon Sep 17 00:00:00 2001 From: Ruud van Asseldonk Date: Thu, 1 May 2025 19:01:42 +0200 Subject: [PATCH 05/14] Integrate album colors into the album list view Testing with Chromium devtools, caching disabled and severely throttling the network, this works amazingly well! The visual pop when a picture loads is still visible, but scrolling the list now feels much more solid, just the color itself already says so much about the album! --- app/src/AlbumListView.purs | 1 + app/src/Dom.js | 6 ++++++ app/src/Dom.purs | 5 +++++ app/src/Html.purs | 8 ++++++++ app/src/Model.purs | 3 +++ 5 files changed, 23 insertions(+) diff --git a/app/src/AlbumListView.purs b/app/src/AlbumListView.purs index 3870deb..60b36f0 100644 --- a/app/src/AlbumListView.purs +++ b/app/src/AlbumListView.purs @@ -141,6 +141,7 @@ renderAlbum :: (Event -> Aff Unit) -> Album -> Html Unit renderAlbum postEvent (Album album) = Html.div $ do Html.addClass "album" Html.img (Model.thumbUrl album.id) (album.title <> " by " <> album.artist) $ do + Html.setBackgroundColor album.color Html.addClass "thumb" Html.span $ do Html.addClass "title" diff --git a/app/src/Dom.js b/app/src/Dom.js index 6deaeae..c5b63be 100644 --- a/app/src/Dom.js +++ b/app/src/Dom.js @@ -207,6 +207,12 @@ export const setMaskImageImpl = function(mask, element) { } } +export const setBackgroundColorImpl = function(hexColor, element) { + return function() { + element.style.backgroundColor = hexColor; + } +} + export const setWidthImpl = function(width, element) { return function() { element.style.width = width; diff --git a/app/src/Dom.purs b/app/src/Dom.purs index 84361bf..eccc57a 100644 --- a/app/src/Dom.purs +++ b/app/src/Dom.purs @@ -33,6 +33,7 @@ module Dom , renderHtml , scrollIntoView , setAttribute + , setBackgroundColor , setHeight , setId , setMaskImage @@ -90,6 +91,7 @@ foreign import onScrollImpl :: Fn2 (Effect Unit) Element (Effect Unit) foreign import removeChildImpl :: Fn2 Element Element (Effect Unit) foreign import removeClassImpl :: Fn2 String Element (Effect Unit) foreign import setAttributeImpl :: Fn3 String String Element (Effect Unit) +foreign import setBackgroundColorImpl :: Fn2 String Element (Effect Unit) foreign import setHeightImpl :: Fn2 String Element (Effect Unit) foreign import setIdImpl :: Fn2 String Element (Effect Unit) foreign import setMaskImageImpl :: Fn2 String Element (Effect Unit) @@ -143,6 +145,9 @@ setMaskImage mask element = runFn2 setMaskImageImpl mask element setScrollTop :: Number -> Element -> Effect Unit setScrollTop off element = runFn2 setScrollTopImpl off element +setBackgroundColor :: String -> Element -> Effect Unit +setBackgroundColor hexColor element = runFn2 setBackgroundColorImpl hexColor element + setAttribute :: String -> String -> Element -> Effect Unit setAttribute attribute value element = runFn3 setAttributeImpl attribute value element diff --git a/app/src/Html.purs b/app/src/Html.purs index 59904f2..7f8f34f 100644 --- a/app/src/Html.purs +++ b/app/src/Html.purs @@ -28,6 +28,7 @@ module Html , p , removeClass , scrollIntoView + , setBackgroundColor , setHeight , setId , setMaskImage @@ -135,6 +136,13 @@ onInput callback = ReaderT $ \container -> scrollIntoView :: Html Unit scrollIntoView = ReaderT Dom.scrollIntoView +-- Set the background color to the given hex color. Should not include the +-- leading `#`, we prepend that here, to avoid having to store and transfer +-- the additional byte. +setBackgroundColor :: String -> Html Unit +setBackgroundColor hexColor = ReaderT $ + \container -> Dom.setBackgroundColor ("#" <> hexColor) container + setScrollTop :: Number -> Html Unit setScrollTop off = ReaderT $ \container -> Dom.setScrollTop off container diff --git a/app/src/Model.purs b/app/src/Model.purs index 3446ae7..afb21dd 100644 --- a/app/src/Model.purs +++ b/app/src/Model.purs @@ -138,6 +138,7 @@ newtype Album = Album , artistIds :: NonEmptyArray ArtistId , releaseDate :: String , firstSeen :: String + , color :: String , discoverScore :: Number , trendingScore :: Number , forNowScore :: Number @@ -155,6 +156,7 @@ instance decodeJsonAlbum :: DecodeJson Album where artist <- Json.getField obj "artist" releaseDate <- Json.getField obj "release_date" firstSeen <- Json.getField obj "first_seen" + color <- Json.getField obj "color" discoverScore <- Json.getField obj "discover_score" trendingScore <- Json.getField obj "trending_score" forNowScore <- Json.getField obj "for_now_score" @@ -165,6 +167,7 @@ instance decodeJsonAlbum :: DecodeJson Album where , artistIds , releaseDate , firstSeen + , color , discoverScore , trendingScore , forNowScore From faf32bc58fa14a8643b9dda9ccf52d0c61f5d343 Mon Sep 17 00:00:00 2001 From: Ruud van Asseldonk Date: Fri, 2 May 2025 11:54:24 +0200 Subject: [PATCH 06/14] Thread searches through the event loop --- app/src/Event.purs | 16 ++++- app/src/Search.purs | 141 +++++++++++++++++++++++++++++--------------- app/src/State.purs | 27 ++++++++- 3 files changed, 134 insertions(+), 50 deletions(-) diff --git a/app/src/Event.purs b/app/src/Event.purs index b7217cc..d62b527 100644 --- a/app/src/Event.purs +++ b/app/src/Event.purs @@ -8,13 +8,14 @@ module Event ( Event (..) , HistoryMode (..) + , SearchSeq (..) , SortField (..) , SortDirection (..) , SortMode ) where import Prelude -import Model (Album, QueuedTrack, ScanStatus) +import Model (Album, QueuedTrack, ScanStatus, SearchResults) import Navigation (Location) data HistoryMode @@ -36,6 +37,13 @@ data SortDirection type SortMode = { field :: SortField, direction :: SortDirection } +-- Search queries contain a sequence number, so we can correlate results with +-- searches and discard results for outdated queries. +data SearchSeq = SearchSeq Int + +derive instance searchSeqEq :: Eq SearchSeq +derive instance searchSeqOrd :: Ord SearchSeq + data Event = Initialize (Array Album) (Array QueuedTrack) | UpdateQueue (Array QueuedTrack) @@ -57,5 +65,11 @@ data Event | UpdateProgress -- The user typed the keyboard shortcut for 'search'. | SearchKeyPressed + -- The user searched for this query. Queries contain a sequence number + -- generated by the search box itself, so that we process results in the + -- right order, even when results or even query events arrive in a different + -- order. + | Search SearchSeq String + | SearchResult SearchSeq SearchResults -- A new scan status was received. | UpdateScanStatus ScanStatus diff --git a/app/src/Search.purs b/app/src/Search.purs index aa803dd..6c78c9c 100644 --- a/app/src/Search.purs +++ b/app/src/Search.purs @@ -10,35 +10,50 @@ module Search , new , focus , clear + , renderSearchResults ) where import Control.Monad.Reader.Class (ask, local) import Data.Array as Array import Data.Foldable (for_) +import Data.Maybe (Maybe (..)) import Data.String.CodeUnits as CodeUnits import Effect (Effect) import Effect.Aff (Aff, launchAff_) import Effect.Class (liftEffect) -import Effect.Class.Console as Console +import Foreign.Object (Object) +import Foreign.Object as Object import Prelude import Dom (Element) import Dom as Dom -import Event (Event, HistoryMode (RecordHistory)) +import Event (Event, HistoryMode (RecordHistory), SearchSeq (SearchSeq)) import Event as Event import Html (Html) import Html as Html -import Model (SearchArtist (..), SearchAlbum (..), SearchTrack (..)) +import Model (Album (..), AlbumId (..), SearchArtist (..), SearchAlbum (..), SearchResults (..), SearchTrack (..)) import Model as Model import Navigation as Navigation +import Var as Var type SearchElements = { searchBox :: Element , resultBox :: Element } -renderSearchArtist :: (Event -> Aff Unit) -> SearchArtist -> Html Unit -renderSearchArtist postEvent (SearchArtist artist) = do +-- Look up the album by id in the album collection. If we found it, set the +-- background color to that album's color. Intended to be curried. +setAlbumColor :: Object Album -> AlbumId -> Html Unit +setAlbumColor albumsById (AlbumId id) = case Object.lookup id albumsById of + Nothing -> pure unit + Just (Album album) -> Html.setBackgroundColor album.color + +renderSearchArtist + :: (Event -> Aff Unit) + -> (AlbumId -> Html Unit) + -> SearchArtist + -> Html Unit +renderSearchArtist postEvent setColor (SearchArtist artist) = do Html.li $ do Html.addClass "artist" Html.div $ do @@ -46,19 +61,26 @@ renderSearchArtist postEvent (SearchArtist artist) = do Html.text artist.name Html.div $ do Html.addClass "discography" - for_ artist.albums $ \albumId -> do - Html.img (Model.thumbUrl albumId) ("An album by " <> artist.name) $ pure unit + for_ artist.albums $ \albumId -> Html.img + (Model.thumbUrl albumId) + ("An album by " <> artist.name) + (setColor albumId) Html.onClick $ launchAff_ $ postEvent $ Event.NavigateTo (Navigation.Artist $ artist.id) RecordHistory -- TODO: Deduplicate between here and album component. -renderSearchAlbum :: (Event -> Aff Unit) -> SearchAlbum -> Html Unit -renderSearchAlbum postEvent (SearchAlbum album) = do +renderSearchAlbum + :: (Event -> Aff Unit) + -> (AlbumId -> Html Unit) + -> SearchAlbum + -> Html Unit +renderSearchAlbum postEvent setColor (SearchAlbum album) = do Html.li $ do Html.addClass "album" Html.img (Model.thumbUrl album.id) (album.title <> " by " <> album.artist) $ do + setColor album.id Html.addClass "thumb" Html.span $ do Html.addClass "title" @@ -77,11 +99,16 @@ renderSearchAlbum postEvent (SearchAlbum album) = do (Navigation.Album $ album.id) RecordHistory -renderSearchTrack :: (Event -> Aff Unit) -> SearchTrack -> Html Unit -renderSearchTrack postEvent (SearchTrack track) = do +renderSearchTrack + :: (Event -> Aff Unit) + -> (AlbumId -> Html Unit) + -> SearchTrack + -> Html Unit +renderSearchTrack postEvent setColor (SearchTrack track) = do Html.li $ do Html.addClass "track" Html.img (Model.thumbUrl track.albumId) track.album $ do + setColor track.albumId Html.addClass "thumb" Html.span $ do Html.addClass "title" @@ -107,47 +134,65 @@ new postEvent = do ask local (const searchBox) $ do + -- We maintain the search sequence number here, because the input handler + -- runs as an Effect rather than Aff, so we can be sure that the sequence + -- numbers match the order of the input events. In the main loop, we only + -- process search results if they are for a newer search than the last one + -- we processed, to ensure that a slow search query that arrives later does + -- not overwrite current search results. (That can happen especially at the + -- beginning, as a short query string matches more, so the response is + -- larger and takes longer to serialize/transfer/deserialize.) + searchSeq <- liftEffect $ Var.create 0 Html.onInput $ \query -> do - -- Fire off the search query and render it when it comes in. - -- TODO: Pass these through the event loop, to ensure that the result - -- matches the query, and perhaps for caching as well. - launchAff_ $ do - Model.SearchResults result <- Model.search query - Console.log $ "Received artists: " <> (show $ Array.length $ result.artists) - Console.log $ "Received albums: " <> (show $ Array.length $ result.albums) - Console.log $ "Received tracks: " <> (show $ Array.length $ result.tracks) - liftEffect $ do - Html.withElement resultBox $ do - Html.clear - Html.div $ do - Html.addClass "search-results-list" - - when (not $ Array.null result.artists) $ do - Html.div $ do - Html.setId "search-artists" - Html.h2 $ Html.text "Artists" - -- Limit the number of results rendered at once to keep search - -- responsive. TODO: Render overflow button. - Html.ul $ for_ (Array.take 10 result.artists) $ renderSearchArtist postEvent - - when (not $ Array.null result.albums) $ do - Html.div $ do - Html.setId "search-albums" - Html.h2 $ Html.text "Albums" - -- Limit the number of results rendered at once to keep search - -- responsive. TODO: Render overflow button. - Html.ul $ for_ (Array.take 25 result.albums) $ renderSearchAlbum postEvent - - when (not $ Array.null result.tracks) $ do - Html.div $ do - Html.setId "search-tracks" - Html.h2 $ Html.text "Tracks" - -- Limit the number of results rendered at once to keep search - -- responsive. TODO: Render overflow button. - Html.ul $ for_ (Array.take 25 result.tracks) $ renderSearchTrack postEvent + currentSeq <- Var.get searchSeq + let nextSeq = currentSeq + 1 + Var.set searchSeq nextSeq + launchAff_ $ postEvent $ Event.Search (SearchSeq nextSeq) query pure $ { searchBox, resultBox } +renderSearchResults + :: (Event -> Aff Unit) + -> SearchElements + -> Object Album + -> SearchResults + -> Effect Unit +renderSearchResults postEvent elements albumsById (SearchResults result) = + let + setColor = setAlbumColor albumsById + in + Html.withElement elements.resultBox $ do + Html.clear + Html.div $ do + Html.addClass "search-results-list" + + when (not $ Array.null result.artists) $ do + Html.div $ do + Html.setId "search-artists" + Html.h2 $ Html.text "Artists" + -- Limit the number of results rendered at once to keep search + -- responsive. TODO: Render overflow button. + Html.ul $ for_ (Array.take 10 result.artists) $ + renderSearchArtist postEvent setColor + + when (not $ Array.null result.albums) $ do + Html.div $ do + Html.setId "search-albums" + Html.h2 $ Html.text "Albums" + -- Limit the number of results rendered at once to keep search + -- responsive. TODO: Render overflow button. + Html.ul $ for_ (Array.take 25 result.albums) $ + renderSearchAlbum postEvent setColor + + when (not $ Array.null result.tracks) $ do + Html.div $ do + Html.setId "search-tracks" + Html.h2 $ Html.text "Tracks" + -- Limit the number of results rendered at once to keep search + -- responsive. TODO: Render overflow button. + Html.ul $ for_ (Array.take 25 result.tracks) $ + renderSearchTrack postEvent setColor + clear :: SearchElements -> Effect Unit clear elements = do Dom.setValue "" elements.searchBox diff --git a/app/src/State.purs b/app/src/State.purs index f45a684..5d214d3 100644 --- a/app/src/State.purs +++ b/app/src/State.purs @@ -39,7 +39,7 @@ import AlbumView (AlbumViewState) import AlbumView as AlbumView import Dom (Element) import Dom as Dom -import Event (Event, HistoryMode, SortMode, SortField (..), SortDirection (..)) +import Event (Event, HistoryMode, SearchSeq (..), SortMode, SortField (..), SortDirection (..)) import Event as Event import History as History import Html as Html @@ -93,6 +93,7 @@ type AppState = , navBar :: NavBarState , statusBar :: StatusBarState , location :: Location + , lastSearchRendered :: SearchSeq , lastArtists :: Array ArtistId , lastAlbum :: Maybe AlbumId , currentTrack :: Maybe QueueId @@ -213,6 +214,7 @@ new bus = do , statusBar: statusBar , location: Navigation.Library , lastArtists: [] + , lastSearchRendered: SearchSeq 0 , lastAlbum: Nothing , currentTrack: Nothing , prefetchedThumb: Nothing @@ -584,6 +586,29 @@ handleEvent event state = case event of _notSearch -> handleEvent (Event.NavigateTo Navigation.Search Event.RecordHistory) state + Event.Search seq query -> do + -- We fork a fiber to run the search query and make it post an event when + -- it's done, we don't block the main event loop on it, so that other logic + -- can continue even when a search is in progress, and also so that multiple + -- search queries can run in parallel when the user is typing fast. + _fiber <- Aff.forkAff $ do + result <- Model.search query + state.postEvent $ Event.SearchResult seq result + pure state + + Event.SearchResult seq results -> + -- We only render the search result if it is for a newer query than what + -- we already rendered. (But it still may not be for the latest query we + -- sent!) This ensures that slow responses get dropped, rather than + -- overwriting later results that we already displayed. + if seq <= state.lastSearchRendered then pure state else do + liftEffect $ Search.renderSearchResults + state.postEvent + state.elements.search + state.albumsById + results + pure $ state { lastSearchRendered = seq } + beforeSwitchPane :: AppState -> Aff AppState beforeSwitchPane state = case state.location of From d28464bfcc6c3d5a46a93c0cdf4d71cf1de54667 Mon Sep 17 00:00:00 2001 From: Ruud van Asseldonk Date: Fri, 2 May 2025 22:02:09 +0200 Subject: [PATCH 07/14] Add reminder about possible optimization --- app/src/Search.purs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/src/Search.purs b/app/src/Search.purs index 6c78c9c..f6a526a 100644 --- a/app/src/Search.purs +++ b/app/src/Search.purs @@ -162,6 +162,12 @@ renderSearchResults postEvent elements albumsById (SearchResults result) = setColor = setAlbumColor albumsById in Html.withElement elements.resultBox $ do + -- TODO: On low-power devices, there can be a brief 1-frame flicker for + -- images to load after extending the query, even if the results were + -- visible previously -- presumably because we delete the nodes, so + -- Chromium deallocates the images, and has to decode them again when we + -- promptly add the nodes again. We could do better by reycling + -- those image nodes. Html.clear Html.div $ do Html.addClass "search-results-list" From e3c95b3d42ccc3f00821690b39d2b7496b0ba307 Mon Sep 17 00:00:00 2001 From: Ruud van Asseldonk Date: Fri, 2 May 2025 22:15:10 +0200 Subject: [PATCH 08/14] Autoformat the prim module Helix formats it on save, and it's annoying to have to "git add -p" all the time. Time to reformat it. --- src/prim.rs | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/prim.rs b/src/prim.rs index 0675b18..2064c25 100644 --- a/src/prim.rs +++ b/src/prim.rs @@ -88,6 +88,7 @@ impl TrackId { } #[inline(always)] + #[rustfmt::skip] pub fn new(album_id: AlbumId, disc_number: u8, track_number: u8) -> TrackId { // Confirm that the numbers are in range so we don't discard anything. debug_assert_eq!(album_id.0 & 0xfff0_0000_0000_0000, 0); @@ -163,17 +164,16 @@ impl Lufs { pub fn new(centi_loudness_units: i16) -> Lufs { Lufs( std::num::NonZeroI16::new(centi_loudness_units) - .expect("A value of 0.0 LUFS is not allowed, use -0.01 LUFS instead.") + .expect("A value of 0.0 LUFS is not allowed, use -0.01 LUFS instead."), ) } - /// Construct a LUFS value from a float. This is in LUFS, not in centi-LUFS /// like `Lufs::new` is. pub fn from_f64(loudness_units: f64) -> Lufs { Lufs( std::num::NonZeroI16::new((loudness_units * 100.0) as i16) - .expect("A value of 0.0 LUFS is not allowed, use -0.01 LUFS instead.") + .expect("A value of 0.0 LUFS is not allowed, use -0.01 LUFS instead."), ) } } @@ -275,11 +275,7 @@ impl Date { debug_assert!(year <= 9999); debug_assert!(month <= 12); debug_assert!(day <= 31); - Date { - year, - month, - day, - } + Date { year, month, day } } } @@ -292,7 +288,9 @@ pub struct Instant { impl Instant { pub fn from_iso8601(t: &str) -> Option { let dt = DateTime::parse_from_rfc3339(t).ok()?; - let result = Instant { posix_seconds_utc: dt.timestamp() }; + let result = Instant { + posix_seconds_utc: dt.timestamp(), + }; Some(result) } @@ -306,7 +304,8 @@ impl Instant { pub fn format_iso8601(&self) -> String { use chrono::SecondsFormat; let use_z = true; - self.to_datetime().to_rfc3339_opts(SecondsFormat::Secs, use_z) + self.to_datetime() + .to_rfc3339_opts(SecondsFormat::Secs, use_z) } } @@ -381,9 +380,13 @@ pub struct Artist { impl fmt::Display for Date { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "{:04}", self.year)?; - if self.month == 0 { return Ok(()) } + if self.month == 0 { + return Ok(()); + } write!(f, "-{:02}", self.month)?; - if self.day == 0 { return Ok(()) } + if self.day == 0 { + return Ok(()); + } write!(f, "-{:02}", self.day) } } From 2fd294f94eedd029f8171e0b0bddcbea79d5bb2f Mon Sep 17 00:00:00 2001 From: Ruud van Asseldonk Date: Fri, 2 May 2025 22:26:31 +0200 Subject: [PATCH 09/14] Remove color alpha hack from formatting Better to deal with this at parse time and store things uniformly in the database. This does require repopulating the database. --- src/prim.rs | 14 +++++++++----- src/thumb_gen.rs | 7 ++++--- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/prim.rs b/src/prim.rs index 2064c25..7306875 100644 --- a/src/prim.rs +++ b/src/prim.rs @@ -320,23 +320,27 @@ pub struct AlbumArtistsRef { } /// An sRGB color (used to approximate thumbnails). +/// +/// The value is a 24-bit color, with R in the most significant bits and B in +/// the least significant bits. The 8 most significant bits are unused and +/// should be set to zero. #[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)] pub struct Color(u32); impl Color { + /// Parse an RGB hex string. + /// + /// The input should be 6 hex characters, we don't support alpha. #[inline] pub fn parse(src: &str) -> Option { + debug_assert_eq!(src.len(), 6); u32::from_str_radix(src, 16).ok().map(Color) } } impl fmt::Display for Color { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - if self.0 > 0xffffff { - write!(f, "{:06x}", self.0 >> 8) - } else { - write!(f, "{:06x}", self.0) - } + write!(f, "{:06x}", self.0) } } diff --git a/src/thumb_gen.rs b/src/thumb_gen.rs index ae09f2a..9a212b0 100644 --- a/src/thumb_gen.rs +++ b/src/thumb_gen.rs @@ -292,9 +292,10 @@ impl<'a> GenThumb<'a> { .take(8) .read_to_string(&mut color_buf)?; - // TODO: Sometimes the color ends in ff, sometimes it does not. What - // gives? We need to normalize this. - let color = Color::parse(&color_buf).ok_or(Error::CommandError( + // Sometimes Magick returns the color in 8 hex digits (ending in ff), + // including alpha, sometimes it returns only 6 charactrs. RGB are the + // first three, so we ignore any trailer. + let color = Color::parse(&color_buf[..6]).ok_or(Error::CommandError( "ImageMagick did not return a valid color.", None, ))?; From 5ef5b9caf1cd7960689809033dd8d0b756cdd25d Mon Sep 17 00:00:00 2001 From: Ruud van Asseldonk Date: Fri, 2 May 2025 22:44:25 +0200 Subject: [PATCH 10/14] Reload index at the end This ensures that we pick up the thumbnail colors after the thumbs are computed, but also the loudness, which previously was not properly reloaded! That meant that loudness normalization didn't work for new albums if you never restarted Musium or re-scanned the collection! --- app/src/About.purs | 1 + app/src/Model.purs | 2 ++ src/main.rs | 2 +- src/scan.rs | 21 +++++++++++++++++++-- src/serialization.rs | 1 + 5 files changed, 24 insertions(+), 3 deletions(-) diff --git a/app/src/About.purs b/app/src/About.purs index 5eb021d..6df1a71 100644 --- a/app/src/About.purs +++ b/app/src/About.purs @@ -101,6 +101,7 @@ updateScanStatus elems (ScanStatus status) = ScanPreProcessingThumbnails -> "Discovering existing thumbnails …" ScanGeneratingThumbnails -> "Generating new thumbnails …" ScanLoadingThumbnails -> "Loading thumbnails …" + ScanReloading -> "Reloading data …" ScanDone -> "Scan complete" valuePair (show status.filesDiscovered) "files discovered" diff --git a/app/src/Model.purs b/app/src/Model.purs index afb21dd..012acb3 100644 --- a/app/src/Model.purs +++ b/app/src/Model.purs @@ -297,6 +297,7 @@ data ScanStage | ScanPreProcessingThumbnails | ScanGeneratingThumbnails | ScanLoadingThumbnails + | ScanReloading | ScanDone derive instance eqScanStage :: Eq ScanStage @@ -315,6 +316,7 @@ instance decodeJsonScanStage :: DecodeJson ScanStage where "preprocessing_thumbnails" -> pure ScanPreProcessingThumbnails "generating_thumbnails" -> pure ScanGeneratingThumbnails "loading_thumbnails" -> pure ScanLoadingThumbnails + "reloading" -> pure ScanReloading "done" -> pure ScanDone _ -> Left $ UnexpectedValue json diff --git a/src/main.rs b/src/main.rs index e4d96d2..192294a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -209,7 +209,7 @@ fn run_scan(config: &Config) -> Result<()> { // its stderr, but this allows the warning to at least be visible // very briefly. let up_clear = "\x1b[F\x1b[K"; - write!(lock, "{0}{0}{0}{0}{0}{0}{1}", up_clear, status).unwrap(); + write!(lock, "{0}{0}{0}{0}{0}{0}{0}{1}", up_clear, status).unwrap(); lock.flush().unwrap(); } } diff --git a/src/scan.rs b/src/scan.rs index 32c48b5..780624a 100644 --- a/src/scan.rs +++ b/src/scan.rs @@ -56,7 +56,7 @@ struct FileMetaId(i64); #[derive(Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd)] pub enum ScanStage { /// Discovering flac files in the library path. - Discovering= 0, + Discovering = 0, /// Determining which files to process. /// @@ -98,8 +98,11 @@ pub enum ScanStage { /// `status.files_to_process_thumbnails` is now final. LoadingThumbnails = 8, + /// Reloading data computed by previous stages. + Reloading = 9, + /// Done. - Done = 9, + Done = 10, } /// Counters to report progress during scanning. @@ -203,6 +206,7 @@ impl fmt::Display for Status { "{} Loading thumbnails", indicator(ScanStage::LoadingThumbnails), )?; + writeln!(f, "{} Reloading new data", indicator(ScanStage::Reloading))?; Ok(()) } } @@ -674,6 +678,19 @@ pub fn run_scan_in_thread( thumb_cache_var.set(thumb_cache_arc); } + // At the end, we refresh the index itself *again*. This is because + // thumbnail generation and loudness measurement insert their + // results into the database "async" (the index is already usable), + // but they only get incorporated into the index at this stage. + // TODO: This is a bit wasteful, we could instead make a copy of the + // existing index, and reload only the loudness and thumbnail color + // fields from the database. + status.stage = ScanStage::Reloading; + tx.send(status).unwrap(); + let mut db_tx = db.begin()?; + let (index, _builder) = MemoryMetaIndex::from_database(&mut db_tx)?; + index_var.set(Arc::new(index)); + status.stage = ScanStage::Done; tx.send(status).unwrap(); Ok(()) diff --git a/src/serialization.rs b/src/serialization.rs index afbf21c..60d2a5c 100644 --- a/src/serialization.rs +++ b/src/serialization.rs @@ -360,6 +360,7 @@ pub fn write_scan_status_json( ScanStage::PreProcessingThumbnails => "preprocessing_thumbnails", ScanStage::GeneratingThumbnails => "generating_thumbnails", ScanStage::LoadingThumbnails => "loading_thumbnails", + ScanStage::Reloading => "reloading", ScanStage::Done => "done", }; From a22a7392ad7ff853265ef537963c6b37ccd8b16e Mon Sep 17 00:00:00 2001 From: Ruud van Asseldonk Date: Fri, 2 May 2025 22:57:16 +0200 Subject: [PATCH 11/14] Handle when Imagemagick outputs a 3-digit color --- src/thumb_gen.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/thumb_gen.rs b/src/thumb_gen.rs index 9a212b0..58406bb 100644 --- a/src/thumb_gen.rs +++ b/src/thumb_gen.rs @@ -294,7 +294,17 @@ impl<'a> GenThumb<'a> { // Sometimes Magick returns the color in 8 hex digits (ending in ff), // including alpha, sometimes it returns only 6 charactrs. RGB are the - // first three, so we ignore any trailer. + // first three, so we ignore any trailer. Sometimes though, it returns + // only 3, presumably a shortcut like CSS allows, in that case we need + // to duplicate every byte. + if color_buf.len() == 3 { + let mut new_buf = String::with_capacity(6); + for ch in color_buf.chars() { + new_buf.push(ch); + new_buf.push(ch); + } + color_buf = new_buf; + } let color = Color::parse(&color_buf[..6]).ok_or(Error::CommandError( "ImageMagick did not return a valid color.", None, From 5e4a9e16199eef17bafc8c5276b124c2be595597 Mon Sep 17 00:00:00 2001 From: Ruud van Asseldonk Date: Fri, 2 May 2025 23:16:27 +0200 Subject: [PATCH 12/14] Fix bug due to SQLite's dynamic typing It interprets the string '413e42' as the number 4.13e+44. Bizarre. --- src/database.rs | 4 ++-- src/database.sql | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/database.rs b/src/database.rs index da6f853..8e6cbb4 100644 --- a/src/database.rs +++ b/src/database.rs @@ -337,9 +337,9 @@ pub fn ensure_schema_exists(tx: &mut Transaction) -> Result<()> { -- storing the 24-bit integer. The data blob is kilobytes anyway, a few bytes -- here makes little difference, and it makes the database more readable for -- humans. - , color string not null + , color text not null , data blob not null - ); + ) strict; "#; let statement = match tx.statements.entry(sql.as_ptr()) { Occupied(entry) => entry.into_mut(), diff --git a/src/database.sql b/src/database.sql index e638275..055cab6 100644 --- a/src/database.sql +++ b/src/database.sql @@ -147,9 +147,9 @@ create table if not exists thumbnails -- storing the 24-bit integer. The data blob is kilobytes anyway, a few bytes -- here makes little difference, and it makes the database more readable for -- humans. -, color string not null +, color text not null , data blob not null -); +) strict; -- @end ensure_schema_exists -- @query insert_file(metadata: InsertFile) ->1 i64 From 8cd49a3b021830097145f23768765756d22d16ab Mon Sep 17 00:00:00 2001 From: Ruud van Asseldonk Date: Fri, 2 May 2025 23:21:12 +0200 Subject: [PATCH 13/14] Update changelog with album color changes And also the search fix that I implemented as a drive-by now that I had to change that logic anyway. --- docs/changelog.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/docs/changelog.md b/docs/changelog.md index 7697157..b16abbe 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -22,6 +22,16 @@ Musium versions are named `MAJOR.MINOR.PATCH`. ## Next +**Compatibility:** + + * The schema of the thumbnail table changed, it gained a `color` column. + The easiest way to deal with this is to drop the table with + `sqlite3 db.sqlite3 'drop table thumbnails;'` and then re-index with + `musium scan`. This will re-compute all thumbnails and analyze their colors, + which may take a while. + +Features: + * A new sort option is available in the album list: _For Now_. This ranking shows you albums that you played at similar times of the day, week, and year in the past. For example, if you tend to listen to more quiet music in the @@ -33,8 +43,21 @@ Musium versions are named `MAJOR.MINOR.PATCH`. to show the most relevant suggestions. * The queue tab in the webinterface is now implemented, including buttons to shuffle and clear the queue. + * Musium now computes and saves the dominant color of album cover art. This + color is used as a placeholder in the webinterface, to reduce visual flicker + when thumbnails are loading. * Add support for Czech diacritics in text normalization. +Bugfixes: + + * When a scan discovered new files and computed loudness, that loudness data + would only be used after a subsequent scan or restart. Now we properly apply + the loudness measurement directly after the scan. + * When a search query in the webinterface was slow to load (which tends to be + the case initially during search, because short queries have many matches), + the results for an earlier query could in rare cases replace the search + results for a later query. Now such late results are properly discarded. + ## 0.16.0 Released 2025-02-02. From ea6cba97bdb2a38bebe2759d0094b74e8241e079 Mon Sep 17 00:00:00 2001 From: Ruud van Asseldonk Date: Sat, 3 May 2025 11:33:46 +0200 Subject: [PATCH 14/14] Clarify changelog --- docs/changelog.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/changelog.md b/docs/changelog.md index b16abbe..a9252f7 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -24,14 +24,16 @@ Musium versions are named `MAJOR.MINOR.PATCH`. **Compatibility:** - * The schema of the thumbnail table changed, it gained a `color` column. - The easiest way to deal with this is to drop the table with - `sqlite3 db.sqlite3 'drop table thumbnails;'` and then re-index with - `musium scan`. This will re-compute all thumbnails and analyze their colors, - which may take a while. + * The schema of the thumbnail table changed. The easiest way to deal with this + is to drop the table with `sqlite3 db.sqlite3 'drop table thumbnails;'` and + then re-index with `musium scan`, where `db.sqlite3` is your database [as + configured](configuration.md#db_path). This will re-compute all thumbnails + and analyze their colors, which may take a while. Features: + * The queue tab in the webinterface is now implemented, including buttons to + shuffle and clear the queue. * A new sort option is available in the album list: _For Now_. This ranking shows you albums that you played at similar times of the day, week, and year in the past. For example, if you tend to listen to more quiet music in the @@ -41,8 +43,6 @@ Features: recent popularity to show you albums worth listening to again. Like the _For Now_ ranking, it takes into account the time of the day, week, and year, to show the most relevant suggestions. - * The queue tab in the webinterface is now implemented, including buttons to - shuffle and clear the queue. * Musium now computes and saves the dominant color of album cover art. This color is used as a placeholder in the webinterface, to reduce visual flicker when thumbnails are loading.