Re: [Libcdio-devel] [Libcdio-help] Custom read_audio function

[Rocky Bernstein]

On Mon, Nov 21, 2011 at 9:09 AM, Bastiaan Timmer <address@hidden>wrote:

> Hi!
>
> After reading an old message on this mailing list (
> http://lists.gnu.org/archive/html/libcdio-devel/2008-02/msg00001.html)
> I wrote a custom read_audio function for ripping audio cd's. The read
> function basically looks like this:
>
> long readRaw2448(cdrom_drive_t *drive, void *pbuf, lsn_t firstsector, long
> sectors)
> {
>  // ... do some allocations ...
>
>  driver_return_code_t ret;
>  if ((ret = mmc_read_cd(drive->p_cdio, s_buffer, firstsector,
> CDIO_MMC_READ_TYPE_ANY, false, false,
>                         0, true, false, false, 1, s_buffersize, sectors))
> != DRIVER_OP_SUCCESS)
>  {
>    std::cout << "RETURN CODE: " << ret << std::endl;
>    return ret; // will be negative if we get here
>  }
>
>  // ... handle the data (handle subframe, copy audio into pbuf) ...
>
>  return sectors;
>
> }
>
> Then, I simply set cdrom_drive_t.read_audio to this function, and read a
> cd with a loop nearly identical to the one
> used in the paranoia.c example:
>
> for (i_cursor = i_first_lsn; i_cursor <= i_last_lsn; i_cursor ++)
> {
>  int16_t *p_readbuf = paranoia_read(p, callback);
>  if (!p_readbuf)
>  {
>    std::cout << "Failed to read frame" << std::endl;
>    return false;
>  else
>  {
>    std::cout << "Got frame" << std::endl;
>    // ... handle returned frame ...
>  }
> }
>
> Now this all worked splendidly, but yesterday I wrote another custom read
> function to be used in some other
> circumstances. This time, I made a slight mistake in the mmc command,
> causing mmc_read_cd() to fail and return
> DRIVER_OP_BAD_PARAMETER. Now I had never seen the read function fail
> before, but simply assumed that if the
> read_audio function would not return a number of sectors, but a negative
> value (an error), paranoia_read() would
> return NULL so that the cd reading loop would break (the 'if (!p_readbuf)'
> part). Instead, I just get the
> following messages repeated:
>
> [MESSAGE] (CdioInternal) : ioctl CDROM_SEND_PACKET failed: Invalid argument
> RETURN CODE: -5
> [WARNING] (CdioCddaInterface::callback()) : paranoia_read() reports `read
> error' at sector 0
>
> The first of these is a message from a log_handler (set with
> cdio_log_set_handler()). The second is the cout seen
> in the readRaw2448() function shown above. The last is a message from my
> callback (given as parameter to
> paranoia_read()). The messages get repeated in series of 4, where the
> sector number changes from 0 to 25 to 50 to 75
> and then back to 0. Calling paranoia_read_limited() instead of
> paranoia_read() in the loop does not seem to help,
> paranoia_read() will just not return. At what point could/should I break
> from the loop if read_audio returns an error
> code? Am I returning the correct values from the read function
> (driver_return_code_t on error, sectors on success)?
> Any other remarks?
>
> thanks,
> Bas Timmer
>


This is a tough one. I'll do my best to describe things as I see it. But
probably a discussion should go on as to how to approach.

paranoia_read() is from Monty/Xiph's paranoia;  thus it follows that API,
which as you have noted is not to return NULL on error.  I don't see why it
couldn't  do so, but that would be changing the paranoia API. So it would
be best to get it changed both in paranoia as well as libcdio's use of it.

The way paranoia expects a program to handle errors is via that callback.
You note that you did get an error through the callback. The part that is
tricky here is that with all this indirection, I'm not sure how the
callback can actually get the error parameter. But we could make sure there
is some way.

As for the multiple level of error messages, well that is all under your
control. The low-level libcdio error messages can be turned off by setting
the log level.  And then there is the std::cout you added. And finally
there is the error message in the callback. Choose however many or which
you want to see.

As for the question of at what point you should bread from the read loop on
an error depends on you. Probably if you get a BAD_PARAMETER_OP, that's
probably not going to change so you could bail immediately. Other errors
may require more discrimination. The whole idea of cd-paranoia though is to
tolerate errors on the hope that some of them are transient. But with that
flexibility comes the responsibility of the program to make the
determination.