Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PICARD-2141: AcousticBrainz Mood-Genre: Save existing genres before overwriting #296

Open
wants to merge 1 commit into
base: 2.0
Choose a base branch
from

Conversation

Sophist-UK
Copy link
Contributor

Save existing genre metadata (i.e. MB genres if "Use genres from MusicBrainz" is enabled) so that scripts can put it back or merge genre values.

Save existing genre metadata (i.e. MB genres if "Use genres from MusicBrainz" is enabled) so that scripts can put it back or merge genre values.
@Sophist-UK Sophist-UK changed the title PICARD-2141: AcousticBrainz Mood-Genre PICARD-2141: AcousticBrainz Mood-Genre: Save existing genres before overwriting Mar 15, 2021
@phw
Copy link
Member

phw commented Mar 18, 2021

I kind of feel that's the wrong way to go. At least we would need to add this to other plugins as well, ideally without breaking things. Also it does not do what one would expect if you have more than one genre plugin enabled (which causes mostly random behavior anyway, another problem). But if you e.g. also enable the last.fm plugin you might end up with ~mbgenre actually being the last.fm genres and not the MB genres.

What about a different approach to solve the issue, that one wants the mood, but not the genre: Add a config page for AcousticBrainz Mood-Genre to enable using mood and/or genre?

For solving the issue of being able to access the MB genres independent of any other genre plugin I'd say we could add a ~musicbrainz_genres variable to Picard directly.

The actual full solution would be to have the ability to have multiple genre plugins that can collect genres, but a single set of settings that handles the final list. E.g. this could also combine genres from multiple sources. There was a short discussion about this at https://community.metabrainz.org/t/acousticbrainz-mood-genre-plugin/404057/7. I thought we also had a ticket, but can't find any right now.

@Sophist-UK
Copy link
Contributor Author

@phw Philipp - I agree with you that genres needs an overhaul (see PICARD-2054).

As and when Picard has the supporting functionality, then plugins like this and lastFm can hook into that.

But right now, I want AcousticBrainz Mood and don't want AcousticBrainz Genre and this is the easiest backwards-compatible solution.

However see also PICARD-2145 - if that is implemented, perhaps with ~musicbrainz_genres then this would be superfluous.

@phw
Copy link
Member

phw commented Mar 20, 2021

Ah yes, PICARD-2054 was the ticket :)

I don't see much that speaks against adding the genre variables for artist, album and track as you suggested. That's probably even better than a single ~musicbrainz_genres. We want to do a beta 3 release soon and soon after the 2.6 final to get this finally out, but maybe we could even include those genre parameters still with 2.6 or at least 2.6.1 as a first step to improve genre handling.

@Sophist-UK
Copy link
Contributor Author

Sophist-UK commented Mar 20, 2021

Hmm ... should we use singular "genre" or plural "genres"?

I am in favour of having three variables holding the genre data specific to artist, release and track and also an overall variable holding the original Picard metadata value of the tag "genre" (which depending on options selected will be some form of combination of the first three).

@phw
Copy link
Member

phw commented Mar 20, 2021

I'd be for "genres" in those tags, as they are supposed to hold multiple. And yes, genre itself would of course stay.

@Sophist-UK
Copy link
Contributor Author

On reflection, I think that genre plugins should always add genres to any existing list rather than replace them.

@Sophist-UK
Copy link
Contributor Author

See also lastFm+ v2 PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants