Support MPD albumart and readpicture command

[adnidor]

Add support to the MPD frontend for the albumart command allowing clients to receive album art directly from mopidy.

Defined here: https://www.musicpd.org/doc/protocol/database.html

[jjok]

Does anyone know how the versioning works with MPD? Currently we declare version 0.19.0.

#: The MPD protocol version is 0.19.0.
VERSION = "0.19.0"

It looks like, from this client, albumart was added in 0.21.0. If we want to support album art, are we meant to, in theory, add everything else from that version? Will it cause problems is we just add one command?

Should clients use the protocol version to find out if a server supports a command, or should they be using the commands command?

[jodal]

If I wrote a client, I'd use commands for feature detection. After 10 years of Mopidy, I believe most MPD client developers are aware of both MPD and Mopidy and hopefully test with both.

That way, we can add new commands one by one, and they'll be useful at least in some clients. To bump the declared version up, I think that we at least should make "empty shells" of any new commands that we don't support, so that the commands are correctly parsed but calling them doesn't change anything.

[kingosticks]

By "empty shells" do you mean they would feature in the commands output so they appear supported, but then don't actually do anything useful? How would clients then correctly detect that we don't actually support something? e.g. #19

[jjok]

There are a few places where we currently a raise NotImplemented exception at the moment.

[jjok]

Here's the relevant documentation for albumart.

albumart {URI} {OFFSET}

Locate album art for the given song and return a chunk of an album art image file at offset OFFSET.

Returns the file size and actual number of bytes read at the requested offset, followed by the chunk requested as raw bytes (see Binary Responses), then a newline and the completion code.

Example:

albumart foo/bar.ogg 0
size: 1024768
binary: 8192
<8192 bytes>
OK

[jodal]

By "empty shells" do you mean they would feature in the commands output so they appear supported, but then don't actually do anything useful? How would clients then correctly detect that we don't actually support something? e.g. #19

We should probably not include those in commands. There has been a very long time since I did any new MPD protocol implementation, so there might be better strategies than what I pull out of my head right now. I guess the most important thing is how clients handle commands missing from commands and/or commands not being implemented.

[BarrettLowe]

I'd like to have this feature also. I would think a NotImplemented would be okay - not great obviously - but I would assume that not all clients would hit the errors. Guess we should figure out what changed from 0.19 and now...

[jjok]

I had a go at implementing this recently, but I couldn't get it to work. We need to be able to allow bytes() in the response message lines, as well as regular strings. I'll see if I can share some code.

[leso-kn]

Hello! I have implemented the albumart command in #57. This does not include the readpicture command and thus works on with media source providing an album art url (e.g. dlna, jellyfin, etc.).

[gvc0]

I tested leso-kn's implementation and did some updates which are waiting for review and commit. I verified the new feature with:

• MAFA (Android client. Works great)
• MALP (Android client with 'experimental' artwork support, so crashes regularly)
• MPDCtlr (Windows client. Has difficulties with Mopidy's MPD protocol implementation, but downloads album art if you get tracks to display)
• Stylophone (Windows client. Is also struggling with Mopidy's limited MPD command set but downloads album art when you play a track)
• Home Assistant server (Web gui and mobile app work fine)
• Persephone (Mac OS client. Works perfectly)

Can't test IOS unfortunately.

[LevitatingBusinessMan]

@gvc0 Just want to say thanks for all the activity from you. Contributors like you keep Mopidy alive.

[leso-kn]

@gvc0 Merged your changes into into leso-kn/mopidy-mpd (see #57). Sorry for the delay, last few months have been quite fluctuative.

Note: ⚠️ There currently seems to be an issue with the Github Actions pipeline of mopidy-mpd.
The pipeline does however pass test_albumart.py and albumart.py, therefor it appears to be an unrelated issue. I am not able to reproduce it locally (in a containerized (clean) local environment, all unit-tests pass and flake8 is satisfied, see steps to do that below).

# Steps to clean-run unit-tests and generate coverage.xml  (Python 3.9.16)
> docker run --rm -ti -v $PWD:/pro --workdir /pro python:3.9-alpine ash -c "set -e
apk add gcc gstreamer gst-plugins-bad gobject-introspection-dev cairo-dev musl-dev
pip install tox PyGObject
python -m tox -e py39 -- --cov-report=xml"

I would like to repeat that this feature is ready for merging! Thanks to @gvc0 it now additionally supports artwork of local tracks through mopidy-local (optional dependency and their changes are covered by the test_albumart.py unit-test).