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

"Catalog Number" incorrectly appended to "artist" instead of the Album name. #127

Closed
lschapker opened this issue Feb 7, 2017 · 8 comments · Fixed by #153
Closed

"Catalog Number" incorrectly appended to "artist" instead of the Album name. #127

lschapker opened this issue Feb 7, 2017 · 8 comments · Fixed by #153
Labels
Bug Generic bug: can be used together with more specific labels

Comments

@lschapker
Copy link

lschapker commented Feb 7, 2017

Whipper version 0.4.2

I am using the following command line for each rip:

whipper cd rip --output-directory "~/Media-Raw/Audio" --track-template "%A/%d - %y/%t - %n" --disc-template "%A/%d - %y"

When I rip a second CD (or more) from the same artist, strange/unexpected directories are created. Example directory structure after second rip:

~/Media-Raw/Audio/Guy Davis
~/Media-Raw/Audio/Guy Davis/Call Down the Thunder - 1996
~/Media-Raw/Audio/Guy Davis/Call Down the Thunder - 1996/<tracks...>
~/Media-Raw/Audio/Guy Davis/Stomp Down Rider - 1995
~/Media-Raw/Audio/Guy Davis/Stomp Down Rider - 1995/<tracks...>
~/Media-Raw/Audio/Guy Davis/<discfiles for "Stomp">
~/Media-Raw/Audio/Guy Davis (RHR CD 89)/<discfiles for "Call">
(the last entry being unexpected)

When needed, the catalog number (or barcode) should be appended to the "disc name" and not anywhere else.

I have a relatively "simple fix" that works in my situation (i.e. with my track and disc templates), but do not know if other combinations have problems. Only changes to cd.py and programs.py (maybe 10 lines total). I will gladly provide, if someone lets me know how the changes should be provided.

@JoeLametta JoeLametta added the Bug Generic bug: can be used together with more specific labels label Feb 12, 2017
@JoeLametta
Copy link
Collaborator

I have a relatively "simple fix" that works in my situation (i.e. with my track and disc templates), but do not know if other combinations have problems. Only changes to cd.py and programs.py (maybe 10 lines total). I will gladly provide, if someone lets me know how the changes should be provided.

You can provide a patch file and share it in this thread or fork whipper and create a pull request against this repository.

@lschapker
Copy link
Author

lschapker commented Feb 12, 2017 via email

@JoeLametta
Copy link
Collaborator

I am currently away from my computer but will be back in March. When back, I will gladly provide a patch file.Thanks!

Hi, don't want to put pressure on you: just wanted to ask you if you're still interested in providing the aforementioned patch.

Thanks in advance,

Joe

@lschapker
Copy link
Author

Hello Joe,
I'm so sorry. Life got in the way and I forgot about this. Thank you for the reminder!

I'm trying to "back out" other change I've made (usage of "mutagen.flac" instead of the original way to encode because I couldn't get the original code to function within PyDev). I only include the code necessary to correct the catalog/directory naming. Please let me know if this looks incomplete/does not function. If it doesn't work (or it looks strange), I'll download the latest code and re-implement the changes I made and send you that code (after almost 3 months not touching this, my memory isn't as sharp as it would like it to be). I patched 2 PY files: morituri/command/cd.py and morituri/common/program.py

2FilesPatched.txt

Please let me know. Thanks, Larry

@Freso
Copy link
Member

Freso commented Apr 28, 2017

I think the approach in the patch is potentially flawed, as someone might well want to do something akin to "%A/%d - %y/%X" or "%A - %d - %y - %X" which would probably throw your (and the original/current!) approach off.

Maybe it's time to implement something like beets' %aunique() disambiguation function?

@lschapker
Copy link
Author

I agree that the "change" (more like a "hack") may not be universal, but in this situation (or maybe just my situation) we do not have 2 albums/discs/etc with the same name. Here we have the album catalog number being appended to the "artist" value instead of the "album name" where (I believe) it belongs (if needed at all).

My presumption is that if we had 2 different albums with the same name, they would have 2 different catalog numbers (i.e. how would the record company be able to track each otherwise?). In that case, the fix would still function as long as the catalog number is always used.

Again, I agree that if the end user wanted a different pattern, my fix wouldn't work correctly (but it's just a "hack" to get me moving in what I felt is a correct direction ).

@Freso
Copy link
Member

Freso commented Apr 29, 2017

I agree that the "change" (more like a "hack") may not be universal, but in this situation (or maybe just my situation) we do not have 2 albums/discs/etc with the same name. Here we have the album catalog number being appended to the "artist" value instead of the "album name" where (I believe) it belongs (if needed at all).

Yes, I understand the issue, and what I'm pointing out is that your hack may work for your template(s), but if someone is using a template of "%A/%d - %y/%X", then the cat. no./disambiguation will get appended to the "%X" part.

Actually, I think I just got an idea for how this could be tackled, while being path level agnostic and without introducing new syntax: looking at the path segments for which one contains "%d" (if any) and append the disambiguation to that. I'll try and whip (pun not fully intended) a PR together for this.


if we had 2 different albums with the same name, they would have 2 different catalog numbers

Not necessarily.

i.e. how would the record company be able to track each otherwise?

  1. Record companies are horrible at keeping track of their stuff.
  2. Not all releases are under record companies, and not all releases get cat. no.'s at all (incl. some releases from record companies).

@Freso
Copy link
Member

Freso commented Apr 29, 2017

@lschapker I'm making a PR at #153 which should fix this issue. If you feel like testing to see if it works for your purposes, that would be great. Thank you. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Generic bug: can be used together with more specific labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants