diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 0957b3403..82c79a023 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -246,7 +246,7 @@ class DiscogsPlugin(BeetsPlugin): # Extract information for the optional AlbumInfo fields, if possible. va = result.data['artists'][0].get('name', '').lower() == 'various' year = result.data.get('year') - mediums = len(set(t.medium for t in tracks)) + mediums = [t.medium for t in tracks] country = result.data.get('country') data_url = result.data.get('uri') @@ -270,12 +270,18 @@ class DiscogsPlugin(BeetsPlugin): # `autotag.apply_metadata`, and set `medium_total`. for track in tracks: track.media = media - track.medium_total = mediums + track.medium_total = mediums.count(track.medium) + # Discogs does not have track IDs. Invent our own IDs as proposed + # in #2336. + track.track_id = str(album_id) + "-" + track.track_alt + + # Retrieve master release id (returns None if there isn't one) + master_id = result.data.get('master_id') return AlbumInfo(album, album_id, artist, artist_id, tracks, asin=None, albumtype=albumtype, va=va, year=year, month=None, - day=None, label=label, mediums=mediums, - artist_sort=None, releasegroup_id=None, + day=None, label=label, mediums=len(set(mediums)), + artist_sort=None, releasegroup_id=master_id, catalognum=catalogno, script=None, language=None, country=country, albumstatus=None, media=media, albumdisambig=None, artist_credit=None, @@ -342,30 +348,33 @@ class DiscogsPlugin(BeetsPlugin): # a 2-sided medium. if ''.join(m) in ascii_lowercase: sides_per_medium = 2 - side_count = 1 # Force for first item, where medium == None for track in tracks: # Handle special case where a different medium does not indicate a # new disc, when there is no medium_index and the ordinal of medium # is not sequential. For example, I, II, III, IV, V. Assume these # are the track index, not the medium. + # side_count is the number of mediums or medium sides (in the case + # of two-sided mediums) that were seen before. medium_is_index = track.medium and not track.medium_index and ( len(track.medium) != 1 or - ord(track.medium) - 64 != medium_count + 1 + # Not within standard incremental medium values (A, B, C, ...). + ord(track.medium) - 64 != side_count + 1 ) if not medium_is_index and medium != track.medium: - if side_count < (sides_per_medium - 1): - # Increment side count: side changed, but not medium. - side_count += 1 - medium = track.medium + side_count += 1 + if sides_per_medium == 2: + if side_count % sides_per_medium: + # Two-sided medium changed. Reset index_count. + index_count = 0 + medium_count += 1 else: - # Increment medium_count and reset index_count and side - # count when medium changes. - medium = track.medium + # Medium changed. Reset index_count. medium_count += 1 index_count = 0 - side_count = 0 + medium = track.medium + index_count += 1 medium_count = 1 if medium_count == 0 else medium_count track.medium, track.medium_index = medium_count, index_count diff --git a/docs/changelog.rst b/docs/changelog.rst index b1c0d4c29..8f45aa69c 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -24,6 +24,10 @@ New features: * A new interoperability plugin to automatically notify Sonos controllers to update the music library once the beets library got updated. Thanks to :user:`cgtobi`. +* :doc:`/plugins/discogs`: The plugin now stores master release ids into + ``mb_releasegroupid`` as well as simulates track ids using release id + and tracklist positions. Track ids are stored in ``mb_trackid``. :bug:`#2336` + Thanks to :user:`dbogdanov`. Fixes: @@ -35,7 +39,9 @@ Fixes: autotagger. Data tracks will always be ignored, but a new option ``ignore_video_tracks`` has been added to control if video tracks should be ignored or not. :bug:`1210` -* :doc:`/plugins/replaygain`: Fix a corner-case with the ``bs1770gain`` backend where ReplayGain values were assigned to the wrong files. Now ``bs1770gain`` version 0.4.6 or later is required. :bug:`2777` +* :doc:`/plugins/replaygain`: Fix a corner-case with the ``bs1770gain`` backend + where ReplayGain values were assigned to the wrong files. Now ``bs1770gain`` + version 0.4.6 or later is required. :bug:`2777` * :doc:`/plugins/lyrics`: The plugin no longer crashes in the Genius source when BeautifulSoup is not found. Instead, it just logs a message and disables the source. @@ -91,6 +97,13 @@ Fixes: * Really fix album replaygain calculation with gstreamer backend. :bug:`2846` * Avoid an error when doing a "no-op" move on non-existent files (i.e., moving a file onto itself). :bug:`2863` +* :doc:`/plugins/discogs`: Fix ``medium`` and ``medium_index`` values which + were occasionally incorrect for releases with two-sided mediums. Fix + ``medium_total`` value. It now contains total number of tracks on the medium + to which a track belongs, not the total number of different mediums present + on the release. :bug:`2887` + Thanks to :user:`dbogdanov`. + For developers: diff --git a/test/test_discogs.py b/test/test_discogs.py index 74f9a5656..8b2eff9f1 100644 --- a/test/test_discogs.py +++ b/test/test_discogs.py @@ -101,9 +101,9 @@ class DGAlbumInfoTest(_common.TestCase): self.assertEqual(d.mediums, 1) self.assertEqual(t[0].medium, 1) - self.assertEqual(t[0].medium_total, 1) + self.assertEqual(t[0].medium_total, 2) self.assertEqual(t[1].medium, 1) - self.assertEqual(t[0].medium_total, 1) + self.assertEqual(t[0].medium_total, 2) def test_parse_medium_numbers_two_mediums(self): release = self._make_release_from_positions(['1-1', '2-1']) @@ -112,9 +112,9 @@ class DGAlbumInfoTest(_common.TestCase): self.assertEqual(d.mediums, 2) self.assertEqual(t[0].medium, 1) - self.assertEqual(t[0].medium_total, 2) + self.assertEqual(t[0].medium_total, 1) self.assertEqual(t[1].medium, 2) - self.assertEqual(t[1].medium_total, 2) + self.assertEqual(t[1].medium_total, 1) def test_parse_medium_numbers_two_mediums_two_sided(self): release = self._make_release_from_positions(['A1', 'B1', 'C1']) @@ -129,7 +129,7 @@ class DGAlbumInfoTest(_common.TestCase): self.assertEqual(t[1].medium_total, 2) self.assertEqual(t[1].medium_index, 2) self.assertEqual(t[2].medium, 2) - self.assertEqual(t[2].medium_total, 2) + self.assertEqual(t[2].medium_total, 1) self.assertEqual(t[2].medium_index, 1) def test_parse_track_indices(self): @@ -139,10 +139,10 @@ class DGAlbumInfoTest(_common.TestCase): self.assertEqual(t[0].medium_index, 1) self.assertEqual(t[0].index, 1) - self.assertEqual(t[0].medium_total, 1) + self.assertEqual(t[0].medium_total, 2) self.assertEqual(t[1].medium_index, 2) self.assertEqual(t[1].index, 2) - self.assertEqual(t[1].medium_total, 1) + self.assertEqual(t[1].medium_total, 2) def test_parse_track_indices_several_media(self): release = self._make_release_from_positions(['1-1', '1-2', '2-1', @@ -153,16 +153,16 @@ class DGAlbumInfoTest(_common.TestCase): self.assertEqual(d.mediums, 3) self.assertEqual(t[0].medium_index, 1) self.assertEqual(t[0].index, 1) - self.assertEqual(t[0].medium_total, 3) + self.assertEqual(t[0].medium_total, 2) self.assertEqual(t[1].medium_index, 2) self.assertEqual(t[1].index, 2) - self.assertEqual(t[1].medium_total, 3) + self.assertEqual(t[1].medium_total, 2) self.assertEqual(t[2].medium_index, 1) self.assertEqual(t[2].index, 3) - self.assertEqual(t[2].medium_total, 3) + self.assertEqual(t[2].medium_total, 1) self.assertEqual(t[3].medium_index, 1) self.assertEqual(t[3].index, 4) - self.assertEqual(t[3].medium_total, 3) + self.assertEqual(t[3].medium_total, 1) def test_parse_position(self): """Test the conversion of discogs `position` to medium, medium_index