search

volumio / Volumio2

Volumio 2 - Audiophile Music Player
http://volumio.org
Other
1.37k stars 315 forks source link

no picture in album view #1161

Closed Warter21 closed 3 years ago

Warter21 commented 7 years ago

Hi!

I have a strange problem. I browse the albums, sometime I cannot see the picture (folder.jpg) on the album view. But if I go into the albums any of these, the picture is magically appear. So I guess it is not a permission issue. Any idea how can I solve it?

Thanks, Warter

xipmix commented 7 years ago

Can't reproduce on 2.129. Can you write a bit more precisely the sequence you follow, eg Browse->Albums->[albumname] or Browse->Artists->[artist]->[albumname]

Very handy if you can tail -f /var/log/volumio.log in a terminal window while you are doing this, so you see exactly when the error comes.

Warter21 commented 7 years ago

it came from the volumio.log. I just added nfs share to volumio and browse the albums in the web interface. In the album view some albums don't show the album picture but If I go into any of these albums, I can see the album picture (folder.jpg). Sequence: Browse->Albums ->[albumname]

xipmix commented 7 years ago

Let me make sure I understand: Browse->Albums : no picture shown for some albums Browse->Albums->[album-with-no-picture] : picture appears next to the album tracks

If you then go back to Browse->Albums, still no picture for the album you visited?

Can you paste a few log lines from before the error, eg from where it says 'info: CURURI: albums://'

Is there something consistent about the albums with no picture, eg album name has multiple words, or artist is more than one word?

Warter21 commented 7 years ago

Yes, you are right. Moreover If I go to "Artists" and "Aerosmith", I can see the right pictures under "Albums" and "Tracks" too. Unfortunately I cannot realize any scheme.

Logs: --- Browse --- [1494431867625] ---------------------------- Client requests Menu Items 2017-05-10T15:57:47.640Z - info: [1494431867635] Listing playlists 2017-05-10T15:57:47.650Z - info: CoreCommandRouter::executeOnPlugin: appearance , getUiSettings 2017-05-10T15:57:47.978Z - info: [1494431867973] ------------------------------ 348ms 2017-05-10T15:57:57.430Z - info: CoreCommandRouter::volumioGetBrowseSources 2017-05-10T15:57:57.444Z - info: CoreCommandRouter::volumioGetQueue 2017-05-10T15:57:57.452Z - info: [1494431877447] CoreStateMachine::getQueue 2017-05-10T15:57:57.458Z - info: [1494431877453] CorePlayQueue::getQueue 2017-05-10T15:57:57.465Z - info: [1494431877458] ---------------------------- Client requests Volumio queue 2017-05-10T15:57:57.473Z - info: [1494431877468] InterfaceWebUI::pushQueue 2017-05-10T15:57:57.503Z - info: [1494431877497] ------------------------------ 58ms

--- Albums --- 2017-05-10T15:58:28.072Z - info: CoreCommandRouter::executeOnPlugin: mpd , handleBrowseUri 2017-05-10T15:58:28.082Z - info: CURURI: albums://

--- Aerosmith / Gold - CD1 --- 2017-05-10T15:59:22.563Z - info: CoreCommandRouter::executeOnPlugin: mpd , handleBrowseUri 2017-05-10T15:59:22.569Z - info: CURURI: albums://Gold%20-%20CD1

--- Back --- 2017-05-10T15:59:54.571Z - info: CoreCommandRouter::executeOnPlugin: mpd , handleBrowseUri 2017-05-10T15:59:54.577Z - info: CURURI: albums://

--- From Artists --- 2017-05-10T16:01:43.359Z - info: CoreCommandRouter::executeOnPlugin: mpd , handleBrowseUri 2017-05-10T16:01:43.365Z - info: CURURI: artists:// 2017-05-10T16:01:47.334Z - info: CoreCommandRouter::executeOnPlugin: mpd , handleBrowseUri 2017-05-10T16:01:47.344Z - info: CURURI: artists://Aerosmith

