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

[Greg Troxel]

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