Address the comments

Extend chn_polltrigger() to cover kqueue+mmap case.

Add code comments explaining why kqueue is special.

Before I do that, let's get the code in perfect shape. I mean, should I change chn_polltrigger and chn_pollreset to handle mmap or should I leave it as it is now?

It's not about mmap, it's about kqueue. Look at the following code:

I think the proper change would be to patch chn_polltrigger to accept pointer to knote and if it's NULL, do what it does now, if it's not, do what this patch does, but that would require changing chn_pollreset and pulling in struct knote into channel.h which I'm not sure we want or not, so please advise.

I was mislead by looking at select/poll and totally forgot that knotes persist in the kernel once registered. @kib thank you for pointing it out. As for title, I forgot to include "kqueue" in it to make it more obvious.

Forgot one of the comments.

Address the comments.

Address the comments.

In previous patch I acidently included kernel and map page patch, so this is a clean one.

My English is terrible. Although man page edit describes what was done, the language is just bad. I even tried to ask AI to improve it and I still don't like it, so I obviously need help with wording. And yes, dsp_oss_syncstart patch fixes LOR we were seeing for so long. It's now part of D57399.

Given it works with umidi, should we close this? I think not as it works for me.

Unification first, as this code is mostly "copy" of the umidi code. I think after unification one place will have the kqueue support and that will probably be copied over from umidi. That being said, do you want me to close this one?

So, is it smart to work on this before umidi/midi unification? I can work on that, I just think it would be better if unification happened first.

I understood that you need to unify umidi and midi, that's why I left this review as is. Please tell me if I got it wrong.

I forgot to change /dev/dsp5 to /dev/dsp. Sorry for the noise.

I followed SOSSO code more closely this time and got rid of the hardcoded 400ms sleep.

Sorry, I completely forgot about this. Give me a week and I'll start working on it again.

What about alternative approach as in "ask the /dev/dsp* what is its control device"? For hardware devices the answer is NULL, for virtual_oss, it's /dev/dsp.ctl (or whatever is configured). Or maybe get devices through nvlist and /dev/sndstat and see which devices are virtual. In any case, my idea is "ask the kernel". I don't have preferences on the exact implementation.

I rebased this patch, but the problem is that I don't have the hardware to test it. I'll try to find cheap PCIe card to test this patch.

In D54360#1244383, @christos wrote:

The USB stack seems to have a default kqfilter method,so I suppose this is why it works. Does it work properly though?

In D54360#1244366, @christos wrote:

I did not try it yet, but there is no way that the code in D54359 is related to this working, because it does not run with USB MIDI. Does it work without D54359?

Oh, it works on 15.0-RELEASE, so no wonder I wrote this patch so easily: it doesn't do anything 😄

In D54360#1244361, @christos wrote:

Does this actually work? snd_uaudio(4) does not seem to implement a kqfilter method. I haven't checked whether the USB stack has a default handler. Also see my comment in D54359.

In D54359#1244357, @christos wrote:

Also I suppose you know this code does not work for USB MIDI, at the moment. I will eventually make USB MIDI attach to it, but currently this only works for physical MIDI.

In D54359#1244356, @christos wrote:

There are a lot of MIDI patches in review that simplify the code, so this won't apply. I can rewrite it on top of them if you want.

As midi_destroy will be removed, adapt this patch for that.

Use lock instead of qlock which will be removed in the future.

The official example at http://manuals.opensound.com/developer/mmap_test.c.html uses 50ms but that proved to be too much, probably because oss_init() sets low sized buffer. I'm not sure how to derive the number of microseconds to sleep, I just used the value that worked for me. I even tried experimenting with different values, but I have no idea how to get to a value that is based on some facts or measurement.

Reverting to a version which just waits for 0.4ms between iterations. I ran this program for ~100 times on my desktop and it didn't produce any glitches. Should I remove verbose or leave it as it is?

Calculate the amount of microseconds to sleep, but keep it in the range between 400 and 800 microseconds. Position is not updated continuously, but at some frequency. That means that value we get from ioctl can not be trusted to be accurate, which is the reason to limit the maximum duration of the sleep. The minimal duration of sleep is put in place so the code doesn't call ioctl too often.