liberatorcnx commented 7 years ago

This is happening because when we view albums mpd is asked for a 'list' of albums (in mpd/index.js). The returned list does not include any information about the path to the album so we cannot look in the album folder for the artwork. What happens is we then do a query through album-art to try to download the artwork. That is why you will see (most) albums have artwork but there may be some where the lookup fails. When you select an album from the list mpd tells us the path to the album so we can use artwork from the album folder.

I have been looking at the possibility of returning a list from mpd that includes the path - that would enable us to use the album art that may well be in the album folder, Alas, the mpd list function does not allow us to do that. The alternative is to do a full query to mpd but that will be much slower than the 'list' function. Anyway, I'm still looking at alternatives to correct this (and save on a lot of duplicated artwork)... Any suggestions welcome!

xipmix commented 7 years ago

function. Anyway, I'm still looking at alternatives to correct this (and save on a lot of duplicated artwork)... Any suggestions welcome!

Kind of ugly, but if there was no albumart returned (I assume this is easy to detect) asking mpd about the first track on an album would more reliably return the albumart?

xipmix commented 7 years ago

The returned list does not include any information about the path to the album so we cannot look in the album folder for the artwork.

Since mpd just had all their issues deleted and moved to github, this might be an opportune time for a feature request ? Or is this misunderstanding the purpose of list ?

liberatorcnx commented 7 years ago

Kind of ugly, but if there was no albumart returned (I assume this is easy to detect) asking mpd about the first track on an album would more reliably return the albumart?

The problem is that you can't ask about the first track on an album. It's all or nothing! mpd returns all tracks for any other type of query. So to find the path for every album you have to deal with a query that returns every track in your collection. For a particular artist or a specific album that's fine because we can ask, for example, for albums by artist 'x' and it will return the result quickly. If we were to wait to see if we find the art via album-art, then query mpd again if not found would just be too slow.

liberatorcnx commented 7 years ago

Since mpd just had all their issues deleted and moved to github, this might be an opportune time for a feature request ? Or is this misunderstanding the purpose of list ?

Good luck with that! I tried to discuss some related issues on their forum and it was not well received by the admin....

mpd really needs more flexibility with the whole searching and listing areas. It's quite crazy that we have to get a full list of every track just to find some simple data.

xipmix commented 7 years ago

If we were to wait to see if we find the art via album-art, then query mpd again if not found would just be too slow.

I take your point, but then I started to wonder - too slow for what? When the user does Browse->Albums they want to see a list of albums, promptly - I get that. What I started wondering is whether people would mind if the display then gradually updates with the missing albumart (via the slower find method) while they browse the list of albums, rather than providing albumart for all titltes at the instant the page loads. Particularly if we can cache the results successfully. A related approach - when the user does the next click: ->[album name], the albumart that is retrieved is made available at the 'Album' level as well (since the same albumart icon applies to all tracks in the one album), so when they next do Browse->Albums they get an albumart icon for the album they recently visited, from the cache. I'm sure I'm missing some things here, I have not spent enough time with the code.

liberatorcnx commented 7 years ago

I take your point, but then I started to wonder - too slow for what? When the user does Browse->Albums they want to see a list of albums, promptly - I get that. What I started wondering is whether people would mind if the display then gradually updates with the missing albumart (via the slower find method) while they browse the list of albums, rather than providing albumart for all titltes at the instant the page loads. Particularly if we can cache the results successfully. A related approach - when the user does the next click: ->[album name], the albumart that is retrieved is made available at the 'Album' level as well (since the same albumart icon applies to all tracks in the one album), so when they next do Browse->Albums they get an albumart icon for the album they recently visited, from the cache. I'm sure I'm missing some things here, I have not spent enough time with the code.

Once the artwork has been loaded by the browser it will be in cache so it's only a problem first time. We ideally want to look for the same artwork that is used once you list the tracks for an album. That means we have to get the path when we do the first listing of albums I think rather than try the current method and then fall back to getting the path....

liberatorcnx commented 7 years ago

I have had a good look at the workings of album artwork now. Currently Volumio will try to download artwork for albums when you first view all albums. If it fails there is no fallback. When you select an Album to view tracks it will look for artwork in the music source album folder (folder.jpg or similar) and then copy it to the artwork folder if there is no artwork already there to use. This is why we can end up having no artwork for an album on the main Albums page but we do have artwork once we select the album to display tracks etc.

I have now modified a version of mpd\index.js so that it does the following;

  1. Gets a list of all album data from mpd when viewing Albums
  2. Searches for artwork for each album in the albumart folder.
  3. If artwork is not already in the albumart folder check to see if there is appropriate artwork in the music source albums own folder (folder.jpg or cover.jpg etc.). If there is artwork there copy it to the artwork folder and use that. If not..
  4. Try to download the artwork using album-art module and if successful copy it to the albumart folder to use.

This produces a consistent result. Albums either have artwork or they don't. If you have artwork in your music source album folders it will be used. If not, it will try to download artwork.

All sounds great so far - but there is a cost:

  1. The first time (only) you try to view all albums it will be slow to populate the artwork on the webpage because it has to either copy the artwork from the music source album folder or try to download artwork. This is a 'one time' hit only. Once this has been done it will only have to repeat this for any new albums added.
  2. The albums page (excluding artwork) will load slower because it is doing a full listing from mpd each time. In my tests on a RPi 3 with some 600 albums and almost 9,000 tracks it takes about 4 seconds to load. Copying and downloading files on the first run took about 5 minutes (my music source is on a slow network mount) but once completed subsequent artwork loading was almost instantaneous thanks to the browser cache.

Do you think people will live with this? Any other thoughts? Maybe a short pop-up message 'Loading Albums'?

xipmix commented 7 years ago

This sounds great. When the albums first load, how long is it before you see anything displayed - 4s or 5m? If it is 4s then there is a specific UI widget for showing 'please wait' that seems appropriate here, I'll try to find out the name. If the wait is long, there is a progress bar thing (used in the plugin upload) that might be appropriate. Meanwhile a toast popup sounds fine.

I think the slowness with large collections (point 2) may need to be addressed at some point, but perhaps as another iteration. It seems to me that it should be possible to collate a stack of promises that will deal with the artwork lookups (steps 3 and 4) in the background. That could continue in the background independent of what is happening in the UI. Which gives me an idea. Why not make the artwork search a background job that runs on startup? Now that you've made the caching consistent, that might be the best place to do it. Again, this seems like another iteration.

volumio commented 7 years ago

You basically came accross the same issues that we did. And the system you see now was the best compromise on albumart completeness and UI reactivity. The timings you mentioned above are not acceptable, we need to put in first place usability and UI smoothness.

Warter21 commented 7 years ago

if there is no lookup value and there is a folder/album.jpg, it should link to the correct location during the background process. I mean link the folder.jpg to the /data/albumart/web/[artist]/[album]/folder.jpg and update the info.json with this information.

liberatorcnx commented 7 years ago

Whilst I agree that usability and UI smoothness is important, for those of us who have spent many hours making sure our album art is correct it is very disappointing to see generic album art icons for our albums. Having spent more time looking at this issue I think there is a workable solution.

Currently we have to do a web download for covers when first viewing albums. When we look at the tracks for any album we then look for art work in the album folder, then copy it to the albumart 'folder' if the album art exists. That means we are storing 2 sets of album art for most albums and the process of copying the artwork happens slowly, as we view individual album tracks. It also means that the album art could be different depending on which artwork you are seeing.

I have looked at ways to speed up the mpd query when requesting full data as is necessary in order to get the path to the album and display the existing artwork. I have put timing tests into the function and my findings with my library are as follows;

Run function including mpd query for all albums based on 10 tests:

Typical Total Function Time: 1804.023ms. (varies between 1503.172ms and 1934.846ms)

Example Console log:

