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 plugin treats headings as index tracks #4234

Closed
musiczetetic opened this issue Jan 15, 2022 · 22 comments
Closed

Discogs plugin treats headings as index tracks #4234

musiczetetic opened this issue Jan 15, 2022 · 22 comments
Labels
needinfo We need more details or follow-up from the filer before this can be tagged "bug" or "feature." stale

Comments

@musiczetetic
Copy link

musiczetetic commented Jan 15, 2022

The Problem

I recently upgraded to a newer version of beets and discovered the implementation of index tracks into the discogs plugin. Great quick implememtation. One problem I quickly discovered is that the plugin treats headings as index tracks. Possibly because the original example from the original request was actually an example of headings and not index tracks.

Quick discogs background: Before index tracks were implemented there were only headings so the original example was a correct submission in the past but would be treated differently if it was submitted now or updated.

Here is an example of index tracks versus the example of headings

While the use of headings in classical music is probably mostly the same as the use of index tracks, for most other genres of music treating headings like index tracks is not useful.

I am assuming the plugin treats actual index tracks the same. It is a bit confusing for those who are well versed in discogs terminology to treat headings as index tracks.

Update: I just used beets for this release with index tracks and it worked fantastic, exactly as I would expect it to.

The Solution

Treat headings differently to index tracks.

1.
index_tracks: yes/no
headings: yes/no

2.
I think headings for most other genres would work better as album subtitles. For example on this realease or this release I discovered the problem on.

index_tracks: yes/no
headings: name/album/no

On both those linked releases the heading for the first disc are the same as the album name, it would be good if that was checked and ignored if that was the case.

@ybnd
Copy link
Contributor

ybnd commented Jan 15, 2022

  1. I'm not sure it's a good idea to essentially drop support for releases like the Handel example; we still need to use the headings in those cases
  2. We already extract disc titles from these headings.
    We'd have to add some extra logic to get rid of the prefix but keep the disc title, but if we do keep the "headings as index tracks" case for compatibility I can't think of a good way to distinguish when we want the prefix... Relying on the name of the album/heading won't ever work 100%

@musiczetetic
Copy link
Author

Thanks for your comments.

  1. This option wouldn't drop support it would just give users the option of treating them differently. In my case I would have headings off for track naming and run a separate process with them on for the few times I needed it. Others might do the opposite.

  2. Can I make use of this disctitle function in any way? I haven't seen it mentioned anywhere. I note that in that code it refers to index tracks rather than headings. Is it referring to index tracks or headings or both? I can't help but think there is still a misunderstanding of the difference between headings and index tracks, there are no prefixes (track numbers?) for headings. Headings and index tracks in discogs are different and used in different ways, it is just that headings existed before index tracks so you will find cases of old submissions using headings where index tracks would be better suited.

@sampsyo sampsyo added the needinfo We need more details or follow-up from the filer before this can be tagged "bug" or "feature." label Jan 16, 2022
@sampsyo
Copy link
Member

sampsyo commented Jan 16, 2022

Thanks for starting the discussion! As an outsider to Discogs standards, can I ask some extremely basic questions about what's currently happening?

Namely, what exactly does beets currently do with index tracks, and what should it do (perhaps behind a config option)? Are we treating these index tracks as normal tracks, which then are guaranteed never to match any files the user has? Or are we treating them only as disc titles, and we should be treating them as tracks?

