Re: [Libcdio-devel] CD_MSF_FORMAT vs LBA on NetBSD

[Thomas Schmitt]

Hi,

lagging behind my promises i today combined vetx01/libcdio with
vext01/libcdio-paranoia.

Bugs found and fixed so far:

- libcdio/lib/driver/gnu_linux.c puts the highest track number into the
  track counter _img_private_t.i_tracks (netbsd.c does it right).
  This caused ill track access, but also enough attempts with cd-paranoia -Q.

- The loop in display_toc() of src/cd-paranoia.c is wrong. It begins to
  count from 1 up to track counter (which was wrongly the highest track
  number on gnu_linux.c).
  It should count from first track to last track plus 1 for lead-out.
  This caused ill track access.

- The loop in lib/cdda_interface/common_interface.c function data_bigendianp()
  should iterate over nominal track numbers but rather does over
  disc_toc[] index plus 1.
  Symptom is the message:
        Cannot determine CDROM drive endianness.

The following run now works for me without raising further suspicions:

  cd-paranoia -d /dev/sr4 -Q

I could need exact instructions about what further tests to run.


Edd: How to proceed git-wise now ?

Shall i branch from your branches and collect fixes ? (New clone from
better address ?)
Shall i produce patches which you put into your branches ?
(Patch content is shown in the text below. Give me instructions about
 their format if you want them.)

------------------------------------------------------------------------
Dirty details in chronological sequence:

  install_dir=/...absolute.path.../install_dir

  git clone https://github.com/vext01/libcdio
  cd libcdio
  git checkout openbsd_fixes_to_master
  sh ./autogen.sh
  ./configure --prefix=$install_dir
  make
  (cd src ; touch cd-drive.1 cd-info.1 cd-read.1 iso-info.1 iso-read.1)
  make
  make install

  cd ../libcdio-paranoia
  make clean
  ./configure --prefix=$install_dir
  make
  make install

without much errors (except the traditional problem with non-existing man
pages which i solved by a run of "touch").

Surprisingly

  ldd "$install_dir"/bin/cd-paranoia

shows two libcdio.so binaries:

  libcdio.so.16 => /usr/lib/libcdio.so.16 (0x00007f7b2970a000)
  libcdio.so.18 => /...install_dir.../lib/libcdio.so.18 (0x00007f7b28821000)

(and also
  libcdio_cdda.so.2 => /...install_dir.../lib/libcdio_cdda.so.2 
(0x00007f7b29502000)
  libcdio_paranoia.so.2 => /...install_dir.../lib/libcdio_paranoia.so.2 
(0x00007f7b292fb000)
)

A test message put into gnu_linux.c of the freshly cloned libcdio does not
show up in a run of cd-paranoia. So it's still using the old libcdio.so.

This libcdio.so.16 is pointed to by the link /usr/lib/libcdio.so.

Is this double libcdio due to my ignorance towards PKG_CONFIG_PATH ?
(To what value should i set it ? I tried "$install_dir", but nothing
 changes after "make clean ; make ; make install" of libcdio-paranoia.)

I tried with hidden /usr/lib/libcdio.so. It tried to use libcdio.a and
failed with relocation incompatibility (because trying to make an .so ?).
When removing /usr/bin/libcdio.a, it failed because not finding libcdio.

I finally had to point /usr/lib/libcdio.so to $install_dir/lib/libcdio.so
before i could see my test message from gnu_linux.c.

--------------------------------------------------------------------------

Whatever, the wrong output still persists

  $ "$install_dir"/bin/cd-paranoia -d /dev/sr4 -Q
  ...
  track        length               begin        copy pre ch
  ===========================================================
    4.     2682 [00:35.57]        0 [00:00.00]    no   no  2
    5.        0 [00:00.00]     2682 [00:35.57]    no   no  2
    6.        0 [00:00.00]     2682 [00:35.57]    no   no  2
  TOTAL    2682 [00:35.57]    (audio only)

But at least it should now run the code that is to be tested.

In src/cd-paranoia.c, d->tracks is 6, i.e. the highest track number.
Its variable name rather suggests that it shall be the track count.
(The loop takes profit.)
In cdio/paranoia/cdda.h the definition of cdrom_drive_t prescibes to
subtract cdio_get_first_track_num() from track number to get the index
for d->disc_toc[]. So d->tracks should be 3 or 4 (modulo lead-out track).

