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.