Handle off-by-1 errors in cdparanoia progress parsing

Fixes the exception I got on ripping The Strokes - Someday

HACKING

@@ -41,3 +41,6 @@ CDROMS
 
 PLEXTOR CD-R   PX-W8432T Read offset of device is: 355.
 
+Test discs
+----------
+The Strokes - Someday (promo): has 1 frame silence marked as SILENCE

morituri/common/common.py

@@ -30,7 +30,7 @@ from morituri.extern.log import log
 
 FRAMES_PER_SECOND = 75
 
-SAMPLES_PER_FRAME = 588
+SAMPLES_PER_FRAME = 588 # a sample is 2 16-bit values, left and right channel
 WORDS_PER_FRAME = SAMPLES_PER_FRAME * 2
 BYTES_PER_FRAME = SAMPLES_PER_FRAME * 4
 

morituri/program/cdparanoia.py

@@ -64,19 +64,21 @@ class ChecksumException(Exception):
     pass
 
 
+# example:
+# ##: 0 [read] @ 24696
 _PROGRESS_RE = re.compile(r"""
-    ^\#\#: (?P<code>.+)\s      # function code
+    ^\#\#: (?P<code>.+)\s         # function code
-    \[(?P<function>.*)\]\s@\s     # function name
+    \[(?P<function>.*)\]\s@\s     # [function name] @
-    (?P<offset>\d+)        # offset
+    (?P<offset>\d+)               # offset in words (2-byte one channel value)
 """, re.VERBOSE)
 
 _ERROR_RE = re.compile("^scsi_read error:")
 
 # from reading cdparanoia source code, it looks like offset is reported in
-# number of single-channel samples, ie. 2 bytes per unit, and absolute
+# number of single-channel samples, ie. 2 bytes (word) per unit, and absolute
 
 
-class ProgressParser(object):
+class ProgressParser(log.Loggable):
     read = 0 # last [read] frame
     wrote = 0 # last [wrote] frame
     errors = 0 # count of number of scsi errors
@@ -128,13 +130,15 @@ class ProgressParser(object):
         # set nframes if not yet set
         if self._nframes is None and self.read != 0:
             self._nframes = frameOffset - self.read
+            self.debug('set nframes to %r', self._nframes)
 
         # set firstFrames if not yet set
         if self._firstFrames is None:
             self._firstFrames = frameOffset - self.start
+            self.debug('set firstFrames to %r', self._firstFrames)
 
         markStart = None
-        markEnd = None
+        markEnd = None # the next unread frame (half-inclusive)
 
         # verify it either read nframes more or went back for verify
         if frameOffset > self.read:
@@ -165,10 +169,11 @@ class ProgressParser(object):
 
         # cdparanoia reads quite a bit beyond the current track before it
         # goes back to verify; don't count those
-        if markEnd > self.stop:
+        # markStart, markEnd of 0, 21 with stop 0 should give 1 read
-            markEnd = self.stop
+        if markEnd > self.stop + 1:
-        if markStart > self.stop:
+            markEnd = self.stop + 1
-            markStart = self.stop
+        if markStart > self.stop + 1:
+            markStart = self.stop + 1
 
         self.reads += markEnd - markStart
 
