Fix work images and artwork precaching#1517
Fix work images and artwork precaching#1517michaelherger merged 2 commits intoLMS-Community:public/9.2from
Conversation
Signed-off-by: darrell-k <darrell@darrell.org.uk>
Signed-off-by: darrell-k <darrell@darrell.org.uk>
|
I'm about to push a change to Unlike I also added logic to the order by in There are still some differences between |
| WHERE tracks.album = ? | ||
| AND tracks.coverid IS NOT NULL | ||
| ORDER BY tracks.disc, tracks.tracknum | ||
| ORDER BY CASE WHEN CAST(CAST(tracks.cover AS INTEGER) AS TEXT) = tracks.cover THEN '1' ELSE '0' END, tracks.disc, tracks.tracknum |
There was a problem hiding this comment.
This prioritisation of external images would only work if the hash didn't happen to be an all numerical value, right? Is it worth the casting overhead? Did you measure the overhead?
There was a problem hiding this comment.
I also added logic to the order by in $sth_get_album_art so that any external artwork is preferred to that embedded in the first track.
In more detail: the double cast converts the column's value to numeric and then back to text. we then compare it to the original value and if they're equal, we know that the original value was an integer, ie the size of the embedded artwork rather than a path to the external image.
This is then enclosed in a CASE statement so as to return 1 if the artwork is embedded and 0 if it is external.
The whole thing is used as the first sort key so that an external image will always take priority over an embedded image.
The existing code just uses the first track's image even if it was embedded and there is also an album-level image in the album's folder.
There was a problem hiding this comment.
I'm sorry, I only saw your initial description after I posted... and then edited my comment to be more specific. And now I see that I confused cover and coverid...
There was a problem hiding this comment.
Run a quick test listing tracks using those orders on my Pi4, with about 22k tracks. Overhead is negligible.
There was a problem hiding this comment.
That's good. What do you say about further work to bring Slim::Music::Artwork::updateStandaloneArtwork and Slim::Music::Artwork::precacheAllArtwork into line?
There was a problem hiding this comment.
Align how the two work? I'm all for that. But this PR started as a fix, which I believe should even go into 9.1. Further alignment might be beyond a stable scope. What do you think?
There was a problem hiding this comment.
It might do and definitely this fix should go into 9.1. Leave it with me for now.
I'm actually having ideas about #1513 now I have studied Slim::Music::Artwork. I think that PR can be simplified.
There was a problem hiding this comment.
Ok, shall we merge this one, and back port in a few if good?
And yes, #1513 needs some cleanup IMHO. There were some no-op changes and probably cruft left behind by the AI assistant forgetting what it had done before...
There was a problem hiding this comment.
Let me check whether the sth_get_album_art processing needs adding to Slim::Music::Artwork::updateStandaloneArtwork. I'm wondering if it needs to be for a album rescan.
There was a problem hiding this comment.
@michaelherger Let's go with this as it is.
|
Thanks! |
2 participants
Ensure that only the image from the first track of a work-instance is used even when there are multiple images for the set of tracks making up the work-instance.
This can happen with embedded track images of different sizes, for example.