python-mpd2 icon indicating copy to clipboard operation
python-mpd2 copied to clipboard

Support new “albumart” command

Open coderkun opened this issue 6 years ago • 7 comments

Is the new “albumart” command (see The music database) already supported or are there any plans to do so?

coderkun avatar Nov 20 '18 23:11 coderkun

You can make a pull request.

Mic92 avatar Nov 21 '18 01:11 Mic92

See #111 for a possible implementation

auchter avatar Apr 18 '20 22:04 auchter

On a related note, are there plans to support the readpicture command, another get-picture command with a binary response format? It seems like it would be a fairly straightforward addition to #111.

glen3b avatar Jun 10 '20 22:06 glen3b

If someone makes a PR with tests.

Mic92 avatar Jun 12 '20 19:06 Mic92

Hi, I see the albumart command is included, since this ticket is still open I guess something must be missing. How do I use this command? Are there already any real-world usecases? I tried calling it like File "examples/asyncio_example.py", line 32, in main print(client.albumart("/ji")) File "/usr/local/lib/python3.7/dist-packages/python_mpd2-1.1.0-py3.7.egg/mpd/base.py", line 361, in albumart AttributeError: 'MPDClient' object has no attribute '_execute_binary'

thouters avatar Oct 05 '20 18:10 thouters

Hi, I'm working on a PR implementing both albumart and readpicture by adding binary payload support on a more fundamental level than is currently done with _execute_binary(). Since I don't see a way to integrate _execute_binary() with the asyncio classes. The unittests for this are thorough, kudos! (it will take me some time to get my (albeit working in blue-sky) code to pass them).

Even though I'm not finished, I'd like to open the discussion on the data returned by the existing non-asyncio albumart call. The albumart command now returns the binary data straight away. Since the readpicture command also has a mime-type keyvalue pair, which should be exposed, I think it makes sense to change the return value of albumart to a dict as well. This is what my work-in-progress is returning: albumart: [{'size': '1925', 'binary': bytearray(b'\xff....')}] readpicture [{'size': '1925'}, {'type': 'image/jpeg', 'binary': bytearray(b'\xff\xd8.....')}] Are there any objections to this?

thouters avatar Oct 15 '20 18:10 thouters

Hi, I'm working on a PR implementing both albumart and readpicture by adding binary payload support on a more fundamental level than is currently done with _execute_binary(). Since I don't see a way to integrate _execute_binary() with the asyncio classes. The unittests for this are thorough, kudos! (it will take me some time to get my (albeit working in blue-sky) code to pass them).

Even though I'm not finished, I'd like to open the discussion on the data returned by the existing non-asyncio albumart call. The albumart command now returns the binary data straight away. Since the readpicture command also has a mime-type keyvalue pair, which should be exposed, I think it makes sense to change the return value of albumart to a dict as well. This is what my work-in-progress is returning: albumart: [{'size': '1925', 'binary': bytearray(b'\xff....')}] readpicture [{'size': '1925'}, {'type': 'image/jpeg', 'binary': bytearray(b'\xff\xd8.....')}] Are there any objections to this?

Isn't size redundant with the bytestream, since Python strings track length? That also seems like a rather large structural overhead (IMO) for the amount of data which is actually being returned, and many of the other calls in this library have a much more direct return.

That said I do agree we need a better way to handle binary returns with arbitrary header sequences. As far as readpicture is concerned I had some thoughts a while back in #121 about how to handle the API signature; I don't know what @Mic92's current thoughts are.

glen3b avatar Oct 16 '20 06:10 glen3b