WIP: Refactor cdrdao toc/table functions into Task and provide progress output (#345)

* Begin work on moving cdrdao to a task

* Add code to start cdrdao task

* Allow cdrdao output to be asynchronously parsable

* Provide progress of cdrdao read toc/table to console

* Flake8 fixes, Freso's advices

whipper/command/cd.py

@@ -94,7 +94,6 @@ class _CD(BaseCommand):
         utils.unmount_device(self.device)
 
         # first, read the normal TOC, which is fast
-        logger.info("reading TOC...")
         self.ittoc = self.program.getFastToc(self.runner, self.device)
 
         # already show us some info based on this

whipper/common/program.py

@@ -27,11 +27,12 @@ import re
 import os
 import time
 
-from whipper.common import accurip, cache, checksum, common, mbngs, path
+from whipper.common import accurip, checksum, common, mbngs, path
 from whipper.program import cdrdao, cdparanoia
 from whipper.image import image
 from whipper.extern import freedb
 from whipper.extern.task import task
+from whipper.result import result
 
 import logging
 logger = logging.getLogger(__name__)
@@ -63,7 +64,6 @@ class Program:
         @param record: whether to record results of API calls for playback.
         """
         self._record = record
-        self._cache = cache.ResultCache()
         self._config = config
 
         d = {}
@@ -95,42 +95,31 @@ class Program:
         if V(version) < V('1.2.3rc2'):
             logger.warning('cdrdao older than 1.2.3 has a pre-gap length bug.'
                            ' See http://sourceforge.net/tracker/?func=detail&aid=604751&group_id=2171&atid=102171')  # noqa: E501
-        toc = cdrdao.ReadTOCTask(device).table
+
+        t = cdrdao.ReadTOC_Task(device)
+        runner.run(t)
+        toc = t.toc.table
+
         assert toc.hasTOC()
         return toc
 
     def getTable(self, runner, cddbdiscid, mbdiscid, device, offset,
-                 out_path):
+                 toc_path):
         """
-        Retrieve the Table either from the cache or the drive.
+        Retrieve the Table from the drive.
 
         @rtype: L{table.Table}
         """
-        tcache = cache.TableCache()
-        ptable = tcache.get(cddbdiscid, mbdiscid)
         itable = None
         tdict = {}
 
-        # Ignore old cache, since we do not know what offset it used.
+        t = cdrdao.ReadTOC_Task(device)
-        if isinstance(ptable.object, dict):
+        t.description = "Reading table"
-            tdict = ptable.object
+        t.toc_path = toc_path
-
+        runner.run(t)
-            if offset in tdict:
+        itable = t.toc.table
-                itable = tdict[offset]
+        tdict[offset] = itable
-
+        logger.debug('getTable: read table %r' % itable)
-        if not itable:
-            logger.debug('getTable: cddbdiscid %s, mbdiscid %s not in cache '
-                         'for offset %s, reading table', cddbdiscid, mbdiscid,
-                         offset)
-            t = cdrdao.ReadTableTask(device, out_path)
-            itable = t.table
-            tdict[offset] = itable
-            ptable.persist(tdict)
-            logger.debug('getTable: read table %r', itable)
-        else:
-            logger.debug('getTable: cddbdiscid %s, mbdiscid %s in cache '
-                         'for offset %s', cddbdiscid, mbdiscid, offset)
-            logger.debug('getTable: loaded table %r', itable)
 
         assert itable.hasTOC()
 
@@ -142,21 +131,15 @@ class Program:
 
     def getRipResult(self, cddbdiscid):
         """
-        Retrieve the persistable RipResult either from our cache (from a
+        Return a RipResult object.
-        previous, possibly aborted rip), or return a new one.
 
         @rtype: L{result.RipResult}
         """
         assert self.result is None
-
+        self.result = result.RipResult()
-        self._presult = self._cache.getRipResult(cddbdiscid)
-        self.result = self._presult.object
 
         return self.result
 
-    def saveRipResult(self):
-        self._presult.persist()
-
     def addDisambiguation(self, template_part, metadata):
         "Add disambiguation to template path part string."
         if metadata.catalogNumber:

whipper/program/cdrdao.py

@@ -2,58 +2,163 @@ import os
 import re
 import shutil
 import tempfile
+import subprocess
 from subprocess import Popen, PIPE
 
-from whipper.common.common import EjectError, truncate_filename
+from whipper.common.common import truncate_filename
 from whipper.image.toc import TocFile
+from whipper.extern.task import task
+from whipper.extern import asyncsub
 
 import logging
 logger = logging.getLogger(__name__)
 
 CDRDAO = 'cdrdao'
 
+_TRACK_RE = re.compile(r"^Analyzing track (?P<track>[0-9]*) \(AUDIO\): start (?P<start>[0-9]*:[0-9]*:[0-9]*), length (?P<length>[0-9]*:[0-9]*:[0-9]*)")  # noqa: E501
+_CRC_RE = re.compile(
+    r"Found (?P<channels>[0-9]*) Q sub-channels with CRC errors")
+_BEGIN_CDRDAO_RE = re.compile(r"-" * 60)
+_LAST_TRACK_RE = re.compile(r"^(?P<track>[0-9]*)")
+_LEADOUT_RE = re.compile(
+    r"^Leadout AUDIO\s*[0-9]\s*[0-9]*:[0-9]*:[0-9]*\([0-9]*\)")
 
-def read_toc(device, fast_toc=False, toc_path=None):
+
+class ProgressParser:
+    tracks = 0
+    currentTrack = 0
+    oldline = ''  # for leadout/final track number detection
+
+    def parse(self, line):
+        cdrdao_m = _BEGIN_CDRDAO_RE.match(line)
+
+        if cdrdao_m:
+            logger.debug("RE: Begin cdrdao toc-read")
+
+        leadout_m = _LEADOUT_RE.match(line)
+
+        if leadout_m:
+            logger.debug("RE: Reached leadout")
+            last_track_m = _LAST_TRACK_RE.match(self.oldline)
+            if last_track_m:
+                self.tracks = last_track_m.group('track')
+
+        track_s = _TRACK_RE.search(line)
+        if track_s:
+            logger.debug("RE: Began reading track: %d",
+                         int(track_s.group('track')))
+            self.currentTrack = int(track_s.group('track'))
+
+        crc_s = _CRC_RE.search(line)
+        if crc_s:
+            print("Track %d finished, "
+                  "found %d Q sub-channels with CRC errors" %
+                  (self.currentTrack, int(crc_s.group('channels'))))
+
+        self.oldline = line
+
+
+class ReadTOCTask(task.Task):
     """
-    Return cdrdao-generated table of contents for 'device'.
+    Task that reads the TOC of the disc using cdrdao
     """
-    # cdrdao MUST be passed a non-existing filename as its last argument
+    description = "Reading TOC"
-    # to write the TOC to; it does not support writing to stdout or
+    toc = None
-    # overwriting an existing file, nor does linux seem to support
-    # locking a non-existant file. Thus, this race-condition introducing
-    # hack is carried from morituri to whipper and will be removed when
-    # cdrdao is fixed.
-    fd, tocfile = tempfile.mkstemp(suffix=u'.cdrdao.read-toc.whipper')
-    os.close(fd)
-    os.unlink(tocfile)
 
-    cmd = [CDRDAO, 'read-toc'] + (['--fast-toc'] if fast_toc else []) + [
+    def __init__(self, device, fast_toc=False, toc_path=None):
-        '--device', device, tocfile]
+        """
-    # PIPE is the closest to >/dev/null we can get
+        Read the TOC for 'device'.
-    logger.debug("executing %r", cmd)
-    p = Popen(cmd, stdout=PIPE, stderr=PIPE)
-    _, stderr = p.communicate()
-    if p.returncode != 0:
-        msg = 'cdrdao read-toc failed: return code is non-zero: ' + \
-              str(p.returncode)
-        logger.critical(msg)
-        # Gracefully handle missing disc
-        if "ERROR: Unit not ready, giving up." in stderr:
-            raise EjectError(device, "no disc detected")
-        raise IOError(msg)
 
-    toc = TocFile(tocfile)
+        @param device:  block device to read TOC from
-    toc.parse()
+        @type  device:  str
-    if toc_path is not None:
+        @param fast_toc:  If to use fast-toc cdrdao mode
-        t_comp = os.path.abspath(toc_path).split(os.sep)
+        @type  fast_toc: bool
-        t_dirn = os.sep.join(t_comp[:-1])
+        @param toc_path: Where to save TOC if wanted.
-        # If the output path doesn't exist, make it recursively
+        @type  toc_path: str
-        if not os.path.isdir(t_dirn):
+        """
-            os.makedirs(t_dirn)
+
-        t_dst = truncate_filename(os.path.join(t_dirn, t_comp[-1] + '.toc'))
+        self.device = device
-        shutil.copy(tocfile, os.path.join(t_dirn, t_dst))
+        self.fast_toc = fast_toc
-    os.unlink(tocfile)
+        self.toc_path = toc_path
-    return toc
+        self._buffer = ""  # accumulate characters
+        self._parser = ProgressParser()
+
+        self.fd, self.tocfile = tempfile.mkstemp(
+            suffix=u'.cdrdao.read-toc.whipper.task')
+
+    def start(self, runner):
+        task.Task.start(self, runner)
+        os.close(self.fd)
+        os.unlink(self.tocfile)
+
+        cmd = ([CDRDAO, 'read-toc']
+               + (['--fast-toc'] if self.fast_toc else [])
+               + ['--device', self.device, self.tocfile])
+
+        self._popen = asyncsub.Popen(cmd,
+                                     bufsize=1024,
+                                     stdin=subprocess.PIPE,
+                                     stdout=subprocess.PIPE,
+                                     stderr=subprocess.PIPE,
+                                     close_fds=True)
+
+        self.schedule(0.01, self._read, runner)
+
+    def _read(self, runner):
+        ret = self._popen.recv_err()
+        if not ret:
+            if self._popen.poll() is not None:
+                self._done()
+                return
+            self.schedule(0.01, self._read, runner)
+            return
+        self._buffer += ret
+
+        # parse buffer into lines if possible, and parse them
+        if "\n" in self._buffer:
+            lines = self._buffer.split('\n')
+            if lines[-1] != "\n":
+                # last line didn't end yet
+                self._buffer = lines[-1]
+                del lines[-1]
+            else:
+                self._buffer = ""
+            for line in lines:
+                self._parser.parse(line)
+                if (self._parser.currentTrack is not 0 and
+                        self._parser.tracks is not 0):
+                    progress = (float('%d' % self._parser.currentTrack) /
+                                float(self._parser.tracks))
+                    if progress < 1.0:
+                        self.setProgress(progress)
+
+        # 0 does not give us output before we complete, 1.0 gives us output
+        # too late
+        self.schedule(0.01, self._read, runner)
+
+    def _poll(self, runner):
+        if self._popen.poll() is None:
+            self.schedule(1.0, self._poll, runner)
+            return
+
+        self._done()
+
+    def _done(self):
+        self.setProgress(1.0)
+        self.toc = TocFile(self.tocfile)
+        self.toc.parse()
+        if self.toc_path is not None:
+            t_comp = os.path.abspath(self.toc_path).split(os.sep)
+            t_dirn = os.sep.join(t_comp[:-1])
+            # If the output path doesn't exist, make it recursively
+            if not os.path.isdir(t_dirn):
+                os.makedirs(t_dirn)
+            t_dst = truncate_filename(
+                os.path.join(t_dirn, t_comp[-1] + '.toc'))
+            shutil.copy(self.tocfile, os.path.join(t_dirn, t_dst))
+        os.unlink(self.tocfile)
+        self.stop()
+        return
 
 
 def DetectCdr(device):
@@ -88,20 +193,6 @@ def version():
     return m.group('version')
 
 
-def ReadTOCTask(device):
-    """
-    stopgap morituri-insanity compatibility layer
-    """
-    return read_toc(device, fast_toc=True)
-
-
-def ReadTableTask(device, toc_path=None):
-    """
-    stopgap morituri-insanity compatibility layer
-    """
-    return read_toc(device, toc_path=toc_path)
-
-
 def getCDRDAOVersion():
     """
     stopgap morituri-insanity compatibility layer