(Experimental) Add Left Only / Right Only channel modes - #235
Open
W-Floyd wants to merge 10 commits into
Open
Conversation
Adds a runtime-settable channel routing mode (Stereo / Left Only / Right Only) that duplicates one input channel to both outputs before DSP processing. Persisted to NVS and exposed in the web UI as a dropdown. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the DSP sample-duplication approach with I2S SLOT_MODE_MONO: - Remove the channel routing loop from dsp_processor_worker - Add make_slot_cfg() helper consolidating MSB/Philips/32-bit slot config - Add decimate_stereo_inplace() which compacts L or R samples in-place and heap_caps_realloc()s each fragment to half size, returning unused bytes to the heap immediately before the chunk enters the queue - Add player_apply_channel_mode() called in main.c after DSP and before insert_pcm_chunk for all three codecs (OPUS, FLAC, PCM) - player_setup_i2s() selects MONO or STEREO slot mode from channel mode - Player task detects channel mode changes at runtime and calls i2s_channel_reconfig_std_slot() with mute/disable/enable bracketing - framesToBytes uses eff_ch=1 in mono mode so byte-to-frame timing is correct In mono mode the I2S driver sends the single decimated sample to both L and R slots. DMA buffers and queued chunk payloads are halved in size (~85 KB heap + ~4.5 KB IRAM saved at 44100 Hz / 1000 ms buffer). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- dsp_processor.c: move s_channel_mode + getter/setter outside #if CONFIG_USE_DSP_PROCESSOR so they always link; protect with a portMUX spinlock so HTTP and player tasks can't race on the value - dsp_types.h: add DSP_CH_MODE_MAX sentinel to the enum - dsp_processor_settings.c: reject out-of-range channel_mode values from both JSON (HTTP POST) and NVS to prevent invalid enums being stored persistently and surviving reboots - player.c: mark s_i2s_mode volatile; make player_apply_channel_mode read s_i2s_mode directly so the decimation decision is always consistent with the framesToBytes calculation in player_task, eliminating the TOCTOU sync drift on mode switches; also fix the dead #else branch which still had ch+bps (addition) instead of ch*bps (multiplication) and lacked the eff_ch mono adjustment - main.c: restructure all three player_apply_channel_mode call sites so the call is outside #if CONFIG_USE_DSP_PROCESSOR — I2S slot reconfiguration is unconditional so decimation must be too, otherwise a non-DSP build sends stereo PCM to a mono-configured DMA slot Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…d expand Replace mono I2S slot approach with: decimate to mono on decode (halves each queued chunk's fragment allocation), then expand back to stereo just before the I2S write (L=R=the selected channel). I2S stays in SLOT_MODE_STEREO so both physical outputs always receive signal, regardless of DAC behaviour. Memory profile: chunks in the PCM queue use ~50% of their normal allocation; only the one chunk currently being played briefly holds a full stereo buffer during expand_mono_to_stereo_inplace. The expand is a single backwards in-place realloc+loop, so no separate bounce buffer is needed. Add bool mono to pcm_chunk_message_t (calloc-zeroed, so existing alloc paths default to false). player_apply_channel_mode sets it after decimation; player_task clears it after expanding. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The root cause of the original issue: I2S_STD_PHILIPS_SLOT_DEFAULT_CONFIG with MONO mode defaults slot_mask to LEFT, so only the left output pin carries the decimated sample — the right is silent. Fix: add cfg.slot_mask = I2S_STD_SLOT_BOTH in make_slot_cfg. With MONO+BOTH the I2S hardware duplicates the single DMA sample to both L and R outputs, which is exactly the desired behaviour for left-only / right-only modes. Also reverts the intermediate decimate+expand approach (no longer needed): removes expand_mono_to_stereo_inplace and the bool mono chunk field, and restores the MONO slot switching + s_i2s_mode tracking that was correct in concept but missing the slot_mask override. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
dsp_processor.c: move dsp_types.h and esp_log.h above the #if CONFIG_USE_DSP_PROCESSOR guard so the channel mode getter/setter (which live outside the guard) can see dsp_channel_mode_t and ESP_LOGW. Drop the now-duplicate esp_log.h from inside the guard. lightsnapcast/CMakeLists.txt: add dsp_processor to REQUIRES so that player.c can include dsp_processor.h. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Channel routing is an I2S hardware concern, not a DSP processing parameter. Remove it from dsp_processor entirely and consolidate in settings_manager alongside other general device settings: - Define dsp_channel_mode_t in player.h (its natural owner) - Remove it from dsp_types.h; drop getter/setter/spinlock from dsp_processor - Remove dsp_settings_save/load_channel_mode and JSON handling from dsp_processor_settings - Add settings_get/set_channel_mode (NVS-backed) to settings_manager; include channel_mode in settings_get/set_from_json - player.c reads via settings_get_channel_mode through a local helper; lightsnapcast now depends on settings_manager instead of dsp_processor - ui_http_server calls settings_set_channel_mode directly instead of routing through a DSP JSON wrapper - Drain the PCM queue before updating s_i2s_mode on slot-mode switch to prevent stereo chunks from being played with mono framesToBytes Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
W-Floyd
commented
Jun 8, 2026
| if (p_payload != NULL) { | ||
| #if 1 | ||
| do { | ||
| size_t framesToBytes = (scSet.ch + (scSet.bits >> 3)); |
Author
There was a problem hiding this comment.
This seems to have been a bug prior?
W-Floyd
commented
Jun 8, 2026
| } | ||
| #endif | ||
| int64_t alreadyWrittenTime_us = 0; | ||
| size_t framesToBytes = (scSet.ch + (scSet.bits >> 3)); |
W-Floyd
marked this pull request as ready for review
June 8, 2026 17:02
Adds a fourth channel mode that averages left and right input samples into a single mono signal routed to both outputs. Implementation follows the same I2S MONO+BOTH-mask path as left/right-only modes. mix_stereo_to_mono_inplace() handles 16-bit (int32 accumulator) and 32-bit (int64 accumulator) samples; falls back to left channel for other bit depths. Validation in settings_set_channel_mode updated to accept values 0-3. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Author
|
For mono speakers, we could also add a MONO mode that mixes to mono before buffering. So CPU overhead but reduced memory usage - thoughts? |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
W-Floyd
force-pushed
the
feat/stereo-split-experimental
branch
from
June 8, 2026 17:36
2c0fca9 to
2a2b6da
Compare
Contributor
|
I think mono mixdown is already available in dsp processor. |
Owner
|
Yes I think I've done this in the past already. |
Author
|
Ah yeah, This differs by mixing down before buffering, rather than after all other DSP. |
…H_MONO The compile-time mono mixdown is redundant now that the runtime channel_mode setting (DSP_CH_MONO) covers the same behavior with more flexibility and correct I2S slot mode reconfiguration. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Alternative to #230, implementing the second suggestion in #230 (comment)
In theory this reduces memory usage a lot (50% of buffer data), would love to quantify exactly how useful this might be.
If/when this is good, I can remove support for run-time changes to help simplify (like #230)
Adds a Channel Mode setting (Stereo / Left Only / Right Only) that routes a single input channel to both outputs.
When a non-stereo mode is active, I2S is configured in MONO slot mode with slot_mask=BOTH, and the selected channel is decimated from each decoded PCM chunk in-place before it enters the playback queue. This halves queue memory consumption for mono modes.
Mode changes take effect immediately without restart via i2s_channel_reconfig_std_slot — the player task detects the change each loop iteration, briefly mutes, drains stale buffered audio, reconfigures the slot, and resumes.