May 22 13:39:44 volumio volumio[4186]: info: CoreCommandRouter::executeOnPlugin: mpd , handleBrowseUri
May 22 13:39:44 volumio volumio[4186]: info: CURURI: albums://
May 22 13:39:44 volumio volumio[4186]: NOT FOUND IN CACHE - need  to query MPD
May 22 13:39:44 volumio volumio[4186]: Sent MPD command : search album ""
May 22 13:39:46 volumio volumio[4186]: TotalFunctionTime: 1849.921ms

The first time it is run we then have to wait some time before the album art placeholders are fully populated - the same as we do currently (time depends on various factors - storage location, network speed, file sizes etc). Subsequent request will load much quicker as the artwork will now be on the Volumio device and likely in the browser cache.

The real issue is with subsequent requests which are currently displayed very quickly, but if a full mpd query is run again we still have to wait the 1804.023ms (average) before the page can be served. One solution is to cache the result of the mpd query that we ran the first time and use that. Then we don't need to run the query again unless we restart Volumio or we update the database. The test results when using a cache are much better of course!

Run function using cached mpd result for all albums based on 10 tests:

Typical Total Function Time: 593.565ms. (varies between 484.070ms and 691.747ms)

Example Console log:

May 22 13:41:13 volumio volumio[4186]: info: CoreCommandRouter::executeOnPlugin: mpd , handleBrowseUri
May 22 13:41:13 volumio volumio[4186]: info: CURURI: albums://
May 22 13:41:13 volumio volumio[4186]: FOUND IN CACHE!
May 22 13:41:13 volumio volumio[4186]: No need to query MPD!
May 22 13:41:13 volumio volumio[4186]: TotalFunctionTime: 564.669ms

This is perfectly acceptable to me and provides a smooth interface. Others may think differently! It may be possible to make this even faster by caching the function but I haven't looked that far yet...

Thoughts??

liberatorcnx commented 7 years ago

Hmm - no response to my last message....

Meantime, I have now cached the response from the first time we select Albums - result is that after the first request the result is virtually instant as there is nothing to do except display the response. It is actually quicker then the current method.

We could also include the first request in the startup process so that the result is already in cache. The only other time a 'new' first request is needed is when the content is updated. Again we could include a process to cache the albums in the background following completion of the update.

I believe that this will satisfy all needs for a quick and smooth interface - it will be even quicker than the current process.

volumio commented 7 years ago

Sorry, but this is not the way to go. We need to have it really smooth. Only way to achieve this is to have our own database

liberatorcnx commented 7 years ago

We need to have it really smooth. Only way to achieve this is to have our own database

Not true! I now have the response loaded to cache during boot and it is 100% smooth. No waiting for pages to load at any point. It works and is quick - quicker than the current version. It solves an issue with missing album art and reduces the need to download (duplicated) art work. What's not to like? Maybe I am missing something here?

CaptainMetal commented 7 years ago

hello volumio,

maybe you can add a button like "scan connected libraries for cover-art", or something like that, instead/alternatively for downloading from internet.....then the folder.jpg or whatever can be copied to the albumart database

or no, wait, take this one:

may be you can do this automatically when the rescan/update button is pressed for scanning the connected network drives !

this would be great !!! and step in the right direction -))))

regards captainmetal

liberatorcnx commented 7 years ago

@xipmix - please take a look at my PR and if you have a chance please test it. I am interested in your feedback. Currently it is caching the response from MPD and doing the lookups and file copies the first time that you view albums. Of course the current version downloads everything from the internet so that will be slower (in most cases). One thing about using existing album art in album folders is that it just takes whatever size that artwork is so it is important to have decent artwork in the folder but not unnecessarily large!

CaptainMetal commented 7 years ago

Hello again,

i tested your amazing work, and it works very well. ALL Albumcovers are displayed correctly like it should ! great work !

But there is also a small bug (?). In the Albumview, on lets say 25% of the Albums, the Artistname is no longer displayed, and when selecting the album for play, it just do nothing. The Albums with Artistname (sometimes the same Artist) are played. image

