From 736db623b3e8dc26c08bb3591a78cf9c3329d869 Mon Sep 17 00:00:00 2001 From: Michele Bigi Date: Tue, 25 Aug 2026 18:33:47 +0200 Subject: [PATCH 1/2] Fix total-audio-silence-on-any-EQ-change: negate ADAU1701 feedback coeffs designPeakingEq() mapped the RBJ cookbook's a1/a2 straight into the ADAU1701 Param EQ cell's A0/A1 registers. The cookbook's difference equation subtracts the feedback terms; the ADAU1701 cell adds them. Any nonzero band gain therefore applied positive instead of negative feedback at the target frequency, so the biquad's state diverged and railed to a constant (inaudible DC) value -- the "any EQ/enhancement change goes completely silent, even the beep test tone" bug. Confirmed live: SigmaStudio's own direct safeload writes to the same registers (correctly signed by its own tool) only distorted, never silenced, which pointed at this driver's own coefficient math rather than the DSP chain or the safeload mechanism itself. Adds a host test asserting Jury stability for the ADD-convention denominator across the actual bass/stereo-enhance gain values and the GainDb range extremes, so a regression trips ctest instead of requiring a live listening test to notice. Co-Authored-By: Claude Sonnet 5 --- Software/components/core/src/BiquadDesign.cpp | 12 ++++- .../core/test/biquad_design_test.cpp | 52 +++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/Software/components/core/src/BiquadDesign.cpp b/Software/components/core/src/BiquadDesign.cpp index 3fd63fc..db76b15 100644 --- a/Software/components/core/src/BiquadDesign.cpp +++ b/Software/components/core/src/BiquadDesign.cpp @@ -82,12 +82,20 @@ BiquadCoefficients designPeakingEq(FrequencyHz center, GainDb gain, const float a1 = -2.0F * cosOmega; const float a2 = 1.0F - alpha / a; + // RBJ's cookbook form subtracts the feedback terms + // (y = ... - (a1/a0) y[n-1] - (a2/a0) y[n-2]); the ADAU1701 Param EQ + // cell's A0/A1 registers add them instead (y = ... + A0 y[n-1] + + // A1 y[n-2]), so the normalized RBJ a1/a2 must be negated when they + // land in A0/A1. Without this, any nonzero gain applies positive + // instead of negative feedback at the band's pole, so the biquad's + // state diverges and rails to a constant (inaudible DC) value -- + // root cause of the 2026-08-25 total-silence-on-any-EQ-change bug. return BiquadCoefficients{ .b0 = b0 / a0, .b1 = b1 / a0, .b2 = b2 / a0, - .a0 = a1 / a0, - .a1 = a2 / a0, + .a0 = -(a1 / a0), + .a1 = -(a2 / a0), }; } diff --git a/Software/components/core/test/biquad_design_test.cpp b/Software/components/core/test/biquad_design_test.cpp index dd771cb..de2a4e1 100644 --- a/Software/components/core/test/biquad_design_test.cpp +++ b/Software/components/core/test/biquad_design_test.cpp @@ -54,6 +54,55 @@ namespace { return EXIT_SUCCESS; } +/* + * The ADAU1701 Param EQ cell's A0/A1 registers ADD the feedback terms + * (y = ... + A0 y[n-1] + A1 y[n-2]), unlike the RBJ cookbook's native + * subtractive form. Denominator 1 - A0 z^-1 - A1 z^-2 = 0 has poles at the + * roots of z^2 - A0 z - A1 = 0; by the Jury test for a monic real quadratic + * z^2 + c1 z + c0 (c1 = -A0, c0 = -A1), both poles lie strictly inside the + * unit circle iff |A1| < 1, A0 + A1 < 1, and A0 - A1 > -1. A missing + * negation when mapping the RBJ a1/a2 into A0/A1 (2026-08-25 total-silence + * regression) fails this for real gain/Q combinations, so this guards + * against that class of bug rather than just checking magnitudes. + */ +[[nodiscard]] bool isAdau1701Stable(const core::BiquadCoefficients &c) noexcept +{ + return std::fabs(c.a1) < 1.0F && (c.a0 + c.a1) < 1.0F + && (c.a0 - c.a1) > -1.0F; +} + +[[nodiscard]] int runPeakingStabilityTest() +{ + const struct + { + std::uint32_t hz; + float dbGain; + float q; + } cases[] = { + {100U, 9.0F, 0.9F}, // bass-enhance band 1 at max level + {400U, 3.0F, 1.0F}, // bass-enhance band 2 at max level + {1000U, -1.5F, 1.0F}, // stereo-enhance band 3 at max level + {3000U, 2.0F, 1.0F}, // stereo-enhance band 4 at max level + {8000U, 4.0F, 1.0F}, // stereo-enhance band 5 at max level + {1000U, 12.0F, 10.0F}, // GainDb::kMaxDb at high Q + {1000U, -96.0F, 0.2F}, // near GainDb::kMinDb at low Q + }; + + for (const auto &tc : cases) { + const auto center = core::FrequencyHz::tryFromHz(tc.hz); + const auto gain = core::GainDb::tryFromDb(tc.dbGain); + const core::BiquadCoefficients peaking = + core::designPeakingEq(*center, *gain, tc.q); + if (!isAdau1701Stable(peaking)) { + std::cerr << "unstable ADAU1701 biquad for " << tc.hz << " Hz, " + << tc.dbGain << " dB, Q=" << tc.q << ": a0=" << peaking.a0 + << " a1=" << peaking.a1 << '\n'; + return EXIT_FAILURE; + } + } + return EXIT_SUCCESS; +} + } // namespace int main() @@ -64,5 +113,8 @@ int main() if (runFlatBiquadTest() != EXIT_SUCCESS) { return EXIT_FAILURE; } + if (runPeakingStabilityTest() != EXIT_SUCCESS) { + return EXIT_FAILURE; + } return EXIT_SUCCESS; } From 75cb147f09b903d52bdac07e7b5ad1e417a15a22 Mon Sep 17 00:00:00 2001 From: Michele Bigi Date: Tue, 25 Aug 2026 18:34:28 +0200 Subject: [PATCH 2/2] Fix SigmaStudio TCP bridge accept() spinning on EBADF forever Every boot path constructs SigmaStudioTcpServer as a named local, start()s it, then moves it into NetBootstrap. The moved-from local's own destructor still runs stop() right after, which used to do an unconditional activeListenFd().store(-1) -- clobbering the singleton the moved-to (real, running) instance had just inherited. From then on acceptLoopTask() called accept(-1, ...) == EBADF forever, on every single boot, breaking every SigmaStudio Remote Connection attempt. stop() now only clears the singleton via compare-exchange against its own listenFd_, so a moved-from husk with no fd of its own leaves the real instance's registration alone. Keeps a recreateListenSocket() self-heal in acceptLoopTask() as a safety net for EBADF from any other future cause, though it's no longer expected to fire in normal operation. Co-Authored-By: Claude Sonnet 5 --- .../net/src/SigmaStudioTcpServer.cpp | 83 ++++++++++++++++++- 1 file changed, 80 insertions(+), 3 deletions(-) diff --git a/Software/components/net/src/SigmaStudioTcpServer.cpp b/Software/components/net/src/SigmaStudioTcpServer.cpp index c0aae3d..0898819 100644 --- a/Software/components/net/src/SigmaStudioTcpServer.cpp +++ b/Software/components/net/src/SigmaStudioTcpServer.cpp @@ -494,8 +494,19 @@ void SigmaStudioTcpServer::stop() noexcept vTaskDelete(task_); task_ = nullptr; } - activeListenFd().store(-1, std::memory_order_release); if (listenFd_ >= 0) { + // Only clear the singleton if it still points at *our* fd: every + // boot path constructs this as a named local, start()s it, then + // moves it into NetBootstrap, so the moved-from local's own + // destructor runs stop() right after. An unconditional + // activeListenFd().store(-1) here used to stomp the atomic the + // moved-to (real, running) instance had just inherited, making + // acceptLoopTask() spin on accept(-1, ...) == EBADF forever from + // the very first boot -- root cause of the 2026-08-25 field + // observation, not a Wi-Fi-layer event. + int expected = listenFd_; + activeListenFd().compare_exchange_strong(expected, -1, + std::memory_order_acq_rel); close(listenFd_); listenFd_ = -1; } @@ -547,6 +558,59 @@ std::expected SigmaStudioTcpServer::start() return {}; } +namespace { + +/** + * @brief recreateListenSocket — rebind a fresh listening socket on kPort. + * + * @dname recreateListenSocket + * @return The new fd on success (also stored in activeListenFd()), or -1. + * @pubstate closes the previous fd read from activeListenFd() if any, then + * publishes the new one. + * + * Self-healing counterpart to SigmaStudioTcpServer::start()'s socket setup. + * The 2026-08-25 field observation (accept() spinning on errno=EBADF + * forever) turned out to be a stop() lifetime bug, now fixed there: this + * function is kept as a safety net in case the singleton is ever cleared + * from underneath a running accept task by some future code path, not + * because it is expected to fire in normal operation. + */ +[[nodiscard]] int recreateListenSocket() noexcept +{ + const int oldFd = activeListenFd().exchange(-1, std::memory_order_acq_rel); + if (oldFd >= 0) { + close(oldFd); + } + + const int fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP); + if (fd < 0) { + ESP_LOGE(kTag, "recreateListenSocket: socket() failed"); + return -1; + } + + const int reuse = 1; + setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &reuse, sizeof(reuse)); + + sockaddr_in addr{}; + addr.sin_family = AF_INET; + addr.sin_addr.s_addr = htonl(INADDR_ANY); + addr.sin_port = htons(kPort); + + if (bind(fd, reinterpret_cast(&addr), sizeof(addr)) != 0 + || listen(fd, 1) != 0) { + ESP_LOGE(kTag, "recreateListenSocket: bind/listen failed, errno=%d", + errno); + close(fd); + return -1; + } + + activeListenFd().store(fd, std::memory_order_release); + ESP_LOGW(kTag, "SigmaStudio TCP listen socket recreated after failure"); + return fd; +} + +} // namespace + void SigmaStudioTcpServer::acceptLoopTask(void* /*arg*/) { while (true) { @@ -556,8 +620,21 @@ void SigmaStudioTcpServer::acceptLoopTask(void* /*arg*/) const int clientFd = accept( listenFd, reinterpret_cast(&clientAddr), &clientLen); if (clientFd < 0) { - ESP_LOGW(kTag, "accept() failed: errno=%d", errno); - vTaskDelay(pdMS_TO_TICKS(100)); + // EBADF means the listen socket itself is gone -- retrying + // accept() on the same fd forever can never recover from this, + // unlike a transient per-call error, so rebuild the socket + // instead of just backing off and looping. + if (errno == EBADF) { + ESP_LOGE(kTag, + "accept() failed: listen socket invalid (errno=%d) " + "-- recreating", + errno); + (void)recreateListenSocket(); + vTaskDelay(pdMS_TO_TICKS(500)); + } else { + ESP_LOGW(kTag, "accept() failed: errno=%d", errno); + vTaskDelay(pdMS_TO_TICKS(100)); + } continue; } ESP_LOGI(kTag, "SigmaStudio client connected");