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

Adding new discogs fields #22

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

Conversation

msil
Copy link

@msil msil commented Oct 24, 2019

This adds some extra tags for discogs identifiers. It is used by the following PR adding extra discogs fields to beets: beetbox/beets#3413

@sampsyo
Copy link
Member

sampsyo commented Oct 24, 2019

Hi! Thanks for looking into this. When we add new tag mappings to MediaFile, it's important to justify them by looking at existing software that uses the tags so we can be sure we maximize compatibility. Have you found other music software that uses Discogs tags?

@msil
Copy link
Author

msil commented Oct 24, 2019

If I'm being totally honest, no I can't justify this at all :) I've yet to see software handle those particular tags. It's more of a personal/selfish thing. I'd to be able to grab the discogs IDs out of my collection without having to query the beets DB for various reasons. I understand if you'd rather not merge this PR.

With regards to beetbox/beets#3413, I would like to see the discogs plugin leave the MusicBrainz fields alone and only use it's own fields to store discogs data. If I were to make that change and this PR is not accepted, then I'm left in a situation where no album/artist/label IDs are tagged in the media files themselves.

@slogsdon7
Copy link
Contributor

I'd actually argue these are almost entirely unnecessary, since if you have the discogs release_id, then you have easy access to the label_id and release_id if you need them.

I know Yate saves discogs label/artist URLs to tags, and I know there are media players which support using links from metadata in the user interface. I'm not sure if that usage is common enough to justify adding those fields, though.

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.

3 participants