Feature/advanced biamp#187
Conversation
…DAC state tracking. Added debug logging.
…mponent to apply stored settings at startup and expose settings structure to http component, Updated html for the UI
…l_settings component architecture. Fully data-driven front-end
…log gain to tas5805m_settings
…equency, analog gain. Added support for readonly parameters
Multiple RESYNCING HARD events triggered a player restart During player restart, the DSP worker function was called paramsChangedSemaphoreHandle was NULL (either not yet created or already deleted) Calling xSemaphoreTake(NULL, ...) caused the LoadProhibited crash The fix ensures we check if the semaphore handle exists before trying to take it.
…es on reboot of device.
The EQ schema and settings JSON were only reading from the driver state, which is uninitialized (all 0s) before tas5805m_settings_apply_delayed() runs when music starts. Now falls back to NVS to show persisted values. Fixed locations: - Channel gains in EQ schema (cur_ch_l/cur_ch_r) - Per-band EQ gains in EQ schema (left and right channels) - Channel gains in settings JSON - Per-band EQ gains in settings JSON
… improvements - Add bi-amp crossover module with Butterworth/Linkwitz-Riley filters (12/24/48 dB/oct) - Implement per-output gain, phase invert, subsonic HPF, and 3-band PEQ - Add loudness compensation with bass/treble shelf filters optimized for bi-amp mode - Restructure EQ settings UI with output-channel layout and PEQ subgroups - Add radio button support for binary phase controls - Hide Channel Gain group in Advanced Bi-Amp mode (uses per-output gains instead) - Support NVS persistence for all bi-amp settings Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add tas5805m_biamp_preset_export() and tas5805m_biamp_preset_import() functions - Add GET/POST /api/biamp/preset HTTP endpoints for download/upload - Add Export Preset and Import Preset buttons to Advanced Bi-Amp UI - JSON preset includes crossover, gains, phase, PEQ, and loudness settings - Enables distributing speaker-specific settings with 3D speaker models
- Fix division by zero crash in player.c when reconnecting (chkInFrames=0) - Add 300ms debouncing to EQ settings sliders to prevent flooding device - Change frequency sliders to 1 Hz increments (crossover, PEQ) - Change gain sliders to 0.5 dB increments using x2 fixed-point storage - Remove debug biquad logging from tas5805m.c and tas5805m_biamp.c - Update preset export/import to handle new gain format - Increase HTTP server stack size for bi-amp schema generation
- Increase PEQ bands from 3 to 6 per channel (12 total) - Update biquad allocation: bands 6-11 for PEQ, band 12 spare - Reorganize loudness UI to group parameters by zone - Each zone shows volume range, threshold, bass, and treble together - Add renderLoudnessZone() function and CSS for grouped layout
Bug fixes: - Fix loudness not applying (static cache never initialized from NVS) - Fix loudness always on when disabled (now resets bands to passthrough) - Fix phase/crossover not working (missing ADVANCED_BIAMP case in EQ mode switch) - Fix I2C mutex race condition with spinlock for initialization - Fix baffle placement showing as slider instead of dropdown Security hardening: - Fix buffer overflow in find_key_value() with size parameter - Add URL decode hex digit validation - Restrict CORS to localhost and private IP ranges - Add gain overflow validation and Q5.27 range clamping - Add crossover frequency bounds validation (20-20000 Hz) Documentation: - Add comprehensive doxygen-style documentation to tas5805m_biamp.h - Add comprehensive doxygen-style documentation to tas5805m_biamp.c - Add ADVANCED_BIAMP_USER_GUIDE.md with full feature documentation
Mobile UI improvements: - Add responsive navigation that stacks vertically on mobile - Add responsive styles for EQ settings (PEQ, radio buttons, presets) - Enhance shared styles.css with mobile breakpoints - Improve touch targets and prevent iOS zoom on input focus Bug fix: - Add defer attribute to all script tags to prevent "settingsUI is not defined" race condition on fast page loads
The function uses tas5805m_write_biquad_coefficients which is only available when EQ support is enabled. Wrap the implementation in a preprocessor conditional to avoid implicit declaration errors when building without EQ support.
- Remove defer from script tags to ensure scripts load before DOMContentLoaded - Add automatic retry with 500ms delay for transient failures - Add 100ms delay between EQ schema/settings requests to reduce ESP32 memory pressure - Show actual error messages and add Retry button for user recovery - Add check for getRequest function availability on DAC page
- Keep max_open_sockets at 7 (LWIP_MAX_SOCKETS limit) - Add recv/send timeouts (5s) to cleanup idle connections faster - Increase backlog_conn to 10 for pending connection queue - Add Connection: close header to large schema responses (DAC/EQ) to free sockets immediately after 36KB/64KB transfers
- Add missing Connection: close header to EQ settings handler (16KB response) - Increase delay between EQ schema and settings requests from 100ms to 300ms - Allows ESP32 memory recovery after 64KB schema response before 16KB settings
The fault monitoring task was crashing with stack overflow when faults were detected. The tas5805m_decode_faults() function can make up to 12 ESP_LOGW calls with formatted strings, each consuming 200-400 bytes of stack for vsnprintf operations. Combined with I2C operations and task overhead, this exceeded the 2048 byte allocation. Increased stack from 2048 to 4096 bytes to provide adequate headroom.
|
@craigmillard86 can you include screeshot here for reference as well. I'm porting some of that into Raspberry Pi kernel driver, looking which options can be of use in Bi-Amp setup. |
No Problem, attached
|
# Conflicts: # components/lightsnapcast/player.c
|
Ok, so I missed that one and now there are conflicts @craigmillard86 |
|
@CarlosDerSeher have removed all the merge conflicts |
There was a problem hiding this comment.
Pull request overview
This PR adds an “Advanced Bi-Amp” DSP configuration path for TAS5805M (active crossover + per-output PEQ + loudness), plus corresponding preset export/import/reset APIs and substantial web UI + HTTP server robustness improvements (mobile reliability, retries, socket cleanup).
Changes:
- Introduces advanced bi-amp DSP + persistence (NVS), loudness compensation, and preset export/import/reset plumbing.
- Extends EQ web UI to render the new Advanced Bi-Amp schema layout (sections/subgroups/radio) and adds mobile responsiveness + retry/debounce behavior.
- Improves HTTP server reliability (timeouts/backlog, large-response connection close) and tightens query parsing/CORS behavior.
Reviewed changes
Copilot reviewed 14 out of 16 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| partitions.csv | Fixes missing partition index for ota_1. |
| docs/ADVANCED_BIAMP_USER_GUIDE.md | Adds end-user documentation for advanced bi-amp crossover/PEQ/loudness/presets. |
| components/ui_http_server/ui_http_server.c | Hardens URL decode + query parsing, modifies CORS handling, adds bi-amp preset/reset endpoints, adjusts HTTP server config/timeouts. |
| components/ui_http_server/html/styles.css | Adds mobile responsiveness improvements for shared styles. |
| components/ui_http_server/html/settings-ui.js | Adds radio controls, conditional visibility, and grouped renderers for bi-amp/loudness UI layouts. |
| components/ui_http_server/html/index.html | Adds responsive navigation styling for mobile. |
| components/ui_http_server/html/eq-settings.html | Adds bi-amp UI rendering, preset buttons, retry logic, and debounced setting updates. |
| components/ui_http_server/html/dac-settings.html | Adds retry logic and better error messaging if scripts aren’t loaded. |
| components/tas5805m_settings/tas5805m_settings.c | Adds advanced bi-amp + loudness settings persistence/apply, schema generation updates, and preset import/export/reset. |
| components/tas5805m_settings/tas5805m_biamp.c | New DSP/biquad coefficient generation and targeted apply helpers for advanced bi-amp. |
| components/tas5805m_settings/include/tas5805m_settings.h | Adds types/NVS keys/APIs for bi-amp, loudness, and presets; introduces new EQ UI mode. |
| components/tas5805m_settings/include/tas5805m_biamp.h | New public bi-amp DSP API and band allocation definitions. |
| components/tas5805m_settings/CMakeLists.txt | Adds tas5805m_biamp.c to the component build. |
| components/lightsnapcast/player.c | Minor formatting change. |
| components/custom_board/tas5805m/tas5805m.c | Adds I2C mutexing, fault task stack increase, loudness-on-volume-change hook, and biquad feedback sign convention adjustment. |
| components/custom_board/CMakeLists.txt | Adds dependency/include path wiring for tas5805m_settings when TAS5805M is enabled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| strncmp(origin, "http://127.", 11) == 0 || | ||
| strncmp(origin, "http://192.168.", 15) == 0 || | ||
| strncmp(origin, "http://10.", 10) == 0 || | ||
| strncmp(origin, "http://172.", 11) == 0) { | ||
| httpd_resp_set_hdr(req, "Access-Control-Allow-Origin", origin); |
There was a problem hiding this comment.
The Origin allowlist check is vulnerable to bypass because it only does a prefix match. For example, an Origin like "http://192.168.evil.com" or "http://10.evil.com" matches these prefixes even though it’s not a private IP, which would unintentionally grant CORS access. Consider properly parsing the Origin (scheme/host/port) and only allowing hostnames == localhost/127.0.0.1 or literal IPv4/IPv6 addresses in RFC1918 ranges (and restrict 172.* to 172.16.0.0–172.31.255.255).
| int ret = httpd_req_recv(req, buf, req->content_len); | ||
| if (ret <= 0) { | ||
| free(buf); | ||
| if (ret == HTTPD_SOCK_ERR_TIMEOUT) { | ||
| httpd_resp_set_status(req, "408 Request Timeout"); | ||
| httpd_resp_sendstr(req, "{\"error\": \"Request timeout\"}"); | ||
| } else { | ||
| httpd_resp_set_status(req, "500 Internal Server Error"); | ||
| httpd_resp_sendstr(req, "{\"error\": \"Failed to read request body\"}"); | ||
| } | ||
| return ESP_OK; | ||
| } | ||
| buf[ret] = '\0'; | ||
|
|
||
| ESP_LOGI(TAG, "%s: Received preset JSON (%d bytes)", __func__, ret); | ||
|
|
There was a problem hiding this comment.
post_biamp_preset_handler calls httpd_req_recv(req, buf, req->content_len) only once. httpd_req_recv can return a partial read, which would truncate the JSON and cause import failures or inconsistent behavior. Consider reading in a loop until req->content_len bytes are received (or error), and validate ret == req->content_len before parsing.
| int ret = httpd_req_recv(req, buf, req->content_len); | |
| if (ret <= 0) { | |
| free(buf); | |
| if (ret == HTTPD_SOCK_ERR_TIMEOUT) { | |
| httpd_resp_set_status(req, "408 Request Timeout"); | |
| httpd_resp_sendstr(req, "{\"error\": \"Request timeout\"}"); | |
| } else { | |
| httpd_resp_set_status(req, "500 Internal Server Error"); | |
| httpd_resp_sendstr(req, "{\"error\": \"Failed to read request body\"}"); | |
| } | |
| return ESP_OK; | |
| } | |
| buf[ret] = '\0'; | |
| ESP_LOGI(TAG, "%s: Received preset JSON (%d bytes)", __func__, ret); | |
| int total_read = 0; | |
| while (total_read < req->content_len) { | |
| int ret = httpd_req_recv(req, buf + total_read, req->content_len - total_read); | |
| if (ret <= 0) { | |
| free(buf); | |
| if (ret == HTTPD_SOCK_ERR_TIMEOUT) { | |
| httpd_resp_set_status(req, "408 Request Timeout"); | |
| httpd_resp_sendstr(req, "{\"error\": \"Request timeout\"}"); | |
| } else { | |
| httpd_resp_set_status(req, "500 Internal Server Error"); | |
| httpd_resp_sendstr(req, "{\"error\": \"Failed to read request body\"}"); | |
| } | |
| return ESP_OK; | |
| } | |
| total_read += ret; | |
| } | |
| if (total_read != req->content_len) { | |
| free(buf); | |
| httpd_resp_set_status(req, "500 Internal Server Error"); | |
| httpd_resp_sendstr(req, "{\"error\": \"Incomplete request body\"}"); | |
| return ESP_OK; | |
| } | |
| buf[total_read] = '\0'; | |
| ESP_LOGI(TAG, "%s: Received preset JSON (%d bytes)", __func__, total_read); |
| #if CONFIG_DAC_TAS5805M | ||
| // Allocate buffer for request body | ||
| char *buf = (char *)malloc(req->content_len + 1); | ||
| if (!buf) { | ||
| ESP_LOGE(TAG, "%s: Failed to allocate buffer for request body", __func__); | ||
| httpd_resp_set_status(req, "500 Internal Server Error"); | ||
| httpd_resp_sendstr(req, "{\"error\": \"Memory allocation failed\"}"); | ||
| return ESP_OK; | ||
| } |
There was a problem hiding this comment.
post_biamp_preset_handler allocates req->content_len + 1 without any upper bound. A large Content-Length could trigger OOM/reset on memory-constrained devices. Consider enforcing a reasonable maximum preset size (and returning 413 Payload Too Large), and/or streaming to a fixed-size buffer with length checks.
| const exportBtn = document.createElement('button'); | ||
| exportBtn.className = 'preset-btn export-btn'; | ||
| exportBtn.textContent = 'Export Preset'; | ||
| exportBtn.onclick = async () => { | ||
| try { | ||
| const response = await fetch('/api/biamp/preset'); | ||
| if (!response.ok) throw new Error('Export failed'); | ||
| const blob = await response.blob(); | ||
| const url = URL.createObjectURL(blob); | ||
| const a = document.createElement('a'); | ||
| a.href = url; | ||
| a.download = 'biamp_preset.json'; | ||
| document.body.appendChild(a); | ||
| a.click(); | ||
| document.body.removeChild(a); | ||
| URL.revokeObjectURL(url); | ||
| } catch (err) { | ||
| alert('Failed to export preset: ' + err.message); | ||
| } | ||
| }; | ||
| presetButtonsDiv.appendChild(exportBtn); | ||
|
|
||
| const importBtn = document.createElement('button'); | ||
| importBtn.className = 'preset-btn import-btn'; | ||
| importBtn.textContent = 'Import Preset'; | ||
| importBtn.onclick = () => { | ||
| const input = document.createElement('input'); | ||
| input.type = 'file'; | ||
| input.accept = '.json'; | ||
| input.onchange = async (e) => { | ||
| const file = e.target.files[0]; | ||
| if (!file) return; | ||
| try { | ||
| const text = await file.text(); | ||
| const response = await fetch('/api/biamp/preset', { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: text | ||
| }); | ||
| if (!response.ok) throw new Error('Import failed'); | ||
| alert('Preset imported successfully! Refreshing...'); | ||
| location.reload(); | ||
| } catch (err) { | ||
| alert('Failed to import preset: ' + err.message); | ||
| } | ||
| }; | ||
| input.click(); | ||
| }; | ||
| presetButtonsDiv.appendChild(importBtn); | ||
|
|
||
| const resetBtn = document.createElement('button'); | ||
| resetBtn.className = 'preset-btn reset-btn'; | ||
| resetBtn.textContent = 'Reset to Defaults'; | ||
| resetBtn.onclick = async () => { | ||
| if (!confirm('Reset all Advanced Bi-Amp settings to defaults?\n\nThis will clear all crossover, PEQ, and loudness settings.')) return; | ||
| try { | ||
| const response = await fetch('/api/biamp/reset', { method: 'POST' }); | ||
| if (!response.ok) throw new Error('Reset failed'); | ||
| alert('Settings reset to defaults! Refreshing...'); |
There was a problem hiding this comment.
The preset Export/Import/Reset actions use relative URLs like /api/biamp/preset and /api/biamp/reset. When the UI is used with the ?backend= query parameter (cross-origin dev setup), these requests will go to the UI origin instead of the selected backend. Use getBackendUrl() consistently for these fetches (and for sendBeacon below) so the feature works in the same scenarios as the rest of the page.
| // Flush any pending debounced updates (called on page unload) | ||
| function flushPendingUpdates() { | ||
| Object.keys(debounceTimers).forEach(key => { | ||
| if (debounceTimers[key]) { | ||
| clearTimeout(debounceTimers[key].timerId); | ||
| // Send the pending update synchronously | ||
| const { value } = debounceTimers[key]; | ||
| navigator.sendBeacon('/api/eq/settings', JSON.stringify({ [key]: value })); |
There was a problem hiding this comment.
flushPendingUpdates() uses navigator.sendBeacon('/api/eq/settings', ...), which ignores the selected backend from getBackendUrl(). In ?backend= mode this will send updates to the wrong host (or fail entirely). Consider targeting the same backend URL as other requests, or disable beacons when a cross-origin backend is configured.
| // Flush any pending debounced updates (called on page unload) | |
| function flushPendingUpdates() { | |
| Object.keys(debounceTimers).forEach(key => { | |
| if (debounceTimers[key]) { | |
| clearTimeout(debounceTimers[key].timerId); | |
| // Send the pending update synchronously | |
| const { value } = debounceTimers[key]; | |
| navigator.sendBeacon('/api/eq/settings', JSON.stringify({ [key]: value })); | |
| // Resolve the settings update endpoint using the configured backend when available | |
| function getSettingsUpdateUrl() { | |
| const backendBase = typeof getBackendUrl === 'function' ? getBackendUrl() : window.location.origin; | |
| return new URL('/api/eq/settings', backendBase || window.location.origin).toString(); | |
| } | |
| // Send a pending update during page unload, honoring the selected backend | |
| function sendPendingUpdateOnUnload(key, value) { | |
| const url = getSettingsUpdateUrl(); | |
| const payload = JSON.stringify({ [key]: value }); | |
| const targetUrl = new URL(url, window.location.href); | |
| const isSameOrigin = targetUrl.origin === window.location.origin; | |
| if (isSameOrigin && typeof navigator.sendBeacon === 'function') { | |
| return navigator.sendBeacon( | |
| targetUrl.toString(), | |
| new Blob([payload], { type: 'application/json' }) | |
| ); | |
| } | |
| fetch(targetUrl.toString(), { | |
| method: 'POST', | |
| headers: { 'Content-Type': 'application/json' }, | |
| body: payload, | |
| keepalive: true, | |
| mode: isSameOrigin ? 'same-origin' : 'cors', | |
| credentials: isSameOrigin ? 'same-origin' : 'omit' | |
| }).catch(() => {}); | |
| return true; | |
| } | |
| // Flush any pending debounced updates (called on page unload) | |
| function flushPendingUpdates() { | |
| Object.keys(debounceTimers).forEach(key => { | |
| if (debounceTimers[key]) { | |
| clearTimeout(debounceTimers[key].timerId); | |
| // Send the pending update to the configured backend | |
| const { value } = debounceTimers[key]; | |
| sendPendingUpdateOnUnload(key, value); |
| esp_err_t ret = ESP_OK; | ||
| tas5805m_biquad_coeffs_t coeffs; | ||
| int num_stages = tas5805m_biamp_get_biquad_count(settings->slope); | ||
| float fc = (float)settings->crossover_freq; | ||
| float fs = (float)validate_sample_rate(settings->sample_rate); | ||
| int band; | ||
|
|
||
| ESP_LOGI(TAG, "Applying bi-amp crossover: fc=%dHz, slope=%ddB/oct, type=%s, fs=%.0fHz", | ||
| settings->crossover_freq, | ||
| num_stages * 12, | ||
| settings->type == BIAMP_TYPE_LINKWITZ_RILEY ? "LR" : "BW", | ||
| fs); | ||
|
|
||
| /* ========== LEFT CHANNEL (WOOFER/LOW OUTPUT) ========== */ | ||
|
|
||
| /* Band 0: Gain + optional phase inversion */ | ||
| float low_total_gain = (float)settings->low_gain / 2.0f; /* Convert from x2 format */ | ||
| if (settings->low_phase_invert) { | ||
| /* Combine gain and phase invert into single coefficient */ | ||
| low_total_gain = -powf(10.0f, low_total_gain / 20.0f); | ||
| coeffs.b0 = low_total_gain; | ||
| coeffs.b1 = 0.0f; | ||
| coeffs.b2 = 0.0f; | ||
| coeffs.a1 = 0.0f; | ||
| coeffs.a2 = 0.0f; | ||
| } else { | ||
| ret = tas5805m_calc_gain(low_total_gain, &coeffs); | ||
| if (ret != ESP_OK) return ret; | ||
| } | ||
| ret = write_biquad_band(TAS5805M_EQ_CHANNELS_LEFT, BIAMP_GAIN_BAND, &coeffs); | ||
| if (ret != ESP_OK) return ret; | ||
|
|
||
| /* Band 1: Subsonic highpass filter (placed early for optimal headroom) */ | ||
| if (settings->subsonic_freq > 0 && settings->subsonic_freq < fs / 2) { | ||
| ret = tas5805m_calc_butterworth_hpf((float)settings->subsonic_freq, fs, &coeffs); | ||
| if (ret != ESP_OK) return ret; | ||
| ESP_LOGI(TAG, "Applying subsonic HPF at %d Hz (band %d)", settings->subsonic_freq, BIAMP_SUBSONIC_BAND); | ||
| } else { | ||
| ret = tas5805m_calc_passthrough(&coeffs); | ||
| if (ret != ESP_OK) return ret; | ||
| } | ||
| ret = write_biquad_band(TAS5805M_EQ_CHANNELS_LEFT, BIAMP_SUBSONIC_BAND, &coeffs); | ||
| if (ret != ESP_OK) return ret; | ||
|
|
||
| /* Bands 2-5: Lowpass crossover filter stages */ | ||
| ret = tas5805m_calc_butterworth_lpf(fc, fs, &coeffs); | ||
| if (ret != ESP_OK) return ret; | ||
|
|
||
| for (int i = 0; i < num_stages; i++) { | ||
| band = BIAMP_CROSSOVER_START_BAND + i; | ||
| ret = write_biquad_band(TAS5805M_EQ_CHANNELS_LEFT, band, &coeffs); | ||
| if (ret != ESP_OK) return ret; |
There was a problem hiding this comment.
settings->type (Butterworth vs Linkwitz-Riley) is currently only used for logging; the actual crossover coefficients always come from tas5805m_calc_butterworth_lpf/hpf cascaded num_stages times. This makes the “Type” setting effectively a no-op, and also means a 4th/8th-order Butterworth response is not implemented correctly (it requires different pole/Q sections per stage, not identical 2nd-order Butterworth sections). Either implement distinct coefficient sets per type/order or remove the UI/type option to avoid misleading users.
| * Band allocation per channel: | ||
| * | ||
| * LEFT (Woofer/Low output): | ||
| * - Band 0: Gain + optional phase inversion | ||
| * - Band 1: Subsonic highpass filter (rumble protection) | ||
| * - Bands 2-5: Lowpass crossover filter stages | ||
| * - Bands 6-8: Parametric EQ (3 bands for driver correction) | ||
| * - Band 12: Baffle step compensation (low shelf boost) | ||
| * - Bands 13-14: Reserved for loudness compensation |
There was a problem hiding this comment.
The band allocation in the function header comment is outdated/inconsistent with the implementation and the new 6-band PEQ UI. The code loops i < BIAMP_PEQ_BANDS (6) and writes bands 6–11, but the comment still says “Bands 6-8: Parametric EQ (3 bands …)”. Please update the comment to match the actual 6-band allocation to avoid confusion when maintaining/debugging the DSP mapping.
| // TAS5805M uses addition convention for feedback: y = b0*x + b1*x1 + b2*x2 + a1*y1 + a2*y2 | ||
| // Standard DSP uses subtraction: y = b0*x + b1*x1 + b2*x2 - a1*y1 - a2*y2 | ||
| // So we negate a1 and a2 to convert from standard DSP convention to TAS5805M convention | ||
| float coeffs[] = {b0, b1, b2, -a1, -a2}; | ||
| const char *names[] = {"B0", "B1", "B2", "A1", "A2"}; |
There was a problem hiding this comment.
tas5805m_write_biquad_coefficients now negates a1/a2 to accept standard DSP “subtract feedback” coefficients, but tas5805m_read_biquad_coefficients still returns the raw stored values (TAS “add feedback” convention). This makes the read/write APIs non-inverses and can confuse any caller that expects to round-trip coefficients. Consider either (a) also negating a1/a2 on read so callers always see standard DSP convention, or (b) documenting in the public header that read returns TAS-convention feedback terms.
| /* Use spinlock to prevent TOCTOU race if init is called concurrently */ | ||
| portENTER_CRITICAL(&tas5805m_init_lock); | ||
| if (tas5805m_i2c_mutex == NULL) { | ||
| tas5805m_i2c_mutex = xSemaphoreCreateRecursiveMutex(); | ||
| if (tas5805m_i2c_mutex == NULL) { | ||
| portEXIT_CRITICAL(&tas5805m_init_lock); | ||
| ESP_LOGE(TAG, "%s: Failed to create I2C mutex", __func__); | ||
| return ESP_ERR_NO_MEM; | ||
| } | ||
| } | ||
| portEXIT_CRITICAL(&tas5805m_init_lock); |
There was a problem hiding this comment.
tas5805m_init() creates the recursive mutex while inside portENTER_CRITICAL(). Creating a semaphore/mutex can allocate memory and call FreeRTOS internals, which is generally discouraged inside a critical section (and can increase interrupt-off time). Consider creating the mutex outside the critical section, then using the critical section only to publish it if still NULL (double-checked init), or initializing it at startup before concurrency is possible.
| /* Use spinlock to prevent TOCTOU race if init is called concurrently */ | |
| portENTER_CRITICAL(&tas5805m_init_lock); | |
| if (tas5805m_i2c_mutex == NULL) { | |
| tas5805m_i2c_mutex = xSemaphoreCreateRecursiveMutex(); | |
| if (tas5805m_i2c_mutex == NULL) { | |
| portEXIT_CRITICAL(&tas5805m_init_lock); | |
| ESP_LOGE(TAG, "%s: Failed to create I2C mutex", __func__); | |
| return ESP_ERR_NO_MEM; | |
| } | |
| } | |
| portEXIT_CRITICAL(&tas5805m_init_lock); | |
| /* Create outside the critical section, then publish under the spinlock. */ | |
| if (tas5805m_i2c_mutex == NULL) { | |
| SemaphoreHandle_t new_mutex = xSemaphoreCreateRecursiveMutex(); | |
| if (new_mutex == NULL) { | |
| ESP_LOGE(TAG, "%s: Failed to create I2C mutex", __func__); | |
| return ESP_ERR_NO_MEM; | |
| } | |
| portENTER_CRITICAL(&tas5805m_init_lock); | |
| if (tas5805m_i2c_mutex == NULL) { | |
| tas5805m_i2c_mutex = new_mutex; | |
| new_mutex = NULL; | |
| } | |
| portEXIT_CRITICAL(&tas5805m_init_lock); | |
| if (new_mutex != NULL) { | |
| vSemaphoreDelete(new_mutex); | |
| } | |
| } |
|
|
||
| #### Treble Boost (per zone) | ||
| - **Range:** -12 to +12 dB | ||
| - **Frequency:** High shelf at ~5 kHz (tweeter channel) |
There was a problem hiding this comment.
The guide states the loudness treble boost uses a high shelf at ~5 kHz, but the implementation in tas5805m_loudness_apply() uses a 4000 Hz shelf. Please align the documentation with the actual corner frequency (or update the code if 5 kHz is intended).
| - **Frequency:** High shelf at ~5 kHz (tweeter channel) | |
| - **Frequency:** High shelf at ~4 kHz (tweeter channel) |
PR: Advanced TAS58505 Bi-Amp Crossover with PEQ, Loudness Compensation, and UI Improvements
Summary
This PR introduces a comprehensive advanced bi-amp crossover system with parametric EQ, loudness compensation, and significant improvements to the web UI and HTTP server reliability.
Key Features
Bug Fixes