From 33b10d60fbb4823c01ac8b781ea33b50867aaa4f Mon Sep 17 00:00:00 2001 From: djl Date: Sun, 13 Sep 2020 20:27:12 +0100 Subject: [PATCH] fetchart: Improve Cover Art Archive source. (#3748) * fetchart: Improve Cover Art Archive source. Instead of blindly selecting the first image, we now treat all "front" images as candidates. This is useful where some digital releases have both an animated cover and a still image and the animated image is the first image returned from the API. --- beetsplug/fetchart.py | 53 +++++++++++----- docs/changelog.rst | 2 + test/test_art.py | 136 ++++++++++++++++++++++++++++++++++++++---- 3 files changed, 163 insertions(+), 28 deletions(-) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index 86c5b958f..1bf8ad428 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -308,16 +308,44 @@ class CoverArtArchive(RemoteArtSource): VALID_THUMBNAIL_SIZES = [250, 500, 1200] if util.SNI_SUPPORTED: - URL = 'https://coverartarchive.org/release/{mbid}/front' - GROUP_URL = 'https://coverartarchive.org/release-group/{mbid}/front' + URL = 'https://coverartarchive.org/release/{mbid}' + GROUP_URL = 'https://coverartarchive.org/release-group/{mbid}' else: - URL = 'http://coverartarchive.org/release/{mbid}/front' - GROUP_URL = 'http://coverartarchive.org/release-group/{mbid}/front' + URL = 'http://coverartarchive.org/release/{mbid}' + GROUP_URL = 'http://coverartarchive.org/release-group/{mbid}' def get(self, album, plugin, paths): """Return the Cover Art Archive and Cover Art Archive release group URLs using album MusicBrainz release ID and release group ID. """ + + def get_image_urls(url, size_suffix=None): + try: + response = self.request(url) + except requests.RequestException: + self._log.debug(u'{0}: error receiving response' + .format(self.NAME)) + return + + try: + data = response.json() + except ValueError: + self._log.debug(u'{0}: error loading response: {1}' + .format(self.NAME, response.text)) + return + + for item in data.get('images', []): + try: + if 'Front' not in item['types']: + continue + + if size_suffix: + yield item['thumbnails'][size_suffix] + else: + yield item['image'] + except KeyError: + pass + release_url = self.URL.format(mbid=album.mb_albumid) release_group_url = self.GROUP_URL.format(mbid=album.mb_releasegroupid) @@ -330,19 +358,12 @@ class CoverArtArchive(RemoteArtSource): size_suffix = "-" + str(plugin.maxwidth) if 'release' in self.match_by and album.mb_albumid: - if size_suffix: - release_thumbnail_url = release_url + size_suffix - yield self._candidate(url=release_thumbnail_url, - match=Candidate.MATCH_EXACT) - yield self._candidate(url=release_url, - match=Candidate.MATCH_EXACT) + for url in get_image_urls(release_url, size_suffix): + yield self._candidate(url=url, match=Candidate.MATCH_EXACT) + if 'releasegroup' in self.match_by and album.mb_releasegroupid: - if size_suffix: - release_group_thumbnail_url = release_group_url + size_suffix - yield self._candidate(url=release_group_thumbnail_url, - match=Candidate.MATCH_FALLBACK) - yield self._candidate(url=release_group_url, - match=Candidate.MATCH_FALLBACK) + for url in get_image_urls(release_group_url): + yield self._candidate(url=url, match=Candidate.MATCH_FALLBACK) class Amazon(RemoteArtSource): diff --git a/docs/changelog.rst b/docs/changelog.rst index a9d2ac540..c89bdb681 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -148,6 +148,8 @@ New features: Thanks to :user:`logan-arens`. :bug:`2947` * Added flac-specific reporting of samplerate and bitrate when importing duplicates. +* :doc:`/plugins/fetchart`: Cover Art Archive source now iterates over + all front images instead of blindly selecting the first one. Fixes: diff --git a/test/test_art.py b/test/test_art.py index f4b3a6e62..51e5a9fe8 100644 --- a/test/test_art.py +++ b/test/test_art.py @@ -76,6 +76,96 @@ class FetchImageHelper(_common.TestCase): file_type, b'').ljust(32, b'\x00')) +class CAAHelper(): + """Helper mixin for mocking requests to the Cover Art Archive.""" + MBID_RELASE = 'rid' + MBID_GROUP = 'rgid' + + RELEASE_URL = 'coverartarchive.org/release/{0}' \ + .format(MBID_RELASE) + GROUP_URL = 'coverartarchive.org/release-group/{0}' \ + .format(MBID_GROUP) + + if util.SNI_SUPPORTED: + RELEASE_URL = "https://" + RELEASE_URL + GROUP_URL = "https://" + GROUP_URL + else: + RELEASE_URL = "http://" + RELEASE_URL + GROUP_URL = "http://" + GROUP_URL + + RESPONSE_RELEASE = """{ + "images": [ + { + "approved": false, + "back": false, + "comment": "GIF", + "edit": 12345, + "front": true, + "id": 12345, + "image": "http://coverartarchive.org/release/rid/12345.gif", + "thumbnails": { + "1200": "http://coverartarchive.org/release/rid/12345-1200.jpg", + "250": "http://coverartarchive.org/release/rid/12345-250.jpg", + "500": "http://coverartarchive.org/release/rid/12345-500.jpg", + "large": "http://coverartarchive.org/release/rid/12345-500.jpg", + "small": "http://coverartarchive.org/release/rid/12345-250.jpg" + }, + "types": [ + "Front" + ] + }, + { + "approved": false, + "back": false, + "comment": "", + "edit": 12345, + "front": false, + "id": 12345, + "image": "http://coverartarchive.org/release/rid/12345.jpg", + "thumbnails": { + "1200": "http://coverartarchive.org/release/rid/12345-1200.jpg", + "250": "http://coverartarchive.org/release/rid/12345-250.jpg", + "500": "http://coverartarchive.org/release/rid/12345-500.jpg", + "large": "http://coverartarchive.org/release/rid/12345-500.jpg", + "small": "http://coverartarchive.org/release/rid/12345-250.jpg" + }, + "types": [ + "Front" + ] + } + ], + "release": "https://musicbrainz.org/release/releaseid" +}""" + RESPONSE_GROUP = """{ + "images": [ + { + "approved": false, + "back": false, + "comment": "", + "edit": 12345, + "front": true, + "id": 12345, + "image": "http://coverartarchive.org/release/releaseid/12345.jpg", + "thumbnails": { + "1200": "http://coverartarchive.org/release/rgid/12345-1200.jpg", + "250": "http://coverartarchive.org/release/rgid/12345-250.jpg", + "500": "http://coverartarchive.org/release/rgid/12345-500.jpg", + "large": "http://coverartarchive.org/release/rgid/12345-500.jpg", + "small": "http://coverartarchive.org/release/rgid/12345-250.jpg" + }, + "types": [ + "Front" + ] + } + ], + "release": "https://musicbrainz.org/release/release-id" + }""" + + def mock_caa_response(self, url, json): + responses.add(responses.GET, url, body=json, + content_type='application/json') + + class FetchImageTest(FetchImageHelper, UseThePlugin): URL = 'http://example.com/test.jpg' @@ -156,15 +246,13 @@ class FSArtTest(UseThePlugin): self.assertEqual(candidates, paths) -class CombinedTest(FetchImageHelper, UseThePlugin): +class CombinedTest(FetchImageHelper, UseThePlugin, CAAHelper): ASIN = 'xxxx' MBID = 'releaseid' AMAZON_URL = 'https://images.amazon.com/images/P/{0}.01.LZZZZZZZ.jpg' \ .format(ASIN) AAO_URL = 'https://www.albumart.org/index_detail.php?asin={0}' \ .format(ASIN) - CAA_URL = 'coverartarchive.org/release/{0}/front' \ - .format(MBID) def setUp(self): super(CombinedTest, self).setUp() @@ -211,17 +299,19 @@ class CombinedTest(FetchImageHelper, UseThePlugin): self.assertEqual(responses.calls[-1].request.url, self.AAO_URL) def test_main_interface_uses_caa_when_mbid_available(self): - self.mock_response("http://" + self.CAA_URL) - self.mock_response("https://" + self.CAA_URL) - album = _common.Bag(mb_albumid=self.MBID, asin=self.ASIN) + self.mock_caa_response(self.RELEASE_URL, self.RESPONSE_RELEASE) + self.mock_caa_response(self.GROUP_URL, self.RESPONSE_GROUP) + self.mock_response('http://coverartarchive.org/release/rid/12345.gif', + content_type='image/gif') + self.mock_response('http://coverartarchive.org/release/rid/12345.jpg', + content_type='image/jpeg') + album = _common.Bag(mb_albumid=self.MBID_RELASE, + mb_releasegroupid=self.MBID_GROUP, + asin=self.ASIN) candidate = self.plugin.art_for_album(album, None) self.assertIsNotNone(candidate) - self.assertEqual(len(responses.calls), 1) - if util.SNI_SUPPORTED: - url = "https://" + self.CAA_URL - else: - url = "http://" + self.CAA_URL - self.assertEqual(responses.calls[0].request.url, url) + self.assertEqual(len(responses.calls), 3) + self.assertEqual(responses.calls[0].request.url, self.RELEASE_URL) def test_local_only_does_not_access_network(self): album = _common.Bag(mb_albumid=self.MBID, asin=self.ASIN) @@ -416,6 +506,28 @@ class GoogleImageTest(UseThePlugin): next(self.source.get(album, self.settings, [])) +class CoverArtArchiveTest(UseThePlugin, CAAHelper): + + def setUp(self): + super(CoverArtArchiveTest, self).setUp() + self.source = fetchart.CoverArtArchive(logger, self.plugin.config) + self.settings = Settings(maxwidth=0) + + @responses.activate + def run(self, *args, **kwargs): + super(CoverArtArchiveTest, self).run(*args, **kwargs) + + def test_caa_finds_image(self): + album = _common.Bag(mb_albumid=self.MBID_RELASE, + mb_releasegroupid=self.MBID_GROUP) + self.mock_caa_response(self.RELEASE_URL, self.RESPONSE_RELEASE) + self.mock_caa_response(self.GROUP_URL, self.RESPONSE_GROUP) + candidates = list(self.source.get(album, self.settings, [])) + self.assertEqual(len(candidates), 3) + self.assertEqual(len(responses.calls), 2) + self.assertEqual(responses.calls[0].request.url, self.RELEASE_URL) + + class FanartTVTest(UseThePlugin): RESPONSE_MULTIPLE = u"""{ "name": "artistname",