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

Ability to skip unrippable track #128

Closed
brendan-pike opened this issue Feb 9, 2017 · 30 comments
Closed

Ability to skip unrippable track #128

brendan-pike opened this issue Feb 9, 2017 · 30 comments
Labels
Accepted Accepted issue on our roadmap Feature New feature Needed: discussion More discussion needed before anything can be done (or still no agreement has been reached) Priority: medium Medium priority
Milestone

Comments

@brendan-pike
Copy link

I have some CDs that have been damaged, whipper stops when it hits a damaged track, ie.
CRITICAL:morituri.command.cd:Giving up on track 4 after 5 times

Could a flag be introduced like -c 'Continue' when a track is unrippable?

@JoeLametta JoeLametta modified the milestones: 2.0, 101010 Feb 12, 2017
@Freso
Copy link
Member

Freso commented Feb 13, 2017

Similar issue in morituri's tracker: thomasvs/morituri#49

@JoeLametta
Copy link
Collaborator

JoeLametta commented Feb 20, 2017

I agree: this is something which is really needed.

To everybody: exactly how would you like to see it implemented?

@istathar
Copy link

@JoeLametta just an option like --keep-going which omits the damaged track and ... keeps going. The accuracy report at the end could say "Track 04: Failed" or something but other than that pretend it doesn't exist.

@istathar
Copy link

(I've got several thousand 20 year old CDs to rip. Most will be difficult to obtain replacements of should they be partially damaged. Some have scratches, others water damaged in a recent storm. Would like to save [at full fidelity] what we can)

@ArchangeGabriel
Copy link

@afcowie Just a little point about scratches: they are almost nothing if they’re only on the surface, you can repair that and have a brand new polished one. ;)

@Freso
Copy link
Member

Freso commented Mar 12, 2017

Currently, if you set --working-directory to the same directory you were ripping to before, morituri/whipper will recognise that there already is a rip there and attempt to continue that rip instead of ripping all of it again.

It would be nice if, however this feature is implemented, it could pick up on the damaged tracks and try to rip them later, like how it can currently resume a rip. So e.g., if you rip tracks 1–3, tracks 4–8 are bad, and then rips 9–11. If you then clean the disc or polish it or otherwise attempt to restore it and try to rip it again, only tracks 4–8 would have to be ripped.

@JoeLametta
Copy link
Collaborator

Currently, if you set --working-directory to the same directory you were ripping to before, morituri/whipper will recognise that there already is a rip there and attempt to continue that rip instead of ripping all of it again.

Didn't even know about this 😄

@vargx
Copy link

vargx commented Jan 16, 2018

This is a must have enhancement. Some cds from the 90s have data tracks in them and whipper fails to rip them. We should have the option to continue ripping after fail and also to skip tracks.

Is there perhaps someway to make a donation to make this happen?

@waweic
Copy link

waweic commented Mar 11, 2018

I have just tried to implement a function to exclude specific tracks which seems to work as expected.
The cuesheet seems to assume, that the missing files are part of the previous file while the log just says that the CRC check failed. Is this something we should implement in the normal version?

@waweic
Copy link

waweic commented Mar 29, 2018

In the last days, I had to deal with some CDs in a pretty bad condition. For them, I implemented an option to be able to exclude tracks from the rip. This would at least get the audio data out of the CD, which will be pretty helpful to the users, that want to have full control over their rips.

@MerlijnWajer
Copy link
Collaborator

Do you have the code for this option somewhere in a PR?

@JoeLametta JoeLametta modified the milestones: backlog, 2.0 Apr 6, 2018
@waweic
Copy link

waweic commented Apr 8, 2018

Added #260

@RecursiveForest
Copy link
Contributor

RecursiveForest commented Jun 8, 2018

I think the best thing to do here would be to implement a --continue-like option to continue ripping after failed tracks, and a --rip-only-these-tracks-like option.

Thanks for the suggestion.

@JoeLametta
Copy link
Collaborator

I think the best thing to do here would be to implement a --continue-like option to continue ripping after failed tracks, and a --rip-only-these-tracks-like option.

I agree.

@xmixahlx
Copy link

Just providing some support for these ideas after I ran into some errors this weekend:
--continue (try to rerip missing tracks)
--keep-going (attempt to rip the next track after a track with errors)
--track ## (a list of tracks to rip in brackets, i.e. {1,2,5,6,7} )
--max-tries (adjustable max tries option, default being 5)

The combination of these options would be quite powerful in addressing problem discs.

@Gooberpatrol66
Copy link

I would like the ability to rip a damaged track and somehow mark it as being corrupted.

@jtl999
Copy link
Contributor

jtl999 commented Nov 5, 2018

Yes, maybe a good idea for the worst case scenario assuming cdparanoia reports the position of errors in a track similar to EAC's "suspicious position"

@JoeLametta JoeLametta added Accepted Accepted issue on our roadmap Priority: medium Medium priority Feature New feature Needed: discussion More discussion needed before anything can be done (or still no agreement has been reached) and removed enhancement labels Nov 12, 2018
@edrozenberg
Copy link

Would be great to be able to include even a track with errors/failed checksum, and continue. i.e. tell the rip to "just give me the best you have and keep going". Would be very useful.

@jtl999
Copy link
Contributor

jtl999 commented Jan 15, 2019

It's planned, and I already have a few ideas regarding the implementation.

@jonpatterns
Copy link

I too would love to see this feature.

@lenzj
Copy link

lenzj commented Jul 1, 2019

This is a much needed feature. Seconding @xmixahlx suggestions. In particular being able to provide a list of tracks to rip in brackets, i.e. {1,2,5,6,7} could be a simple solution.

In terms of a "nice to have" feature it might be useful if whipper could identify the specific position(s) within a track where the read errors are occurring. Running checksums on smaller chunks of the track could identify the location(s) where errors are occurring.

Thank you for your efforts on whipper! Best cd ripper I have found.

@srussel
Copy link

srussel commented Nov 24, 2019

I would like an option as @edrozenberg suggested to allow keeping tracks with bad checksums. I have found that around 1% of my CDs have an unrippable track. These unrippable tracks play fine as an audio CD in the same drive. Obtaining a replacement CD is either not possible or not worth the cost. In one case I had an unrippable track on a brand new shrinkwrapped CD.

I would like to be able to rip these unrippable tracks on a best efforts basis. It is better to have something rather than nothing. Whipper could perhaps include metadata in the flac about the quality of the rip so if at some point in the future I want to be certain about the quality of the rip or if I have duplicate songs and want to keep the best I can do so.

Also, I have noticed in some cases that when retrying I see one checksum that is more common than the others, just not in the same read/verify try. Maybe the there could be an option to do a number of reads and pick the read with the most commonly occurring checksum.

@Perdellian
Copy link

Perdellian commented Mar 9, 2020

Also, I have noticed in some cases that when retrying I see one checksum that is more common than the others, just not in the same read/verify try. Maybe the there could be an option to do a number of reads and pick the read with the most commonly occurring checksum.

I believe that's how RubyRipper did it, meaning that you would need, say, 3/5 total tries matching rather than two consecutive tries.

At some point in the project's history, I think it even switched over to doing partial-track rerips to narrow down where the error was and to save time by not reripping/analyzing sections that were already known to match (with, perhaps, some minimum size to make sure they were still beating the cache?).

@hschmidt
Copy link

Agreed, sorely needed. To get me over the hump of one stubborn track, I did the patch below. Probably naive, but better than nothing.

Index: whipper/command/cd.py
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/whipper/command/cd.py b/whipper/command/cd.py
--- a/whipper/command/cd.py	(revision 4fb9d99dddd45ca84bf0adc6c4cb09ce2e27c7c3)
+++ b/whipper/command/cd.py	(date 1609379659979)
@@ -290,6 +290,10 @@
                                  action="store_true", dest="unknown",
                                  help="whether to continue ripping if "
                                  "the CD is unknown", default=False)
+        self.parser.add_argument('--ignore-checksums',
+                                 action="store_true", dest="ignore_checksums",
+                                 help="whether to continue ripping if "
+                                 "a checksum mismatch occurs", default=False)
         self.parser.add_argument('--cdr',
                                  action="store_true", dest="cdr",
                                  help="whether to continue ripping if "
@@ -465,7 +469,8 @@
                                                   number,
                                                   len(self.itable.tracks),
                                                   extra),
-                                              coverArtPath=self.coverArtPath)
+                                              coverArtPath=self.coverArtPath,
+                                              ignoreChecksums=self.options.ignore_checksums)
                         break
                     # FIXME: catching too general exception (Exception)
                     except Exception as e:
@@ -482,9 +487,10 @@
                 if trackResult.testcrc == trackResult.copycrc:
                     logger.info('CRCs match for track %d', number)
                 else:
-                    raise RuntimeError(
-                        "CRCs did not match for track %d" % number
-                    )
+                    if not self.options.ignore_checksums:
+                        raise RuntimeError(
+                            "CRCs did not match for track %d" % number
+                        )
 
                 print('Peak level: %.6f' % (trackResult.peak / 32768.0))
                 print('Rip quality: {:.2%}'.format(trackResult.quality))
