Date: Fri, 22 Oct 2021 10:08:43 GMT From: Thomas Zander <riggs@FreeBSD.org> To: ports-committers@FreeBSD.org, dev-commits-ports-all@FreeBSD.org, dev-commits-ports-main@FreeBSD.org Subject: git: 059ad2ed29b7 - main - audio/openal-soft: Fix crashes in the sndio backend. Message-ID: <202110221008.19MA8hEX039047@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by riggs: URL: https://cgit.FreeBSD.org/ports/commit/?id=059ad2ed29b7ca685d2b088296728e99fc5c4e11 commit 059ad2ed29b7ca685d2b088296728e99fc5c4e11 Author: Thomas Zander <riggs@FreeBSD.org> AuthorDate: 2021-10-22 10:04:38 +0000 Commit: Thomas Zander <riggs@FreeBSD.org> CommitDate: 2021-10-22 10:08:41 +0000 audio/openal-soft: Fix crashes in the sndio backend. Details: - Backport the upstream commits at https://github.com/kcat/openal-soft/commit/620836f173ae6fc4505d0634984e0f2c46166367 https://github.com/kcat/openal-soft/commit/1fd4c865fc084f134363db5155361d5483679235 https://github.com/kcat/openal-soft/commit/a4b0a3d7b3ec271243cfda4780e567e49f2b37b7 MFH: 2021Q4 --- audio/openal-soft/Makefile | 2 +- .../openal-soft/files/patch-alc_backends_sndio.cpp | 455 +++++++++++++++++++++ 2 files changed, 456 insertions(+), 1 deletion(-) diff --git a/audio/openal-soft/Makefile b/audio/openal-soft/Makefile index 70c163170550..69987e4d8ffa 100644 --- a/audio/openal-soft/Makefile +++ b/audio/openal-soft/Makefile @@ -1,6 +1,6 @@ PORTNAME= openal-soft PORTVERSION= 1.21.1 -PORTREVISION= 1 +PORTREVISION= 2 CATEGORIES= audio MASTER_SITES= https://openal-soft.org/openal-releases/ diff --git a/audio/openal-soft/files/patch-alc_backends_sndio.cpp b/audio/openal-soft/files/patch-alc_backends_sndio.cpp new file mode 100644 index 000000000000..dba1079c4249 --- /dev/null +++ b/audio/openal-soft/files/patch-alc_backends_sndio.cpp @@ -0,0 +1,455 @@ +--- alc/backends/sndio.cpp.orig 2021-02-04 19:09:06 UTC ++++ alc/backends/sndio.cpp +@@ -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; + +@@ -74,39 +81,29 @@ SndioPlayback::~SndioPlayback() + + int SndioPlayback::mixerProc() + { +- sio_par par; +- sio_initpar(&par); +- if(!sio_getpar(mSndHandle, &par)) +- { +- 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)) + { +- al::byte *WritePtr{mBuffer.data()}; +- size_t len{mBuffer.size()}; ++ al::span<al::byte> buffer{mBuffer}; + +- 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); + } + } + +@@ -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; +- +- // 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; +@@ -159,7 +142,6 @@ bool SndioPlayback::reset() + par.bits = 8; + par.sig = 0; + break; +- case DevFmtFloat: + 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; + ++ 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; + +- 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) +- { +- 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) +- || par.pchan == 5 +- || (par.pchan == 6 && mDevice->FmtChans != DevFmtX51 && mDevice->FmtChans != DevFmtX51Rear) +- || (par.pchan == 7 && mDevice->FmtChans != DevFmtX61) +- || (par.pchan == 8 && mDevice->FmtChans != DevFmtX71) +- || par.pchan > 8) +- { +- WARN("Got %u channels for %s\n", par.pchan, DevFmtChannelsString(mDevice->FmtChans)); +- mDevice->FmtChans = DevFmtStereo; ++ catch(al::backend_exception &e) { ++ if(tryfmt == DevFmtShort) ++ throw; ++ par.clear(); ++ tryfmt = DevFmtShort; ++ goto retry_params; + } + +- 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()) + { +- 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}; +@@ -323,40 +306,65 @@ int SndioCapture::recordProc() + + const uint frameSize{mDevice->frameSizeFromFmt()}; + ++ int nfds_pre{sio_nfds(mSndHandle)}; ++ if (nfds_pre <= 0) ++ { ++ mDevice->handleDisconnect("Incorrect return value from sio_nfds(): %d", nfds_pre); ++ return 1; ++ } ++ ++ mFds.resize(nfds_pre); ++ + 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 +379,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(); +
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202110221008.19MA8hEX039047>