For local music libraries, queue tab cannot load album art covers from music file's directory

[TwelveP]

Hi, thanks for sharing this client. Playback and library scanning work like a charm! However I want to report two issues I'm having regarding the cover art support. I would like to help as well, but first things first; I think acknowledging the root of the problem is top priority.

A little filesystem context
I'm running MPD and this client/server stack in a Win10 machine. My music library resides looks like this Music/${audio-format}/${album-folder}/${music-file} and each album folder name is formatted like ${artist} [${year} ${release-type}] ${release-name}/. Each release-name folder may have a front.jpg file acting like the sole cover art.

I run the server with most default settings like so java -jar "-Dmpd.music.directory=F:/Benjamin/Music" .\ampd-1.3.4.jar

The problems

1. If I play my music files, their local cover art file is ignored and instead the server always tries to fetch another one from the cache, or the web and then caching it that's enabled. Gotta say, that doesn't always work or even matches the album. However when I go browsing each folder, the aforementioned cover art file is correctly read and displayed in the client web interface with no issue whatsoever.

2. If the album or band name contains symbols that are forbidden to filenames, the server it fails to cache the cover art at all. Take for example, post-punk band DEVO's debut album "Q: Are We Not Men? A: We Are Devo!" and you'll surely understand the issue. The raised exception goes as follows:

2021-10-25 00:22:24.942 ERROR 17328 --- [0.0-8080-exec-4] o.a.c.c.C.[.[.[/].[dispatcherServlet]    : Servlet.service() for servlet [dispatcherServlet] in context with path [] threw exception [Request processing failed; nested exception is java.nio.file.InvalidPathException: Illegal char <:> at index 51: C:\Users\Benjamin\.local\share\ampd\covers\a_DEVO_Q: Are We Not Men? A: We Are Devo!.jpg] with root cause

java.nio.file.InvalidPathException: Illegal char <:> at index 51: C:\Users\Benjamin\.local\share\ampd\covers\a_DEVO_Q: Are We Not Men? A: We Are Devo!.jpg
        at java.base/sun.nio.fs.WindowsPathParser.normalize(WindowsPathParser.java:182) ~[na:na]
        at java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:153) ~[na:na]
        at java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:77) ~[na:na]
        at java.base/sun.nio.fs.WindowsPath.parse(WindowsPath.java:92) ~[na:na]
        at java.base/sun.nio.fs.WindowsFileSystem.getPath(WindowsFileSystem.java:229) ~[na:na]
        at java.base/java.nio.file.Path.of(Path.java:147) ~[na:na]
        at java.base/java.nio.file.Paths.get(Paths.java:69) ~[na:na]
        at org.hihn.ampd.server.service.CoverCacheService.buildCacheFullPath(CoverCacheService.java:209) ~[ampd-1.3.4.jar:1.3.4]
        at org.hihn.ampd.server.service.CoverCacheService.loadCover(CoverCacheService.java:94) ~[ampd-1.3.4.jar:1.3.4]
        at org.hihn.ampd.server.service.CoverService.findAlbumCoverForTrack(CoverService.java:74) ~[ampd-1.3.4.jar:1.3.4]
        at org.hihn.ampd.server.controller.CoverController.findAlbumCoverForTrack(CoverController.java:57) ~[ampd-1.3.4.jar:1.3.4]
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:na]
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[na:na]
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:na]
        at java.base/java.lang.reflect.Method.invoke(Method.java:566) ~[na:na]
        at org.springframework.web.method.support.InvocableHandlerMethod.doInvoke(InvocableHandlerMethod.java:197) ~[spring-web-5.3.8.jar:5.3.8]
        [...]

Hope this can be made sense of. And again, I'd be glad to help if it's not overly complicated. I have a couple ideas on how to fix these, but obviously a lot of questions to make before that.

[rain0r]

Hi there,

happy to hear you're enjoying ampd!

To problem 1:

Things concering the cover cache and loading covers from MusicBrainz:

You're totally right, I also got some weird covers from the MusicBrainz API. Especially for non-popular tracks. I just checked the code and the "min score" for a cover to appear in the search query results is 60. The higher it is the higher is the chance it's the correct cover. To be honest I don't know which value would give us well balanced search results.

I made it configurable via the application.properties, the name is min.score. You could try different values with the -Dmin.score=XX flag when starting ampd.

You could also turn off the local cache completely via -Dlocal.cover.cache=false. ampd won't make any calls to the MusicBrainz API in that case.

No cover in the queue

This is indeed a bug, thanks for reporting it! Fixed that.