Index: whipper/program/cdparanoia.py
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/whipper/program/cdparanoia.py b/whipper/program/cdparanoia.py
--- a/whipper/program/cdparanoia.py	(revision 4fb9d99dddd45ca84bf0adc6c4cb09ce2e27c7c3)
+++ b/whipper/program/cdparanoia.py	(date 1609374978899)
@@ -440,7 +440,8 @@
     _tmppath = None
 
     def __init__(self, path, table, start, stop, overread, offset=0,
-                 device=None, taglist=None, what="track", coverArtPath=None):
+                 device=None, taglist=None, what="track", coverArtPath=None,
+                 ignoreChecksums=False):
         """
         Init ReadVerifyTrackTask.
 
@@ -460,6 +461,7 @@
         :type taglist: dict
         """
         task.MultiSeparateTask.__init__(self)
+        self.ignoreChecksums = ignoreChecksums
 
         logger.debug('creating read and verify task on %r', path)
 
@@ -537,12 +539,14 @@
                     # FIXME: detect this before encoding
                     logger.info('checksums do not match, %08x %08x',
                                 c1, c2)
-                    self.exception = ChecksumException(
-                        'read and verify failed: test checksum')
+                    if not self.ignoreChecksums:
+                        self.exception = ChecksumException(
+                            'read and verify failed: test checksum')
 
                 if self.tasks[5].checksum != self.checksum:
-                    self.exception = ChecksumException(
-                        'Encoding failed, checksum does not match')
+                    if not self.ignoreChecksums:
+                        self.exception = ChecksumException(
+                            'Encoding failed, checksum does not match')
 
                 # delete the unencoded file
                 os.unlink(self._tmpwavpath)
Index: whipper/common/program.py
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/whipper/common/program.py b/whipper/common/program.py
--- a/whipper/common/program.py	(revision 4fb9d99dddd45ca84bf0adc6c4cb09ce2e27c7c3)
+++ b/whipper/common/program.py	(date 1609374877535)
@@ -534,7 +534,7 @@
         return ret
 
     def ripTrack(self, runner, trackResult, offset, device, taglist,
-                 overread, what=None, coverArtPath=None):
+                 overread, what=None, coverArtPath=None, ignoreChecksums=False):
         """
         Rip and store a track of the disc.
 
@@ -576,7 +576,8 @@
                                            device=device,
                                            taglist=taglist,
                                            what=what,
-                                           coverArtPath=coverArtPath)
+                                           coverArtPath=coverArtPath,
+                                           ignoreChecksums=ignoreChecksums)
 
         runner.run(t)
 

@ghost
Copy link

ghost commented Jan 30, 2021

Is there any appetite for the other options suggested here, such as the --track ##/--rip-only-these-tracks or an option to keep tracks with bad checksums? I was considering implementing those also, but wanted to hear suggestions from users/developers first.

@aereaux
Copy link

aereaux commented Jan 31, 2021

I'd personally love a way to keep tracks with bad checksums. I've resorted to using another program to rip them.

@ssokolow
Copy link

Is there any appetite for the other options suggested here, such as the --track ##/--rip-only-these-tracks

If I can ever make time and nobody else had done it, I was planning to implement those for getting good-quality backups of Red Book BGM tracks on CD-ROM games.

I've resorted to using another program to rip them.

I do that, but I'm not sure a "keep tracks with bad checksums" would help. Exact Audio Copy (which I keep installed in Wine for things whipper fails) just has a more robust system for ripping problem tracks, even if I've never figured out how to get it to use MusicBrainz for metadata instead of GnuDB.

@aereaux
Copy link

aereaux commented Apr 12, 2021

Exact Audio Copy (which I keep installed in Wine for things whipper fails) just has a more robust system for ripping problem tracks

Is there any documentation on what EAC does for this? Maybe it would be possible to implement that sort of thing in whipper?

@JoeLametta
Copy link
Collaborator

Is there any documentation on what EAC does for this? Maybe it would be possible to implement that sort of thing in whipper?

Some information here:

@jonpatterns
Copy link

jonpatterns commented Dec 13, 2022

--keep-going is not the same as skip track.

If I know a track is unrippable then it would be good to be able to specify skipping that track. Instead of waiting a long time for it to fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted Accepted issue on our roadmap Feature New feature Needed: discussion More discussion needed before anything can be done (or still no agreement has been reached) Priority: medium Medium priority
Projects
None yet
Development

No branches or pull requests