On Tue, Nov 22, 2011 at 10:20 AM, Bastiaan Timmer
<address@hidden> wrote:
Thanks for the quick reply!--- On Tue, 11/22/11, Rocky Bernstein <address@hidden> wrote:
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.
Right, that would seem like a good idea, but it is a long-term solution
(especially considering the slow changes to Xiph's paranoia). Also, not returning NULL is only part of the problem (see below).
|
Ok. I also want to make this observation more explicit. Returning NULL is not inconsistent with the existing paranoia API. (And hope in the long run cdparanoia agrees and changes also.)
But before making this change, I'd like to hear from others and possibly cdparanoia.
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.
Well I'm not completely sure having the error parameter in the callback would help that much, because the callback can't make read_paranoia() return, right? As I said, the problem is not so much that paranoia_read() does not return NULL (which would help me decide to break out of the reading-loop), the big problem is is that paranoia_read() doesn't return at all! And as far as I can tell there nothing I can do (returning something, setting some variable) in functions accessible to me (the callback and the read function) that will force paranoia_read to return.
|
Ok. Again this is a paranoia API issue. Or it is an issue in the way the lower-level libcdio disc reading interacts with paranoia. I'd be interested in how paranoia suggests how one solve this. If it something paranoia guarantees will never happen if you use the paranoia I/O routine how would it guarantee this?
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.
Yes, I knew all that, my program accepts a verbosity option which I had cranked up to get this output (plus the added std::cout). The logging options in libcdio are nice and much appreciated.
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.
So, the only solution I have thought of until now, is to have read_audio(), when it fails, not _return_ the error code but _throw_ it. Then I can just catch it right outside of the read_paranoia() loop, and inform the user of the problem.
|
Seems reasonable to me. More generally, I'd really appreciate it if someone would completely redo the C++ interface in a way that is more sensible from the C++ standpoint (hiding my my ignorance of how to do things properly in C++.)
The Perl, Python and Ruby interfaces probably make more sense with respect to doing things in a way that is more consistent with those languages.
But indeed, I'm wondering if I should throw on all possible errors. Looking at the list in device.h, I guess the program might as well exit on DRIVER_OP_BAD_PARAMETER, BAD_POINTER, UNSUPPORTED and NOT_PERMITTED.
|
Perhaps. These are not transient errors, i.e. things which can change depending on the ability to read damaged media, or whether there is other activity/access on the CD-ROM like a CD loading/ejecting or being used by another process.
I suppose I could fix the problems indicated by UNINT and NO_DRIVER (after breaking the loop of course). |
Perhaps. I don't see how it is that fixing NO_DRIVER ,or UNINIT is any different than fixing a BAD_POINTER. But since you are coding this, the choice of what can be fixed or not is totally up to you and the program.
I am not sure about DRIVER_OP_ERROR and MMC_SENSE_DATA because I really don't know what they mean. Any thoughts?
|
DRIVER_OP_ERROR is a bit generic. It means you tried to do some sort of operation and we couldn't do that operation. It could be like the operation is unsupported or not permitted, but there are special codes for that. You can get an OP_ERROR for example if you couldn't open the CD-ROM drive (perhaps it wasn't found), but as I write this I would have thought I could have returned UNINIT in these cases as well.
Also, please let me know if you have any other solution than throwing, I have actually managed
to never write throwing functions up til now, so now I think I have to read up on C++ exception handling. Also, C programmers in my situation wouldn't have this luxury.
By the way, when I made the typo in the mmc_read command that resulted in this issue, the typo had the effect of passing a NULL pointer as 2nd parameter ('Place to store data.'). Shouldn't it have returned DRIVER_OP_BAD_POINTER instead of BAD_PARAMETER?
|
If you create a small test case that exhibits this, I'll look at it and possibly adjust things and then add that example as a test case.
Thanks.
_______________________________________________
Libcdio-help mailing list
address@hidden
https://lists.gnu.org/mailman/listinfo/libcdio-help