whipper icon indicating copy to clipboard operation
whipper copied to clipboard

AccurateRip checks failed on CD with data track

Open gorgobacka opened this issue 7 years ago • 11 comments

While testing the updated version on a CD with a data track at the end, whipper seems to crash when checking the tracks against AccurateRip.

Log file attached.

@RecursiveForest This might be related to your recent AccurateRip V2 PR. Maybe you have an idea why this error occurs.

whipper.log.gz

gorgobacka avatar Sep 18 '17 10:09 gorgobacka

It looks like the issue is it can't find the AccurateRip entry; it's definitely a bug if whipper crashes (can you paste console output?), but can you confirm what happens with a version of whipper before the commit came in? (I understand this may be difficult because JoeLametta neglected to squash merge…)

RecursiveForest avatar Sep 18 '17 16:09 RecursiveForest

Here is the console output (the interesting part):

....
CRCs match for track 16                    
Peak level: 92.72% 
Rip quality: 100.00%
AccurateRip entry not found                       
Traceback (most recent call last):
  File "/usr/local/bin/whipper", line 11, in <module>
    load_entry_point('whipper', 'console_scripts', 'whipper')()
  File "/home/XXXXX/Documents/Programming/whipper/whipper/command/main.py", line 32, in main
    ret = cmd.do()
  File "/home/XXXXX/Documents/Programming/whipper/whipper/command/basecommand.py", line 124, in do
    return self.cmd.do()
  File "/home/XXXXX/Documents/Programming/whipper/whipper/command/basecommand.py", line 124, in do
    return self.cmd.do()
  File "/home/XXXXX/Documents/Programming/whipper/whipper/command/cd.py", line 190, in do
    self.doCommand()
  File "/home/XXXXX/Documents/Programming/whipper/whipper/command/cd.py", line 492, in doCommand
    accurip.print_report(self.program.result)
  File "/home/XXXXX/Documents/Programming/whipper/whipper/common/accurip.py", line 249, in print_report
    if track.AR['DBMaxConfidence'] is not None:
TypeError: 'NoneType' object has no attribute '__getitem__'

Tomorrow, I will try again with a previous commit and will report back. (However, it has worked before, since it's my "data track CD". for testing.)

gorgobacka avatar Sep 18 '17 16:09 gorgobacka

(I understand this may be difficult because JoeLametta neglected to squash merge…)

Well, that was on purpose. I've decided I'm not going to squash merge anything big (or) important enough to cause troubles. Of course this choice won't disallow the user who submits the PR to work in his branch as he prefers (squash, rebase, rewrite, etc.): that's fine as long as the final PR doesn't resemble a fat monolithic blob.

After a quick check, I've found the two following suspects:

  1. Uncertain about this one.
  2. I'm not sure but I don't think that ConnectionError handles the HTTP 404 status code (doesn't this require HTTPError?). Code here.

Tomorrow, I will try again with a different commit and will report back. (However, I think I tried this CD before and it worked.)

Thanks for the bug report. Going back to 7ae27de32bcd3cc59b98a8388aa0f48e48120c8e should be enough.

JoeLametta avatar Sep 18 '17 17:09 JoeLametta

I'm pretty confident I know what's causing the bug, so I'll try to work a fix tonight. The issue is related to the code not supporting discs with data tracks properly (because I don't have any to test with).

RecursiveForest avatar Sep 18 '17 17:09 RecursiveForest

@gorgobacka , does whipper report the CD as found in the accuraterip database in previous versions? If so, could you paste the accuraterip URL? (Or better yet, paste the entire console output & debug log from a succesful run with a previous version.)

RecursiveForest avatar Sep 19 '17 18:09 RecursiveForest

@RecursiveForest unfortunately, it seems that the CD is not present in the accuraterip database.

I went back to commit https://github.com/JoeLametta/whipper/commit/7ae27de32bcd3cc59b98a8388aa0f48e48120c8e and the rip worked fine.

Here are the console output & debug log: whipper_console_output.log.gz whipper.log.gz

gorgobacka avatar Sep 20 '17 07:09 gorgobacka

@gorgobacka Does it still happen?

To me it seems related to #196.

JoeLametta avatar Apr 06 '18 13:04 JoeLametta

I will check again.

gorgobacka avatar Apr 06 '18 17:04 gorgobacka

I get the same error as before with the same CD. Logfiles added.

console_output.txt.gz whipper.log.gz

(If it's the same error, one issue can be close. But I'm not sure.)

gorgobacka avatar Apr 06 '18 20:04 gorgobacka

I should have code to fix this somewhere...

MerlijnWajer avatar Apr 06 '18 20:04 MerlijnWajer

I get the same error as before with the same CD. Logfiles added.

Thanks! Please see @RecursiveForest's comment: to test that hypothesis could you backup your whipper cache folder and check if moving it away solves the issue?

EDIT: I've just seen @MerlijnWajer's previous comment. :smile:

JoeLametta avatar Apr 06 '18 20:04 JoeLametta