[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
>
>
> ᐧ
- Re: [Libcdio-devel] CD_MSF_FORMAT vs LBA on NetBSD, (continued)
- Re: [Libcdio-devel] CD_MSF_FORMAT vs LBA on NetBSD, Edd Barrett, 2018/12/09
- Re: [Libcdio-devel] CD_MSF_FORMAT vs LBA on NetBSD, Thomas Schmitt, 2018/12/09
- Re: [Libcdio-devel] CD_MSF_FORMAT vs LBA on NetBSD, Edd Barrett, 2018/12/09
- Re: [Libcdio-devel] CD_MSF_FORMAT vs LBA on NetBSD, Thomas Schmitt, 2018/12/09
- Re: [Libcdio-devel] CD_MSF_FORMAT vs LBA on NetBSD, Edd Barrett, 2018/12/09
- Re: [Libcdio-devel] CD_MSF_FORMAT vs LBA on NetBSD, Thomas Schmitt, 2018/12/09
- Re: [Libcdio-devel] CD_MSF_FORMAT vs LBA on NetBSD, Thomas Schmitt, 2018/12/09
- Re: [Libcdio-devel] CD_MSF_FORMAT vs LBA on NetBSD, Edd Barrett, 2018/12/10
- Re: [Libcdio-devel] CD_MSF_FORMAT vs LBA on NetBSD, Thomas Schmitt, 2018/12/10
- Re: [Libcdio-devel] CD_MSF_FORMAT vs LBA on NetBSD, Thomas Schmitt, 2018/12/11
- Re: [Libcdio-devel] CD_MSF_FORMAT vs LBA on NetBSD,
Rocky Bernstein <=
- Re: [Libcdio-devel] CD_MSF_FORMAT vs LBA on NetBSD, Thomas Schmitt, 2018/12/11
- Re: [Libcdio-devel] CD_MSF_FORMAT vs LBA on NetBSD, Edd Barrett, 2018/12/11
- Re: [Libcdio-devel] CD_MSF_FORMAT vs LBA on NetBSD, Thomas Schmitt, 2018/12/12
- Re: [Libcdio-devel] CD_MSF_FORMAT vs LBA on NetBSD, Thomas Schmitt, 2018/12/12
- Re: [Libcdio-devel] CD_MSF_FORMAT vs LBA on NetBSD, Edd Barrett, 2018/12/12
- Re: [Libcdio-devel] CD_MSF_FORMAT vs LBA on NetBSD, Thomas Schmitt, 2018/12/12
- Re: [Libcdio-devel] CD_MSF_FORMAT vs LBA on NetBSD, Rocky Bernstein, 2018/12/12
- Re: [Libcdio-devel] CD_MSF_FORMAT vs LBA on NetBSD, Thomas Schmitt, 2018/12/12
- Re: [Libcdio-devel] CD_MSF_FORMAT vs LBA on NetBSD, Rocky Bernstein, 2018/12/12
- Re: [Libcdio-devel] CD_MSF_FORMAT vs LBA on NetBSD, Edd Barrett, 2018/12/15