Robert William Fuller writes: > Eric Shattow wrote: > > Would you suggest removing it or making it configurable at run-time? > > I would remove it. The cd-paranoia included with libcdio already has a > read offset correction option (-O). Similarly, my own ripper, cued, > does read offset correction as well. It is easy enough for the > application programmer to do read offset correction. > > If we make it configurable, then we need to add it to NRG and all the > other drivers as well. Adding it to the operating system drivers will > be a real pain because then we have to apply an offset across sector > boundaries, which means reading additional sectors. This is best done > at a higher level, like the application level, which is how libcdio's > cd-paranoia and cued manage it. > > The most painless fix is to not change the model, but fix the bug. Even > if in the long term it is decided to change the model, the bug needs to > be fixed in the short term. By the way, it looks like the cdrdao driver > also has a 272 byte offset, probably for the same reason the bincue > driver has it.
Without a doubt the 272-byte offset is a hack that I put in for lack of understanding what was going on. I think I did notice that the drive on a Solaris box gave a different offset. Since the next release will have API changes, it would be a good time, release-wise, to fix this as well. cd-read will need an "audio offset-correction" option and possibly the reference doc should be updated and some regression tests may need correcting. > > > On Mon, Mar 17, 2008 at 12:27 AM, Robert William Fuller < > > [EMAIL PROTECTED]> wrote: > > > >> Does anybody know what this is about? I'm pretty sure I know what it's > >> about, but I want to confirm. I spent some time trying to figure out > >> why every time I re-created an image file from a rip and re-ripped from > >> the new image file, each new rip was progressively off by 68 more audio > >> samples, i.e. 272 bytes (2 channels * 2 bytes per channel * 68 samples > >> == 272 bytes.) So I carefully debugged my ripper and couldn't find > >> anything wrong with it, so I turned to libcdio and found the source of > >> my troubles. The libcdio code in question is at the end of this message. > >> > >> My suspicion is that the author of the libcdio code was comparing a CD > >> rip with an image rip and was trying to compensate for a CD drive with a > >> 68 audio sample read offset. The author may not have known at the time > >> that almost all CD drives have some read offset, with the exception of a > >> few Plextors, which is why EAC, cued, and cdparanoia support read offset > >> correction. If that is the case, the 272 byte offset in bincue.c for > >> audio sectors is emulating a CD drive with a 68 audio sample read > >> offset, which is probably the author's original CD drive! I'm almost > >> certain we need to remove the 272 byte. Note that this only affects > >> audio sectors. Thoughts? > >> > >> static driver_return_code_t > >> _read_audio_sectors_bincue (void *p_user_data, void *data, lsn_t lsn, > >> unsigned int nblocks) > >> { > >> _img_private_t *p_env = p_user_data; > >> int ret; > >> > >> /* Why the adjustment of 272, I don't know. It seems to work though */ > >> if (lsn != 0) { > >> ret = cdio_stream_seek (p_env->gen.data_source, > >> (lsn * CDIO_CD_FRAMESIZE_RAW) - 272, SEEK_SET); > >> > >> > >> > > > > >