Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mrp: Handle missing artwork gracefully #1033

Merged
merged 1 commit into from Apr 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 10 additions & 4 deletions docs/api/pyatv/interface.html
Expand Up @@ -857,7 +857,9 @@ <h1 class="title">Module <code>pyatv.interface</code></h1>

@abstractmethod
@feature(30, &#34;Artwork&#34;, &#34;Playing media artwork.&#34;)
async def artwork(self, width=512, height=None) -&gt; Optional[ArtworkInfo]:
async def artwork(
self, width: Optional[int] = 512, height: Optional[int] = None
) -&gt; Optional[ArtworkInfo]:
&#34;&#34;&#34;Return artwork for what is currently playing (or None).

The parameters &#34;width&#34; and &#34;height&#34; makes it possible to request artwork of a
Expand Down Expand Up @@ -2145,7 +2147,9 @@ <h3>Methods</h3>

@abstractmethod
@feature(30, &#34;Artwork&#34;, &#34;Playing media artwork.&#34;)
async def artwork(self, width=512, height=None) -&gt; Optional[ArtworkInfo]:
async def artwork(
self, width: Optional[int] = 512, height: Optional[int] = None
) -&gt; Optional[ArtworkInfo]:
&#34;&#34;&#34;Return artwork for what is currently playing (or None).

The parameters &#34;width&#34; and &#34;height&#34; makes it possible to request artwork of a
Expand Down Expand Up @@ -2244,7 +2248,7 @@ <h3>Instance variables</h3>
<h3>Methods</h3>
<dl>
<dt id="pyatv.interface.Metadata.artwork"><code class="name flex">
<span>async def <span class="ident">artwork</span></span>(<span>self, width=512, height=None) -> Optional[<a title="pyatv.interface.ArtworkInfo" href="#pyatv.interface.ArtworkInfo">ArtworkInfo</a>]</span>
<span>async def <span class="ident">artwork</span></span>(<span>self, width: Optional[int] = 512, height: Optional[int] = None) -> Optional[<a title="pyatv.interface.ArtworkInfo" href="#pyatv.interface.ArtworkInfo">ArtworkInfo</a>]</span>
</code></dt>
<dd>
<section class="desc"><p>Return artwork for what is currently playing (or None).</p>
Expand All @@ -2259,7 +2263,9 @@ <h3>Methods</h3>
</summary>
<pre><code class="python">@abstractmethod
@feature(30, &#34;Artwork&#34;, &#34;Playing media artwork.&#34;)
async def artwork(self, width=512, height=None) -&gt; Optional[ArtworkInfo]:
async def artwork(
self, width: Optional[int] = 512, height: Optional[int] = None
) -&gt; Optional[ArtworkInfo]:
&#34;&#34;&#34;Return artwork for what is currently playing (or None).

The parameters &#34;width&#34; and &#34;height&#34; makes it possible to request artwork of a
Expand Down
8 changes: 6 additions & 2 deletions pyatv/dmap/__init__.py
Expand Up @@ -205,7 +205,9 @@ async def playstatus(self, use_revision=False, timeout=None):
self.latest_hash = self.latest_playing.hash
return self.latest_playing

async def artwork(self, width, height) -> Optional[ArtworkInfo]:
async def artwork(
self, width: Optional[int], height: Optional[int]
) -> Optional[ArtworkInfo]:
"""Return artwork for what is currently playing (or None)."""
url = _ARTWORK_CMD.format(width=width or 0, height=height or 0)
art = await self.daap.get(url, daap_data=False)
Expand Down Expand Up @@ -405,7 +407,9 @@ def __init__(self, identifier, apple_tv):
self.apple_tv = apple_tv
self.artwork_cache = Cache(limit=4)

async def artwork(self, width=512, height=None) -> Optional[ArtworkInfo]:
async def artwork(
self, width: Optional[int] = 512, height: Optional[int] = None
) -> Optional[ArtworkInfo]:
"""Return artwork for what is currently playing (or None).

The parameters "width" and "height" makes it possible to request artwork of a
Expand Down
4 changes: 3 additions & 1 deletion pyatv/interface.py
Expand Up @@ -629,7 +629,9 @@ def device_id(self) -> Optional[str]:

@abstractmethod
@feature(30, "Artwork", "Playing media artwork.")
async def artwork(self, width=512, height=None) -> Optional[ArtworkInfo]:
async def artwork(
self, width: Optional[int] = 512, height: Optional[int] = None
) -> Optional[ArtworkInfo]:
"""Return artwork for what is currently playing (or None).

The parameters "width" and "height" makes it possible to request artwork of a
Expand Down
17 changes: 11 additions & 6 deletions pyatv/mrp/__init__.py
Expand Up @@ -420,7 +420,9 @@ def __init__(self, protocol, psm, identifier):
self.psm = psm
self.artwork_cache = Cache(limit=4)

async def artwork(self, width=512, height=None) -> Optional[ArtworkInfo]:
async def artwork(
self, width: Optional[int] = 512, height: Optional[int] = None
) -> Optional[ArtworkInfo]:
"""Return artwork for what is currently playing (or None).

The parameters "width" and "height" makes it possible to request artwork of a
Expand All @@ -438,14 +440,17 @@ async def artwork(self, width=512, height=None) -> Optional[ArtworkInfo]:
_LOGGER.debug("Retrieved artwork %s from cache", identifier)
return self.artwork_cache.get(identifier)

artwork = await self._fetch_artwork(width or 0, height or -1)
if artwork:
artwork: Optional[ArtworkInfo] = None
try:
artwork = await self._fetch_artwork(width or 0, height or -1)
except Exception:
_LOGGER.warning("Artwork not present in response")
else:
self.artwork_cache.put(identifier, artwork)
return artwork

return None
return artwork

async def _fetch_artwork(self, width, height):
async def _fetch_artwork(self, width, height) -> Optional[ArtworkInfo]:
playing = self.psm.playing
resp = await self.psm.protocol.send_and_receive(
messages.playback_queue_request(playing.location, width, height)
Expand Down
15 changes: 9 additions & 6 deletions tests/fake_device/mrp.py
Expand Up @@ -480,12 +480,15 @@ def handle_playback_queue_request(self, message, inner):
setstate = messages.create(
protobuf.SET_STATE_MESSAGE, identifier=message.identifier
)
queue = setstate.inner().playbackQueue
queue.location = 0
item = queue.contentItems.add()
item.artworkData = self.state.states[self.state.active_player].artwork
item.artworkDataWidth = state.artwork_width or 456
item.artworkDataHeight = state.artwork_height or 789

artwork_data = self.state.states[self.state.active_player].artwork
if artwork_data:
queue = setstate.inner().playbackQueue
queue.location = 0
item = queue.contentItems.add()
item.artworkData = artwork_data
item.artworkDataWidth = state.artwork_width or 456
item.artworkDataHeight = state.artwork_height or 789
self.send_to_client(setstate)

def handle_wake_device(self, message, inner):
Expand Down
13 changes: 13 additions & 0 deletions tests/mrp/test_mrp_functional.py
Expand Up @@ -168,6 +168,19 @@ async def test_metadata_artwork_id_no_identifier(self):
await self.playing(title="dummy")
self.assertEqual(self.atv.metadata.artwork_id, "some_id")

@unittest_run_loop
async def test_metadata_artwork_erroneously_available(self):
self.usecase.example_video()

# Metadata suggests that artwork is available but no artwork is available
# when requested by client
self.usecase.change_artwork(None, ARTWORK_MIMETYPE, ARTWORK_ID)

await self.playing(title="dummy")

artwork = await self.atv.metadata.artwork(width=123, height=456)
self.assertIsNone(artwork)

@unittest_run_loop
async def test_metadata_artwork_width_and_height(self):
self.usecase.example_video()
Expand Down