Merge pull request #2222 from diego-plan9/discogs-fixes

Several discogs fixes

beetsplug/discogs.py

@@ -34,6 +34,8 @@
 import json
 import socket
 import os
+import traceback
+from string import ascii_lowercase
 
 
 # Silence spurious INFO log lines generated by urllib3.
@@ -234,6 +236,11 @@ def get_album_info(self, result):
             catalogno = None
         country = result.data.get('country')
         media = result.data['formats'][0]['name']
+        # Explicitly set the `media` for the tracks, since it is expected by
+        # `autotag.apply_metadata`, and set `medium_total`.
+        for track in tracks:
+            track.media = media
+            track.medium_total = mediums
         data_url = result.data['uri']
         return AlbumInfo(album, album_id, artist, artist_id, tracks, asin=None,
                          albumtype=albumtype, va=va, year=year, month=None,
@@ -269,10 +276,19 @@ def get_artist(self, artists):
     def get_tracks(self, tracklist):
         """Returns a list of TrackInfo objects for a discogs tracklist.
         """
+        try:
+            clean_tracklist = self.coalesce_tracks(tracklist)
+        except Exception as exc:
+            # FIXME: this is an extra precaution for making sure there are no
+            # side effects after #2222. It should be removed after further
+            # testing.
+            self._log.debug(u'{}', traceback.format_exc())
+            self._log.error(u'uncaught exception in coalesce_tracks: {}', exc)
+            clean_tracklist = tracklist
         tracks = []
         index_tracks = {}
         index = 0
-        for track in tracklist:
+        for track in clean_tracklist:
             # Only real tracks have `position`. Otherwise, it's an index track.
             if track['position']:
                 index += 1
@@ -283,7 +299,19 @@ def get_tracks(self, tracklist):
         # Fix up medium and medium_index for each track. Discogs position is
         # unreliable, but tracks are in order.
         medium = None
-        medium_count, index_count = 0, 0
+        medium_count, index_count, side_count = 0, 0, 0
+        sides_per_medium = 1
+
+        # If a medium has two sides (ie. vinyl or cassette), each pair of
+        # consecutive sides should belong to the same medium.
+        if all([track.medium is not None for track in tracks]):
+            m = sorted(set([track.medium.lower() for track in tracks]))
+            # If all track.medium are single consecutive letters, assume it is
+            # 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
@@ -295,11 +323,17 @@ def get_tracks(self, tracklist):
             )
 
             if not medium_is_index and medium != track.medium:
-                # Increment medium_count and reset index_count when medium
-                # changes.
-                medium = track.medium
-                medium_count += 1
-                index_count = 0
+                if side_count < (sides_per_medium - 1):
+                    # Increment side count: side changed, but not medium.
+                    side_count += 1
+                    medium = track.medium
+                else:
+                    # Increment medium_count and reset index_count and side
+                    # count when medium changes.
+                    medium = track.medium
+                    medium_count += 1
+                    index_count = 0
+                    side_count = 0
             index_count += 1
             track.medium, track.medium_index = medium_count, index_count
 
@@ -315,30 +349,109 @@ def get_tracks(self, tracklist):
 
         return tracks
 
