Bug 9757 - Scanner does not pre-cache artwork for controller
: Scanner does not pre-cache artwork for controller
Status: CLOSED FIXED
Product: Logitech Media Server
Classification: Unclassified
Component: Scanner
: 7.4.0
: Linkstation Debian Linux
: P1 normal with 2 votes (vote)
: 7.4.0
Assigned To: Andy Grundman
:
Depends on: 9919 13454
Blocks:
  Show dependency treegraph
 
Reported: 2008-10-18 17:48 UTC by Martin Schwenke
Modified: 2011-01-07 04:36 UTC (History)
9 users (show)

See Also:
Category: Bug


Attachments
Hack to Music::Artwork to attempt pre-caching of artwork for controller (1.68 KB, patch)
2008-10-19 18:40 UTC, Martin Schwenke
Details | Diff
Patch to add backend for preferTrackArtwork option. (2.91 KB, patch)
2008-10-21 19:17 UTC, Martin Schwenke
Details | Diff
Add precaching for controller artwork and preferTrackArtwork option (1.92 KB, patch)
2009-08-03 16:14 UTC, Martin Schwenke
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Schwenke 2008-10-18 17:48:35 UTC
The artwork requested by the controller does not appear to be pre-cached when the scanner runs.

I run squeezecenter on a Linkstation Pro (400MHz ARM CPU?) and it generally works beautifully.  However, I get very bad skips/resets in my audio at the beginning of each album.  I stream FLACs (yep, I realise this might be a little bit adventurous via a Linkstation) and when the controller tries to fetch the artwork the audio stops... sometimes for a few seconds but often for up to a minute.  This seems to be because squeezecenter is struggling to generate the artwork at the required sizes.

So...

I did a bunch of stracing on the squeezecenter process and found that these were being requested:

  cover_186x186_o.jpg
  cover_154x154_o.jpg
  cover_56x56_o.jpg

Only the last seems to be generated by Music::Artwork->findArtwork().  However, I'm not convinced because when I list albums the artwork thumbnails take a long time to display and, while I was strace-ing, seemed to be generated when the controller requested them.

So, I've hacked the above function to generate the above sizes (along with a full-sized PNG and a 50x50 JPEG for the web interface - seems to be what it's asking for).  I'll post here when I know if it works...

A related problem I'm seeing is that it is taking about 2.5 minutes to generate the artwork for each album.  For >600 albums, the full artwork scan will take over 25 hours!  That even seems like a long time on my little ARM CPU.  /proc/.../maps shows that squeezecenter has GD.so (and libgd.so.2.0.33) (and bunch of probably irrelevant shared libraries) mapped.  Is there anything related that I might have missed and have not compiled/installed on my system?

Thanks...

peace & happiness,
martin
Comment 1 Martin Schwenke 2008-10-19 18:40:00 UTC
Created attachment 4155 [details]
Hack to Music::Artwork to attempt pre-caching of artwork for controller

This patch attempts to add some extra artwork combinations to the cache, so they're not generated when the controller requests them.

It also turns off a couple of the conditions, so artwork is generated for albums that aren't new.
Comment 2 Martin Schwenke 2008-10-19 18:57:02 UTC
In ran a scan over my 625 albums and it took 17.5 hours to process the artwork alone.  The patch in the previous comment doesn't seem to do the whole job.  At the moment I'm at least managing to pre-cache the thumbnails for the controller.  Viewing a list of albums was blazingly fast, whereas it used to sometimes be painfully slow.

However, the main 186x186 JPEG reequired by the controller isn't cached properly.  One file was generated during the pre-caching run: music/2026/cover_186x186_o.jpg.  However, another file was created when the controller requested the artwork: music/2027/cover_186x186_o.jpg.  I'm guessing these names (taken from inside the files that have hashes for names) will be more useful for diagnosing what's happening than the hashes themselves.

Interestingly, if I diff hexdumps of both cache files, the differences are minimal:

--- /tmp/x1     2008-10-20 12:27:23.752019939 +1100
+++ /tmp/x2     2008-10-20 12:27:48.962018851 +1100
@@ -1,10 +1,10 @@
 00000000  05 07 02 00 00 00 02 0a  1e 6d 75 73 69 63 2f 32  |.........music/2|
