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

discogs_importer: subtracks in Discogs to separete tracks in Musicbrainz #281

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

hirokobayashi
Copy link

Hi
I modified Discogs importer to generate separate tracks for Discogs subtracks instead of merging them to one track in musicbrainz.
The track format will be "trackname: subtrackname" which would match the classical music guideline.

@hirokobayashi hirokobayashi changed the title discogs_importer: subtracks in Discogs to separete tracks in Musicbrainz discogs_importer: subtracks in Discogs to separete tracks in Musicbrainz Jun 19, 2020
@jesus2099
Copy link
Contributor

Hello @hirokobayashi,
But MusicBrainz is supposed to represent the CD faithfully, without adding fictitious tracks, isn't it?

@hirokobayashi
Copy link
Author

Hello @hirokobayashi,
But MusicBrainz is supposed to represent the CD faithfully, without adding fictitious tracks, isn't it?

This commit fixes #138

Please check the discogs document about track and heading here.
https://support.discogs.com/hc/en-us/articles/360005055373-Database-Guidelines-12-Tracklisting#Index_Tracks_And_Headings

Original discogs importer uses a heading as a track and subsequent tracks are merged into the heading.

@jesus2099
Copy link
Contributor

Oh thanks, in fact you already found a doc about the distinction between indeed tracks (that are 1 CD track) and headings (CD track set, logical group of several CD tracks).

I would like to test your patch with the two cases and come back here afterwards. ☺️

@ke4roh
Copy link

ke4roh commented May 17, 2021

I've run into the bug this fixes. Here's a vote for merge!

@kaysond
Copy link

kaysond commented May 15, 2022

Please merge this!

@kaysond
Copy link

kaysond commented May 15, 2022

Pinging @jesus2099 @murdos @Schweinepriester @kellnerd

I'm happy to rebase this if someone will commit to merging it in the next week.

@kaysond
Copy link

kaysond commented May 23, 2022

I've fixed this for classical albums in a really hacked together fork here: https://github.com/kaysond/musicbrainz-userscripts/blob/discogs-classical/discogs_importer.user.js

@jesus2099
Copy link
Contributor

jesus2099 commented May 23, 2022

Oh, is that patch to old now?
I think Discogs had recently changed their pages.

Sorry @hirokobayashi, only now I took time to test it:

I first tested this 2CD release from this post .
It works good with latest version of the script, splitting it in 2 mediums.
But this PR patched script does not load at all on the Discogs page.

@kaysond, is that what you meant by rebasing?
Redo this patch but in the latest importer version?

Next tests will be this single track with indexes release from this other post.

@jesus2099
Copy link
Contributor

jesus2099 commented May 23, 2022

Wow!
It seems better!

Discogs release Current importer @hirokobayashi + @kaysond version
2017-02-19: trackset 🔴 🟢
2017-02-21: strange LP bugged tracklist what is expected? different, I find it better, but what is expected?
2017-02-21: multi disc tracksets 🔴 🟢
2020-05-10: single split track 🔴 🟢
2020-06-06: complex multi-disc tracksets 🔴 🟢
2020-12-17: A/B LP 🟢 🟢
2020-12-17: CD+CD+DVD 🔴 🔴
2020-12-27: A/B+C/D with bad? subtracks 🔴 🔴
2020-12-27: A/B+C/D with good? subtracks 🔴 🔴
2020-12-27: split track on second CD 🔴 🔴

(I will update this table with more tests)

Wow, it must be so difficult to manage all the cases.

@kaysond
Copy link

kaysond commented May 23, 2022

@jesus2099 - correct. This PR is based on a very old discogs import script. Mine was based on the latest one, but its not very good code!

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.

4 participants