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

[Robert William Fuller]

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.

> 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);