-00000010  30 32 37 2f 63 6f 76 65  72 5f 31 38 36 78 31 38  |027/cover_186x18|
+00000010  30 32 36 2f 63 6f 76 65  72 5f 31 38 36 78 31 38  |026/cover_186x18|
 00000020  36 5f 6f 2e 6a 70 67 04  11 0d 43 61 63 68 65 3a  |6_o.jpg...Cache:|
 00000030  3a 4f 62 6a 65 63 74 03  00 00 00 06 05 00 00 00  |:Object.........|
 00000040  05 5f 53 69 7a 65 0a 05  6e 65 76 65 72 00 00 00  |._Size..never...|
 00000050  0b 5f 45 78 70 69 72 65  73 5f 41 74 05 00 00 00  |._Expires_At....|
-00000060  04 5f 4b 65 79 09 48 fb  ba 11 00 00 00 0b 5f 43  |._Key.H......._C|
+00000060  04 5f 4b 65 79 09 48 fa  b5 67 00 00 00 0b 5f 43  |._Key.H..g...._C|
 00000070  72 65 61 74 65 64 5f 41  74 04 03 00 00 00 05 04  |reated_At.......|
 00000080  01 00 00 30 d5 ff d8 ff  e0 00 10 4a 46 49 46 00  |...0.......JFIF.|
 00000090  01 01 00 00 01 00 01 00  00 ff fe 00 3b 43 52 45  |............;CRE|
@@ -796,6 +796,6 @@
 000031c0  67 0a 0a 69 6d 61 67 65  2f 6a 70 65 67 00 00 00  |g..image/jpeg...|
 000031d0  0b 63 6f 6e 74 65 6e 74  54 79 70 65 05 00 00 00  |.contentType....|
 000031e0  04 73 69 7a 65 00 00 00  05 5f 44 61 74 61 09 48  |.size...._Data.H|
-000031f0  fb ba 11 00 00 00 0c 5f  41 63 63 65 73 73 65 64  |......._Accessed|
+000031f0  fa b5 67 00 00 00 0c 5f  41 63 63 65 73 73 65 64  |..g...._Accessed|
 00003200  5f 41 74                                          |_At|
 00003203

The only meaningful change appears to be the 2026 versus 2027 in that path name.

So the scanning run didn't generate the desired filename.  Any clues?

Thanks...
Comment 3 Martin Schwenke 2008-10-21 17:29:18 UTC
Alright, delving slightly deeper:

mysql> select title from albums where artwork = 2027;
Empty set (0.00 sec)

mysql> select title from albums where artwork = 2026;
+--------------------------+
| title                    |
+--------------------------+
| Early 21st Century Blues | 
+--------------------------+
1 row in set (0.01 sec)

As you can see, I have no album with artwork 2027 but I do have an album with artwork 2026.

So, 2 questions:

* Why is the controller requesting artwork for 2027?

* How does squeezecenter know which cover to use when 2027 is requested?

Thanks...
Comment 4 Martin Schwenke 2008-10-21 17:32:35 UTC
Ahhh!  Now I see what is happening...  It is generating new cache/Artwork files for *every track*.  That seems like complete overkill, since an album usually only has 1 cover...  ;-)

Is there any way of turning off this behaviour and just have the controller request the cover for the album?
Comment 5 Martin Schwenke 2008-10-21 19:17:13 UTC
Created attachment 4164 [details]
Patch to add backend for preferTrackArtwork option.

OK, here's a patch that adds a server config setting called 'preferTrackArtwork', which defaults to 1, so you get the current behaviour.  If you set it to 0 it turns off the "fix" to Bug 7443, which makes squeezecenter prefer track artwork over album artwork.  A low end server like mine just can't afford to keep generating album art for every track, so that's the justification for making it a server config option.

The patch also includes the part of the patch from comment #1 that causes album art for the controller to be pre-cached.

This only does the backend part: defines the option and checks its value.  Right now I haven't added the option to the web interface but I'm happy to do so if it has a high likelyhood of being accepted.  For now I'm happy to hack server.prefs to set this to 0.

peace & happiness,
martin
Comment 6 Martin Schwenke 2008-10-21 19:22:44 UTC
OK, this looks like it will never end...

Now that I've hacked various things (probably hand-hacking server.prefs?) the web interface insists on going into the setup wizard.  It seems to think squeezeserver isn't configured at all

Any clues on how to avoid that?
Comment 7 Martin Schwenke 2008-10-21 21:14:36 UTC
Added:

wizardDone: 1

to end of server.prefs and it is all fine.