@@ -185,8 +190,9 @@ class ProgressParser(object):
         Each frame gets read twice.
         More than two reads for a frame reduce track quality.
         """
-        frames = self.stop - self.start + 1
+        frames = self.stop - self.start + 1 # + 1 since stop is inclusive
         reads = self.reads
+        self.debug('getTrackQuality: frames %d, reads %d' % (frames, reads))
 
         # don't go over a 100%; we know cdparanoia reads each frame at least
         # twice

morituri/test/Makefile.am

@@ -44,8 +44,10 @@ EXTRA_DIST = \
 	track-single.cue \
 	cdparanoia.progress \
 	cdparanoia.progress.error \
+	cdparanoia.progress.strokes \
 	cdrdao.readtoc.progress \
 	silentalarm.result.pickle \
+	strokes-someday.toc \
 	totbl.fast.toc \
 	track.flac \
 	cache/result/fe105a11.pickle \

morituri/test/cdparanoia.progress.strokes

@@ -0,0 +1,111 @@
+Sending all callbacks to stderr for wrapper script
+cdparanoia III release 10.2 (September 11, 2008)
+
+Ripping from sector       0 (track  0 [0:00.00])
+	  to sector       0 (track  0 [0:00.00])
+
+outputting to cdda.wav
+
+##: 0 [read] @ 24696
+##: 0 [read] @ 56448
+##: 0 [read] @ 88200
+##: 0 [read] @ 119952
+##: 0 [read] @ 151704
+##: 0 [read] @ 183456
+##: 0 [read] @ 215208
+##: 0 [read] @ 246960
+##: 0 [read] @ 278712
+##: 0 [read] @ 310464
+##: 0 [read] @ 342216
+##: 0 [read] @ 373968
+##: 0 [read] @ 405720
+##: 0 [read] @ 437472
+##: 0 [read] @ 469224
+##: 0 [read] @ 500976
+##: 0 [read] @ 532728
+##: 0 [read] @ 564480
+##: 0 [read] @ 596232
+##: 0 [read] @ 627984
+##: 0 [read] @ 659736
+##: 0 [read] @ 691488
+##: 0 [read] @ 723240
+##: 0 [read] @ 754992
+##: 0 [read] @ 786744
+##: 0 [read] @ 818496
+##: 0 [read] @ 850248
+##: 0 [read] @ 882000
+##: 0 [read] @ 913752
+##: 0 [read] @ 945504
+##: 0 [read] @ 977256
+##: 0 [read] @ 1009008
+##: 0 [read] @ 1040760
+##: 0 [read] @ 1072512
+##: 0 [read] @ 1104264
+##: 0 [read] @ 1136016
+##: 0 [read] @ 1167768
+##: 0 [read] @ 1199520
+##: 0 [read] @ 1231272
+##: 0 [read] @ 1263024
+##: 0 [read] @ 1294776
+##: 0 [read] @ 1326528
+##: 0 [read] @ 1358280
+##: 0 [read] @ 1390032
+##: 0 [read] @ 1410024
+##: 0 [read] @ 23520
+##: 0 [read] @ 55272
+##: 0 [read] @ 87024
+##: 0 [read] @ 118776
+##: 0 [read] @ 150528
+##: 0 [read] @ 182280
+##: 0 [read] @ 214032
+##: 0 [read] @ 245784
+##: 0 [read] @ 277536
+##: 0 [read] @ 309288
+##: 0 [read] @ 341040
+##: 0 [read] @ 372792
+##: 0 [read] @ 404544
+##: 0 [read] @ 436296
+##: 0 [read] @ 468048
+##: 0 [read] @ 499800
+##: 0 [read] @ 531552
+##: 0 [read] @ 563304
+##: 0 [read] @ 595056
+##: 0 [read] @ 626808
+##: 0 [read] @ 658560
+##: 0 [read] @ 690312
+##: 0 [read] @ 722064
+##: 0 [read] @ 753816
+##: 0 [read] @ 785568
+##: 0 [read] @ 817320
+##: 0 [read] @ 849072
+##: 0 [read] @ 880824
+##: 0 [read] @ 912576
+##: 0 [read] @ 944328
+##: 0 [read] @ 976080
+##: 0 [read] @ 1007832
+##: 0 [read] @ 1039584
+##: 0 [read] @ 1071336
+##: 0 [read] @ 1103088
+##: 0 [read] @ 1134840
+##: 0 [read] @ 1166592
+##: 0 [read] @ 1198344
+##: 0 [read] @ 1230096
+##: 0 [read] @ 1261848
+##: 0 [read] @ 1293600
+##: 0 [read] @ 1325352
+##: 0 [read] @ 1357104
+##: 0 [read] @ 1388856
+##: 0 [read] @ 1410024
+##: 1 [verify] @ 0
+##: 3 [correction] @ 1005459
+##: 3 [correction] @ 1005480
+##: 1 [verify] @ 1005480
+##: 1 [verify] @ 1005480
+##: -2 [wrote] @ 1175
+##: -2 [wrote] @ 1176
+##: -1 [finished] @ 1175
+
+
+Done.
+
+

morituri/test/strokes-someday.toc

@@ -0,0 +1,12 @@
+CD_DA
+
+
+// Track 1
+TRACK AUDIO
+COPY
+NO PRE_EMPHASIS
+TWO_CHANNEL_AUDIO
+SILENCE 00:00:01
+FILE "data.wav" 0 03:06:59
+START 00:00:01
+

morituri/test/test_image_toc.py

@@ -318,3 +318,21 @@ class TOTBLTestCase(common.TestCase):
     def testCDDBId(self):
         self.toc.table.absolutize()
         self.assertEquals(self.toc.table.getCDDBDiscId(), '810b7b0b')
+
+
+# The Strokes - Someday has a 1 frame SILENCE marked as such in toc
+
+
+class StrokesTestCase(common.TestCase):
+
+    def setUp(self):
+        self.path = os.path.join(os.path.dirname(__file__),
+            u'strokes-someday.toc')
+        self.toc = toc.TocFile(self.path)
+        self.toc.parse()
+        self.assertEquals(len(self.toc.table.tracks), 1)
+
+    def testIndexes(self):
+        t = self.toc.table.tracks[0]
+        self.assertEquals(t.getIndex(0).relative, 0)
+        self.assertEquals(t.getIndex(1).relative, 1)

morituri/test/test_program_cdparanoia.py

@@ -25,7 +25,23 @@ class ParseTestCase(common.TestCase):
             self._parser.parse(line)
 
         q = '%.01f %%' % (self._parser.getTrackQuality() * 100.0, )
-        self.assertEquals(q, '99.7 %')
+        self.assertEquals(q, '99.6 %')
+
+class Parse1FrameTestCase(common.TestCase):
+
+    def setUp(self):
+        path = os.path.join(os.path.dirname(__file__),
+            'cdparanoia.progress.strokes')
+        self._parser = cdparanoia.ProgressParser(start=0, stop=0)
+
+        self._handle = open(path)
+
+    def testParse(self):
+        for line in self._handle.readlines():
+            self._parser.parse(line)
+
+        q = '%.01f %%' % (self._parser.getTrackQuality() * 100.0, )
+        self.assertEquals(q, '100.0 %')
 
 
 class ErrorTestCase(common.TestCase):