libcdio-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Rocky Bernstein
Subject: Re: [Libcdio-devel] CD_MSF_FORMAT vs LBA on NetBSD
Date: Tue, 11 Dec 2018 15:27:18 -0500

On Tue, Dec 11, 2018 at 1:48 PM Thomas Schmitt <address@hidden> wrote:

> 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.
>

For libcdoi changes, when you and Edd are satisfied that things are the way
they should be merge that into a branch in savannah libcdio and then merge
that branch into master.

For cdio_paranoia changes, use the usual github process of submitting pull
request.

Also, if Edd hasn't filled out a FSF copyright release form, he should do
that.


>
> Have a nice day :)
>
> Thomas
>
>
>


reply via email to

[Prev in Thread] Current Thread [Next in Thread]