regards, captainmetal

CaptainMetal commented 7 years ago

Oh and i forget to add: the Albums are now sorted by Artistname and no longer by Albumname. This is wonderfull -)))

liberatorcnx commented 7 years ago

@CaptainMetal - do you have AlbumArtist tags for those albums missing Artist name? Albums are listed by AlbumArtist so if this tag is missing you will get this result...

CaptainMetal commented 7 years ago

oh yes, i am sorry. that was the problem ! my fault.

xipmix commented 7 years ago

I finally found some time to try this. Preparation for testing with trunk volumio 2.201 and this PR added on top:

To check the time it took to get all the artwork I used the mtimes on the downloaded files

$ find /data/albumart/web -type f -exec stat -c "%Y" {} \; >/tmp/times;
$ start=`sort -n /tmp/times|head -1`
$ finish=`sort -n /tmp/times|tail -1`

As a cross-check I also noted the wall clock time (date) just after the first albumart loaded and when the last one did. This check confirmed the numbers below are not drastically in error.

The timing results are interesting but there are differences in behaviour that may not make them directly comparable. First the raw numbers:

Now the differences that your PR shows:

I hope this is of some help. I think it is clear your download method is fast enough for practical use but there seem to be a few surprises in how the cached images are getting used. Perhaps that can be improved later PRs.

Having written all this, I found a mistake in my analysis. I wanted to check how many duplicate files there were, so I did the obvious

 find /data/albumart/web/ -type f -exec md5sum {} \; >/tmp/md5

and then nearly fell over backwards

awk '{print $1}' /tmp/md5|sort|uniq -c|wc  
     23      46     943

There are 362 files with the same checksum! That turns out to be because they are all info.json files containing

{}

Checking for other file types than .json:

$ find /data/albumart/web/ -type f ! -name "*.json"|wc
     14      14    1308

So I really don't understand what is going on here. There are waaay more than 14 distinct album art images displayed in the web ui. I have to leave this for a bit now but soon I will go back to plain 2.201 and check what happens there.

xipmix commented 7 years ago

I reimaged a 2.201 image (i.e. vanilla volumio) and did the preparation steps as above. I left it running for a few hours (with nothing in the play queue) before looking at the albumart issue.

Turned on albumart and went to Browse->Artists -- all the albumart was there already in the web UI !?? Same for Browse->Albums. But no files in /data/albumart/web.

When I clicked on an album, it took a couple of seconds for the same albumart shown for the album to appear next to each track title. And in /data/albumart/web I get a PNG file with the image and an info.json pointing to it.

It seems that while I was gone volumio quietly downloaded all the imagery but there doesn't seem to be any on-disk cache. The src location of the albumart images in the Browse->Albums and Browse->Artists views are a url pointing at http://volumio.local/albumart?<artist>/<album>/<artsize>/yadayada. I find this quite surprising - that once the image has been obtained and was visible in the UI, there was no attempt to cache it any other place than the browser. But see below.

I opened another browser window in private mode and checked Browse->Albums and Browse->Artists. I also had something in the queue this time. With a clean browser cache, the albumart downloads started over again and I timed them coming in (the Artists view makes this easier as they come roughly in order). In /data/albumart/web there appeared 483 PNG images and 819 json files, taking 369 seconds for the whole process. It seems having something in the play queue is required to trigger this, which seems like a bug to me. Note that, judged by their md5 sums, there were 466 unique PNG files. The duplicates were things like multiple downloads of the same image for a various artists album, or the same image for slightly different artist names ('A and B', vs 'A & B').

liberatorcnx commented 7 years ago

I don't really understand your results @xipmix - with vanilla volumio(2.201), browse artist and browse albums both rely on downloading images to display. This is because the query made to mpd is 'list artist' or 'list album' which does not return the path to albums, so library artwork cannot be used.