I can't find anything in the code that would have reset that option...  :-(
Comment 8 Jim McAtee 2008-10-21 22:48:26 UTC
See some of the comments in bug 6225.  It would be nice if there were an option to not use any type of per-track artwork.  That way the album artwork would be used for all tracks on the album and it wouldn't require generating all those identical files.
Comment 9 Martin Schwenke 2008-10-22 01:34:13 UTC
I agree Jim.

Unless I've missed something obvious, the current patch attached to this bug provides the option you want.  It allows you to turn off the per-track artwork.  It is against 7.2.0.

I wonder if you can try it out?  You'll need to add:

preferTrackArtwork: 0

to server.prefs to get the desired behaviour.  It seems to work for me.

It seems like a simpler short-term solution than changing the DB schema.  If we can get some traction for this patch, perhaps in Bug 6225, I'll try to work out how to put the config editing for this option into the web interface.

That said, I agree that thinking carefully about the problem and fixing the schema is the right way to go in the long-term.  ;-)

Note: After hand-hacking server.prefs, I've ended up losing a bunch of settings and SqueezeCenter has insisted on re-scanning my collection.  I think that's because audiodir got unset.  So, that'll take about 20 hours for me (again).  So, take care when hand-hacking server.prefs...  unless you have a nice fast machine doing the scan...  :-)
Comment 10 Martin Schwenke 2008-10-25 15:08:33 UTC
My patch seems to work for all of the obvious artwork for the controller.  

However, it doesn't fix the problem for the thumbnails in the playlist in the SqueezeCenter player.  I've searched through the code and can't think of an obvious way of fixing this...  :-(
Comment 11 James Richardson 2008-11-03 09:23:58 UTC
Andy: would this one be yours to handle? if not please reassign appropriately.
Comment 12 Michael Herger 2008-11-06 12:36:31 UTC
Martin - did you consider using resize instead of resample for the artwork? It would be a huge improvement in performance at the cost of slightly worse quality of the output files (Settings/Advanced/Performance). And no code change needed.

I assume your CPU doesn't have a floating-point unit, does it?
Comment 13 Martin Schwenke 2008-11-06 13:55:25 UTC
Hi Michael,

I used that for a while and things were somewhat better... but I still had the occasional audio skip.

If I used resize instead of resample, would the artwork quality be noticeably worse at the sizes (154, 186) the controller uses?  I didn't take much notice last time I tried it because I was focusing on solving the audio performance problem.

