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

Thanks for the information. Since cdda_player doesn't work on NetBSD, and
no one wants to fix it, the
simplest thing then is just to disable building it on NetBSD. That's a
trivial change on configure.ac, which has now been done in commit 123db461 .


> 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,  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.
>
> Hope this helps and if you need me to test more or review code changes
> please feel free to ask.
>
>