beets icon indicating copy to clipboard operation
beets copied to clipboard

Fix Hybrid SACD importing

Open InvisibleFunction opened this issue 1 year ago • 7 comments

Description

Fixes #5148.

When importing, the code that matches tracks does not consider the medium number. This causes problems on Hybrid SACDs where the artists, track numbers, titles, and lengths are the same on both layers.

I added a distance penalty for mismatching medium numbers.

Before:

$ beet imp .

/Volumes/Music/ti/Red Garland/1958 - All Mornin' Long - 1 (6 items)

  Match (95.4%):
  The Red Garland Quintet - All Mornin' Long
  ≠ media, year
  MusicBrainz, 2xHybrid SACD (CD layer), 2013, US, Analogue Productions, CPRJ 7130 SA, mono
  https://musicbrainz.org/release/6a584522-58ea-470b-81fb-e60e5cd7b21e
  * Artist: The Red Garland Quintet
  * Album: All Mornin' Long
  * Hybrid SACD (CD layer) 1
     ≠ (#2-1) All Mornin' Long (20:21) -> (#1-1) All Mornin' Long (20:21)
     ≠ (#2-2) They Can't Take That Away From Me (10:24) -> (#1-2) They Can't Take That Away From Me (10:27)
     ≠ (#2-3) Our Delight (6:23) -> (#1-3) Our Delight (6:23)
  * Hybrid SACD (CD layer) 2
     ≠ (#1-1) All mornin' long (20:21) -> (#2-1) All Mornin' Long (20:21)
     ≠ (#1-2) They can't take that away from me (10:27) -> (#2-2) They Can't Take That Away From Me (10:25)
     ≠ (#1-3) Our delight (6:23) -> (#2-3) Our Delight (6:23)
➜ [A]pply, More candidates, Skip, Use as-is, as Tracks, Group albums,
Enter search, enter Id, aBort, eDit, edit Candidates?

Note that all tracks tagged with disc 1 get moved to disc 2 and vice versa.

After:

$ beet-test imp .

/Volumes/Music/ti/Red Garland/1958 - All Mornin' Long - 1 (6 items)

  Match (95.4%):
  The Red Garland Quintet - All Mornin' Long
  ≠ media, year
  MusicBrainz, 2xMedia, 2013, US, Analogue Productions, CPRJ 7130 SA, mono
  https://musicbrainz.org/release/6a584522-58ea-470b-81fb-e60e5cd7b21e
  * Artist: The Red Garland Quintet
  * Album: All Mornin' Long
  * Hybrid SACD (CD layer) 1
     ≠ (#1-1) All mornin' long (20:21) -> (#1-1) All Mornin' Long (20:21)
     ≠ (#1-2) They can't take that away from me (10:27) -> (#1-2) They Can't Take That Away From Me (10:27)
     ≠ (#1-3) Our delight (6:23) -> (#1-3) Our Delight (6:23)
  * Hybrid SACD (SACD layer) 2
     * (#2-1) All Mornin' Long (20:21)
     * (#2-2) They Can't Take That Away From Me (10:24)
     * (#2-3) Our Delight (6:23)
➜ [A]pply, More candidates, Skip, Use as-is, as Tracks, Group albums,
Enter search, enter Id, aBort, eDit, edit Candidates?

Yay!

To Do

  • [] ~Documentation. (If you've added a new command-line flag, for example, find the appropriate page under docs/ to describe it.)~
  • [X] Changelog. (Add an entry to docs/changelog.rst to the bottom of one of the lists near the top of the document.)
  • [ ] Tests. (Very much encouraged but not strictly required.) (I don't really know how to do this effectively and would appreciate pointers.)

InvisibleFunction avatar Mar 17 '24 02:03 InvisibleFunction

Can I get any feedback on this PR?

InvisibleFunction avatar Mar 30 '24 19:03 InvisibleFunction

uhoh, I tried to get fancy with git :( let me fix that

InvisibleFunction avatar Mar 30 '24 19:03 InvisibleFunction

Okay, I think I successfully got git fancy. Would love some feedback on this PR!

InvisibleFunction avatar May 15 '24 02:05 InvisibleFunction

Hey, before this slips off the front page of PRs, what do we have to do to implement this fix?

InvisibleFunction avatar Jun 28 '24 15:06 InvisibleFunction

Looks good to me. I'd love to see this merged, as I have this exact problem. It also seems reasonable in general to consider the medium number even for other media.

emiham avatar Aug 21 '24 11:08 emiham

It has come in handy for me when I want to import just one disk of a release that has both cds and bluerays or something.

InvisibleFunction avatar Aug 22 '24 19:08 InvisibleFunction

Here is an example of a non-SACD release which can't be properly imported right now: https://musicbrainz.org/release/ea92a194-2d60-35c7-9d56-0e1dba20cd45

emiham avatar Aug 29 '24 23:08 emiham