(Again, apologies for being slow, but I don't quite understand yet exactly what the current behavior is…)

@musiczetetic
Copy link
Author

Hi Adrian,
As someone who has been using Discogs for 20 years I am happy to answer any questions you might have.

Currently with the "index_tracks" config option turned on beets adds the index tracks to the beginning of the sub tracks name, which seems like the correct way to handle them. They are being treated as part of the name of the sub tracks. They are part of the name of the sub tracks most commonly in classical music. We shouldn't be treating them as separate tracks, I think the way they are handled now is correct.

I can't describe the purpose of index tracks any better than the Discogs guidelines does:
Index Tracks are used whenever the Index Track Title is the title of a musical piece and the Sub-tracks below are parts or movements of that piece.

The problem I am raising is that headings are being treated the same way as index tracks which is wrong except for legacy discogs submissions that were made before index tracks were introduced. Index tracks and Headings have different purposes.

As per Discogs guidelines Use a "Heading" whenever there is a block of text on the release that is descriptive of the tracks below, but is not the title of a musical piece.

We might not be able to do anything useful with them because they are used for varying diffferent purposes. Sometimes for disc titles on multidisc sets, sometimes for a sub heading on an album. I could probably find examples if required.

The best option may be just to leave how it works as is but add an option to turn off treating Headings the same way.

I am not sure what the disc titles function is doing. I have never seen it added to my tags, maybe it is something in the beets database only.

@ybnd
Copy link
Contributor

ybnd commented Jan 17, 2022

AFAIK disctitle is taken pretty loosely in beets already, so it's probably not a big issue if it's not super consistent

We should be able to address this by making our configuration a bit more fine-grained, e.g. with 4 options:

  • Index tracks
    • Use for prefix?
    • Use for disctitle
  • Headings
    • Use for prefix?
    • Use for disctitle?

Given how index tracks are described above and the fact how headings like the ones on the Bowie & Lynch examples you gave above don't make much sense with the headings as a prefix, I think it's reasonable to go with the following defaults:

  • Index tracks -> prefix, but not disctitle
  • Headings -> disctitle, but not prefix

I'd suggest postponing this until I can fix the data loss issue in #4235 though, since it involves a lot of the same functionality & seems more critical

@ybnd
Copy link
Contributor

ybnd commented Jan 18, 2022

Can I make use of this disctitle function in any way? I haven't seen it mentioned anywhere.

It's one of our metadata fields, you can use it in queries and formatting.
I suppose a common usage would be for path configuration, e.g. to divide albums into multiple folders

@ybnd
Copy link
Contributor

ybnd commented Jan 18, 2022

There's also a small bug right now that causes disctitle to only appear on the first track of each section

@stale
Copy link

stale bot commented Mar 20, 2022

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Mar 20, 2022
@musiczetetic
Copy link
Author

musiczetetic commented Mar 20, 2022

Yes, still relevant.

@stale stale bot removed the stale label Mar 20, 2022
@sampsyo
Copy link
Member

sampsyo commented Mar 20, 2022

It would be great to hear from other folks on the thread about what exactly the new behavior should be… @musiczetetic says:

The best option may be just to leave how it works as is but add an option to turn off treating Headings the same way.

Is that what folks want? If so, how would Headings be treated?

@emanuele-virgillito
Copy link

hi @sampsyo, as a heavy discogs user i would find perfect the behavior proposed by @ybnd !

@stale
Copy link

stale bot commented Jun 12, 2022

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 12, 2022
@ybnd
Copy link
Contributor

ybnd commented Jun 13, 2022

Shouldn't be closed; there's still an in-progress PR to try and adress this & in the meantime this issue should stay open to gather more feedback from users

@stale stale bot removed the stale label Jun 13, 2022
@musiczetetic
Copy link
Author

Given how index tracks are described above and the fact how headings like the ones on the Bowie & Lynch examples you gave above don't make much sense with the headings as a prefix, I think it's reasonable to go with the following defaults:

* Index tracks -> prefix, but not `disctitle`

* Headings -> `disctitle`, but not prefix

I agree with this proposal for defaults settings.

It would be good when manually approving each match that an option to use or ignore them would be presented. Not sure how this would look or if it is even possible. Because of the inability to turn off MusicBrainz matches I have to manually select the discogs option for every search at the moment anyhow.

@stale
Copy link

stale bot commented Sep 24, 2022

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Sep 24, 2022
@musiczetetic
Copy link
Author

Still relevant.

@stale stale bot removed the stale label Oct 6, 2022
@JOJ0
Copy link
Member

JOJ0 commented Nov 7, 2022

Hi, thanks for reporting this and starting a PR, I'm just here to report examples of where the current implementation renders using Discogs as the source plugin unusable. These 2 releases both would lead to the importer grabbing just two "tracks":

  • I'm not sure why the first one fails
  • In the second one the headings "Part One" and "Part Two" are the problem.

Example 1:

https://www.discogs.com/release/831995-Various-The-Eclectic-Sound-Of-Vienna-1-2

The API response actually looks correct, no heading, just regular tracks:

"tracklist": [
    {
      "position": "1.01",
      "type_": "track",
      "artists": [
        {
          "name": "Tosca",
          "anv": "",
          "join": "",
          "role": "",
          "tracks": "",
          "id": 3212,
          "resource_url": "https://api.discogs.com/artists/3212"
        }
      ],
      "title": "Fuckdubworksong (Uptight Mix)",
      "extraartists": [
        {
          "name": "Demon Flowers",
          "anv": "",
          "join": "",
          "role": "Remix",
          "tracks": "",
          "id": 166513,
          "resource_url": "https://api.discogs.com/artists/166513"
        },
        {
          "name": "Rodney Hunter",
          "anv": "",
          "join": "",
          "role": "Remix",
          "tracks": "",
          "id": 46624,
          "resource_url": "https://api.discogs.com/artists/46624"
        }
      ],
      "duration": ""
    },
...
...

It's interesting why it's messed up like this. Is this by any chance the issue being talked about here or something else?

beet import eclectic volume 1 -L -t -S 831995

/remote/data/music-library/Christl CD collection/Compilations/1997 - The Eclectic Sound of Vienna, Volume 1 (10 items)
Correcting tags from:
    The Eclectic Sound of Vienna, Volume 1
To:
    The Eclectic Sound Of Vienna 1 + 2
URL:
    https://www.discogs.com/release/831995-Various-The-Eclectic-Sound-Of-Vienna-1-2
(Similarity: 28.6%) (id, unmatched tracks, tracks, catalognum, album, year) (Discogs, CD, 2003, Austria, Spray Records, 82876 51669 2)
 * Fuckdubworksong (Uptight mix) -> Fuckdubworksong (Uptight Mix) / Bazaar / Closer To God / Elephants Walk / Close2eve / Si Commandante! (Real Shit Mix) / The Wanna Do Track / Speechless Drum & Bass / Aquaribass / How Will He Touch? (id, title)
 * Bazaar                        -> Children Of The Ghetto (Eclectic Mix) / Keepin' Da Funk Alive / Happy (Uptight Anti-Stress Workout) / Get Off My Wall (All American Soulstar Radio Edit) / Sand Rabbit / What Goes Up (FM4 Edit) / The Plan (Aphrodite Remix) / Jewel Of The Day (Earth Edit) / Blue / Rollin' On Chrome (Wild Motherfucker Dub) / Holy Smoke / Carpenter / Let The Peace Come On / 10 / Traces (id, artist, title)
Unmatched tracks (8):
 ! Closer to God                  (# 3) (6:55)
 ! Elephants Walk                 (# 4) (5:35)
 ! Close2eve (feat. Timea)        (# 5) (3:28)
 ! Si Commandante (Real Shit mix) (# 6) (5:28)
 ! The Wanna do Track             (# 7) (5:25)
 ! Speechless (Drum & Bass)       (# 8) (6:51)
 ! Aquaribass                     (# 9) (8:34)
 ! How Will He Touch?             (#10) (7:58)
Apply, More candidates, Skip, Use as-is, as Tracks, Group albums,
Enter search, enter Id, aBort, eDit, edit Candidates, plaYi     

Example 2:

https://www.discogs.com/release/1125261-Kruder-Dorfmeister-The-KD-Sessions

 "tracklist": [
    {
      "position": "",
      "type_": "heading",
      "title": "Part One",
      "duration": ""
    },
...
...

Let's ignore the fact that the files are not the correct ones for the release!

Enter release ID: 1125261
Correcting tags from:
    Various Artists - The Eclectic Sound of Vienna, Volume 1
To:
    Kruder & Dorfmeister - The K&D Sessions™
URL:
    https://www.discogs.com/release/1125261-Kruder-Dorfmeister-The-KD-Sessions
(Similarity: 16.9%) (id, unmatched tracks, tracks, album, artist, catalognum, label, year) (Discogs, CD, 1998, Germany, !K7, !K7073CD)
 * Fuckdubworksong (Uptight mix) -> Part One (id, title)
 * Bazaar                        -> Part Two (id, title)
Unmatched tracks (8):
 ! Closer to God                  (# 3) (6:55)
 ! Elephants Walk                 (# 4) (5:35)
 ! Close2eve (feat. Timea)        (# 5) (3:28)
 ! Si Commandante (Real Shit mix) (# 6) (5:28)
 ! The Wanna do Track             (# 7) (5:25)
 ! Speechless (Drum & Bass)       (# 8) (6:51)
 ! Aquaribass                     (# 9) (8:34)
 ! How Will He Touch?             (#10) (7:58)
Apply, More candidates, Skip, Use as-is, as Tracks, Group albums,
Enter search, enter Id, aBort, eDit, edit Candidates, plaY? 

@ybnd
Copy link
Contributor

ybnd commented Nov 7, 2022

@JOJ0 looks like in both examples the tracks in each disc/section are handled as subtracks.

  • in the first example the sections are untitled, so the names of the individual tracks are combined here
  • in the second release the section titles are used for the names of the "two tracks"; AFAIK

The headings aren't the problem, it's the 1.01 format of the track numbers (see here).
Tracks numbered in this way are combined into one track according to the Discogs guidelines

Sub tracks, for example, DJ mixes that comprise one track on a CD: Separate songs or tunes that are rolled into one track on a CD, LP, etc., should be listed using a point and then a number: 1, 2, 3.1, 3.2, 3.3, 4,…

I see two sensible ways for us to approach this:

  • Do nothing: assume it's up to Discogs users to format track numbers differently, not our problem
  • A neat way to be a bit more flexible while still retaining this functionality could be to generate two candidates for such albums: one where we combine the "subtracks" and one where we don't. Then we can leave it up to the matcher to figure out which representation fits best with the files we're importing.

@ybnd
Copy link
Contributor

ybnd commented Nov 7, 2022

@JOJ0 IMO this should probably be handled in a separate issue/PR though, as to avoid more scope creep & delays with this one (mostly on me, but still)

@JOJ0
Copy link
Member

JOJ0 commented Nov 7, 2022

Yeah sure, totally agree! I dont want to pollute the current pr's fix. Was just wondering if I'm hitting the same thing. thanks for clarifying! appreciated!

@stale
Copy link

stale bot commented Jan 16, 2023

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 16, 2023
@musiczetetic
Copy link
Author

  • A neat way to be a bit more flexible while still retaining this functionality could be to generate two candidates for such albums: one where we combine the "subtracks" and one where we don't. Then we can leave it up to the matcher to figure out which representation fits best with the files we're importing.

This sounds like a good solution

@stale stale bot closed this as completed Apr 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needinfo We need more details or follow-up from the filer before this can be tagged "bug" or "feature." stale
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants