On Thu, Oct 25, 2018 at 8:30 PM Greg Troxel <g...@lexort.com> wrote: > Edd Barrett <e...@theunixzoo.co.uk> 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. > >
