beetbox / beets

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

discogs: Incorrect disc numbering on two-sided media such as vinyl #2887

Closed dbogdanov closed 6 years ago

dbogdanov commented 6 years ago

Problem

There is an issue with annotating vinyl releases from Discogs that have multiple discs (sides A/B, C/D, etc.).

For example, take this release.

The playlist metadata on Discogs:

A - Damon Wild vs. Function - Friction
B1 - Damon Wild - Skyline
B2 - Norman - Fortnight
C - C D C Crew - Flightdance  
D1 - Chris Sattinger - Decoder Ring 
D2 - Function - CNTRL

Beet import (tracks for Vinyl 1 and Vinyl 2 are split incorrectly):

Tagging:
    Various Artists - SW33
URL:
    https://www.discogs.com/Various-SW33/release/32654
(Similarity: 76.5%) (id, source) (Discogs, 2xVinyl, 1997, US, Synewave)
Vinyl 1
 * Friction (#0)     -> Friction (#1-1)
 * Skyline (#0)      -> Skyline (#1-2)
 * Fortnight (#0)    -> Fortnight (#1-3)
 * Flightdance (#0)  -> Flightdance (#1-4)
Vinyl 2
 * Decoder Ring (#0) -> Decoder Ring (#2-1)
 * CNTRL (#0)        -> CNTRL (#2-2)
[A]pply, More candidates, Skip, Use as-is, as Tracks, Group albums,
Enter search, enter Id, aBort, plaY?

The output of beet info after importing:

beet info SW33 | grep -E "disc:|track:|tracktotal:|title:|artist:"
     albumartist: Various Artists
          artist: Damon Wild vs. Function
            disc: 1
           title: Friction
           track: 1
      tracktotal: 2
     albumartist: Various Artists
          artist: Damon Wild
            disc: 1
           title: Skyline
           track: 2
      tracktotal: 2
     albumartist: Various Artists
          artist: Norman
            disc: 1
           title: Fortnight
           track: 3
      tracktotal: 2
     albumartist: Various Artists
          artist: C D C Crew
            disc: 1
           title: Flightdance
           track: 4
      tracktotal: 2
     albumartist: Various Artists
          artist: Chris Sattinger
            disc: 2
           title: Decoder Ring
           track: 1
      tracktotal: 2
     albumartist: Various Artists
          artist: Function
            disc: 2
           title: CNTRL
           track: 2
      tracktotal: 2

The output tracktotal is incorrect and it breaks the behavior of the missing plugin. Also the split into disc 1 and disc 2 is also incorrect, therefore the disc and trackvalues are erroneous. Those are two vinyls, and by conventions, A and B sides are vinyl 1, C and D should be vinyl 2.

sampsyo commented 6 years ago

Interesting—so would you summarize the problem as:

Is that an accurate diagnosis? Does it appear this way for other albums too?

Also, do you have per_disc_numbering enabled? If so, what happens if you turn it off?

dbogdanov commented 6 years ago

Side index would mean there should be disc1, disc2, disc3, disc4 which is not the case in my example, so no. Also in this example track C is incorrectly assigned to disc 1 instead of disc 2.

dbogdanov commented 6 years ago

Disabling per_disc_numbering makes tracktotal correct (the total number of tracks on the release). Still, the disc numbers are messed up. So in short:

dbogdanov commented 6 years ago

Related to per_disc_numbering, the issue is here.

sampsyo commented 6 years ago

Side index would mean there should be disc1, disc2, disc3, disc4 which is not the case in my example, so no. Also in this example track C is incorrectly assigned to disc 1 instead of disc 2.

Right—I meant the disc-relative side index. So side A on any disc is “disc 1,” side B anywhere is “disc 2,” etc. Is that right?

Disabling per_disc_numbering makes tracktotal correct (the total number of tracks on the release). Still, the disc numbers are messed up.

Indeed; I think the discogs problem is the root cause—if that were fixed, I think per_disc_numbering would behave correctly.

So if anyone has the time, I think the thing to do would be to look carefully at the disc assignment code in discogs and run a few tests with examples you provide.

dbogdanov commented 6 years ago

There are two independent issues:

1) incorrect tracktotal. The error is here:

item.tracktotal = track_info.medium_total or len(album_info.tracks)

This will result in the medium_total value instead of len(album_info.tracks). The order of operands should be changed.


2) incorrect medium and medium index numbering, but only in some cases:

A - Damon Wild vs. Function - Friction
B1 - Damon Wild - Skyline
B2 - Norman - Fortnight
C - C D C Crew - Flightdance  
D1 - Chris Sattinger - Decoder Ring 
D2 - Function - CNTRL

Medium C does not have medium_index and is erroneously considered as the medium_is_index == True case. This is because medium_count is not increased after each side, but two sides (the medium is considered two-sided).

    medium_is_index = track.medium and not track.medium_index and (
                len(track.medium) != 1 or
                ord(track.medium) - 64 != medium_count + 1
    )
sampsyo commented 6 years ago

This will result in the medium_total value instead of len(album_info.tracks). The order of operands should be changed.

Hmm… this is the part I'm not sure I agree with. The idea behind the the medium_total is that it contains exactly what tracktotal wants: the number of tracks on the medium (e.g., disc or side) that the track appears on. See the MusicBrainz backend, for example, in beets.autotag.mb: that value comes from the length of the track listing on each medium.

On the other hand, I think I see what you mean about the problem in discogs. Could you open a pull request, preferably with a few tests, to demonstrate the proposed change?

dbogdanov commented 6 years ago

Hmm… this is the part I'm not sure I agree with. The idea behind the the medium_total is that it contains exactly what tracktotal wants: the number of tracks on the medium (e.g., disc or side) that the track appears on.

I see. So then it's the missing plugin's fault to identify those vinyl releases as having missing files.

sampsyo commented 6 years ago

Yeah, good point. Is it the case that missing is confused on any release tagged with per_disc_numbering, perhaps?

dbogdanov commented 6 years ago

It appears that in the case of discogs medium_total contains the number of mediums, not the number of tracks on each medium. The same happens in tests for discogs, so it seems to be done intentionally.

I'll create a pull request after you confirm that this definition of medium_total isn't the correct one.

sampsyo commented 6 years ago

Aha! Yes indeed; that seems like a mistake to me. Thank you!