lib/cdda_interface/cddap_interface.c has:
    d->tracks = cdio_get_num_tracks(d->p_cdio) ;
libcdio/.../gnu_linux.c points to
    get_num_tracks_generic()
whereas netbsd.c points to
    get_num_tracks_netbsd()

get_num_tracks_generic() returns generic_img_private_t.i_tracks .
get_num_tracks_netbsd() returns TOTAL_TRACKs which in netbsd.c is the same
as .i_tracks.

But gnu_linux.c happily puts the highest track number into .i_tracks.
The number of tracks is computed from both. E.g. in read_toc_linux():
   u_tracks = p_env->gen.i_tracks - p_env->gen.i_first_track + 1;


The definition of generic_img_private_t in libcdio/lib/driver/generic.h
says:

    track_t i_first_track;  /**< The starting track number. */
    track_t i_tracks;       /**< The number of tracks. */

So gnu_linux.c is wrong.

--------------------------------------------------------------------------

Then of course, the loop in display_toc() of src/cd-paranoia.c is wrong:

  for( i=1; i<=d->tracks; i++)
    if ( cdda_track_audiop(d,i) > 0 ) {

      lsn_t sec=cdda_track_firstsector(d,i);

cdda_track_audiop() in libcdio-paranoia/lib/cdda_interface/toc.c uses "i"
for cdio_get_track_format() which in gnu_linux.c and in netbsd.c expects
nominal track numbers.
cdda_track_firstsector() subtracts from "i" the first nominal track number
and uses the result as index to cdrom_drive_t.disc_toc[].

So the loop should rather be

  for( i = cdio_get_first_track_num(d->p_cdio);
       i <= cdio_get_last_track_num(d->p_cdio); i++)

But with gnu_linux.c as it is now, i get

  d->tracks= 6, first= 4, last= 9

So cdio_get_last_track_num() is broken on gnu_linux.c.i
Probably by the .i_tracks bug.

And access to the first three tracks is still not correct:

    4.     2682 [00:35.57]        0 [00:00.00]    no   no  2
    5.        0 [00:00.00]     2682 [00:35.57]    no   no  2
    6.        0 [00:00.00]     2682 [00:35.57]    no   no  2
    7.        0 [00:00.00]     2682 [00:35.57]    no   no  4
    8.        0 [00:00.00]     2682 [00:35.57]    no   no  4
    9.     5364 [01:11.39]     2682 [00:35.57]    no   no  4
  TOTAL    8046 [01:47.21]    (audio only)

I have to fix gnu_linux.c first:
============================================================================
--- lib/driver/gnu_linux.c.orig 2018-12-11 13:59:31.849293595 +0100
+++ lib/driver/gnu_linux.c      2018-12-11 15:58:51.897320630 +0100
@@ -1126,7 +1126,7 @@ static bool
 read_toc_linux (void *p_user_data)
 {
   _img_private_t *p_env = p_user_data;
-  int i;
+  int i, i_last_track;
   unsigned int u_tracks;
 
   /* read TOC header */
@@ -1137,9 +1137,10 @@ read_toc_linux (void *p_user_data)
   }
 
   p_env->gen.i_first_track = p_env->tochdr.cdth_trk0;
-  p_env->gen.i_tracks      = p_env->tochdr.cdth_trk1;
+  i_last_track             = p_env->tochdr.cdth_trk1;
+  p_env->gen.i_tracks      = i_last_track - p_env->gen.i_first_track + 1;
 
-  u_tracks = p_env->gen.i_tracks - p_env->gen.i_first_track + 1;
+  u_tracks = p_env->gen.i_tracks;
 
   if (u_tracks > CDIO_CD_MAX_TRACKS) {
      cdio_log(CDIO_LOG_WARN, "Number of tracks exceeds maximum (%d vs. %d)\n",
@@ -1149,7 +1150,7 @@ read_toc_linux (void *p_user_data)
 
 
   /* read individual tracks */
-  for (i= p_env->gen.i_first_track; i<=p_env->gen.i_tracks; i++) {
+  for (i= p_env->gen.i_first_track; i <= i_last_track; i++) {
     struct cdrom_tocentry *p_toc =
       &(p_env->tocent[i-p_env->gen.i_first_track]);

============================================================================

Next i fix cd-paranoia.c:

============================================================================

--- src/cd-paranoia.c.orig      2018-12-09 21:59:02.956749766 +0100
+++ src/cd-paranoia.c   2018-12-11 19:09:31.601363825 +0100
@@ -251,7 +251,8 @@ display_toc(cdrom_drive_t *d)
          "track        length               begin        copy pre ch\n"
          "===========================================================");
 
-  for( i=1; i<=d->tracks; i++)
+  for( i=cdio_get_first_track_num(d->p_cdio);
+       i<=cdio_get_last_track_num(d->p_cdio); i++)
     if ( cdda_track_audiop(d,i) > 0 ) {
 
       lsn_t sec=cdda_track_firstsector(d,i);

============================================================================

Now i get a much better result with cd-paranoia -Q:

    4.     2682 [00:35.57]        0 [00:00.00]    no   no  2
    5.     2682 [00:35.57]     2682 [00:35.57]    no   no  2
    6.     2682 [00:35.57]     5364 [01:11.39]    no   no  2
  TOTAL    8046 [01:47.21]    (audio only)

(d->tracks= 3, first= 4, last= 6)

But now the CD with track 1 to 3 gets stuck for quite a while and then
reports

  006: Could not read any data from drive

  Cdparanoia could not find a way to read audio from this drive.

This is reproducible until i eject and reload the tray.
Then i get the correct track list:

  ts_debug: d->tracks= 3, first= 1, last= 3
    1.     2682 [00:35.57]        0 [00:00.00]    no   no  2
    2.     2682 [00:35.57]     2682 [00:35.57]    no   no  2
    3.     2682 [00:35.57]     5364 [01:11.39]    no   no  2
  TOTAL    8046 [01:47.21]    (audio only)

More runs succeed, but last very long. What does it do for 45 seconds ?
The drive is busy blinking.
With option -v the last message before the long waiting is

  Attempting to determine drive endianness from data....

when it continues, it says

        Data appears to be coming back Little Endian.
        certainty: 100%

libcdio-paranoia/lib/cdda_interface/common_interface.c has

  /* find a block with nonzero data *

This explains a lot. My data are dummies, not real audio.

At least it nothing to do with track numbers.
But why does it sometimes last 15 seconds and sometimes 45 ?

And why is the CD with track 4 to 6 reliably fast ?
Aha:
        Cannot determine CDROM drive endianness.

Reason is that data_bigendianp() in common_interface.c loops over
track indice from 0 to (d->tracks - 1) and converts them into track
numbers by adding 1.

============================================================================

It seems odd to count from first track minus 1 to last track minus 1
only to add 1 with any usage of "i". So i also changed the "i+1" gestures.

--- lib/cdda_interface/common_interface.c.orig  2018-12-09 21:57:17.716749368 
+0100
+++ lib/cdda_interface/common_interface.c       2018-12-11 18:56:01.209360765 
+0100
@@ -70,12 +70,13 @@ data_bigendianp(cdrom_drive_t *d)
   
   cdmessage(d,"\nAttempting to determine drive endianness from data...");
   d->enable_cdda(d,1);
-  for(i=0,checked=0;i<d->tracks;i++){
+  for(i=cdio_get_first_track_num(d->p_cdio), checked=0;
+      i<=cdio_get_last_track_num(d->p_cdio); i++){
     float lsb_energy=0;
     float msb_energy=0;
-    if(cdda_track_audiop(d,i+1)==1){
-      long firstsector=cdda_track_firstsector(d,i+1);
-      long lastsector=cdda_track_lastsector(d,i+1);
+    if(cdda_track_audiop(d,i)==1){
+      long firstsector=cdda_track_firstsector(d,i);
+      long lastsector=cdda_track_lastsector(d,i);
       int zeroflag=-1;
       long beginsec=0;

============================================================================

Now i get from the CD with track 4 to 6:

  Attempting to determine drive endianness from data......
        Data appears to be coming back Little Endian.
        certainty: 100%

But this decision is made in less than a second, each time i try.
Shrug.

I close the cd-paranoia shop for today. Instructions about how to submit
my changes would be appreciated.


Have a nice day :)

Thomas