From ca6e77b1d5e9a047c4dd4623b6a767bf05b9c645 Mon Sep 17 00:00:00 2001 From: darrell-k Date: Mon, 16 Mar 2026 17:54:19 +0000 Subject: [PATCH 1/4] Alternative artwork/box set approach Signed-off-by: darrell-k --- Slim/Control/Commands.pm | 1 + Slim/Music/Artwork.pm | 258 +++++++++++++++++++++++++++++---------- Slim/Schema.pm | 22 +++- Slim/Schema/Track.pm | 20 ++- 4 files changed, 231 insertions(+), 70 deletions(-) diff --git a/Slim/Control/Commands.pm b/Slim/Control/Commands.pm index 5089ecd4f49..de5c38981bf 100644 --- a/Slim/Control/Commands.pm +++ b/Slim/Control/Commands.pm @@ -2779,6 +2779,7 @@ sub rescanCommand { types => 'audio', recursive => 0, } ) if scalar @paths; + Slim::Music::Artwork->precacheAllArtwork if scalar @paths; } else { # In-process scan diff --git a/Slim/Music/Artwork.pm b/Slim/Music/Artwork.pm index 790d317083b..61d0fb74abd 100644 --- a/Slim/Music/Artwork.pm +++ b/Slim/Music/Artwork.pm @@ -23,7 +23,8 @@ use Digest::MD5 qw(md5_hex); use File::Basename qw(basename dirname); use File::Slurp; use File::Path qw(mkpath rmtree); -use File::Spec::Functions qw(catfile catdir); +use File::Spec::Functions qw(catdir canonpath splitdir); +use List::Util qw(min); use Path::Class; use Scalar::Util qw(blessed); use Tie::Cache::LRU; @@ -58,25 +59,34 @@ my %findArtCache; # Public class methods sub findStandaloneArtwork { - my ( $class, $trackAttributes, $deferredAttributes, $dirurl ) = @_; + my ( $class, $trackAttributes, $deferredAttributes, $dirurl, $commonParent ) = @_; + + my $discNumber = $trackAttributes->{disc}; return 0 if !Slim::Music::Info::isFileURL($dirurl); my $isInfo = main::INFOLOG && $log->is_info; - my $art = $findArtCache{$dirurl}; - - # Files to look for - my @files = qw(cover folder album thumb); - - # User-defined artwork format - my $coverFormat = $prefs->get('coverArt'); + my $discCacheKey = $dirurl; + $discCacheKey .= "|disc:$discNumber" if $discNumber; + my $art = $findArtCache{$discCacheKey}; if ( !defined $art ) { my $parentDir = Path::Class::dir( Slim::Utils::Misc::pathFromFileURL($dirurl) ); + # Files to look for + my @files = ( _discSpecificArtworkNames($discNumber||0), qw(cover folder album thumb) ); + + # User-defined artwork format + my $coverFormat = $prefs->get('coverArt'); + # coverArt/artfolder pref support - if ( $coverFormat ) { + + # if called with the commonParent parameter instead of a Track attribute hash, only accept a hardcoded user pref. + if ($commonParent && $coverFormat && $coverFormat !~ /^%/) { + push @files, $coverFormat; + } + elsif ( $coverFormat ) { # If the user has specified a pattern to match the artwork on, we need # to generate that pattern. This is nasty. if ( $coverFormat =~ /^%(.*?)(\..*?){0,1}$/ ) { @@ -129,6 +139,8 @@ sub findStandaloneArtwork { if ( !$art ) { # Find all image files in the file directory + $parentDir = $commonParent if $commonParent; + my $types = qr/\.(?:jpe?g|png|gif)$/i; my $files = File::Next::files( { @@ -137,6 +149,7 @@ sub findStandaloneArtwork { }, $parentDir ); my @found; + while ( my $image = $files->() ) { push @found, $image; } @@ -147,7 +160,15 @@ sub findStandaloneArtwork { $art = $preferred[0]; } else { - $art = $found[0] || 0; + # exclude disc-specific artwork (if we had wanted them, they'd have been picked up already, above) + my @discSpecificNames = _discSpecificArtworkNames(); + my $excludedList = join( '|', @discSpecificNames ); + if ( my @preferred = grep { basename($_) !~ qr/^(?:$excludedList)\w+\./i } @found ) { + $art = $preferred[0]; + } + else { + $art = $found[0] || 0; + } } } @@ -156,7 +177,7 @@ sub findStandaloneArtwork { # files in a single directory with different artwork if ( !$coverFormat ) { %findArtCache = () if scalar keys %findArtCache > 32; - $findArtCache{$dirurl} = $art; + $findArtCache{$discCacheKey} = $art; } } @@ -210,27 +231,34 @@ sub updateStandaloneArtwork { tracks.coverid, albums.id AS albumid, albums.title AS album_title, - albums.artwork AS album_artwork + albums.discc AS disc_count, + albums.artwork AS album_artwork, + -- This is the Sqlite equivalent of dirname() + rtrim(tracks.url, replace(tracks.url, '/', '')) AS dirname, + tracks.disc FROM tracks JOIN albums ON (tracks.album = albums.id) WHERE $where - GROUP BY tracks.cover, tracks.album + GROUP BY tracks.cover, tracks.album, dirname, tracks.disc + ORDER BY tracks.album, tracks.disc, dirname }; + my $sth_album_urls = $dbh->prepare("SELECT url FROM tracks WHERE tracks.album = ? AND tracks.url LIKE 'file://%'"); + my $sth_update_tracks = $dbh->prepare( qq{ UPDATE tracks SET cover = ?, coverid = ?, cover_cached = NULL - WHERE album = ? + WHERE album = ? AND url like ? AND disc = ? } ); - my $sth_update_albums = $dbh->prepare( qq{ - UPDATE albums - SET artwork = ? - WHERE id = ? + my $sth_update_track_cover_cached_null = $dbh->prepare( qq{ + UPDATE tracks + SET cover_cached = NULL + WHERE album = ? } ); my ($count) = $dbh->selectrow_array( qq{ - SELECT COUNT(*) FROM ( $sql ) AS t1 + SELECT COUNT(DISTINCT albumid) FROM ( $sql ) AS t1 } ); $log->error("Starting updateStandaloneArtwork for $count albums"); @@ -251,15 +279,20 @@ sub updateStandaloneArtwork { my $sth = $dbh->prepare($sql); $sth->execute; - my ($trackid, $url, $cover, $coverid, $albumid, $album_title, $album_artwork); - $sth->bind_columns(\$trackid, \$url, \$cover, \$coverid, \$albumid, \$album_title, \$album_artwork); + my ($trackid, $url, $cover, $coverid, $albumid, $album_title, $disc_count, $album_artwork, $dirname, $disc_number); + $sth->bind_columns(\$trackid, \$url, \$cover, \$coverid, \$albumid, \$album_title, \$disc_count, \$album_artwork, \$dirname, \$disc_number); my $i = 0; my $t = 0; + my $previousAlbum; my $work = sub { if ( $sth->fetch ) { + my $newCoverId; + my $newCover; + my $newAlbumCover; + my $urlDir = dirname(Slim::Utils::Misc::pathFromFileURL($url)) if $url =~ /^file:/; $progress->update( $album_title ); @@ -268,18 +301,49 @@ sub updateStandaloneArtwork { $t = time + 5; } + if ( $previousAlbum != $albumid ) { + $previousAlbum = $albumid; + if ( $url =~ /^file:/ ) { + my @currentAlbumUrls = @{$dbh->selectall_arrayref($sth_album_urls, {}, $albumid)}; + $sth_album_urls->finish; + my @paths = Slim::Utils::Misc::uniq(map( dirname(Slim::Utils::Misc::pathFromFileURL($_->[0])), @currentAlbumUrls )); + + my $commonParent; + if (@paths == 1) { + $commonParent = $paths[0]; + } + elsif (scalar @paths) { + $commonParent = _findCommonParent(\@paths); + } + + if ( my $parentArtwork = Slim::Music::Artwork->findStandaloneArtwork(undef, undef, Slim::Utils::Misc::fileURLFromPath($urlDir), $commonParent) ) { + my $parentUrl = Slim::Utils::Misc::fileURLFromPath($parentArtwork); + $newAlbumCover = Slim::Music::Artwork->generateImageId({ + image => $parentArtwork, + url => $parentUrl, + }); + } + # parent artwork changed so make sure cache update is triggered + if ( $newAlbumCover ne $album_artwork ) { + $sth_update_track_cover_cached_null->execute($albumid); + } + } + } + # check for updated artwork if ( $cover ) { - $newCoverId = Slim::Schema::Track->generateCoverId({ - cover => $cover, - url => $url, + $newCoverId = Slim::Music::Artwork->generateImageId({ + image => $cover, + url => Slim::Utils::Misc::fileURLFromPath($cover), }); + $newCover = $cover if $newCoverId; } # check for new artwork to unchanged file # - !$cover: there wasn't any previously # - !$newCoverId: existing file has disappeared - if ( (!$cover || !$newCoverId) && Slim::Music::Info::isFileURL($url) ) { + + if ( Slim::Music::Info::isFileURL($url) && (!$cover || !$newCoverId || $urlDir ne dirname($cover) || $disc_count > 1) ) { # store properties in a hash my $track = Slim::Schema->find('Track', $trackid); @@ -287,20 +351,16 @@ sub updateStandaloneArtwork { my %columnValueHash = map { $_ => $track->$_() } keys %{$track->attributes}; $columnValueHash{primary_artist} = $columnValueHash{primary_artist}->id if $columnValueHash{primary_artist}; - my $newCover = Slim::Music::Artwork->findStandaloneArtwork( + $newCover = Slim::Music::Artwork->findStandaloneArtwork( \%columnValueHash, {}, - Slim::Utils::Misc::fileURLFromPath( - dirname(Slim::Utils::Misc::pathFromFileURL($url)) - ), + Slim::Utils::Misc::fileURLFromPath($urlDir), ); - if ($newCover) { - $cover = $newCover; - - $newCoverId = Slim::Schema::Track->generateCoverId({ - cover => $newCover, - url => $url, + if ($newCover && $newCover ne $cover) { + $newCoverId = Slim::Music::Artwork->generateImageId({ + image => $newCover, + url => Slim::Utils::Misc::fileURLFromPath($newCover), }); } } @@ -310,13 +370,7 @@ sub updateStandaloneArtwork { # Make sure album.artwork points to this track, as it may not # be pointing there now because we did not join tracks via the # artwork column. - if ( ($album_artwork || '') ne $newCoverId ) { - $sth_update_albums->execute( $newCoverId, $albumid ); - } - - # Update the rest of the tracks on this album - # to use the same coverid and cover_cached status - $sth_update_tracks->execute( $cover, $newCoverId, $albumid ); + $sth_update_tracks->execute( $newCover, $newCoverId, $albumid, $dirname . "%", $disc_number ); if ( ++$i % 50 == 0 ) { Slim::Schema->forceCommit; @@ -326,7 +380,7 @@ sub updateStandaloneArtwork { Slim::Utils::Scheduler::unpause() if !main::SCANNER; } elsif ( $cover =~ /^https?:/ && (!$album_artwork || $album_artwork ne $cover) ) { - $sth_update_albums->execute( $newCoverId, $albumid ); + $sth_update_track_cover_cached_null->execute($albumid); if ( ++$i % 50 == 0 ) { Slim::Schema->forceCommit; @@ -337,8 +391,7 @@ sub updateStandaloneArtwork { } # cover art has disappeared elsif ( !$newCoverId ) { - $sth_update_albums->execute( undef, $albumid ); - $sth_update_tracks->execute( 0, undef, $albumid ); + $sth_update_tracks->execute( 0, undef, $albumid, $dirname . "%", $disc_number ); $log->warn('Artwork has been removed for ' . $album_title); } @@ -661,12 +714,11 @@ sub precacheAllArtwork { albums.artwork AS album_artwork FROM tracks JOIN albums ON (tracks.album = albums.id) - WHERE tracks.cover != '0' - AND tracks.coverid IS NOT NULL } - . ($force ? '' : ' AND tracks.cover_cached IS NULL') + . ($force ? '' : ' WHERE tracks.cover_cached IS NULL') . qq{ GROUP BY tracks.cover, tracks.album + ORDER BY tracks.album }; my $sth_update_tracks = $dbh->prepare( qq{ @@ -676,6 +728,13 @@ sub precacheAllArtwork { AND (cover = ? OR cover LIKE 'http%') } ); + my $sth_update_tracks_with_parent = $dbh->prepare( qq{ + UPDATE tracks + SET cover = ?, coverid = ?, cover_cached = 1 + WHERE album = ? + AND (cover = ? OR cover LIKE 'http%') + } ); + my $sth_update_albums = $dbh->prepare( qq{ UPDATE albums SET artwork = ? @@ -683,7 +742,7 @@ sub precacheAllArtwork { } ); my ($count) = $dbh->selectrow_array( qq{ - SELECT COUNT(*) FROM ( $sql ) AS t1 + SELECT COUNT(DISTINCT albumid) FROM ( $sql ) AS t1 } ); $log->error("Starting precacheArtwork for $count albums"); @@ -728,24 +787,25 @@ sub precacheAllArtwork { my $i = 0; my %artCount; + my $parentArtwork; + my $parentArtworkId; + my $previousAlbum; + my $sth_album_urls = $dbh->prepare_cached("SELECT url FROM tracks WHERE tracks.album = ? AND tracks.url LIKE 'file://%'"); my $work = sub { if ( $sth->fetch ) { - # Make sure album.artwork points to this track, as it may not - # be pointing there now because we did not join tracks via the - # artwork column. - if ( $album_artwork && $album_artwork ne $coverid ) { - $sth_update_albums->execute( $coverid, $albumid ); - } - - $artCount{$albumid}++; # Callback after resize is finished, needed for async resizing my $finished = sub { if ($isEnabled) { # Update the rest of the tracks on this album # to use the same coverid and cover_cached status - $sth_update_tracks->execute( $coverid, $albumid, $cover ); + if ( $parentArtwork && $parentArtworkId && !$coverid ) { + $sth_update_tracks_with_parent->execute( $parentArtwork, $parentArtworkId, $albumid, $cover ); + } + else { + $sth_update_tracks->execute( $coverid, $albumid, $cover ); + } } $progress->update( $album_title ); @@ -757,6 +817,39 @@ sub precacheAllArtwork { Slim::Utils::Scheduler::unpause() if !main::SCANNER; }; + if ( $previousAlbum != $albumid ) { + $previousAlbum = $albumid; + if ( $url =~ /^file:/ ) { + my @currentAlbumUrls = @{$dbh->selectall_arrayref($sth_album_urls, {}, $albumid)}; + $sth_album_urls->finish; + my @paths = Slim::Utils::Misc::uniq(map( dirname(Slim::Utils::Misc::pathFromFileURL($_->[0])), @currentAlbumUrls )); + + my $commonParent; + if (@paths == 1) { + $commonParent = $paths[0]; + } + elsif (scalar @paths) { + $commonParent = _findCommonParent(\@paths); + } + my $urlDir = dirname(Slim::Utils::Misc::pathFromFileURL($url)); + if ( $parentArtwork = Slim::Music::Artwork->findStandaloneArtwork(undef, undef, Slim::Utils::Misc::fileURLFromPath($urlDir), $commonParent) ) { + $parentArtworkId = Slim::Music::Artwork->generateImageId({ + image => $parentArtwork, + url => Slim::Utils::Misc::fileURLFromPath($parentArtwork), + }) || ''; + $sth_update_albums->execute( $parentArtworkId, $albumid ); + Slim::Utils::ImageResizer->resize($parentArtwork, "music/$parentArtworkId/cover_", join(',', @specs), undef); + } + else { + $parentArtwork = undef; + $parentArtworkId = undef; + } + } + } + + + $artCount{$albumid}++ unless $parentArtwork; + # Do the actual pre-caching only if the pref for it is enabled if ( $isEnabled ) { # let's grab external images when run in the scanner @@ -824,12 +917,10 @@ sub precacheAllArtwork { while ( my ($albumId, $trackCount) = each %artCount ) { - next unless $trackCount > 1; - $sth_get_album_art->execute($albumId); my ($coverId) = $sth_get_album_art->fetchrow_array; - $sth_update_albums->execute( $coverId, $albumId ) if $coverId; + $sth_update_albums->execute( $coverId, $albumId ); } @@ -904,4 +995,47 @@ sub getResizeSpecs { return @specs; } +sub _findCommonParent { + my $paths = shift; + return undef unless ref $paths eq 'ARRAY' && @$paths >= 2; + + my @split_paths = map { + my $normalized = canonpath($_); + [splitdir($normalized)]; + } @$paths; + + my @common; + my $min_len = min(map { scalar @$_ } @split_paths); + + for my $i (0 .. $min_len - 1) { + my $component = $split_paths[0][$i]; + my $all_same = 1; + + for my $path (@split_paths) { + if ($path->[$i] ne $component) { + $all_same = 0; + last; + } + } + + last unless $all_same; + push @common, $component; + } + + return @common ? catdir(@common) : undef; +} + +sub _discSpecificArtworkNames { + my $discId = shift; + + return () if defined($discId) && !$discId; + + my @discSpecificNames = ( + 'cover-disc%s', 'cover_disc%s', 'coverdisc%s', + 'folder-disc%s', 'folder_disc%s', 'folderdisc%s', + 'disc%s', 'cd%s' + ); + return ( map {sprintf($_, $discId)} @discSpecificNames ); +} + 1; diff --git a/Slim/Schema.pm b/Slim/Schema.pm index a04164313de..5bb39d8adc5 100644 --- a/Slim/Schema.pm +++ b/Slim/Schema.pm @@ -1815,12 +1815,25 @@ sub _newTrack { } if ( $columnValueHash{cover} ) { - # Generate coverid value based on artwork, mtime, filesize + # Generate coverid value based on artwork, mtime, filesize: + # if cover is embedded in the file (ie it's a number, the size of the embedded image) use the track URL, + # if cover is external use the URL of the image. + # This avoids generating multiple cover ids for the same image. I'm not sure if they would ever get cached + # because Slim::Music::Artwork would consolidate them, but this seems cleaner. + my ($coverUrl, $mtime, $size); + if ( $columnValueHash{cover} =~ /^\d+$/ ) { + $coverUrl = $url; + $mtime = $columnValueHash{timestamp}; + $size = $columnValueHash{filesize}; + } + else { + $coverUrl = Slim::Utils::Misc::fileURLFromPath($columnValueHash{cover}); + } $columnValueHash{coverid} = Slim::Schema::Track->generateCoverId( { cover => $columnValueHash{cover}, - url => $url, - mtime => $columnValueHash{timestamp}, - size => $columnValueHash{filesize}, + url => $coverUrl, + mtime => $mtime, + size => $size, } ); } @@ -2011,6 +2024,7 @@ sub updateOrCreateBase { grouping => 1, discsubtitle => 1, musicbrainz_id => 1, + cover => 1, }; # Some taggers will not supply blank tags so create the attributes for columns which need to be nulled. foreach my $col (keys %$nullableColumns) { diff --git a/Slim/Schema/Track.pm b/Slim/Schema/Track.pm index 57dc4802ed9..cc28aa515bc 100644 --- a/Slim/Schema/Track.pm +++ b/Slim/Schema/Track.pm @@ -728,13 +728,25 @@ sub coverid { if ( !defined $val ) { # Initialize coverid value if ( $self->cover ) { + my ($coverUrl, $mtime, $size); + # if cover is embedded in the file (ie it's a number, the size of the embedded image) use the track URL, + # if cover is external use the URL of the image. + # This avoids generating multiple cover ids for the same image. I'm not sure if they would ever get cached + # because Slim::Music::Artwork would consolidate them, but this seems cleaner. + if ( $self->cover =~ /^\d+$/ ) { + $coverUrl = $self->url; + $mtime = $self->timestamp; + $size = $self->filesize; + } + else { + $coverUrl = Slim::Utils::Misc::fileURLFromPath($self->cover); + } $val = $self->generateCoverId( { cover => $self->cover, - url => $self->url, - mtime => $self->timestamp, - size => $self->filesize, + url => $coverUrl, + mtime => $mtime, + size => $size, } ); - $self->_coverid($val); $self->update; } From f9e9f279875c9ef64c7925cda18b482dab97639b Mon Sep 17 00:00:00 2001 From: darrell-k Date: Tue, 17 Mar 2026 15:23:34 +0000 Subject: [PATCH 2/4] tidy some things up Signed-off-by: darrell-k --- Slim/Music/Artwork.pm | 18 +++++++++++------- Slim/Schema.pm | 15 +++------------ Slim/Schema/Track.pm | 15 +++------------ 3 files changed, 17 insertions(+), 31 deletions(-) diff --git a/Slim/Music/Artwork.pm b/Slim/Music/Artwork.pm index 61d0fb74abd..ee73c314882 100644 --- a/Slim/Music/Artwork.pm +++ b/Slim/Music/Artwork.pm @@ -59,6 +59,10 @@ my %findArtCache; # Public class methods sub findStandaloneArtwork { + # PR https://github.com/LMS-Community/slimserver/pull/1536 + # new parameter $commonParent flag introduced. If this is passed $dirurl should be the parent directory and we don't need + # to have received anything in $trackAttributes or $deferredAttributes as track-based variable cover art format is bypassed. + my ( $class, $trackAttributes, $deferredAttributes, $dirurl, $commonParent ) = @_; my $discNumber = $trackAttributes->{disc}; @@ -139,7 +143,6 @@ sub findStandaloneArtwork { if ( !$art ) { # Find all image files in the file directory - $parentDir = $commonParent if $commonParent; my $types = qr/\.(?:jpe?g|png|gif)$/i; @@ -289,9 +292,9 @@ sub updateStandaloneArtwork { my $work = sub { if ( $sth->fetch ) { - my $newCoverId; - my $newCover; - my $newAlbumCover; + my $newCoverId = undef; + my $newCover = undef; + my $newAlbumCover = undef; my $urlDir = dirname(Slim::Utils::Misc::pathFromFileURL($url)) if $url =~ /^file:/; $progress->update( $album_title ); @@ -316,7 +319,7 @@ sub updateStandaloneArtwork { $commonParent = _findCommonParent(\@paths); } - if ( my $parentArtwork = Slim::Music::Artwork->findStandaloneArtwork(undef, undef, Slim::Utils::Misc::fileURLFromPath($urlDir), $commonParent) ) { + if ( my $parentArtwork = Slim::Music::Artwork->findStandaloneArtwork(undef, undef, Slim::Utils::Misc::fileURLFromPath($commonParent), 1) ) { my $parentUrl = Slim::Utils::Misc::fileURLFromPath($parentArtwork); $newAlbumCover = Slim::Music::Artwork->generateImageId({ image => $parentArtwork, @@ -832,7 +835,7 @@ sub precacheAllArtwork { $commonParent = _findCommonParent(\@paths); } my $urlDir = dirname(Slim::Utils::Misc::pathFromFileURL($url)); - if ( $parentArtwork = Slim::Music::Artwork->findStandaloneArtwork(undef, undef, Slim::Utils::Misc::fileURLFromPath($urlDir), $commonParent) ) { + if ( $parentArtwork = Slim::Music::Artwork->findStandaloneArtwork(undef, undef, Slim::Utils::Misc::fileURLFromPath($commonParent), 1) ) { $parentArtworkId = Slim::Music::Artwork->generateImageId({ image => $parentArtwork, url => Slim::Utils::Misc::fileURLFromPath($parentArtwork), @@ -997,7 +1000,8 @@ sub getResizeSpecs { sub _findCommonParent { my $paths = shift; - return undef unless ref $paths eq 'ARRAY' && @$paths >= 2; + return undef if ref $paths ne 'ARRAY'; + return $paths->[0] if scalar @$paths == 1; my @split_paths = map { my $normalized = canonpath($_); diff --git a/Slim/Schema.pm b/Slim/Schema.pm index 5bb39d8adc5..f901e2bc18c 100644 --- a/Slim/Schema.pm +++ b/Slim/Schema.pm @@ -1820,20 +1820,11 @@ sub _newTrack { # if cover is external use the URL of the image. # This avoids generating multiple cover ids for the same image. I'm not sure if they would ever get cached # because Slim::Music::Artwork would consolidate them, but this seems cleaner. - my ($coverUrl, $mtime, $size); - if ( $columnValueHash{cover} =~ /^\d+$/ ) { - $coverUrl = $url; - $mtime = $columnValueHash{timestamp}; - $size = $columnValueHash{filesize}; - } - else { - $coverUrl = Slim::Utils::Misc::fileURLFromPath($columnValueHash{cover}); - } $columnValueHash{coverid} = Slim::Schema::Track->generateCoverId( { cover => $columnValueHash{cover}, - url => $coverUrl, - mtime => $mtime, - size => $size, + url => $columnValueHash{cover} =~ /^\d+$/ ? $url : Slim::Utils::Misc::fileURLFromPath($columnValueHash{cover}), + mtime => $columnValueHash{timestamp}, + size => $columnValueHash{filesize}, } ); } diff --git a/Slim/Schema/Track.pm b/Slim/Schema/Track.pm index cc28aa515bc..8b50cc4e7d4 100644 --- a/Slim/Schema/Track.pm +++ b/Slim/Schema/Track.pm @@ -728,24 +728,15 @@ sub coverid { if ( !defined $val ) { # Initialize coverid value if ( $self->cover ) { - my ($coverUrl, $mtime, $size); # if cover is embedded in the file (ie it's a number, the size of the embedded image) use the track URL, # if cover is external use the URL of the image. # This avoids generating multiple cover ids for the same image. I'm not sure if they would ever get cached # because Slim::Music::Artwork would consolidate them, but this seems cleaner. - if ( $self->cover =~ /^\d+$/ ) { - $coverUrl = $self->url; - $mtime = $self->timestamp; - $size = $self->filesize; - } - else { - $coverUrl = Slim::Utils::Misc::fileURLFromPath($self->cover); - } $val = $self->generateCoverId( { cover => $self->cover, - url => $coverUrl, - mtime => $mtime, - size => $size, + url => $self->cover =~ /^\d+$/ ? $self->url : Slim::Utils::Misc::fileURLFromPath($self->cover), + mtime => $self->timestamp, + size => $self->filesize, } ); $self->_coverid($val); $self->update; From 2c26e5174f1a09b7acb080756d5e631c34150cd8 Mon Sep 17 00:00:00 2001 From: darrell-k Date: Tue, 17 Mar 2026 18:54:40 +0000 Subject: [PATCH 3/4] SQL formatting Signed-off-by: darrell-k --- Slim/Music/Artwork.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Slim/Music/Artwork.pm b/Slim/Music/Artwork.pm index ee73c314882..c01f3cc0740 100644 --- a/Slim/Music/Artwork.pm +++ b/Slim/Music/Artwork.pm @@ -237,7 +237,7 @@ sub updateStandaloneArtwork { albums.discc AS disc_count, albums.artwork AS album_artwork, -- This is the Sqlite equivalent of dirname() - rtrim(tracks.url, replace(tracks.url, '/', '')) AS dirname, + RTRIM(tracks.url, REPLACE(tracks.url, '/', '')) AS dirname, tracks.disc FROM tracks JOIN albums ON (tracks.album = albums.id) From 9abe2741a1021bbd10b91ec07b52015907aade8e Mon Sep 17 00:00:00 2001 From: darrell-k Date: Thu, 26 Mar 2026 16:45:03 +0000 Subject: [PATCH 4/4] efficiency changes Signed-off-by: darrell-k --- Slim/Music/Artwork.pm | 114 +++++++++++++++++++++--------------------- 1 file changed, 56 insertions(+), 58 deletions(-) diff --git a/Slim/Music/Artwork.pm b/Slim/Music/Artwork.pm index c01f3cc0740..acd07fb3ebe 100644 --- a/Slim/Music/Artwork.pm +++ b/Slim/Music/Artwork.pm @@ -63,11 +63,11 @@ sub findStandaloneArtwork { # new parameter $commonParent flag introduced. If this is passed $dirurl should be the parent directory and we don't need # to have received anything in $trackAttributes or $deferredAttributes as track-based variable cover art format is bypassed. - my ( $class, $trackAttributes, $deferredAttributes, $dirurl, $commonParent ) = @_; + my ( $class, $trackAttributes, $deferredAttributes, $dirurl, $commonParent, $trackUrl, $trackDisc ) = @_; - my $discNumber = $trackAttributes->{disc}; + my $discNumber = $trackAttributes->{disc} || $trackDisc; - return 0 if !Slim::Music::Info::isFileURL($dirurl); + return 0 if !Slim::Music::Info::isFileURL($dirurl) || (!$trackAttributes && !$commonParent && !$trackUrl); my $isInfo = main::INFOLOG && $log->is_info; @@ -100,7 +100,7 @@ sub findStandaloneArtwork { # XXX This may break for some people as it's not using a Track object anymore my $meta = { %{$trackAttributes}, %{$deferredAttributes} }; - if ( my $prefix = Slim::Music::TitleFormatter::infoFormat( undef, $1, undef, $meta ) ) { + if ( my $prefix = Slim::Music::TitleFormatter::infoFormat( $trackUrl, $1, undef, $trackUrl ? undef : $meta ) ) { $coverFormat = $prefix . $suffix; if ( main::ISWINDOWS ) { @@ -222,8 +222,8 @@ sub updateStandaloneArtwork { }; } - # Find all tracks with un-cached artwork: - # * All distinct cover values where cover isn't 0 and cover_cached is null + # Find all tracks with un-cached internal artwork or external artwork: + # * All distinct cover values where cover is an external image, or isn't 0 and cover_cached is null # * Tracks share the same cover art when the cover field is the same # (same path or same embedded art length). my $sql = qq{ @@ -246,12 +246,10 @@ sub updateStandaloneArtwork { ORDER BY tracks.album, tracks.disc, dirname }; - my $sth_album_urls = $dbh->prepare("SELECT url FROM tracks WHERE tracks.album = ? AND tracks.url LIKE 'file://%'"); - my $sth_update_tracks = $dbh->prepare( qq{ UPDATE tracks SET cover = ?, coverid = ?, cover_cached = NULL - WHERE album = ? AND url like ? AND disc = ? + WHERE album = ? AND url LIKE ? AND disc = ? } ); my $sth_update_track_cover_cached_null = $dbh->prepare( qq{ @@ -288,14 +286,35 @@ sub updateStandaloneArtwork { my $i = 0; my $t = 0; my $previousAlbum; + my $prevAlbumArtwork; + my @albumDirs; + + my $processAlbum = sub { + if ( scalar @albumDirs ) { + my $newAlbumCover = $prevAlbumArtwork; # assume it hasn't changed + @albumDirs = Slim::Utils::Misc::uniq(@albumDirs); + my $commonParent = _findCommonParent(\@albumDirs); + + if ( my $parentArtwork = Slim::Music::Artwork->findStandaloneArtwork(undef, undef, Slim::Utils::Misc::fileURLFromPath($commonParent), 1) ) { + my $parentUrl = Slim::Utils::Misc::fileURLFromPath($parentArtwork); + $newAlbumCover = Slim::Music::Artwork->generateImageId({ + image => $parentArtwork, + url => $parentUrl, + }); + } + # parent artwork changed so make sure cache update is triggered + if ( $newAlbumCover ne $prevAlbumArtwork ) { + $sth_update_track_cover_cached_null->execute($previousAlbum); + } + } + }; my $work = sub { if ( $sth->fetch ) { my $newCoverId = undef; my $newCover = undef; - my $newAlbumCover = undef; - my $urlDir = dirname(Slim::Utils::Misc::pathFromFileURL($url)) if $url =~ /^file:/; + my $urlDir = dirname(Slim::Utils::Misc::pathFromFileURL($url)) if Slim::Music::Info::isFileURL($url); $progress->update( $album_title ); @@ -305,33 +324,14 @@ sub updateStandaloneArtwork { } if ( $previousAlbum != $albumid ) { - $previousAlbum = $albumid; - if ( $url =~ /^file:/ ) { - my @currentAlbumUrls = @{$dbh->selectall_arrayref($sth_album_urls, {}, $albumid)}; - $sth_album_urls->finish; - my @paths = Slim::Utils::Misc::uniq(map( dirname(Slim::Utils::Misc::pathFromFileURL($_->[0])), @currentAlbumUrls )); - - my $commonParent; - if (@paths == 1) { - $commonParent = $paths[0]; - } - elsif (scalar @paths) { - $commonParent = _findCommonParent(\@paths); - } - - if ( my $parentArtwork = Slim::Music::Artwork->findStandaloneArtwork(undef, undef, Slim::Utils::Misc::fileURLFromPath($commonParent), 1) ) { - my $parentUrl = Slim::Utils::Misc::fileURLFromPath($parentArtwork); - $newAlbumCover = Slim::Music::Artwork->generateImageId({ - image => $parentArtwork, - url => $parentUrl, - }); - } - # parent artwork changed so make sure cache update is triggered - if ( $newAlbumCover ne $album_artwork ) { - $sth_update_track_cover_cached_null->execute($albumid); - } + if ( $previousAlbum ) { + $processAlbum->(); } + $prevAlbumArtwork = $album_artwork; + $previousAlbum = $albumid; + @albumDirs = (); } + push @albumDirs, $urlDir; # check for updated artwork if ( $cover ) { @@ -347,25 +347,22 @@ sub updateStandaloneArtwork { # - !$newCoverId: existing file has disappeared if ( Slim::Music::Info::isFileURL($url) && (!$cover || !$newCoverId || $urlDir ne dirname($cover) || $disc_count > 1) ) { - # store properties in a hash - my $track = Slim::Schema->find('Track', $trackid); - - if ($track) { - my %columnValueHash = map { $_ => $track->$_() } keys %{$track->attributes}; - $columnValueHash{primary_artist} = $columnValueHash{primary_artist}->id if $columnValueHash{primary_artist}; - - $newCover = Slim::Music::Artwork->findStandaloneArtwork( - \%columnValueHash, - {}, - Slim::Utils::Misc::fileURLFromPath($urlDir), - ); - - if ($newCover && $newCover ne $cover) { - $newCoverId = Slim::Music::Artwork->generateImageId({ - image => $newCover, - url => Slim::Utils::Misc::fileURLFromPath($newCover), - }); - } + # pass the url, much faster! If the user has a wildcard image name pref, the Track will still be instantiated further + # down the line, but this change avoids it being done when not neccessary. + $newCover = Slim::Music::Artwork->findStandaloneArtwork( + {}, + {}, + Slim::Utils::Misc::fileURLFromPath($urlDir), + undef, + $url, + $disc_number, + ); + + if ($newCover && $newCover ne $cover) { + $newCoverId = Slim::Music::Artwork->generateImageId({ + image => $newCover, + url => Slim::Utils::Misc::fileURLFromPath($newCover), + }); } } @@ -393,7 +390,7 @@ sub updateStandaloneArtwork { Slim::Utils::Scheduler::unpause() if !main::SCANNER; } # cover art has disappeared - elsif ( !$newCoverId ) { + elsif ( $coverid && !$newCoverId ) { $sth_update_tracks->execute( 0, undef, $albumid, $dirname . "%", $disc_number ); $log->warn('Artwork has been removed for ' . $album_title); @@ -402,6 +399,8 @@ sub updateStandaloneArtwork { return 1; } + $processAlbum->(); + $progress->final; $cb && $cb->(); @@ -822,9 +821,8 @@ sub precacheAllArtwork { if ( $previousAlbum != $albumid ) { $previousAlbum = $albumid; - if ( $url =~ /^file:/ ) { + if ( Slim::Music::Info::isFileURL($url) ) { my @currentAlbumUrls = @{$dbh->selectall_arrayref($sth_album_urls, {}, $albumid)}; - $sth_album_urls->finish; my @paths = Slim::Utils::Misc::uniq(map( dirname(Slim::Utils::Misc::pathFromFileURL($_->[0])), @currentAlbumUrls )); my $commonParent;