This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

• Add capture support to the sndio backend (patch-src_audio_sndio, text/plain, 2017-07-20 01:43:22 +0000, 4376 bytes)
• Fixes for the sndio backend (file_3705.txt, text/plain, 2017-07-20 20:57:01 +0000, 142 bytes)
• Fixes for the sndio backend (patch-src_audio_sndio, text/plain, 2017-07-20 21:16:53 +0000, 3183 bytes)
• Fix the incorrect check on poll(2) (patch-src_audio_sndio, text/plain, 2017-07-20 22:46:40 +0000, 626 bytes)

Reported in version: HG 2.1
Reported for operating system, platform: OpenBSD, All

Comments on the original bug report:

On 2017-07-20 01:43:22 +0000, wrote:

Created attachment 2801
Add capture support to the sndio backend

The attached patch adds capture support to the sndio backend.

The patch also allows the `OpenDevice' function to accept arbitrary device names.

On 2017-07-20 17:39:59 +0000, Sam Lantinga wrote:

Added, thanks!
https://hg.libsdl.org/SDL/rev/9bdb766ec7e5

On 2017-07-20 18:10:27 +0000, Ryan C. Gordon wrote:

Some minor nitpicks on this patch:

• while (SNDIO_sio_read(this->hidden->dev, buf, sizeof(buf)) != 0) {

•    /* do nothing */;
  

• }

Can this return -1 if there's an error? If so, this will loop forever if a USB audio device is unplugged or whatever, so we should loop while sio_read() > 0.

•    SNDIO_sio_open(devname != NULL ? SIO_DEVANY : devname,
  

That probably was meant to be "devname == NULL", right?

Also, CaptureFromDevice() needs to block enough to return a value > 0, but it doesn't have to return a full buffer; the calling code knows to loop again to get more data, but it considers a zero return value as a fatal error. I don't know if that simplifies the code much to only block until something is available instead of enough is available, but that option is available to you if it makes things cleaner.

--ryan.

On 2017-07-20 20:53:54 +0000, wrote:

Can this return -1 if there's an error? If so, this will loop forever if a USB audio
device is unplugged or whatever, so we should loop while sio_read() > 0.

`sio_read' returns an unsigned value with zero indicating an error.

That probably was meant to be "devname == NULL", right?

Right, sorry.

On 2017-07-20 20:57:01 +0000, wrote:

Created attachment 2803
Fixes for the sndio backend

On 2017-07-20 21:02:58 +0000, Ryan C. Gordon wrote:

(In reply to kdrakehp from comment # 4)

Created attachment 2803 [details]
Fixes for the sndio backend

(this wasn't the patch, can't you attach again? Sorry.)

--ryan.

On 2017-07-20 21:03:42 +0000, Ryan C. Gordon wrote:

`sio_read' returns an unsigned value with zero indicating an error.

Oh, ok, cool. :)

--ryan.

On 2017-07-20 21:16:53 +0000, wrote:

Created attachment 2804
Fixes for the sndio backend

On 2017-07-20 21:45:26 +0000, wrote:

(In reply to Ryan C. Gordon from comment # 5)

(In reply to kdrakehp from comment # 4)

Created attachment 2803 [details]
Fixes for the sndio backend

(this wasn't the patch, can't you attach again? Sorry.)

--ryan.

That is odd. I have added the patch again.

On 2017-07-20 22:22:54 +0000, Ryan C. Gordon wrote:

That is odd. I have added the patch again.

Thanks!

This patch is now https://hg.libsdl.org/SDL/rev/dfa9358ae3e0 ...

One last question (just to make sure)...in SNDIO_CaptureFromDevice, this doesn't block at all; this happens in a background thread in SDL, but I assume this is burning the CPU core at full speed waiting for more data to arrive; should we put a sleep in there? (and/or can the device's file descriptor just be set to block, so reads don't return until there is data or an error?).

--ryan.

On 2017-07-20 22:45:22 +0000, wrote:

One last question (just to make sure)...in SNDIO_CaptureFromDevice, this
doesn't block at all; this happens in a background thread in SDL, but I
assume this is burning the CPU core at full speed waiting for more data to
arrive; should we put a sleep in there? (and/or can the device's file
descriptor just be set to block, so reads don't return until there is data
or an error?).

--ryan.

No, poll(2) waits for an event on the file-descriptor list set by
sio_pollfd'. The way I used it in the CaptureFromDevice' function
waits for the `POLLIN' event to signal that there is data available
to read. It should only ever execute the loop body once, unless an
interrupt is received.

The `FlushCapture' function is what requires the device to be
non-blocking, because sndio itself does not provide a way to flush
the buffer.

Now that I think about it, I forgot something else. The comparison
for poll should be < 0'' not <= 0'', an interrupt would cause
the `CaptureFromDevice' function to incorrectly return zero.

Sorry about that.

On 2017-07-20 22:46:40 +0000, wrote:

Created attachment 2806
Fix the incorrect check on poll(2)

On 2017-07-21 00:43:19 +0000, Ryan C. Gordon wrote:

No, poll(2) waits for an event on the file-descriptor list set by

Oh, sorry, you're right. I saw "poll" and ignored the INFTIM argument.

Your poll() fix is now https://hg.libsdl.org/SDL/rev/41d93b7fd207, thanks!

I think this resolves everything, so I'm marking this bug resolved again. Thanks again for working through this with me. :)

--ryan.