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

[Rocky Bernstein]

On Thu, Oct 25, 2018 at 8:30 PM Greg Troxel <address@hidden> wrote:

> Edd Barrett <address@hidden> writes:
>
> > Hi Greg,
> >
> > Thanks for getting back to us. I'm an OpenBSD developer trying to get
> > the NetBSD driver to work for OpenBSD.
> >
> > On Fri, Oct 12, 2018 at 10:02:16PM -0400, Greg Troxel wrote:
> >>   405: Option not supported by drive
> >
> > I've seen this. If I remember correctly (but please double check), I
> > traced it to setting the drive speed. I think it's harmless.
> >
> >> So it seems that moving NetBSD to CD_LBA_FORMAT at best needs more work,
> >> and that the LBA change should be ifdef'd for OpenBSD for now.
> >
> > Great. That's exactly what I've done in my branch. Thanks for trying
> > this out.
>
> so specifically your ifdef openbsd around this seems ok on netbsd (as
> built and tested from your branch).
>
> >> (If lib/driver/netbsd.c is actually used on OpenBSD, it would be nice to
> >> have a comment at the front of the file explaining this.)
> >
> > The build system will choose the NetBSD backend for OpenBSD, but it
> > currently doesn't work.
>
> ok, but when this is merged a comment is in order, so that people
> starting to read the code will get that.
>
> > If you get time, would you be able to try my branch on NetBSD? The only
> > outstanding issue for OpenBSD on this branch is cdda-player track
> > skipping is broken.
> >
> > I'd be interested to see if I've introduced regressions.
> >
> > https://github.com/vext01/libcdio/tree/openbsd_fixes_to_master
>
> I built the branch and am currently using cd-paranoia with it to rip a
> disc, and I'm getting files with the same hash as before.
>
> cdda-player doesn't work, but it doesn't on master either, and it seems
> roughly the same.  It gets errors, lists the tracks, and indicates that
> it's playing but keyboard input does nothing, and ^C exits.  (I
> understand that I almost certainly have no audio path, but I expect the
> curses ui to sort of work.)
>
> Reading the diff, there are some changes whose comments don't explain
> the change and why it's necessary (vs sort of restating what I absorb
> from the diff).  Specifically these
>
>   8207ace8914c418dea9d626d7839bec0f7202fab
>   946ebc924ddeb7b26badfdcc7f29f0f5373ff027
>   a7fc90e1ce7a70a9efde4d1ca4711b20c5b2c8e5
>
>
> Most of the changes look ok.  And then there's the dropping of using
> sysctl in favor of probing by name - and it's not clear why that should
> be changed on NetBSD even if the sysctl approach doesn't/can't work on
> OpenBSD.  (It's also not clear why it shouldn't change, but for a big
> change like that there should be rationale in the commit message.)
>
> Some of the changes look unrelated to OpenBSD (unbreak cdda-player?,
> Ensure that stdin..., missing set of b_cd, add missing -d)
>
> Also, I don't get this:
>
>   ef44a97af6430107f09e27a31576c4adfa36ac09
>
> It does not make sense to me to be compiling the netbsd.c file on
> operating systems where that isn't the plan,



Elsewhere there are routines which itereative over a fixed enum of driver
types. So that has to be negotiated when the driver isn't appropriate or
for some other reason available.


or if it is compiled
> because ifdefing the whole file is easier than not including it via
> automake, then a giant ifdef around the file.
>

If you think you can do better, then by all means do so. At this stage in
this project change is more likely to happen when folks get involved to
make it happen. I don't think it is all that hard to all imagine how things
might be done better, or differently.


> Hope this helps and if you need me to test more or review code changes
> please feel free to ask.
>
>