diff --git a/Slim/Control/Commands.pm b/Slim/Control/Commands.pm index 5089ecd4f4..de5c38981b 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 790d317083..acd07fb3eb 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,38 @@ my %findArtCache; # Public class methods sub findStandaloneArtwork { - my ( $class, $trackAttributes, $deferredAttributes, $dirurl ) = @_; + # 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. - return 0 if !Slim::Music::Info::isFileURL($dirurl); + my ( $class, $trackAttributes, $deferredAttributes, $dirurl, $commonParent, $trackUrl, $trackDisc ) = @_; - my $isInfo = main::INFOLOG && $log->is_info; + my $discNumber = $trackAttributes->{disc} || $trackDisc; - my $art = $findArtCache{$dirurl}; + return 0 if !Slim::Music::Info::isFileURL($dirurl) || (!$trackAttributes && !$commonParent && !$trackUrl); - # Files to look for - my @files = qw(cover folder album thumb); + my $isInfo = main::INFOLOG && $log->is_info; - # 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}$/ ) { @@ -86,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 ) { @@ -129,6 +143,7 @@ sub findStandaloneArtwork { if ( !$art ) { # Find all image files in the file directory + my $types = qr/\.(?:jpe?g|png|gif)$/i; my $files = File::Next::files( { @@ -137,6 +152,7 @@ sub findStandaloneArtwork { }, $parentDir ); my @found; + while ( my $image = $files->() ) { push @found, $image; } @@ -147,7 +163,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 +180,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; } } @@ -198,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{ @@ -210,27 +234,32 @@ 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_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 +280,41 @@ 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 $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; + + my $newCoverId = undef; + my $newCover = undef; + my $urlDir = dirname(Slim::Utils::Misc::pathFromFileURL($url)) if Slim::Music::Info::isFileURL($url); $progress->update( $album_title ); @@ -268,41 +323,46 @@ sub updateStandaloneArtwork { $t = time + 5; } + if ( $previousAlbum != $albumid ) { + if ( $previousAlbum ) { + $processAlbum->(); + } + $prevAlbumArtwork = $album_artwork; + $previousAlbum = $albumid; + @albumDirs = (); + } + push @albumDirs, $urlDir; + # 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) ) { - # 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}; - - my $newCover = Slim::Music::Artwork->findStandaloneArtwork( - \%columnValueHash, - {}, - Slim::Utils::Misc::fileURLFromPath( - dirname(Slim::Utils::Misc::pathFromFileURL($url)) - ), - ); - - if ($newCover) { - $cover = $newCover; - - $newCoverId = Slim::Schema::Track->generateCoverId({ - cover => $newCover, - url => $url, - }); - } + + if ( Slim::Music::Info::isFileURL($url) && (!$cover || !$newCoverId || $urlDir ne dirname($cover) || $disc_count > 1) ) { + # 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), + }); } } @@ -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; @@ -336,9 +390,8 @@ sub updateStandaloneArtwork { Slim::Utils::Scheduler::unpause() if !main::SCANNER; } # cover art has disappeared - elsif ( !$newCoverId ) { - $sth_update_albums->execute( undef, $albumid ); - $sth_update_tracks->execute( 0, undef, $albumid ); + elsif ( $coverid && !$newCoverId ) { + $sth_update_tracks->execute( 0, undef, $albumid, $dirname . "%", $disc_number ); $log->warn('Artwork has been removed for ' . $album_title); } @@ -346,6 +399,8 @@ sub updateStandaloneArtwork { return 1; } + $processAlbum->(); + $progress->final; $cb && $cb->(); @@ -661,12 +716,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 +730,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 +744,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 +789,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 +819,38 @@ sub precacheAllArtwork { Slim::Utils::Scheduler::unpause() if !main::SCANNER; }; + if ( $previousAlbum != $albumid ) { + $previousAlbum = $albumid; + if ( Slim::Music::Info::isFileURL($url) ) { + my @currentAlbumUrls = @{$dbh->selectall_arrayref($sth_album_urls, {}, $albumid)}; + 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($commonParent), 1) ) { + $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 +918,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 +996,48 @@ sub getResizeSpecs { return @specs; } +sub _findCommonParent { + my $paths = shift; + return undef if ref $paths ne 'ARRAY'; + return $paths->[0] if scalar @$paths == 1; + + 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 a04164313d..f901e2bc18 100644 --- a/Slim/Schema.pm +++ b/Slim/Schema.pm @@ -1815,10 +1815,14 @@ 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. $columnValueHash{coverid} = Slim::Schema::Track->generateCoverId( { cover => $columnValueHash{cover}, - url => $url, + url => $columnValueHash{cover} =~ /^\d+$/ ? $url : Slim::Utils::Misc::fileURLFromPath($columnValueHash{cover}), mtime => $columnValueHash{timestamp}, size => $columnValueHash{filesize}, } ); @@ -2011,6 +2015,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 57dc4802ed..8b50cc4e7d 100644 --- a/Slim/Schema/Track.pm +++ b/Slim/Schema/Track.pm @@ -728,13 +728,16 @@ sub coverid { if ( !defined $val ) { # Initialize coverid value if ( $self->cover ) { + # 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. $val = $self->generateCoverId( { cover => $self->cover, - url => $self->url, + url => $self->cover =~ /^\d+$/ ? $self->url : Slim::Utils::Misc::fileURLFromPath($self->cover), mtime => $self->timestamp, size => $self->filesize, } ); - $self->_coverid($val); $self->update; }