Check if input and output config are the same by checking number of bytes and number of input/output channels. I had cases where input and output channel numbers are different, as well as difference in buffer_info.bytes when the number of input and output channels is the same. I'm afraid that handling all possible cases would make this example complicated, so I chose to write the code that insists that input and output configs are the same.

Warn only if sample format/rate/channels were not set to desired values, but fail only in simple.c if the format is not the requested one.

Lowering usleep time and checking if ci.ptr is zero made all the glitches in the sound go away. I do need to run it for quite a few times to validate that, but in the worst case I significantly lowered the probability of gibberish on the output. Next, I need to calculate the usleep time based on ci.ptr.

The usleep based mmap example based on official OSS example. There are three big things about it right now that make it not-so-perfect:

I think I'll try my luck with http://manuals.opensound.com/developer/mmap_test.c.html first and fit it into our example and once the usleep version works, see what are the values of ptr compared to the ones in kqueue scenario. My thinking is "if I can't fix it, move to simpler example", which I'm hoping the official docs are. Anyway, I didn't give up on this code, I just need to approach it from the different angle, for now.

Give more info on failed set attempt

Fix copy/paste typo.

Check sample format, rate and channels after they are set.

Check sample rate and format after ioctl call. While here, print sample size and number of channels of configured device.

I am looking into the code in general and I want to figure out how to display information so it makes more sense, not just dump printf.

Rebase after oss.h fix

Remove needless include

Map or allocate buffer in oss_init. For some reason with this patch probability of producing proper sound is way higher. I still have to add GETIPTR/GETOPTR checks and usleep based on the position.

Let me try to compensate the drift by reading SNDCTL_DSP_GETIPTR and SNDCTL_DSP_GETOPTR and experiment with that for a bit.

I forgot to edit Makefile. Fixed now.

Remove extra functions as they are very simple

That's the strange thing. If I run it 20-30 times, I would get roughly half of the times distorted sound, half of the times normal sound. Also, I tried having one config and two events for read and write, but I couldn't produce any sound like that. Anyway, let me fix this example to conform to the comments and lets go from there.

I tested simple, select and poll on the real hardware. As that machine is not running CURRENT, I can't test kqueue. @christos, do you have any means on testing it on real hardware? I tried to test it in bhyve with -s 6,hda,play=/dev/dsp,rec=/dev/dsp but none of the examples work properly that way.

Reword comments and fix Makefile

I already fixed those as well as share/examples/Makefile, I just want to check if everything is working, and that has to wait for my day job to finish.

I just realized I copy/pasted the same first license comment everywhere. Can you tell me what should I put in the "Copyright" lines, please?

Improve comment regarding usage of interleaved format and channels.

I tested all examples with real hardware. Well, all except kqueue, as the machine with the actual hardware is 14.3 based.

Fix to_channels and to_interleaved and address other comments.

Move contents of README into comments, mostly in oss.h

More cleanup from old example

I just remembered one more comment I can't find. It is about setting fragments. I'd like to leave it there and in the comments mention that setting fragments is optional. I'd like to have a full examples so people interested in sound don't have to wonder how things are done. To be honest, official OSS documentation could use some improvement. At least I found it hard to understand what exactly I need to do.

I hope I found all comments. It was not obvious I would have to click on a line number beside comment to get old comments.

I missed old comments as I moved files from oss directory. It took me a while to realize how to read them. Sorry for the noise.

I really hope this is OK way to handle 24 bits.

The directory where these files live is "oss". As everything is about OSS in "sound" directory, should I perhaps rename oss to audio? My reasoning is that there are audio and MIDI examples that we can show, so "oss" is a bit missleading. Or maybe have share/examples/oss and audio and midi under it? Anyway, when I started writing the examples I only wrote audio part, so "sound" was appropriate, which probably isn't the case any more.

Use per-config buffer.

Address the rest of the comments (without moving README to comments).

I hope I addressed all the comments except showing example for both polling directions and readme. I think if a person understands polling in one direction, it is easy to extrapolate how to work with two directions. As these are examples, not complete guides, I would argue that this is good enough, especially in kqueue case where both directions would create two events and example would become more complicated. As I do plan to add mmap example once this is merged, that example will have input device and output device so even that configuration will be shown, I just don't think that all examples should split the sound card into two directions. As for the readme, good catch, I didn't think of that, but let's make the code in the examples good and I'll change the readme to correspond to those changes.

Add triggering.

One more wrong device fixed

Use default DSP device