To problem 2:

It looks like ampd fails to read the file. Java cannot read files on Windows that contain a colon (:).
Just tested it on OSX and it works, so that's something I can't fix.
Personally, I would recommed to abstain from these "problematic" characters (:*\ etc.) in filenames.

For now, I placed a try catch around it.

I'm glad to hear your ideas on it!

[TwelveP]

Sorry, I think I didn't make myself clear. My issue was thinking that the queue tab should display cover arts from local filesystem only. If I disable caching with MusicBrainz it just shows nothing. Is this by design as of right now?

About the second issue, I thought a simple way to avoid illegal characters in those filenames: calculating a cheap hash function from the artist/album name and using said hash to create and read files. So, instead of creating a cache file named DEVO - Q: Are We Not Men? [...].jpg, it could be created like as89g63490g23as9fa8[...].jpg and from the artist/album, refer to that same file by calculating said hash every time. That's my quick thought; I know it may not look clean...

Also btw some artists like Caetano Veloso happen to have released three, four albums named exactly the same, only in different years. This will fool the algorithm into thinking they're the same album when they're not. Maybe a better solution is to use the MusicBrainz album ID.

[rain0r]

Hi there!

Sorry, I think I didn't make myself clear. My issue was thinking that the queue tab should display cover arts from local filesystem only. If I disable caching with MusicBrainz it just shows nothing. Is this by design as of right now?

This was indeed a bug in ampd. I fixed it in this commit.

About the second issue, I thought a simple way to avoid illegal characters in those filenames: calculating a cheap hash function from the artist/album name and using said hash to create and read files. So, instead of creating a cache file named DEVO - Q: Are We Not Men? [...].jpg, it could be created like as89g63490g23as9fa8[...].jpg and from the artist/album, refer to that same file by calculating said hash every time. That's my quick thought; I know it may not look clean...

For this, we could just use String.hashCode() and it would be a better solution than what we have now.

Also btw some artists like Caetano Veloso happen to have released three, four albums named exactly the same, only in different years. This will fool the algorithm into thinking they're the same album when they're not. Maybe a better solution is to use the MusicBrainz album ID.

Yes, this would also be my preferred solution. Unfortunately, the mpd library in use (javampd) does not offer an option to read specific id3 tags.

[TwelveP]

This was indeed a bug in ampd. I fixed it in this commit.

OK, tested it and now it finds my cover art files alright. Thanks for fixing!

However I suggest letting users configure the cover art filename pattern(s) they wish to use. This may come as a surprise, but in my case, within some folders I have more than one image file lying around; these can be back covers, cd pictures, liner notes etc. So using astherisks matches the first one in alphabetical order, regardless of which one it is.

For this, we could just use String.hashCode() and it would be a better solution than what we have now.

Yes, that's simple enough.

Unfortunately, the mpd library in use (javampd) does not offer an option to read specific id3 tags.

That's too bad. Think it could be requested as a new feature for javampd?

Also, sorry if my next question is dumb, but I really need some orientation in regards to mpd in general, so here goes:
Can this client integrate a plugin to scrobble to last.fm? Or that's a concern for mpd itself?

[rain0r]

This was indeed a bug in ampd. I fixed it in this commit.

OK, tested it and now it finds my cover art files alright. Thanks for fixing!

You're welcome!

However I suggest letting users configure the cover art filename pattern(s) they wish to use. This may come as a surprise, but in my case, within some folders I have more than one image file lying around; these can be back covers, cd pictures, liner notes etc. So using astherisks matches the first one in alphabetical order, regardless of which one it is.

In my last commit I've added the property artwork.filename.pattern which can be used to specify the glob when looking for artwork.

But it's only used when browsing and not when viewing the queue.

This is because javampd can only return the artwork for the file currently played. For browsing, ampd itself looks for artwork using the value of mpd.music.directory.

For this, we could just use String.hashCode() and it would be a better solution than what we have now.

Yes, that's simple enough.

Implemented the use of String.hashCode() on the associated branch

Unfortunately, the mpd library in use (javampd) does not offer an option to read specific id3 tags.

That's too bad. Think it could be requested as a new feature for javampd?

You could do that, yes. The repository is https://github.com/finnyb/javampd
But it seems inactive at the moment.

Also, sorry if my next question is dumb, but I really need some orientation in regards to mpd in general, so here goes: Can this client integrate a plugin to scrobble to last.fm? Or that's a concern for mpd itself?

There is no system for plugins in ampd. Also, scrobbling is something which should be done on the server-side, in my opinion.

For example, I'm using mpdscribble