The downloaded images are stored in /data/albumart/web/(artist or albumartist). The number of artists downloaded will depend on the 'Sort Artists By' setting (in Settings - My Music - Music Library Settings). If set to Artist, all Artists are downloaded. If set to AlbumArtist then all AlbumArtists are downloaded. Browse albums always uses AlbumArtist (it's the only logical tag to use given that an album can have multiple Artists). In order for this to work you must ensure that AlbumArtist tags are populated for albums. The results you quoted in your previous post suggest that maybe this is not the case with your albums.

If you browse artists and then select an artist, volumio will check if there is local album artwork for the artists albums (with a file name that matches the correct pattern - set in plugins/miscellanea/albumart/albumart.js 184). If it finds a relevant file, it is then copied to /data/albumart/folder/artist/album and used. Strangely, if it fails to find a matching file but finds ANY jpg, jpeg or png file it will use the first one it finds but DOES NOT COPY the file to disk cache. Otherwise it will try to download artwork and store it at /data/albumart/web/artist/album_name.

A similar process is used for browse albums - covers are downloaded and saved to /data/albumart/web/artist/album name. These downloaded images are used to display at browse albums. If you then select an album then volumio checks for local artwork as per with artists and copies relevant files to /data/albumart/folder/artist/album.

This all works 100% for me with vanilla volumio(2.201) on Raspberry Pi 3B and was the starting point for my PR.

xipmix commented 7 years ago

Which parts do you find confusing?

Perhaps a better question is what things would you like me to test for? I think we've covered the question of whether the speed is adequate.

It might be worth writing down the expected behaviour in a slightly more formal way, to focus the discussion. Down the track this might be useful for figuring out how to write proper tests.

Example, which may or may not resemble reality

Starting conditions

My feeling is there are maybe a half dozen of these that are worth elucidating. I guess the minimum would be the cases where your PR differs from vanilla. Do you think this is useful?

On the question of AlbumArtist tags, I haven't checked the source files (all .flac) but will do so soon.

xipmix commented 7 years ago

I checked a half dozen source files, none have AlbumArtist (or 'Album Artist', etc).

liberatorcnx commented 6 years ago

I am no longer working on development of Volumio, but if you copy cover.jpg from the root ‘Ella Fitzgerald Sings the Duke Ellington Songbook’ folder and paste a copy into each of the Disk folders (Disk 1 / Disk 2 / Disk 3) it should work.

--

Liberator

From: GuidoParlato notifications@github.com Sent: 20 March 2018 15:08 To: volumio/Volumio2 Volumio2@noreply.github.com Cc: Liberator steve@stevemalone.com; Comment comment@noreply.github.com Subject: Re: [volumio/Volumio2] no picture in album view (#1161)

Hi all, i'm very sorry to bother you with this topic, i have read hundreds of threads and i'm really not able to understand:

How is this things of the album art woking?

I'm definitively not the most technical person, but i'm sure about two things:

This are my settings: https://user-images.githubusercontent.com/37587579/37662854-4adde7cc-2c58-11e8-8739-ecc7ac5a6044.png

and this is the result: https://user-images.githubusercontent.com/37587579/37662909-5ce35c7c-2c58-11e8-86b3-f1952f6c7ffa.png

and this the two folders of the first two Ella Fitzgerald albums:

https://user-images.githubusercontent.com/37587579/37663055-ae7efe1a-2c58-11e8-94ee-7f26ab2ac263.png

https://user-images.githubusercontent.com/37587579/37663056-aec4d5a2-2c58-11e8-9208-b97173ae82f4.png

I have really tried any possible combination, and i have edited repetitively metadata, but i'm not able to find a pattern.

Thank you

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/volumio/Volumio2/issues/1161#issuecomment-374632887 , or mute the thread https://github.com/notifications/unsubscribe-auth/ALpNU-JPAabiXoMjhJttX7ws65C20B6fks5tgRtHgaJpZM4NOGjD . https://github.com/notifications/beacon/ALpNU7XIt7jEWTox26QBHd0XAW2GaFrWks5tgRtHgaJpZM4NOGjD.gif