libcdio-paranoia instead of cdparanoia

[45054]

libcdio-paranoia appears to be maintained: https://github.com/rocky/libcdio-paranoia

It would probably be easier to file bugs against this project compared to the original cdparanoia so in case we see a patch for the Julie Roberts bug, it could end up in the official repositories of the distributions.

[MerlijnWajer]

Is rocky a person who just maintains a fork? Or an actual maintainer? The URL for libcdio-paranoia seems to be: https://www.gnu.org/software/libcdio/

Presumably they use the same paranoia implementation, just differ in how they interface with the lower levels of the operating system? I know that my OS (gentoo) uses cdparanoia from xiph by default, but allows me to switch to the libcdio one.

[45054]

http://git.savannah.gnu.org/cgit/libcdio.git/log/ lists a R. Bernstein as does https://github.com/rocky
Another contributor on both repositories - https://github.com/enzo1982 - is the author of http://www.freac.org/

All the main distros appear to provide the libcdio packages and the executable would change from cdparanoia to cd-paranoia

Arch: https://www.archlinux.org/packages/extra/x86_64/libcdio-paranoia/
Debian: https://packages.debian.org/de/sid/libcdio-utils
Ubuntu: http://packages.ubuntu.com/de/xenial/libcdio-utils
Fedora: https://apps.fedoraproject.org/packages/libcdio-paranoia

[zacklb]

+1. These maintainers may help us solve #74 or at least add fixes upstream.

[RecursiveForest]

I certainly like the idea of using a maintained libparanoia implementation.

[JoeLametta]

If this one is interface compatible with xiph's cdparanoia and it's still maintained (unfortunately cdparanoia seems abandonware), then switching to it is probably a good idea.

[ArchangeGabriel]

cdparanoia is definitively abandonware judging by the ML archive. Only emails in more than two years from… @RecursiveForest. And even before that, in has not been very active for quite more years.

Switching to libcdio implementation looks a good idea. ;)

[ArchangeGabriel]

So, as said by @JoeLametta in #74, we definitively need to switch to libcdio-paranoia now that they have the fix for this bug thanks to @a10footsquirrel.

[JoeLametta]

Something as easy as what's shown below should be enough to make a painless switch:

--- whipper-master-f873bd0/morituri/program/cdparanoia.py
+++ whipper-edited-f873bd0/program/cdparanoia.py
@@ -282,10 +282,10 @@
 
         bufsize = 1024
         if self._overread:
