Re: [Libcdio-devel] unexpected offset in bincue.c for audio sectors

[R. Bernstein]

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 <
 > > address@hidden> 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);
 > >>
 > >>
 > >>
 > > 
 > 
 > 
 >