Support new “albumart” command #102

Activity

1. Mic92 commented on Nov 21, 2018

   

   Mic92

   on Nov 21, 2018

   Owner

   More actions

   You can make a pull request.
2. auchter commented on Apr 18, 2020

   

   auchter

   on Apr 18, 2020

   Contributor

   More actions

   See #111 for a possible implementation
3. glen3b commented on Jun 10, 2020

   

   glen3b

   on Jun 10, 2020

   Contributor

   More actions

   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.
4. Mic92 commented on Jun 12, 2020

   

   Mic92

   on Jun 12, 2020

   Last edited by Mic92

   Owner

   More actions

   If someone makes a PR with tests.
5. thouters commented on Oct 5, 2020

   

   thouters

   on Oct 5, 2020

   More actions

   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'

   👍React with 👍1Reacted by Martin Weinelt
6. thouters commented on Oct 15, 2020

   

   thouters

   on Oct 15, 2020

   More actions

   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?
7. glen3b commented on Oct 16, 2020

   

   glen3b

   on Oct 16, 2020

   Contributor

   More actions

   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.