diff --git a/beets/autotag/hooks.py b/beets/autotag/hooks.py index 7f400e4ed..57537a36d 100644 --- a/beets/autotag/hooks.py +++ b/beets/autotag/hooks.py @@ -69,12 +69,13 @@ class TrackInfo(object): may be None. """ def __init__(self, title, track_id, artist=None, artist_id=None, - length=None): + length=None, medium_index=None): self.title = title self.track_id = track_id self.artist = artist self.artist_id = artist_id self.length = length + self.medium_index = medium_index # Aggregation of sources. diff --git a/beets/autotag/match.py b/beets/autotag/match.py index e63770b13..ecf9bebd9 100644 --- a/beets/autotag/match.py +++ b/beets/autotag/match.py @@ -75,7 +75,10 @@ STRONG_REC_THRESH = 0.04 MEDIUM_REC_THRESH = 0.25 REC_GAP_THRESH = 0.25 -# Artist signals that indicate "various artists". +# Artist signals that indicate "various artists". These are used at the +# album level to determine whether a given release is likely a VA +# release and also on the track level to to remove the penalty for +# differing artists. VA_ARTISTS = (u'', u'various artists', u'va', u'unknown') # Autotagging exceptions. @@ -227,14 +230,15 @@ def track_distance(item, track_info, track_index=None, incl_artist=False): # Attention: MB DB does not have artist info for all compilations, # so only check artist distance if there is actually an artist in # the MB track data. - if incl_artist and track_info.artist: + if incl_artist and track_info.artist and \ + item.artist.lower() not in VA_ARTISTS: dist += string_dist(item.artist, track_info.artist) * \ TRACK_ARTIST_WEIGHT dist_max += TRACK_ARTIST_WEIGHT # Track index. if track_index and item.track: - if track_index != item.track: + if item.track not in (track_index, track_info.medium_index): dist += TRACK_INDEX_WEIGHT dist_max += TRACK_INDEX_WEIGHT diff --git a/beets/autotag/mb.py b/beets/autotag/mb.py index adac36286..7efae92c2 100644 --- a/beets/autotag/mb.py +++ b/beets/autotag/mb.py @@ -35,12 +35,24 @@ RELEASE_INCLUDES = ['artists', 'media', 'recordings', 'release-groups', 'labels', 'artist-credits'] TRACK_INCLUDES = ['artists'] -def track_info(recording): +# python-musicbrainz-ngs search functions: tolerate different API versions. +if hasattr(musicbrainzngs, 'release_search'): + # Old API names. + _mb_release_search = musicbrainzngs.release_search + _mb_recording_search = musicbrainzngs.recording_search +else: + # New API names. + _mb_release_search = musicbrainzngs.search_releases + _mb_recording_search = musicbrainzngs.search_recordings + +def track_info(recording, medium_index=None): """Translates a MusicBrainz recording result dictionary into a beets - ``TrackInfo`` object. + ``TrackInfo`` object. ``medium_index``, if provided, is the track's + index (1-based) on its medium. """ info = beets.autotag.hooks.TrackInfo(recording['title'], - recording['id']) + recording['id'], + medium_index=medium_index) # Get the name of the track artist. if recording.get('artist-credit-phrase'): @@ -83,7 +95,7 @@ def album_info(release): track_infos = [] for medium in release['medium-list']: for track in medium['track-list']: - ti = track_info(track['recording']) + ti = track_info(track['recording'], int(track['position'])) if track.get('title'): # Track title may be distinct from underling recording # title. @@ -145,7 +157,7 @@ def match_album(artist, album, tracks=None, limit=SEARCH_LIMIT): if not any(criteria.itervalues()): return - res = musicbrainzngs.release_search(limit=limit, **criteria) + res = _mb_release_search(limit=limit, **criteria) for release in res['release-list']: # The search result is missing some data (namely, the tracks), # so we just use the ID and fetch the rest of the information. @@ -163,7 +175,7 @@ def match_track(artist, title, limit=SEARCH_LIMIT): if not any(criteria.itervalues()): return - res = musicbrainzngs.recording_search(limit=limit, **criteria) + res = _mb_recording_search(limit=limit, **criteria) for recording in res['recording-list']: yield track_info(recording) diff --git a/docs/changelog.rst b/docs/changelog.rst index 111c927e2..b10819e01 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -6,6 +6,12 @@ Changelog * The :doc:`/plugins/lyrics`, originally by `Peter Brunner`_, is revamped and included with beets, making it easy to fetch **song lyrics**. +* The autotagger now tolerates tracks on multi-disc albums that are numbered + per-disc. For example, if track 24 on a release is the first track on the + second disc, then it is not penalized for having its track number set to 1 + instead of 24. +* The autotagger now also tolerates tracks whose track artists tags are set + to "Various Artists". * Fix a bug in the ``rewrite`` plugin that broke the use of multiple rules for a single field. * Fix a crash with non-ASCII characters in bytestring metadata fields (e.g., diff --git a/test/test_autotag.py b/test/test_autotag.py index f98e1feb3..c5af0b461 100644 --- a/test/test_autotag.py +++ b/test/test_autotag.py @@ -68,31 +68,58 @@ class PluralityTest(unittest.TestCase): self.assertEqual(l_album, 'The White Album') self.assertTrue(artist_consensus) -class AlbumDistanceTest(unittest.TestCase): - def item(self, title, track, artist='some artist'): - return Item({ - 'title': title, 'track': track, - 'artist': artist, 'album': 'some album', - 'length': 1, - 'mb_trackid': '', 'mb_albumid': '', 'mb_artistid': '', - }) +def _make_item(title, track, artist='some artist'): + return Item({ + 'title': title, 'track': track, + 'artist': artist, 'album': 'some album', + 'length': 1, + 'mb_trackid': '', 'mb_albumid': '', 'mb_artistid': '', + }) - def trackinfo(self): - ti = [] - ti.append(TrackInfo('one', None, 'some artist', length=1)) - ti.append(TrackInfo('two', None, 'some artist', length=1)) - ti.append(TrackInfo('three', None, 'some artist', length=1)) - return ti +def _make_trackinfo(): + ti = [] + ti.append(TrackInfo('one', None, 'some artist', length=1)) + ti.append(TrackInfo('two', None, 'some artist', length=1)) + ti.append(TrackInfo('three', None, 'some artist', length=1)) + return ti +class TrackDistanceTest(unittest.TestCase): + def test_identical_tracks(self): + item = _make_item('one', 1) + info = _make_trackinfo()[0] + dist = match.track_distance(item, info, incl_artist=True) + self.assertEqual(dist, 0.0) + + def test_different_title(self): + item = _make_item('foo', 1) + info = _make_trackinfo()[0] + dist = match.track_distance(item, info, incl_artist=True) + self.assertNotEqual(dist, 0.0) + + def test_different_artist(self): + item = _make_item('one', 1) + item.artist = 'foo' + info = _make_trackinfo()[0] + dist = match.track_distance(item, info, incl_artist=True) + self.assertNotEqual(dist, 0.0) + + def test_various_artists_tolerated(self): + item = _make_item('one', 1) + item.artist = 'Various Artists' + info = _make_trackinfo()[0] + dist = match.track_distance(item, info, incl_artist=True) + self.assertEqual(dist, 0.0) + +class AlbumDistanceTest(unittest.TestCase): def test_identical_albums(self): items = [] - items.append(self.item('one', 1)) - items.append(self.item('two', 2)) - items.append(self.item('three', 3)) + items.append(_make_item('one', 1)) + items.append(_make_item('two', 2)) + items.append(_make_item('three', 3)) info = AlbumInfo( artist = 'some artist', album = 'some album', - tracks = self.trackinfo(), + tracks = _make_trackinfo(), va = False, album_id = None, artist_id = None, ) @@ -100,12 +127,12 @@ class AlbumDistanceTest(unittest.TestCase): def test_incomplete_album(self): items = [] - items.append(self.item('one', 1)) - items.append(self.item('three', 3)) + items.append(_make_item('one', 1)) + items.append(_make_item('three', 3)) info = AlbumInfo( artist = 'some artist', album = 'some album', - tracks = self.trackinfo(), + tracks = _make_trackinfo(), va = False, album_id = None, artist_id = None, ) @@ -115,13 +142,13 @@ class AlbumDistanceTest(unittest.TestCase): def test_global_artists_differ(self): items = [] - items.append(self.item('one', 1)) - items.append(self.item('two', 2)) - items.append(self.item('three', 3)) + items.append(_make_item('one', 1)) + items.append(_make_item('two', 2)) + items.append(_make_item('three', 3)) info = AlbumInfo( artist = 'someone else', album = 'some album', - tracks = self.trackinfo(), + tracks = _make_trackinfo(), va = False, album_id = None, artist_id = None, ) @@ -129,13 +156,13 @@ class AlbumDistanceTest(unittest.TestCase): def test_comp_track_artists_match(self): items = [] - items.append(self.item('one', 1)) - items.append(self.item('two', 2)) - items.append(self.item('three', 3)) + items.append(_make_item('one', 1)) + items.append(_make_item('two', 2)) + items.append(_make_item('three', 3)) info = AlbumInfo( artist = 'should be ignored', album = 'some album', - tracks = self.trackinfo(), + tracks = _make_trackinfo(), va = True, album_id = None, artist_id = None, ) @@ -144,13 +171,13 @@ class AlbumDistanceTest(unittest.TestCase): def test_comp_no_track_artists(self): # Some VA releases don't have track artists (incomplete metadata). items = [] - items.append(self.item('one', 1)) - items.append(self.item('two', 2)) - items.append(self.item('three', 3)) + items.append(_make_item('one', 1)) + items.append(_make_item('two', 2)) + items.append(_make_item('three', 3)) info = AlbumInfo( artist = 'should be ignored', album = 'some album', - tracks = self.trackinfo(), + tracks = _make_trackinfo(), va = True, album_id = None, artist_id = None, ) @@ -161,18 +188,69 @@ class AlbumDistanceTest(unittest.TestCase): def test_comp_track_artists_do_not_match(self): items = [] - items.append(self.item('one', 1)) - items.append(self.item('two', 2, 'someone else')) - items.append(self.item('three', 3)) + items.append(_make_item('one', 1)) + items.append(_make_item('two', 2, 'someone else')) + items.append(_make_item('three', 3)) info = AlbumInfo( artist = 'some artist', album = 'some album', - tracks = self.trackinfo(), + tracks = _make_trackinfo(), va = True, album_id = None, artist_id = None, ) self.assertNotEqual(match.distance(items, info), 0) + def test_tracks_out_of_order(self): + items = [] + items.append(_make_item('one', 1)) + items.append(_make_item('three', 2)) + items.append(_make_item('two', 3)) + info = AlbumInfo( + artist = 'some artist', + album = 'some album', + tracks = _make_trackinfo(), + va = False, + album_id = None, artist_id = None, + ) + dist = match.distance(items, info) + self.assertTrue(0 < dist < 0.2) + + def test_two_medium_release(self): + items = [] + items.append(_make_item('one', 1)) + items.append(_make_item('two', 2)) + items.append(_make_item('three', 3)) + info = AlbumInfo( + artist = 'some artist', + album = 'some album', + tracks = _make_trackinfo(), + va = False, + album_id = None, artist_id = None, + ) + info.tracks[0].medium_index = 1 + info.tracks[1].medium_index = 2 + info.tracks[2].medium_index = 1 + dist = match.distance(items, info) + self.assertEqual(dist, 0) + + def test_per_medium_track_numbers(self): + items = [] + items.append(_make_item('one', 1)) + items.append(_make_item('two', 2)) + items.append(_make_item('three', 1)) + info = AlbumInfo( + artist = 'some artist', + album = 'some album', + tracks = _make_trackinfo(), + va = False, + album_id = None, artist_id = None, + ) + info.tracks[0].medium_index = 1 + info.tracks[1].medium_index = 2 + info.tracks[2].medium_index = 1 + dist = match.distance(items, info) + self.assertEqual(dist, 0) + def _mkmp3(path): shutil.copyfile(os.path.join(_common.RSRC, 'min.mp3'), path) class AlbumsInDirTest(unittest.TestCase): diff --git a/test/test_mb.py b/test/test_mb.py index b66a68d2e..fa8d097c3 100644 --- a/test/test_mb.py +++ b/test/test_mb.py @@ -35,9 +35,13 @@ class MBAlbumInfoTest(unittest.TestCase): 'medium-list': [], } if tracks: - release['medium-list'].append({ - 'track-list': [{'recording': track} for track in tracks] - }) + track_list = [] + for i, track in enumerate(tracks): + track_list.append({ + 'recording': track, + 'position': str(i+1), + }) + release['medium-list'].append({ 'track-list': track_list }) return release def _make_track(self, title, tr_id, duration): @@ -85,6 +89,16 @@ class MBAlbumInfoTest(unittest.TestCase): self.assertEqual(t[1].track_id, 'ID TWO') self.assertEqual(t[1].length, 200.0) + def test_parse_track_indices(self): + tracks = [self._make_track('TITLE ONE', 'ID ONE', 100.0 * 1000.0), + self._make_track('TITLE TWO', 'ID TWO', 200.0 * 1000.0)] + release = self._make_release(tracks=tracks) + + d = mb.album_info(release) + t = d.tracks + self.assertEqual(t[0].medium_index, 1) + self.assertEqual(t[1].medium_index, 2) + def test_parse_release_year_month_only(self): release = self._make_release('1987-03') d = mb.album_info(release)