Ensure that multi-session CDs are still read correctly by whipper

[MerlijnWajer]

@RecursiveForest reworked the cdrdao parsing logic, and I am kind of afraid that that broken multisession CDs. We need to double-check that this is not the case, or fix it.

[BonkaBonka]

Not sure if this is what you're talking about but when attempting to rip the Quake II or Fantasy General discs whipper is crashing with this assertion error:

Using configured read offset 6
Checking device /dev/sr0
CDDB disc id: bb0feb0b
Traceback (most recent call last):
  File "/usr/sbin/whipper", line 11, in <module>
    load_entry_point('whipper==0.5.1', 'console_scripts', 'whipper')()
  File "/usr/lib/python2.7/site-packages/morituri/command/main.py", line 31, in main
    ret = cmd.do()
  File "/usr/lib/python2.7/site-packages/morituri/command/basecommand.py", line 123, in do
    return self.cmd.do()
  File "/usr/lib/python2.7/site-packages/morituri/command/basecommand.py", line 123, in do
    return self.cmd.do()
  File "/usr/lib/python2.7/site-packages/morituri/command/cd.py", line 122, in do
    self.mbdiscid = self.ittoc.getMusicBrainzDiscId()
  File "/usr/lib/python2.7/site-packages/morituri/image/table.py", line 348, in getMusicBrainzDiscId
    values = self._getMusicBrainzValues()
  File "/usr/lib/python2.7/site-packages/morituri/image/table.py", line 464, in _getMusicBrainzValues
    assert not self.tracks[-1].audio
AssertionError

[MerlijnWajer]

This is slightly different - and instead related to data tracks being at the start of a CD, I think. Can you tell us which CD this is, and give the cdrdao TOC?

[MerlijnWajer]

Perhaps in a different issue.

[RecursiveForest]

If you can give me a bin/cue of a multisession CD, or direct me to where I can find one, I can test what changed. But can you show me what broke exactly? There's not much to the cdrdao logic, it just calls cdrdao read-toc, then passes the output to whipper.image.toc.TocFile. Was it broken before a1eb3377 and d7f85574 ?

[RecursiveForest]

A brief review of d7f85574 leads me to believe I probably broke it with that commit. If I can get my hands on a multisession CD I'd be very happy to fix this.

[MerlijnWajer]

@RecursiveForest - I missed your comments before, sorry. I can give you a bin/cue tomorrow. I'm sure it's broken as the whole --session code got removed. We need to use the disk-info and read each session separately and then merge them. I can find my morituri-fork code for it. I'm in the process of working on some other data track and toc related things, though, so it may take a little while.

[RecursiveForest]

Yeah I definitely am guilty of an overzealous delete here; if you at least give me the toc I can add a cleaner version of it back in, and this time with tests.

tests tests tests. I'm no longer going to push changes without tests.

[BonkaBonka]

@RecursiveForest were the TOCs I provided helpful to you or do you need something else to move this forward?

[MerlijnWajer]

No, they were not yet provided. I will self assign until I upload them, which I hope to do this week. Extremely busy schedule.

[MerlijnWajer]

Adding to 1.0 milestone. This is important.

[RecursiveForest]

I still need a copy of those toc files.

[BonkaBonka]

@RecursiveForest, Issue #183 has the link to a gist with TOCs of discs that fail for me. Also, @ubitux added a TOC that tails for him in the comments on #183.

[MerlijnWajer]

I've commented on PR #286. Let me see if I can take care of this soon.

[JoeLametta]

This is not code you can copy/paste or use-as-is (it won't work), but I used something like this a while ago. Note that this code doesn't rip all the data tracks if there are multiple separate sessions (I haven't figured out an easy way, but it's also out of scope of this issue, and perhaps in general for audio rippers):

    def rip_tracks(self):
        self.rs.profile = self.ripper.setup_things()

        # TODO: rip impl
        self.rs.htoapath = self.ripper.rip_htoa(self.rs.profile,
                                                self.rs.disambiguate)

        self.data_tracks_ripped = False

        for i, track in enumerate(self.ripper.itable.tracks):
            logger.debug(u'Ripping track: %d %s', i, track)
            if not track.audio:
                if self.data_tracks_ripped:
                    logging.debug(u'Not ripping data tracks again, already'
                                  ' ripped once')

                else:
                    self.rip_data_track()
                    self.data_tracks_ripped = True

                track.indexes[1].relative = 0
            else:
                audio_tracks_left = filter(lambda x: x.audio,
                                           self.ripper.itable.tracks[i:])

                # is_last is only used to control the speed of the rip when we're doing fast rips - to use -Y at the last track instead of -Z
                is_last = len(audio_tracks_left) == 1

                self.rip_audio_track(i, last=is_last)

                ## Previous a data track?
                if i > 0 and not self.ripper.itable.tracks[i-1].audio:
                    if track.getPregap() > 0:
                        index = track.getFirstIndex()
                        index.relative = 0

            self.progress['total_progress'] = \
                    float(i + 1) / len(self.ripper.itable.tracks)
            self.report()

            if self.skip_fast_mode_oracle():
                self._allow_fast_rip = False

Originally posted by @MerlijnWajer in https://github.com/whipper-team/whipper/pull/286#issuecomment-441020978