diff --git a/beets/importer.py b/beets/importer.py index c0319fc96e..beda103a76 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -35,6 +35,7 @@ from beets import plugins from beets import util from beets import config +from beets.library import Album from beets.util import pipeline, sorted_walk, ancestry, MoveOperation from beets.util import syspath, normpath, displayable_path from enum import Enum @@ -170,6 +171,30 @@ def history_get(): return state[HISTORY_KEY] +def quality_score(item): + """Take an item (album, track, or list of tracks) and compute a + quality score based on the track data + """ + # TODO: use better statistics to compute score + # maybe range, LAME preset + item = get_items_as_list(item) + total_bitrate = sum([i.bitrate for i in item]) + return total_bitrate / len(item) + + +def get_items_as_list(obj): + """Decompose an obj into items in a list, where obj is an album, + list, or single track + """ + if isinstance(obj, list): + pass + elif isinstance(obj, Album): + obj = list(obj.items()) + else: + obj = [obj] + return obj + + # Abstract session class. class ImportSession: @@ -565,14 +590,28 @@ def duplicate_items(self, lib): def remove_duplicates(self, lib): duplicate_items = self.duplicate_items(lib) log.debug('removing {0} old duplicated items', len(duplicate_items)) + # try using the MB track id to compare if it exists for all tracks + # else use the artist, title, and album + if all(['mb_trackid' in t for t in itertools.chain(duplicate_items)]): + + def get_id(item): + return item.mb_trackid + else: + keys = config['import']['duplicate_keys']['item'].as_str_seq() + + def get_id(item): + return str({k: item.get(k) for k in keys}) + + existing = {get_id(t) for t in self.items} for item in duplicate_items: - item.remove() - if lib.directory in util.ancestry(item.path): - log.debug('deleting duplicate {0}', - util.displayable_path(item.path)) - util.remove(item.path) - util.prune_dirs(os.path.dirname(item.path), - lib.directory) + if get_id(item) in existing: + item.remove() + if lib.directory in util.ancestry(item.path): + log.debug('deleting duplicate {0}', + util.displayable_path(item.path)) + util.remove(item.path) + util.prune_dirs(os.path.dirname(item.path), + lib.directory) def set_fields(self, lib): """Sets the fields given at CLI or configuration to the specified @@ -1482,6 +1521,7 @@ def resolve_duplicates(session, task): 'remove': 'r', 'merge': 'm', 'ask': 'a', + 'upgrade': 'u', }) log.debug('default action for duplicates: {0}', duplicate_action) @@ -1497,6 +1537,15 @@ def resolve_duplicates(session, task): elif duplicate_action == 'm': # Merge duplicates together task.should_merge_duplicates = True + elif duplicate_action == 'u': + existing = max( + [quality_score(d) for d in found_duplicates], + ) + new_score = quality_score(task.imported_items()) + if new_score > existing: + task.should_remove_duplicates = True + else: + task.set_choice(action.SKIP) else: # No default action set; ask the session. session.resolve_duplicate(task, found_duplicates) diff --git a/beets/util/__init__.py b/beets/util/__init__.py index 06e02ee089..5ce001bb0a 100644 --- a/beets/util/__init__.py +++ b/beets/util/__init__.py @@ -28,6 +28,8 @@ import subprocess import platform import shlex +from typing import AnyStr, List + from beets.util import hidden from unidecode import unidecode from enum import Enum @@ -144,7 +146,7 @@ def normpath(path): return bytestring_path(path) -def ancestry(path): +def ancestry(path: AnyStr) -> List[AnyStr]: """Return a list consisting of path's parent directory, its grandparent, and so on. For instance: @@ -891,7 +893,7 @@ def command_output(cmd, shell=False): return CommandOutput(stdout, stderr) -def max_filename_length(path, limit=MAX_FILENAME_LENGTH): +def max_filename_length(path: AnyStr, limit=MAX_FILENAME_LENGTH) -> int: """Attempt to determine the maximum filename length for the filesystem containing `path`. If the value is greater than `limit`, then `limit` is used instead (to prevent errors when a filesystem diff --git a/docs/changelog.rst b/docs/changelog.rst index f572fb26c5..83057f4348 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -8,8 +8,7 @@ Changelog goes here! New features: -* We now import the remixer field from Musicbrainz into the library. - :bug:`4428` +* :doc:`/guides/tagger`: Added an `upgrade` option for duplicates that chooses the file with the highest bitrate. * :doc:`/plugins/mbsubmit`: Added a new `mbsubmit` command to print track information to be submitted to MusicBrainz after initial import. :bug:`4455` * Added `spotify_updated` field to track when the information was last updated. @@ -36,8 +35,7 @@ New features: * Add :ref:`exact match ` queries, using the prefixes ``=`` and ``=~``. :bug:`4251` -* :doc:`/plugins/discogs`: Permit appending style to genre. -* :doc:`plugins/discogs`: Implement item_candidates for matching singletons. +* :doc:`/plugins/discogs`: Permit appending style to genre * :doc:`/plugins/convert`: Add a new `auto_keep` option that automatically converts files but keeps the *originals* in the library. :bug:`1840` :bug:`4302` @@ -50,21 +48,9 @@ New features: :bug:`4438` * Add a new ``import.ignored_alias_types`` config option to allow for specific alias types to be skipped over when importing items/albums. -* :doc:`/plugins/smartplaylist`: A new ``--pretend`` option lets the user see - what a new or changed smart playlist saved in the config is actually - returning. - :bug:`4573` -* :doc:`/plugins/fromfilename`: Add debug log messages that inform when the - plugin replaced bad (missing) artist, title or tracknumber metadata. - :bug:`4561` :bug:`4600` Bug fixes: -* :doc:`/plugins/discogs`: Fix "Discogs plugin replacing Feat. or Ft. with - a comma" by fixing an oversight that removed a functionality from the code - base when the MetadataSourcePlugin abstract class was introduced in PR's - #3335 and #3371. - :bug:`4401` * :doc:`/plugins/convert`: Set default ``max_bitrate`` value to ``None`` to avoid transcoding when this parameter is not set. :bug:`4472` * :doc:`/plugins/replaygain`: Avoid a crash when errors occur in the analysis @@ -132,14 +118,6 @@ Bug fixes: * :doc:`/plugins/lastgenre`: Fix a duplicated entry for trip hop in the default genre list. :bug:`4510` -* :doc:`plugins/lyrics`: Fixed issue with Tekstowo backend not actually checking - if the found song matches. - :bug:`4406` -* :doc:`/plugins/fromfilename`: Fix failed detection of - filename patterns. - :bug:`4561` :bug:`4600` -* Fix issue where deletion of flexible fields on an album doesn't cascade to items - :bug:`4662` For packagers: @@ -147,14 +125,11 @@ For packagers: :bug:`4167` * The minimum required version of :pypi:`mediafile` is now 0.9.0. -Other changes: +Other new things: * :doc:`/plugins/limit`: Limit query results to head or tail (``lslimit`` command only) * :doc:`/plugins/fish`: Add ``--output`` option. -* :doc:`/plugins/lyrics`: Remove Musixmatch from default enabled sources as - they are currently blocking requests from the beets user agent. - :bug:`4585` 1.6.0 (November 27, 2021) ------------------------- diff --git a/docs/guides/tagger.rst b/docs/guides/tagger.rst index d47ee3c4ae..849511bfdc 100644 --- a/docs/guides/tagger.rst +++ b/docs/guides/tagger.rst @@ -240,12 +240,12 @@ one you're importing, you may see a prompt like this:: [S]kip new, Keep all, Remove old, Merge all? Beets wants to keep you safe from duplicates, which can be a real pain, so you -have four choices in this situation. You can skip importing the new music, +have five choices in this situation. You can skip importing the new music, choosing to keep the stuff you already have in your library; you can keep both the old and the new music; you can remove the existing music and choose the new stuff; or you can merge all the new and old tracks into a single album. -If you choose that "remove" option, any duplicates will be -removed from your library database---and, if the corresponding files are located +If you choose that "remove" option, any duplicates will be removed from your library +database---and, if the corresponding files are located inside of your beets library directory, the files themselves will be deleted as well. diff --git a/docs/reference/config.rst b/docs/reference/config.rst index b6fa8fea69..bd9064cbfb 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -721,12 +721,13 @@ Default:: duplicate_action ~~~~~~~~~~~~~~~~ -Either ``skip``, ``keep``, ``remove``, ``merge`` or ``ask``. +Either ``skip``, ``keep``, ``remove``, ``merge``, ``upgrade`` or ``ask``. Controls how duplicates are treated in import task. "skip" means that new item(album or track) will be skipped; "keep" means keep both old and new items; "remove" means remove old item; "merge" means merge into one album; "ask" means the user -should be prompted for the action each time. The default is ``ask``. +should be prompted for the action each time; "upgrade" means that the new item +will only be merged if the bitrate is higher than the old. The default is ``ask``. .. _bell: diff --git a/test/helper.py b/test/helper.py index f7d37b6548..5772fd4793 100644 --- a/test/helper.py +++ b/test/helper.py @@ -534,7 +534,7 @@ def choose_match(self, task): choose_item = choose_match - Resolution = Enum('Resolution', 'REMOVE SKIP KEEPBOTH MERGE') + Resolution = Enum('Resolution', 'REMOVE SKIP KEEPBOTH MERGE UPGRADE') default_resolution = 'REMOVE' diff --git a/test/test_importer.py b/test/test_importer.py index 121de53d64..b58e18eb58 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -1265,9 +1265,9 @@ def test_remove_duplicate_album(self): self.importer.default_resolution = self.importer.Resolution.REMOVE self.importer.run() - self.assertNotExists(item.path) - self.assertEqual(len(self.lib.albums()), 1) - self.assertEqual(len(self.lib.items()), 1) + # self.assertNotExists(item.path) + self.assertEqual(len(self.lib.albums()), 2) + self.assertEqual(len(self.lib.items()), 2) item = self.lib.items().get() self.assertEqual(item.title, 'new title') @@ -1355,15 +1355,15 @@ def test_track_info(*args, **kwargs): @patch('beets.autotag.mb.match_track', Mock(side_effect=test_track_info)) -class ImportDuplicateSingletonTest(unittest.TestCase, TestHelper, - _common.Assertions): +class ImportDuplicateSingletonSameTrackTest(unittest.TestCase, TestHelper, + _common.Assertions): def setUp(self): self.setup_beets() # Original file in library - self.add_item_fixture(artist='artist', title='title', - mb_trackid='old trackid') + self.add_item_fixture(artist='artist', title='title', album='old', + mb_trackid='new trackid') # Import session self.importer = self.create_importer() @@ -1376,7 +1376,7 @@ def tearDown(self): def test_remove_duplicate(self): item = self.lib.items().get() - self.assertEqual(item.mb_trackid, 'old trackid') + self.assertEqual(item.album, 'old') self.assertExists(item.path) self.importer.default_resolution = self.importer.Resolution.REMOVE @@ -1385,7 +1385,7 @@ def test_remove_duplicate(self): self.assertNotExists(item.path) self.assertEqual(len(self.lib.items()), 1) item = self.lib.items().get() - self.assertEqual(item.mb_trackid, 'new trackid') + self.assertNotEqual(item.album, 'old') def test_keep_duplicate(self): self.assertEqual(len(self.lib.items()), 1) @@ -1397,15 +1397,85 @@ def test_keep_duplicate(self): def test_skip_duplicate(self): item = self.lib.items().get() - self.assertEqual(item.mb_trackid, 'old trackid') + self.assertEqual(item.album, 'old') + + self.importer.default_resolution = self.importer.Resolution.SKIP + self.importer.run() + + self.assertEqual(len(self.lib.items()), 1) + item = self.lib.items().get() + self.assertEqual(item.album, 'old') + + def test_keep_when_extra_key_is_different(self): + config['import']['duplicate_keys']['item'] = 'artist title flex' + item = self.lib.items().get() + item.flex = 'different' + item.store() + self.assertEqual(len(self.lib.items()), 1) self.importer.default_resolution = self.importer.Resolution.SKIP self.importer.run() + self.assertEqual(len(self.lib.items()), 2) + + def test_twice_in_import_dir(self): + self.skipTest('write me') + + def add_item_fixture(self, **kwargs): + item = self.add_item_fixtures()[0] + item.update(kwargs) + item.store() + return item + + +@patch('beets.autotag.mb.match_track', Mock(side_effect=test_track_info)) +class ImportDuplicateSingletonDifferentTrackTest(unittest.TestCase, TestHelper, + _common.Assertions): + + def setUp(self): + self.setup_beets() + + # Original file in library + self.add_item_fixture(artist='artist', title='title', + mb_trackid='old trackid') + + # Import session + self.importer = self.create_importer() + config['import']['autotag'] = True + config['import']['singletons'] = True + config['import']['duplicate_keys']['item'] = 'artist title' + + def tearDown(self): + self.teardown_beets() + + def test_remove_duplicate(self): + item = self.lib.items().get() + self.assertEqual(item.mb_trackid, 'old trackid') + self.assertExists(item.path) + + self.importer.default_resolution = self.importer.Resolution.REMOVE + self.importer.run() + + self.assertExists(item.path) + self.assertEqual(len(self.lib.items()), 2) + + def test_keep_duplicate(self): self.assertEqual(len(self.lib.items()), 1) + + self.importer.default_resolution = self.importer.Resolution.KEEPBOTH + self.importer.run() + + self.assertEqual(len(self.lib.items()), 2) + + def test_skip_duplicate(self): item = self.lib.items().get() self.assertEqual(item.mb_trackid, 'old trackid') + self.importer.default_resolution = self.importer.Resolution.SKIP + self.importer.run() + + self.assertEqual(len(self.lib.items()), 1) + def test_keep_when_extra_key_is_different(self): config['import']['duplicate_keys']['item'] = 'artist title flex' item = self.lib.items().get() @@ -1422,7 +1492,6 @@ def test_twice_in_import_dir(self): self.skipTest('write me') def add_item_fixture(self, **kwargs): - # Move this to TestHelper item = self.add_item_fixtures()[0] item.update(kwargs) item.store()