-            argv = ["cdparanoia", "--stderr-progress",
+            argv = ["cd-paranoia", "--stderr-progress",
                 "--sample-offset=%d" % self._offset, "--force-overread", ]
         else:
-            argv = ["cdparanoia", "--stderr-progress",
+            argv = ["cd-paranoia", "--stderr-progress",
                 "--sample-offset=%d" % self._offset, ]
         if self._device:
             argv.extend(["--force-cdrom-device", self._device, ])
@@ -302,7 +302,7 @@
         except OSError, e:
             import errno
             if e.errno == errno.ENOENT:
-                raise common.MissingDependencyException('cdparanoia')
+                raise common.MissingDependencyException('libcdio-paranoia')
 
             raise
 
@@ -561,7 +561,7 @@
 
 def getCdParanoiaVersion():
     getter = common.VersionGetter('cdparanoia',
-        ["cdparanoia", "-V"],
+        ["cd-paranoia", "-V"],
         _VERSION_RE,
         "%(version)s %(release)s")
 
@@ -586,7 +586,7 @@
     def __init__(self, device=None):
         # cdparanoia -A *always* writes cdparanoia.log
         self.cwd = tempfile.mkdtemp(suffix='.morituri.cache')
-        self.command = ['cdparanoia', '-A']
+        self.command = ['cd-paranoia', '-A']
         if device:
             self.command += ['-d', device]
 

What still needs to be done:

• Check if the version reporting code needs to be updated.
• Investigate if the cdparanoia-overread patch (or something similar) is still useful for libcdio-paranoia
• Update whipper's README
• Update Travis CI's config

[peaveyman]

Is this slated for the 1.0 release?

[MerlijnWajer]

Is it not possible to make this an option? --use-paranoia libcdio-paranoia or --use-paranoia cdparanoia-paranoia ?

[JoeLametta]

Is this slated for the 1.0 release?

Currently, it is.

Is it not possible to make this an option? --use-paranoia libcdio-paranoia or --use-paranoia cdparanoia-paranoia ?

It can be done but, apart from covering a greater pool of users, would there be any advantage?

[MerlijnWajer]

Yes - allowing people to use the standard cdparanoia instead of requiring the libcdio one. This is a matter of: being able to use whipper on older/lts distributions, and the fact that the normal cdparanoia is also fine for most use cases - as accuraterip will usually also tell you if something went wrong. We could default to libcdio-cdparanoia, but allow users to set a different binary if they want to. Should not be too hard to add in, especially if the implementations really are that compatible.

[Freso]

I think we should just drop cdparanoia and only use libcdio-paranoia going forward. If you use a distro that does have cdparanoia and doesn't have libcdio-paranoia, it will likely still have gstreamer 0.10 as well and then you can probably stick to morituri or old whipper versions for your ripping needs.

[tobbez]

Arch, Debian, Fedora, openSUSE, Ubuntu all ship the binary as cd-paranoia.

Gentoo is the odd one out, and ships it as libcdio-paranoia, but manages a symlink called cdparanoia pointing to the currently chosen implementation.

[MerlijnWajer]

I would like to pick a default binary name, eg. 'cd-paranoia', and then allow people to specify an alternative implementation via flags/config. That should also take care of distributions naming the implementations differently. It would be nicest if the distributions followed the (clear) Gentoo naming, but that's not going to happen methinks.

[Freso]

Or if Gentoo followed the naming from upstream like the other distributions.

[rocky]

libcdio-paranioa is the project name. And various OS distributions use that name as the package name, e.g. debian and redhat. There is a library for those who want use from a programming language and not shell out. But, of course, the package also includes the command-line utility as well. And that's what most people use.

Obviously libcdio-paranoia can't install a binary called cdparanoia by default. It does however have a configure option, --with-cd-paranoia-name, which allows you to specify the name of the command-line utility. The default is cd-paranoia.

[tobbez]

The pragmatic approach would be something like

def executable_exists(exename):
    for p in os.environ['PATH']:
        ep = os.path.join(p, exename)
        if os.path.isfile(ep) and os.access(ep, os.X_OK):
            return True
    return False

def find_paranoia():
    if config.get('paranoia_exe', None) is not None:
        return config['paranoia_exe']
    for paranoia in ['libcdio-paranoia', 'cd-paranoia', 'cdparanoia']:
        if executable_exists(paranoia):
            return paranoia
    return None

and issue a warning if the version found has issues (i.e. cdparanoia, or old version of libcdio-paranoia).

Long term, a better plan would likely be to utilize the library instead (e.g. using ctypes).

[mbelow]

Using libcdio fixes issues with my CD drive similar to #146 . I can't rip the last track of a CD using Xiph cdparanoia on this drive, but the libcdio cd-paranoia works.

whipper shows repeated warnings for the last track like:

WARNING:whipper.program.cdparanoia:file size 62455852 did not match expected size 62535020

The file size expected/received depends on the CD, but the kind of message is always the same. After a number of tries it stops.

whipper lists the drive as: drive: /dev/cdrom, vendor: HL-DT-ST, model: DVD-RAM GH40L , release: RB02

cdparanoia -B returns a long list of error messages like:

scsi_read error: sector=313555 length=1 retry=8
Sense key: 5 ASC: 21 ASCQ: 0
Transport error: Illegal SCSI request (rejected by target)
System error: Invalid argument

cdparanoia -A returns "PARANOIA MAY NOT BE TRUSTWORTHY WITH THIS DRIVE!"

libcdio's cd-paranoia -B simply works. It shows a small "e" in the progress bar for the last track. According to the docs that is a corrected error.

[JoeLametta]

Old cdparanoia's overread patch has been merged into libcdio's cd-paranoia: see here (available since release-10.2+0.94+2).

Here's why it's useful:

Without this patch, cdparanoia always tries to read into the leadout when a positive sample offset is given.

The patch changes the default behaviour to simply provide null samples for leadout sectors instead of actually reading them. Only when --force-overread is given, it actually tries to read these sectors.

enzo1982's comment

[peaveyman]

Any idea when the change to libcdio-paranoia will happen?

[MerlijnWajer]

I will try to do this ASAP.

[MerlijnWajer]

(Feel free to poke me here or in IRC if I forget)

[peaveyman]

MerlijnWajer: POKE!!! :)

[MerlijnWajer]

Changed my opinion about supporting cdparanoia (instead of libcdio-cdparanoia). Let's not do it now, and instead only support the 'accurate' tool.

[MerlijnWajer]

That means that we should probably check for the exact version in the near future. I think #213 is ready to be merged.

[JoeLametta]

I've just merged #213 by @MerlijnWajer which hard switches whipper to cd-paranoia (from libcdio-paranoia).

[MerlijnWajer]

Would be good to have a few people jump on this merged PR and test master.

Furthermore, let's open a ticket to address:

1. Strict checking/requiring (or not) the cd-paranoia version
2. Allow specifying alternative cdparanoia implementations (without version checking, e.g. --force-paranoia /usr/bin/cdparanoia would remove all version checks and use this paranoia implementation.

[MerlijnWajer]

(And close this ticket)