Backport the following two commits from upstream, which fixes the sndio

backend changes that were introduced in the 1.21.1 update.

Simplify channel handling in the sndio backend
620836f173ae6fc4505d0634984e0f2c46166367

Use non-block mode for sndio capture
1fd4c865fc084f134363db5155361d5483679235

From Brad, tested by me

ok maintainer (David CARLIER)
This commit is contained in:
brynet 2021-08-20 21:54:22 +00:00
parent 42a9b0f316
commit 28a8fba46b
2 changed files with 386 additions and 123 deletions

View File

@ -1,4 +1,4 @@
# $OpenBSD: Makefile,v 1.56 2021/07/30 12:47:53 brynet Exp $
# $OpenBSD: Makefile,v 1.57 2021/08/20 21:54:22 brynet Exp $
COMMENT = cross-platform 3D audio API
@ -6,7 +6,7 @@ V = 1.21.1
EPOCH = 0
DISTNAME = openal-soft-$V
PKGNAME = openal-$V
REVISION = 0
REVISION = 1
CATEGORIES = audio
SHARED_LIBS = openal 4.1

View File

@ -1,30 +1,45 @@
$OpenBSD: patch-alc_backends_sndio_cpp,v 1.2 2021/07/31 00:27:59 brynet Exp $
$OpenBSD: patch-alc_backends_sndio_cpp,v 1.3 2021/08/20 21:54:22 brynet Exp $
Revert..
Simplify channel handling in the sndio backend
620836f173ae6fc4505d0634984e0f2c46166367
sndio: Support more than 2 channels
5cffe7e50a2885d9b392c15f2cd3941cac49692c
Use non-block mode for sndio capture
1fd4c865fc084f134363db5155361d5483679235
and
Use a safer layout if sndio changes the channel count
b4a52321c4ad8e8bc0eb29d2cdae2c043fc405e1
which break the backend.
Index: alc/backends/sndio.cpp
--- alc/backends/sndio.cpp.orig
+++ alc/backends/sndio.cpp
@@ -57,7 +57,7 @@ struct SndioPlayback final : public BackendBase {
@@ -22,6 +22,7 @@
#include "backends/sndio.h"
+#include <poll.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
@@ -43,7 +44,12 @@ namespace {
static const char sndio_device[] = "SndIO Default";
+struct SioPar : public sio_par {
+ SioPar() { sio_initpar(this); }
+ void clear() { sio_initpar(this); }
+};
+
struct SndioPlayback final : public BackendBase {
SndioPlayback(ALCdevice *device) noexcept : BackendBase{device} { }
~SndioPlayback() override;
@@ -56,6 +62,7 @@ struct SndioPlayback final : public BackendBase {
void stop() override;
sio_hdl *mSndHandle{nullptr};
+ uint mFrameStep{};
- al::vector<al::byte> mBuffer;
+ al::vector<ALubyte> mBuffer;
al::vector<al::byte> mBuffer;
std::atomic<bool> mKillNow{true};
std::thread mThread;
@@ -74,24 +74,16 @@ SndioPlayback::~SndioPlayback()
@@ -74,39 +81,29 @@ SndioPlayback::~SndioPlayback()
int SndioPlayback::mixerProc()
{
@ -35,106 +50,133 @@ Index: alc/backends/sndio.cpp
- mDevice->handleDisconnect("Failed to get device parameters");
- return 1;
- }
-
+ const size_t frameStep{mFrameStep};
+ const size_t frameSize{frameStep * mDevice->bytesFromFmt()};
- const size_t frameStep{par.pchan};
- const size_t frameSize{frameStep * par.bps};
-
SetRTPriority();
althrd_setname(MIXER_THREAD_NAME);
- while(!mKillNow.load(std::memory_order_acquire)
- && mDevice->Connected.load(std::memory_order_acquire))
+ const size_t frameStep{mDevice->channelsFromFmt()};
+ const uint frameSize{mDevice->frameSizeFromFmt()};
+
+ while(!mKillNow.load(std::memory_order_acquire) &&
+ mDevice->Connected.load(std::memory_order_acquire))
while(!mKillNow.load(std::memory_order_acquire)
&& mDevice->Connected.load(std::memory_order_acquire))
{
- al::byte *WritePtr{mBuffer.data()};
+ ALubyte *WritePtr{mBuffer.data()};
size_t len{mBuffer.size()};
- size_t len{mBuffer.size()};
+ al::span<al::byte> buffer{mBuffer};
mDevice->renderSamples(WritePtr, static_cast<uint>(len/frameSize), frameStep);
@@ -135,47 +127,35 @@ bool SndioPlayback::reset()
sio_initpar(&par);
- mDevice->renderSamples(WritePtr, static_cast<uint>(len/frameSize), frameStep);
- while(len > 0 && !mKillNow.load(std::memory_order_acquire))
+ mDevice->renderSamples(buffer.data(), static_cast<uint>(buffer.size() / frameSize),
+ frameStep);
+ while(!buffer.empty() && !mKillNow.load(std::memory_order_acquire))
{
- size_t wrote{sio_write(mSndHandle, WritePtr, len)};
+ size_t wrote{sio_write(mSndHandle, buffer.data(), buffer.size())};
if(wrote == 0)
{
ERR("sio_write failed\n");
mDevice->handleDisconnect("Failed to write playback samples");
break;
}
-
- len -= wrote;
- WritePtr += wrote;
+ buffer = buffer.subspan(wrote);
}
}
par.rate = mDevice->Frequency;
@@ -131,26 +128,12 @@ void SndioPlayback::open(const char *name)
bool SndioPlayback::reset()
{
- sio_par par;
- sio_initpar(&par);
+ SioPar par;
- par.rate = mDevice->Frequency;
- switch(mDevice->FmtChans)
- {
+ auto tryfmt = mDevice->FmtType;
+retry_params:
+ switch(tryfmt)
{
- case DevFmtMono : par.pchan = 1; break;
- case DevFmtQuad : par.pchan = 4; break;
- case DevFmtX51Rear: // fall-through - "Similar to 5.1, except using rear channels instead of sides"
- case DevFmtX51 : par.pchan = 6; break;
- case DevFmtX61 : par.pchan = 7; break;
- case DevFmtX71 : par.pchan = 8; break;
+ par.pchan = ((mDevice->FmtChans != DevFmtMono) ? 2 : 1);
-
- // fall back to stereo for Ambi3D
- case DevFmtAmbi3D : // fall-through
- case DevFmtStereo : par.pchan = 2; break;
- }
-
switch(mDevice->FmtType)
{
- case DevFmtByte:
- par.bits = 8;
- par.sig = 1;
- break;
- case DevFmtUByte:
- par.bits = 8;
- par.sig = 0;
- break;
- switch(mDevice->FmtType)
- {
case DevFmtByte:
par.bits = 8;
par.sig = 1;
@@ -159,7 +142,6 @@ bool SndioPlayback::reset()
par.bits = 8;
par.sig = 0;
break;
- case DevFmtFloat:
- case DevFmtShort:
- par.bits = 16;
- par.sig = 1;
- break;
- case DevFmtUShort:
- par.bits = 16;
- par.sig = 0;
- break;
- case DevFmtInt:
- par.bits = 32;
- par.sig = 1;
- break;
- case DevFmtUInt:
- par.bits = 32;
- par.sig = 0;
- break;
+ case DevFmtByte:
+ par.bits = 8;
+ par.sig = 1;
+ break;
+ case DevFmtUByte:
+ par.bits = 8;
+ par.sig = 0;
+ break;
+ case DevFmtFloat:
+ case DevFmtShort:
+ par.bits = 16;
+ par.sig = 1;
+ break;
+ case DevFmtUShort:
+ par.bits = 16;
+ par.sig = 0;
+ break;
+ case DevFmtInt:
+ par.bits = 32;
+ par.sig = 1;
+ break;
+ case DevFmtUInt:
+ par.bits = 32;
+ par.sig = 0;
+ break;
case DevFmtShort:
par.bits = 16;
par.sig = 1;
@@ -168,6 +150,7 @@ bool SndioPlayback::reset()
par.bits = 16;
par.sig = 0;
break;
+ case DevFmtFloat:
case DevFmtInt:
par.bits = 32;
par.sig = 1;
@@ -177,70 +160,64 @@ bool SndioPlayback::reset()
par.sig = 0;
break;
}
+ par.bps = SIO_BPS(par.bits);
par.le = SIO_LE_NATIVE;
+ par.msb = 1;
@@ -202,28 +182,8 @@ bool SndioPlayback::reset()
}
+ par.rate = mDevice->Frequency;
+ par.pchan = mDevice->channelsFromFmt();
+
par.round = mDevice->UpdateSize;
par.appbufsz = mDevice->BufferSize - mDevice->UpdateSize;
if(!par.appbufsz) par.appbufsz = mDevice->UpdateSize;
mDevice->Frequency = par.rate;
+ mDevice->FmtChans = ((par.pchan==1) ? DevFmtMono : DevFmtStereo);
- if(!sio_setpar(mSndHandle, &par) || !sio_getpar(mSndHandle, &par))
- {
- ERR("Failed to set device parameters\n");
- return false;
- }
+ try {
+ if(!sio_setpar(mSndHandle, &par))
+ throw al::backend_exception{al::backend_error::DeviceError,
+ "Failed to set device parameters"};
- if(par.bits != par.bps*8)
- {
- ERR("Padded samples not supported (%u of %u bits)\n", par.bits, par.bps*8);
- return false;
- }
- if(par.le != SIO_LE_NATIVE)
- {
- ERR("Non-native-endian samples not supported (got %s-endian)\n",
- par.le ? "little" : "big");
- return false;
- }
+ par.clear();
+ if(!sio_getpar(mSndHandle, &par))
+ throw al::backend_exception{al::backend_error::DeviceError,
+ "Failed to get device parameters"};
- mDevice->Frequency = par.rate;
-
- if(par.pchan < 2)
- {
- if(mDevice->FmtChans != DevFmtMono)
@ -142,7 +184,16 @@ Index: alc/backends/sndio.cpp
- WARN("Got %u channel for %s\n", par.pchan, DevFmtChannelsString(mDevice->FmtChans));
- mDevice->FmtChans = DevFmtMono;
- }
- }
+ if(par.bps > 1 && par.le != SIO_LE_NATIVE)
+ throw al::backend_exception{al::backend_error::DeviceError,
+ "%s-endian samples not supported", par.le ? "Little" : "Big"};
+ if(par.bits < par.bps*8 && !par.msb)
+ throw al::backend_exception{al::backend_error::DeviceError,
+ "MSB-padded samples not supported (%u of %u bits)", par.bits, par.bps*8};
+ if(par.pchan < 1)
+ throw al::backend_exception{al::backend_error::DeviceError,
+ "No playback channels on device"};
}
- else if((par.pchan == 2 && mDevice->FmtChans != DevFmtStereo)
- || par.pchan == 3
- || (par.pchan == 4 && mDevice->FmtChans != DevFmtQuad)
@ -154,37 +205,249 @@ Index: alc/backends/sndio.cpp
- {
- WARN("Got %u channels for %s\n", par.pchan, DevFmtChannelsString(mDevice->FmtChans));
- mDevice->FmtChans = DevFmtStereo;
- }
-
if(par.bits == 8 && par.sig == 1)
mDevice->FmtType = DevFmtByte;
else if(par.bits == 8 && par.sig == 0)
@@ -247,15 +207,8 @@ bool SndioPlayback::reset()
mDevice->UpdateSize = par.round;
mDevice->BufferSize = par.bufsz + par.round;
+ catch(al::backend_exception &e) {
+ if(tryfmt == DevFmtShort)
+ throw;
+ par.clear();
+ tryfmt = DevFmtShort;
+ goto retry_params;
}
- mBuffer.resize(mDevice->UpdateSize * par.pchan*par.bps);
- if(par.sig == 1)
- std::fill(mBuffer.begin(), mBuffer.end(), al::byte{});
- else if(par.bits == 8)
- std::fill_n(mBuffer.data(), mBuffer.size(), al::byte(0x80));
- else if(par.bits == 16)
- std::fill_n(reinterpret_cast<uint16_t*>(mBuffer.data()), mBuffer.size()/2, 0x8000);
- else if(par.bits == 32)
- std::fill_n(reinterpret_cast<uint32_t*>(mBuffer.data()), mBuffer.size()/4, 0x80000000u);
+ mBuffer.resize(mDevice->UpdateSize * mDevice->frameSizeFromFmt());
+ std::fill(mBuffer.begin(), mBuffer.end(), 0);
return true;
}
@@ -323,8 +276,8 @@ int SndioCapture::recordProc()
const uint frameSize{mDevice->frameSizeFromFmt()};
- while(!mKillNow.load(std::memory_order_acquire)
- && mDevice->Connected.load(std::memory_order_acquire))
+ while(!mKillNow.load(std::memory_order_acquire) &&
+ mDevice->Connected.load(std::memory_order_acquire))
- if(par.bits == 8 && par.sig == 1)
- mDevice->FmtType = DevFmtByte;
- else if(par.bits == 8 && par.sig == 0)
- mDevice->FmtType = DevFmtUByte;
- else if(par.bits == 16 && par.sig == 1)
- mDevice->FmtType = DevFmtShort;
- else if(par.bits == 16 && par.sig == 0)
- mDevice->FmtType = DevFmtUShort;
- else if(par.bits == 32 && par.sig == 1)
- mDevice->FmtType = DevFmtInt;
- else if(par.bits == 32 && par.sig == 0)
- mDevice->FmtType = DevFmtUInt;
+ if(par.bps == 1)
+ mDevice->FmtType = (par.sig==1) ? DevFmtByte : DevFmtUByte;
+ else if(par.bps == 2)
+ mDevice->FmtType = (par.sig==1) ? DevFmtShort : DevFmtUShort;
+ else if(par.bps == 4)
+ mDevice->FmtType = (par.sig==1) ? DevFmtInt : DevFmtUInt;
else
+ throw al::backend_exception{al::backend_error::DeviceError,
+ "Unhandled sample format: %s %u-bit", (par.sig?"signed":"unsigned"), par.bps*8};
+
+ mFrameStep = par.pchan;
+ if(par.pchan != mDevice->channelsFromFmt())
{
auto data = mRing->getWriteVector();
size_t todo{data.first.len + data.second.len};
- ERR("Unhandled sample format: %s %u-bit\n", (par.sig?"signed":"unsigned"), par.bits);
- return false;
+ WARN("Got %u channel%s for %s\n", par.pchan, (par.pchan==1)?"":"s",
+ DevFmtChannelsString(mDevice->FmtChans));
+ if(par.pchan < 2) mDevice->FmtChans = DevFmtMono;
+ else mDevice->FmtChans = DevFmtStereo;
}
+ mDevice->Frequency = par.rate;
setDefaultChannelOrder();
@@ -287,6 +264,11 @@ void SndioPlayback::stop()
}
+/* TODO: This could be improved by avoiding the ring buffer and record thread,
+ * counting the available samples with the sio_onmove callback and reading
+ * directly from the device. However, this depends on reasonable support for
+ * capture buffer sizes apps may request.
+ */
struct SndioCapture final : public BackendBase {
SndioCapture(ALCdevice *device) noexcept : BackendBase{device} { }
~SndioCapture() override;
@@ -301,6 +283,7 @@ struct SndioCapture final : public BackendBase {
sio_hdl *mSndHandle{nullptr};
+ al::vector<struct pollfd> mFds;
RingBufferPtr mRing;
std::atomic<bool> mKillNow{true};
@@ -326,37 +309,53 @@ int SndioCapture::recordProc()
while(!mKillNow.load(std::memory_order_acquire)
&& mDevice->Connected.load(std::memory_order_acquire))
{
- auto data = mRing->getWriteVector();
- size_t todo{data.first.len + data.second.len};
- if(todo == 0)
+ /* Wait until there's some samples to read. */
+ const int nfds{sio_pollfd(mSndHandle, mFds.data(), POLLIN)};
+ if(nfds <= 0)
{
- static char junk[4096];
- sio_read(mSndHandle, junk,
- minz(sizeof(junk)/frameSize, mDevice->UpdateSize)*frameSize);
+ mDevice->handleDisconnect("Failed to get polling fds: %d", nfds);
+ break;
+ }
+ int pollres{::poll(mFds.data(), static_cast<uint>(nfds), 2000)};
+ if(pollres < 0)
+ {
+ if(errno == EINTR) continue;
+ mDevice->handleDisconnect("Poll error: %s", strerror(errno));
+ break;
+ }
+ if(pollres == 0)
continue;
+
+ const int revents{sio_revents(mSndHandle, mFds.data())};
+ if((revents&POLLHUP))
+ {
+ mDevice->handleDisconnect("Got POLLHUP from poll events");
+ break;
}
+ if(!(revents&POLLIN))
+ continue;
- size_t total{0u};
- data.first.len *= frameSize;
- data.second.len *= frameSize;
- todo = minz(todo, mDevice->UpdateSize) * frameSize;
- while(total < todo)
+ auto data = mRing->getWriteVector();
+ al::span<al::byte> buffer{data.first.buf, data.first.len*frameSize};
+ while(!buffer.empty())
{
- if(!data.first.len)
- data.first = data.second;
+ size_t got{sio_read(mSndHandle, buffer.data(), buffer.size())};
+ if(got == 0) break;
- size_t got{sio_read(mSndHandle, data.first.buf, minz(todo-total, data.first.len))};
- if(!got)
+ mRing->writeAdvance(got / frameSize);
+ buffer = buffer.subspan(got);
+ if(buffer.empty())
{
- mDevice->handleDisconnect("Failed to read capture samples");
- break;
+ data = mRing->getWriteVector();
+ buffer = {data.first.buf, data.first.len*frameSize};
}
-
- data.first.buf += got;
- data.first.len -= got;
- total += got;
}
- mRing->writeAdvance(total / frameSize);
+ if(buffer.empty())
+ {
+ /* Got samples to read, but no place to store it. Drop it. */
+ static char junk[4096];
+ sio_read(mSndHandle, junk, sizeof(junk) - (sizeof(junk)%frameSize));
+ }
}
return 0;
@@ -371,76 +370,80 @@ void SndioCapture::open(const char *name)
throw al::backend_exception{al::backend_error::NoDevice, "Device name \"%s\" not found",
name};
- mSndHandle = sio_open(nullptr, SIO_REC, 0);
+ mSndHandle = sio_open(nullptr, SIO_REC, true);
if(mSndHandle == nullptr)
throw al::backend_exception{al::backend_error::NoDevice, "Could not open backend device"};
- sio_par par;
- sio_initpar(&par);
-
+ SioPar par;
switch(mDevice->FmtType)
{
case DevFmtByte:
- par.bps = 1;
+ par.bits = 8;
par.sig = 1;
break;
case DevFmtUByte:
- par.bps = 1;
+ par.bits = 8;
par.sig = 0;
break;
case DevFmtShort:
- par.bps = 2;
+ par.bits = 16;
par.sig = 1;
break;
case DevFmtUShort:
- par.bps = 2;
+ par.bits = 16;
par.sig = 0;
break;
case DevFmtInt:
- par.bps = 4;
+ par.bits = 32;
par.sig = 1;
break;
case DevFmtUInt:
- par.bps = 4;
+ par.bits = 32;
par.sig = 0;
break;
case DevFmtFloat:
throw al::backend_exception{al::backend_error::DeviceError,
"%s capture samples not supported", DevFmtTypeString(mDevice->FmtType)};
}
- par.bits = par.bps * 8;
+ par.bps = SIO_BPS(par.bits);
par.le = SIO_LE_NATIVE;
- par.msb = SIO_LE_NATIVE ? 0 : 1;
+ par.msb = 1;
par.rchan = mDevice->channelsFromFmt();
par.rate = mDevice->Frequency;
par.appbufsz = maxu(mDevice->BufferSize, mDevice->Frequency/10);
- par.round = minu(par.appbufsz, mDevice->Frequency/40);
+ par.round = minu(par.appbufsz/2, mDevice->Frequency/40);
- mDevice->UpdateSize = par.round;
- mDevice->BufferSize = par.appbufsz;
-
if(!sio_setpar(mSndHandle, &par) || !sio_getpar(mSndHandle, &par))
throw al::backend_exception{al::backend_error::DeviceError,
"Failed to set device praameters"};
- if(par.bits != par.bps*8)
+ if(par.bps > 1 && par.le != SIO_LE_NATIVE)
throw al::backend_exception{al::backend_error::DeviceError,
+ "%s-endian samples not supported", par.le ? "Little" : "Big"};
+ if(par.bits < par.bps*8 && !par.msb)
+ throw al::backend_exception{al::backend_error::DeviceError,
"Padded samples not supported (got %u of %u bits)", par.bits, par.bps*8};
- if(!((mDevice->FmtType == DevFmtByte && par.bits == 8 && par.sig != 0)
- || (mDevice->FmtType == DevFmtUByte && par.bits == 8 && par.sig == 0)
- || (mDevice->FmtType == DevFmtShort && par.bits == 16 && par.sig != 0)
- || (mDevice->FmtType == DevFmtUShort && par.bits == 16 && par.sig == 0)
- || (mDevice->FmtType == DevFmtInt && par.bits == 32 && par.sig != 0)
- || (mDevice->FmtType == DevFmtUInt && par.bits == 32 && par.sig == 0))
- || mDevice->channelsFromFmt() != par.rchan || mDevice->Frequency != par.rate)
+ auto match_fmt = [](DevFmtType fmttype, const sio_par &par) -> bool
+ {
+ return (fmttype == DevFmtByte && par.bps == 1 && par.sig != 0)
+ || (fmttype == DevFmtUByte && par.bps == 1 && par.sig == 0)
+ || (fmttype == DevFmtShort && par.bps == 2 && par.sig != 0)
+ || (fmttype == DevFmtUShort && par.bps == 2 && par.sig == 0)
+ || (fmttype == DevFmtInt && par.bps == 4 && par.sig != 0)
+ || (fmttype == DevFmtUInt && par.bps == 4 && par.sig == 0);
+ };
+ if(!match_fmt(mDevice->FmtType, par) || mDevice->channelsFromFmt() != par.rchan
+ || mDevice->Frequency != par.rate)
throw al::backend_exception{al::backend_error::DeviceError,
"Failed to set format %s %s %uhz, got %c%u %u-channel %uhz instead",
DevFmtTypeString(mDevice->FmtType), DevFmtChannelsString(mDevice->FmtChans),
- mDevice->Frequency, par.sig?'s':'u', par.bits, par.rchan, par.rate};
+ mDevice->Frequency, par.sig?'s':'u', par.bps*8, par.rchan, par.rate};
mRing = RingBuffer::Create(mDevice->BufferSize, par.bps*par.rchan, false);
+ mDevice->BufferSize = static_cast<uint>(mRing->writeSpace());
+ mDevice->UpdateSize = par.round;
setDefaultChannelOrder();