beetbox / beets

music library manager and MusicBrainz tagger
http://beets.io/
MIT License
12.76k stars 1.82k forks source link

Optionally disable the MusicBrainz data source #400

Closed alastair closed 2 years ago

alastair commented 10 years ago

I'd like to perform an series of imports using beets but instead of using the musicbrainz autotagger I'd like to write my own matcher as a plugin. There is a config option import.autotag, but this disables the tagger completely.

@sampsyo has suggested another config option to handle this, but it might be a little messy to implement, so I'm asking for some feedback. My current idea would be to put a config option check in beets/autotag/hooks.py, in each of the methods that can be extended in the plugins, e.g. https://github.com/sampsyo/beets/blob/master/beets/autotag/hooks.py#L534 The alternative would be to move the check higher up to somewhere like https://github.com/sampsyo/beets/blob/master/beets/autotag/match.py#L409, but this might require some larger changes to the plugin system.

The other problem I can see is that there is a significant reliance on musicbrainz track ids, e.g. https://github.com/sampsyo/beets/blob/master/beets/autotag/match.py#L450. Any chance of working around these without a large rewrite?

sampsyo commented 10 years ago

Yes, I think the first place you mentioned (in hooks.py) is ideal. In short, since beets now supports pluggable metadata sources, the MusicBrainz source is very much like a plugin that can't be disabled. And the hooks.py functions are the place where the MB source is aggregated with the plugin source. (Of course, a longer-term way to solve this would be to turn MB into a real plugin that's enabled by default but can be disabled.)

And the other issue you pointed out is a good one—that deduplication step will behave incorrectly for matches without MBIDs. Maybe a separate ticket is in order?

funkyfuture commented 5 years ago

question: why isn't the Musicbrainz related mechanic a (default, opt-outable) plugin?

sampsyo commented 5 years ago

Inertia, basically. :smiley: It started out as the only data source, before we even had plugins. It could be separated out into its own plugin, but that would take some effort—and we'd need to add some edge-case error handling to deal with the event that you have no metadata sources enabled.

At the moment, we're pretty comfortable with the notion that it's abstracted into its own component and could fairly easily be its own plugin.

funkyfuture commented 5 years ago

i had the chance to try beets for my needs again after my former post and the mb-centrism is really a showstopper for me. (background: i was quiet involved w/ the Discogs community, my record collection is reflected there and so are the sources of digitized artifacts.)

regarding the configuration, what about prioritizing the sources w/ a list, eg:

sources:
  - discogs
  - musicbrainz

this should imply these to be added to the plugins value.

sampsyo commented 5 years ago

For what it’s worth, it’s possible to achieve some prioritization like this using the source_weight option on most (all?) data source plugins.

funkyfuture commented 5 years ago

sorry, i forgot to mention that this would allow an easy config transition as the default would be sources: ["musicbrainz"].

01tot10 commented 5 years ago

Just wanted to join the discussion as well, since this is a feature I find myself wishing for almost everytime I start using beets after a longer break!

As far as I'm concerned, Discogs is the de-facto-standard database for commercially released music and has a huge growing community backing it. Since it can be seamlessly integrated into the beets workflow as a plugin it would be awesome to be able to disable MusicBrainz as a source since I always prioritize Discogs as a source and the MusicBrainz-query only slows down my beets usage. More motivation - now when I'm importing some new releases into my library, https://musicbrainz.org/ is down resulting in a time-out with every single import that I'm doing, adding a minute to the query time for every single item in my list! Pretty frustrating for something you never had a use of in the first place!

Other than that, beets has pretty much revolutionized how I manage my music library, and is super awesome of a program, so thanks for that!

colin-marshall commented 5 years ago

@sampsyo if you could list out all the tasks that need to be done to separate it out into its own plugin, I might be able to help move this feature request along.

sampsyo commented 5 years ago

Good question. It might actually be far easier to just add a baked-in config option for enabling MB search by default. So many other plugins use MusicBrainz queries that it would be pretty awkward to redirect them to use a module that’s relocated to a third plugin.

The place to get started with this would be in the beets.autotag.hooks module, which contains the high-level calls that call into both MB and the plugin-provided backends. Adding a conditional around the calls to the MB backend in each such function might be the thing to do.

wwsh commented 4 years ago

You can always disable Musicbrainz using /etc/hosts. It still performs requests and spams with errors messages, though.

lev777 commented 4 years ago

Configuring a local musicbrainz server that does not exist accomplished the same without disabling access to the musicbrainz.org site.

musicbrainz: host: localhost:5000

lev777 commented 2 years ago

Retested. This issue is no longer reproducible in beets v1.6.0.

On Sun, Dec 26, 2021, 4:12 AM J0J0 T @.***> wrote:

Hi, just stumbled accross this issue accidentally. We implemented rate limiting in python3-discogs-client around June 2021. Is this still an issue?

snejus commented 2 years ago

I've had this implemented in a semi-dirty way through quitting early in beets/mb.py depending on a new musicbrainz.enabled config option:

diff --git a/beets/autotag/mb.py b/beets/autotag/mb.py
index e6a2e277..9a6a7e7f 100644
--- a/beets/autotag/mb.py
+++ b/beets/autotag/mb.py
@@ -482,6 +482,9 @@ def match_album(artist, album, tracks=None, extra_tags=None):
     The query consists of an artist name, an album name, and,
     optionally, a number of tracks on the album and any other extra tags.
     """
+    if not config["musicbrainz"]["enabled"].get(bool):
+        return None
+
     # Build search criteria.
     criteria = {'release': album.lower().strip()}
     if artist is not None:
@@ -558,6 +561,9 @@ def album_for_id(releaseid):
     object or None if the album is not found. May raise a
     MusicBrainzAPIError.
     """
+    if not config["musicbrainz"]["enabled"].get(bool):
+        return None
+
     log.debug('Requesting MusicBrainz release {}', releaseid)
     albumid = _parse_id(releaseid)
     if not albumid:
@@ -579,6 +585,9 @@ def track_for_id(releaseid):
     """Fetches a track by its MusicBrainz ID. Returns a TrackInfo object
     or None if no track is found. May raise a MusicBrainzAPIError.
     """
+    if not config["musicbrainz"]["enabled"].get(bool):
+        return None
+
     trackid = _parse_id(releaseid)
     if not trackid:
         log.debug('Invalid MBID ({0}).', releaseid)
diff --git a/beets/config_default.yaml b/beets/config_default.yaml
index 74540891..518e086a 100644
--- a/beets/config_default.yaml
+++ b/beets/config_default.yaml
@@ -108,6 +108,7 @@ musicbrainz:
     searchlimit: 5
     extra_tags: []
     genres: no
+    enabled: yes

 match:
     strong_rec_thresh: 0.04

See the commit here: https://github.com/snejus/beets/commit/ffe63a64f5dc6724054e22dd88d4181068c5f4e4

Happy opening a PR if that seems reasonable at all.

sampsyo commented 2 years ago

Sure; a PR along these lines would be a good way to get started, at the very least!