Re: [Libcdio-devel] check_cue.sh failing with cdda_4_5 image on MinGW

[Rocky Bernstein]

On Wed, May 20, 2020 at 6:56 AM Pete Batard <address@hidden> wrote:

> Hi Thomas,
>
> Thanks a lot for looking into this.
>

And my thanks,Thomas, as well.


> I can understand why you are being confused, and I guess I have to
> apologize for making you waste your time here, because it turns out that
> the real issue is that cdda_4_5.bin was registered in git as a symbolic
> link to cdda.bin, which of course works great on platforms that do
> support symbolic links (Linux) but not so much on Windows...
>
> Of course, once you start testing properly and pay attention to the
> helpful cd-info warnings, things become quite obvious:
>
> ++ WARN: image D:/Projects/libcdio/test/data/cdda_4_5.bin size (10) not
> multiple of blocksize (2352)
>
> The above is because on windows (msysgit), the symlink is converted to a
> text file with the path to the file it points to.
>
> And once you use an actual cdda.bin duplicate for cdda_4_5.bin, then the
> test passes...
>
> Now, whereas there is *some* support for symlinks with recent versions
> of msysgit, this is not currently enabled by default therefore I don't
> think we want to count on that to try to work around the issue.
>
> Instead I suggest is that we just duplicate the file. Considering that,
> internally, this does not increase the size of the git repository (since
>   git is smart enough to store a single object for both files
> regardless), I don't see much of a drawback doing this. Especially this
> will not increase the amount of content needing to be downloaded when
> cloning the repo.


> Therefore, unless someone objects to removing the symlink and simply
> duplicating cdda.bin as cdda_4_5.bin, I will push a commit that does
> just that within the next 24 hours.
>

I'm okay with this. And your other changes as well.

The code is there to be used, and played with and fixed when it is lacking.

Thanks Pete and Thomas for keeping this alive.



> Regards,
>
> /Pete
>
> On 2020.05.20 08:29, Thomas Schmitt wrote:
> > Hi,
> >
> > i had a look at _cdio_get_track_msf in lib/driver/MSWindows, because i
> > assumed that this would be used with MinGW.
> >
> > The driver seems only partly prepared for start track number > 1.
> > But i cannot derive the particular mistake of cd-info from what i see in
> > the code.
> >
> > So which driver is in effect with that failed test ?
> >
> > -------------------------------------------------------------------------
> > What's wrong with lib/driver/MSWindows:
> >
> > The implementation of .get_track_msf does not subtract the track number
> > offset, but rather subtracts 1 before using its parameter i_tracks as
> > index to p_env->tocent[].
> >
> >    ...
> >      The "leadout" track is specified either by
> >      using i_tracks LEADOUT_TRACK or the total tracks+1.
> >    ...
> >    */
> >    static bool
> >    _cdio_get_track_msf(void *p_user_data, track_t i_tracks, msf_t *p_msf)
> >    {
> >      ...
> >      if (i_tracks == CDIO_CDROM_LEADOUT_TRACK) i_tracks =
> p_env->gen.i_tracks+1;
> >
> >      if (i_tracks > p_env->gen.i_tracks+1 || i_tracks == 0) {
> >        return false;
> >      } else {
> >        cdio_lsn_to_msf(p_env->tocent[i_tracks-1].start_lsn, p_msf);
> >        return true;
> >      }
> >
> > The two implementations of reading the Table-Of-Content subtract the
> > track number offset when filling p_env->tocent.
> >
> > lib/driver/MSWindows/aspi32.c : read_toc_aspi
> >
> >      p_env->gen.i_tracks  = tocheader[3] - tocheader[2] + 1;
> >      ...
> >        for( i = 0 ; i <= p_env->gen.i_tracks ; i++, j++ ) {
> >          ...
> >          p_env->tocent[ i ].start_lsn = ...
> >
> > lib/driver/MSWindows/win32_ioctl.c : read_toc_win32ioctl
> >
> >      p_env->gen.i_tracks  = cdrom_toc.LastTrack - cdrom_toc.FirstTrack +
> 1;
> >      ...
> >      for( i = 0 ; i <= p_env->gen.i_tracks ; i++, j++ ) {
> >        p_env->tocent[ i ].start_lsn =
> >
> > So to deliver the correct entries, _cdio_get_track_msf() has to be called
> > with i_tracks = tocent index + 1. I.e. first valid i_tracks is 1, not
> > the nominal first track number.
> >
> > But src/cd-info.c calls cdio_get_track_msf and thus win32.c
> > _cdio_get_track_msf with indice from a loop starting at the nominal
> > first track number:
> >
> >      for (i = i_first_track; i <= CDIO_CDROM_LEADOUT_TRACK; i++) {
> >        ...
> >        if (!cdio_get_track_msf(p_cdio, i, &msf)) {
> >          ...
> >        ...
> >        /* skip to leadout */
> >        if (i == i_first_track + i_tracks - 1) {
> >          i = CDIO_CDROM_LEADOUT_TRACK-1;
> >        }
> >      }
> >
> > So it seems not much of a riddle why the leadout LSN and MSF are wrong.
> > But why then is the cd-info line for track 5 correct ?
> > Why does win32.c:_cdio_get_track_msf not bail out by this test
> > when i_tracks is 4 or 5 ? (p_env->gen.i_tracks is supposed to be 2.)
> >
> >        if (i_tracks > p_env->gen.i_tracks+1 || i_tracks == 0) {
> >          return false;
> >
> > -------------------------------------------------------------------------
> >
> >
> > Have a nice day :)
> >
> > Thomas
> >
> >
>
>
>