One issue is that I already have 250MB of cached artwork - most of this is the pre-cached 154 and 186 pixel JPEGs for the controller, courtesy of my hack.  If the cached artwork doesn't expire (and for performance reasons I don't want it to) then, at say 16 tracks per album (many have less but I have some boxed sets with 50-100 tracks), I'll probably have 4GB of cached artwork.  My / filesystem doesn't have enough space for that much cached artwork.  :-(  I could bind mount or symlink the artwork cache to somewhere else... but it seems pointless because it seems crazy to cache 10, 16 or 94 (for Lennon's Anthology) cached copies of the same artwork!

I really do wonder what percentage of people use per-track artwork?  I'd imagine that most people just have 1 cover artwork image per album.

I'm happy with my current hack and I am happy to re-apply it to upgrades (or simply not upgrade for a while).  However, I think the long-term solution is to fix the DB schema to reference the  artwork for each song but, if there is only 1 piece of artwork for an album, reference the same artwork URL.  That way, for my case, both the controller and the web interface would always attempt to load the same artwork for every song...

peace & happiness,
martin
Comment 14 Andy Grundman 2009-07-29 15:03:49 UTC
Need to review pre-caching for 7.4.
Comment 15 Martin Schwenke 2009-08-03 16:11:48 UTC
The precaching part seems like it should be much simpler these days:

---------------
--- /usr/share/perl5/Slim/Music/Artwork.pm.orig 2009-01-20 12:13:59.000000000 +1
100
+++ /usr/share/perl5/Slim/Music/Artwork.pm      2009-03-18 13:36:35.000000000 +1
100
@@ -367,6 +367,8 @@
                        50 => '_o',
                        $coversize => '_o',
                        56 => '_o.jpg',
+                       154 => '_o.jpg',
+                       186 => '_o.jpg',
                );
        
                for my $dim ( keys %dims ) {
--------------

I haven't carefully checked if this causes the right images to get precached.  Doing this, adding the preferTrackArwork option and also turning off resampling is causing things to play without any skipping at all.  That never used to work even with just resampling off, so I assume the precaching is working...

I'll attach a new patch.  It's what I did for 7.3.2 and it applies (with slight offset) to 7.3.3.

I think the important thing is to either add a hack like the preferTrackArtwork option... or fix the schema so that things are better normalised.  However, fixing the schema will be a big job...  and it will touch everything...
Comment 16 Martin Schwenke 2009-08-03 16:14:17 UTC
Created attachment 5559 [details]
Add precaching for controller artwork and preferTrackArtwork option

New-ish patch.  Still no preferences UI code for preferTrackArtwork option.
Comment 17 Andy Grundman 2009-08-12 07:09:47 UTC
Ben: can you give me a quick inventory of all image sizes where cover art is 
used in all skins?
Comment 18 Ben Klaas 2009-08-18 06:54:52 UTC
artwork sizes used in squeezeplay: 40, 41, 64, 143, 180, 240, 272
Comment 19 Michael Herger 2009-08-18 14:04:25 UTC
After some discussion with Ben we came to the conclusion that a static list of sizes won't do. A minor change on the SqueezePlay side of things and it's a waste of resources. We must come up with something more dynamic. Here goes my suggestion:

Let's introduce a registerartworkparams or similar command which allows to register artwork size and resizing parameters. Any software requiring artwork in a given size (Squeezeplay desktop & Controller, iPeng, your greatest and latest plugin) can tell SC to pre-cache a list of given artwork types:

registerartworkparams jive ['120x120_m','300x140_m.jpg']

I'd add the client type to allow updating this setting. If we had eg. the following defaults:

{
   web => ['50x50_o.jpg','100x100_o.jpg'],
   jive => ['120x120_m','300x150_m.jpg'],
}

the client could specifically update its own parameters without invalidating the others, or end up in a ever growing list of formats. The caller just has to make sure it provides the full list of formats it wants.

The client type isn't used but for this update filtering. Nor is the default list exhaustive. Any app/plugin could register its own sizes, eg. iPeng. This would allow to start with a minimum default set, but allow for growth with new apps as needed.

Thoughts?

(I know, this doesn't cover the artwork per track vs. artwork per album issue)
Comment 20 Joerg Schwieder 2009-08-18 14:19:01 UTC
Sounds good to me.

Is an app expected to register all sizes it will eventually need? How's the process then dynamically: When the app registers a size for the first time, the thumbs are being generated and this is being remembered for subsequent scans until a newer version of the app probably replaces the size info?

And what is the strategy as towards what should be registered and what not.
Without thinking it over I would say for iPeng I would register 75x75 thumbs but NOT register the 320x320 and 480x480 fullscreen images I use since the latter ones are not time critical (IMHO).

Correct?
Comment 21 Martin Schwenke 2009-08-18 15:23:06 UTC
Michael, that sounds excellent!  This is an important 1/2 of the issue and this seems like a really good independent fix.

Joerg makes some interesting points.  My view is that the current precaching logic is a bit too strict.  If my memory and current reading of the code is correct, precaching is only done if there is no artwork already in the DB.  This doesn't handle the case where you improve the quality of some artwork.  I think it also checks that the timestamp (of the artwork?) and checks that it is more recent than the current scan.  This doesn't work well if, for example, I download/scan some artwork and then forget to sync my artwork directory on my workstation up to the server.

So, there will be lots of cases to consider... but I think this will be an important fix.

Thanks!  :-)
Comment 22 Michael Herger 2009-08-18 23:16:52 UTC
> Is an app expected to register all sizes it will eventually need?

No. It's about the pre-caching, thus optional optimization. Eg. it does
make sense to pre-cache browse size artwork, but not the largest version a
user probably _might_ need by turning the "Fit full art" option in iPeng
on (as you mentioned below).

> When the app registers a size for the first time, the
> thumbs are being generated and this is being remembered for subsequent  
> scans until a newer version of the app probably replaces the size info?

That's indeed not part of my proposal yet. The new sizes would only be
pre-cached the next time a full scan is run.

Andy is planning on improving the overall artwork strategy. I could
imagine to run an artwork-only scan if one of these parameters are
changed. But that would be part of bug 9919.

> And what is the strategy as towards what should be registered and what  
> not.
> Without thinking it over I would say for iPeng I would register 75x75  
> thumbs
> but NOT register the 320x320 and 480x480 fullscreen images I use since  
> the latter ones are not time critical (IMHO).
> Correct?

Exactly.
Comment 23 Michael Herger 2009-08-19 00:03:29 UTC
> If my memory and current reading of the code is
> correct, precaching is only done if there is no artwork already in the  
> DB.

That's correct. This is part of what bug 9919 is about. Artwork handling  
needs more improvement. I'm only trying to improve the currently broken  
pre-caching (due to format changes) handling right now.
Comment 24 Richard Titmuss 2009-08-19 01:19:20 UTC
Not a 7.4 task, but I'm planning on implementing better image resizing in SP. When that's done SC should only need to cache one size (optimized for network download) and SP will resize it. Therefore I am not sure effort improving the SC caching is needed for 7.4.
Comment 25 Joerg Schwieder 2009-08-19 01:23:34 UTC
As long as that size is 75x75 that's fine with me...
Comment 26 SVN Bot 2009-08-19 01:47:39 UTC
 == Auto-comment from SVN commit #28225 to the slim repo by michael ==
 == https://svn.slimdevices.com/slim?view=revision&revision=28225 ==

Bug: 9757
Description: fix pre-cached artwork size; full fix will require more work, probably post 7.4
Comment 27 Joerg Schwieder 2009-08-19 01:58:18 UTC
Could you probably already define how the command is supposed to look like so I could implement the registration and it will work as soon as the feature's available.
Comment 28 Martin Schwenke 2009-08-19 03:28:09 UTC
(In reply to comment #23)
> That's correct. This is part of what bug 9919 is about. Artwork handling  
> needs more improvement. I'm only trying to improve the currently broken  
> pre-caching (due to format changes) handling right now.

Cool, thanks!

I've also added a comment to bug 9919.
Comment 29 Anders Markstrom 2009-08-19 03:38:24 UTC
This is excellent. 

This cover my bug 10751.

The registerartworkparams also covers my thoughts that it should work without
any interaction from the user.
Comment 30 Michael Herger 2009-08-20 02:53:35 UTC
I'm closing this bug as fixed, as artwork in Controller size is pre-cached again.

I've moved the "register artwork parameters to be pre-cached" proposal to bug 10751.
Comment 31 James Richardson 2009-10-05 14:26:55 UTC
This bug has been marked as fixed in the 7.4.0 release version of SqueezeBox Server!
    * SqueezeCenter: 28672
    * Squeezebox 2 and 3: 130
    * Transporter: 80
    * Receiver: 65
    * Boom: 50
    * Controller: 7790
    * Radio: 7790  

Please see the Release Notes for all the details: http://wiki.slimdevices.com/index.php/Release_Notes

If you haven't already, please download and install the new version from http://www.logitechsqueezebox.com/support/download-squeezebox-server.html

If you are still experiencing this problem, feel free to reopen the bug with your new comments and we'll have another look.
Comment 32 Mikael Nyberg 2011-01-07 04:36:37 UTC
In Version: 7.6.0 - r31707 etc.

Some sizes are not pre-cached for all my albums ?:

Example:

11-01-06 19:58:24.6779] Slim::Web::Graphics::artworkRequest (83) Artwork request: music/1a987dd0/cover_240x240_m
[11-01-06 19:58:24.6814] Slim::Web::Graphics::artworkRequest (122)   Resize specification: 240x240_m
[11-01-06 19:58:24.6867] Slim::Web::Graphics::artworkRequest (272)   Resizing: /media/music/music2/Lossles Music/Loudon Wainwright III - 10 Songs for the New Depression/Cash for Clunkers.flac using spec 240x240_m
[11-01-06 19:58:24.8068] Slim::Web::Graphics::_cached (68)   from cache: jpg (22372 bytes for music/1a987dd0/cover_240x240_m)
[11-01-06 19:58:26.9025] Slim::Web::Graphics::artworkRequest (83) Artwork request: music/1a987dd0/cover_190x190_m
[11-01-06 19:58:26.9050] Slim::Web::Graphics::artworkRequest (122)   Resize specification: 190x190_m
[11-01-06 19:58:26.9091] Slim::Web::Graphics::artworkRequest (272)   Resizing: /media/music/music2/Lossles Music/Loudon Wainwright III - 10 Songs for the New Depression/Cash for Clunkers.flac using spec 190x190_m
[11-01-06 19:58:26.9809] Slim::Web::Graphics::_cached (68)   from cache: jpg (14298 bytes for music/1a987dd0/cover_190x190_m)