+    def coalesce_tracks(self, raw_tracklist):
+        """Pre-process a tracklist, merging subtracks into a single track. The
+        title for the merged track is the one from the previous index track,
+        if present; otherwise it is a combination of the subtracks titles.
+        """
+        def add_merged_subtracks(tracklist, subtracks):
+            """Modify `tracklist` in place, merging a list of `subtracks` into
+            a single track into `tracklist`."""
+            # Calculate position based on first subtrack, without subindex.
+            idx, medium_idx, _ = self.get_track_index(subtracks[0]['position'])
+            position = '%s%s' % (idx or '', medium_idx or '')
+
+            if len(tracklist) > 1 and not tracklist[-1]['position']:
+                # Assume the previous index track contains the track title, and
+                # "convert" it to a real track. The only exception is if the
+                # index track is the only one on the tracklist, as it probably
+                # is a medium title.
+                tracklist[-1]['position'] = position
+            else:
+                # Merge the subtracks, pick a title, and append the new track.
+                track = subtracks[0].copy()
+                track['title'] = ' / '.join([t['title'] for t in subtracks])
+                tracklist.append(track)
+
+        # Pre-process the tracklist, trying to identify subtracks.
+        subtracks = []
+        tracklist = []
+        prev_subindex = ''
+        for track in raw_tracklist:
+            # Regular subtrack (track with subindex).
+            if track['position']:
+                _, _, subindex = self.get_track_index(track['position'])
+                if subindex:
+                    if subindex.rjust(len(raw_tracklist)) > prev_subindex:
+                        # Subtrack still part of the current main track.
+                        subtracks.append(track)
+                    else:
+                        # Subtrack part of a new group (..., 1.3, *2.1*, ...).
+                        add_merged_subtracks(tracklist, subtracks)
+                        subtracks = [track]
+                    prev_subindex = subindex.rjust(len(raw_tracklist))
+                    continue
+
+            # Index track with nested sub_tracks.
+            if not track['position'] and 'sub_tracks' in track:
+                # Append the index track, assuming it contains the track title.
+                tracklist.append(track)
+                add_merged_subtracks(tracklist, track['sub_tracks'])
+                continue
+
+            # Regular track or index track without nested sub_tracks.
+            if subtracks:
+                add_merged_subtracks(tracklist, subtracks)
+                subtracks = []
+                prev_subindex = ''
+            tracklist.append(track)
+
+        # Merge and add the remaining subtracks, if any.
+        if subtracks:
+            add_merged_subtracks(tracklist, subtracks)
+
+        return tracklist
+
     def get_track_info(self, track, index):
         """Returns a TrackInfo object for a discogs track.
         """
         title = track['title']
         track_id = None
-        medium, medium_index = self.get_track_index(track['position'])
+        medium, medium_index, _ = self.get_track_index(track['position'])
         artist, artist_id = self.get_artist(track.get('artists', []))
         length = self.get_track_length(track['duration'])
         return TrackInfo(title, track_id, artist, artist_id, length, index,
                          medium, medium_index, artist_sort=None,
                          disctitle=None, artist_credit=None)
 
     def get_track_index(self, position):
-        """Returns the medium and medium index for a discogs track position.
-        """
-        # medium_index is a number at the end of position. medium is everything
-        # else. E.g. (A)(1), (Side A, Track )(1), (A)(), ()(1), etc.
-        match = re.match(r'^(.*?)(\d*)$', position.upper())
+        """Returns the medium, medium index and subtrack index for a discogs
+        track position."""
+        # Match the standard Discogs positions (12.2.9), which can have several
+        # forms (1, 1-1, A1, A1.1, A1a, ...).
+        match = re.match(
+            r'^(.*?)'           # medium: everything before medium_index.
+            r'(\d*?)'           # medium_index: a number at the end of
+                                # `position`, except if followed by a subtrack
+                                # index.
+                                # subtrack_index: can only be matched if medium
+                                # or medium_index have been matched, and can be
+            r'((?<=\w)\.[\w]+'  # - a dot followed by a string (A.1, 2.A)
+            r'|(?<=\d)[A-Z]+'   # - a string that follows a number (1A, B2a)
+            r')?'
+            r'$',
+            position.upper()
+        )
+
         if match:
-            medium, index = match.groups()
+            medium, index, subindex = match.groups()
+
+            if subindex and subindex.startswith('.'):
+                subindex = subindex[1:]
         else:
             self._log.debug(u'Invalid position: {0}', position)
-            medium = index = None
-        return medium or None, index or None
+            medium = index = subindex = None
+        return medium or None, index or None, subindex or None
 
     def get_track_length(self, duration):
         """Returns the track length in seconds for a discogs duration.

docs/changelog.rst

@@ -71,7 +71,10 @@ And there are a few bug fixes too:
 * :doc:`/plugins/lyrics`: Search for lyrics using the title part preceding the 
   colon character. :bug:`2206`
 * Fix a crash when a query contains a date field that is not set for all
-  the items. :bug:`1938` 
+  the items. :bug:`1938`
+* :doc:`/plugins/discogs`: Subtracks are now detected and combined into a
+  single track, two-sided mediums are treated as single discs, and tracks
+  have ``media`` and ``medium_total`` set correctly. :bug:`2222`
 
 The last release, 1.3.19, also erroneously reported its version as "1.3.18"
 when you typed ``beet version``. This has been corrected.

setup.py

@@ -106,6 +106,7 @@ def build_manpages():
         'pyxdg',
         'pathlib',
         'python-mpd2',
+        'discogs-client'
     ],
 
     # Plugin (optional) dependencies: