Audio: Selector: Add support for multiple up/down-mix profiles by singalsu · Pull Request #10613 · thesofproject/sof

singalsu · 2026-03-10T12:47:56Z

No description provided.

singalsu · 2026-03-10T12:51:31Z

WIP - I seem to have still incorrect channels order assumption for 5.1 format.

Update - it wasn't wrong, the cplay in tinycompress wasn't parsing the extended wav header that is used with more than two channels: alsa-project/tinycompress#32

lyakh · 2026-03-11T07:17:19Z

src/audio/selector/selector.c

+					  source_channels, sink_channels);
+				cd->coeffs_config = &cd->multi_coeffs_config[i];
+				break;
+			}


do you need to check the "not found" case?

I could add a warn trace that the default first mix coefficients set is used, or let the prepare fail. I'm not sure what is best. Maybe fail loudly to not have slightly wrong sounding use case, maybe even clipping samples or strange left/right "sound image" shift.

Copilot

Pull request overview

This PR extends the IPC4 selector/micsel coefficient blob format and firmware handling to support receiving multiple up/down-mix profiles in a single set_config() call, and adds a new combined topology blob intended for playback endpoint decoder-offload conversions.

Changes:

Add support in selector IPC4 path for multiple coefficient configurations and runtime selection by (source_channels, sink_channels), plus a “passthrough copy” fast-path.
Update the Octave blob-generation script to emit packed headers per profile and to generate a combined multi-profile blob.
Replace/remove legacy single-purpose downmix blob config files and add a new stereo_endpoint_playback_updownmix.conf multi-profile blob.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tools/topology/topology2/include/components/micsel/stereo_endpoint_playback_updownmix.conf	Adds a new multi-profile selector_config blob for playback endpoint conversions.
tools/topology/topology2/include/components/micsel/downmix_71_to_stereo.conf	Removes legacy single-profile blob (no-LFE variant).
tools/topology/topology2/include/components/micsel/downmix_71_to_mono.conf	Removes legacy single-profile blob (no-LFE variant).
tools/topology/topology2/include/components/micsel/downmix_51_to_stereo.conf	Removes legacy single-profile blob (no-LFE variant).
tools/topology/topology2/include/components/micsel/downmix_51_to_mono.conf	Removes legacy single-profile blob (no-LFE variant).
src/include/sof/audio/selector.h	Extends coeff config header layout and adds constants for Q10 unity and max config count.
src/audio/selector/tune/sof_selector_blobs.m	Generates packed profile headers and a combined multi-profile blob.
src/audio/selector/selector_generic.c	Updates processing calls to use a pointer to the selected coefficient config.
src/audio/selector/selector.c	Allocates storage for multiple configs, parses multi-config blobs, selects profile at prepare time, and adds passthrough fast-path.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/audio/selector/selector.c

src/audio/selector/tune/sof_selector_blobs.m

src/audio/selector/selector.c

This patch extends the struct ipc4_selector_coeffs_config. The first 16-bit reserved field is split into to 8-bit field for source and sink channels count. The configuration is changed to array of the previous structs instead of single. When preparing for stream the array is checked for matching number of channels for source and sink and if found the coefficients for the channels counts is selected into use. The change avoids the need for user space to update the configuration in runtime if the blob used for initialization contains all the needed channels up/down-mix profiles. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

This patch modifies the configuration blobs build script sof_selector_blobs.m to create one blob that contains several of them for the purpose to up or down-mix DSP offloaded decoder output. The blob uses the reserved fields of selector configuration to describe source and sink channel counts and channel config values. The exported blob is "stereo_endpoint_playback_updownmix.conf". The single configuration blobs continue to be with the reserved fields as zeros. The .conf blobs may be removed later when other topologies are modified to not use them. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

This patch adds for micsel component configuration blob "stereo_endpoint_playback_updownmix.conf". It contains the needed up-mix and down-mix coefficients to handle playback of mono, 5.1 and 7.1 channels into stereo endpoint. The old configuration blobs with five and seven channels without LFE channel are removed. Despite the file names with "51" and "71" the blobs are not suitable for 6ch or 8ch handling. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-11T17:30:39Z

src/audio/selector/selector.c

+	if (cd->num_configs > 1) {
+		/* A pass-through blob with two or more configurations is for the max
+		 * channels (8) amount.
+		 */
+		if (source_channels == sink_channels) {
+			source_channels = SEL_SOURCE_CHANNELS_MAX;
+			sink_channels = SEL_SINK_CHANNELS_MAX;
+		}


When cd->num_configs > 1 and source_channels == sink_channels, the code forces both to 8 before searching. This makes the selection logic depend on a hardcoded assumption that the pass-through profile is always 8->8, and it also makes the later log message report 8->8 even if the actual stream is e.g. 2->2. A safer approach is to first try an exact match, and only fall back to 8->8 (or a dedicated passthrough profile) if no exact match exists.

Copilot · 2026-03-11T17:30:39Z

src/include/sof/audio/selector.h

+	uint8_t source_channels_count; /**< source channels count when num_configs != 0 */
+	uint8_t sink_channels_count; /**< sink channels count when num_configs != 0 */
+	uint8_t source_channel_config; /**< source channels config when num_configs != 0 */
+	uint8_t sink_channel_config; /**< sink channels config when num_configs != 0 */


The field comments refer to num_configs, but that variable is not part of struct ipc4_selector_coeffs_config (it's tracked in struct comp_data). Reword these comments to describe when the fields are expected to be populated (e.g., "used when multiple profiles are packed into one blob") to avoid confusion for blob producers/consumers.

Suggested change

uint8_t source_channels_count; /**< source channels count when num_configs != 0 */

uint8_t sink_channels_count; /**< sink channels count when num_configs != 0 */

uint8_t source_channel_config; /**< source channels config when num_configs != 0 */

uint8_t sink_channel_config; /**< sink channels config when num_configs != 0 */

uint8_t source_channels_count; /**< source channels count, used when multiple profiles are packed into one blob */

uint8_t sink_channels_count; /**< sink channels count, used when multiple profiles are packed into one blob */

uint8_t source_channel_config; /**< source channel configuration, used when multiple profiles are packed into one blob */

uint8_t sink_channel_config; /**< sink channel configuration, used when multiple profiles are packed into one blob */

Copilot · 2026-03-11T17:30:39Z

src/audio/selector/tune/sof_selector_blobs.m

+	coeffs_vec = reshape(sel.coeffs, 1, []); % convert to row vector
+	coeffs_q10 = int16(round(coeffs_vec .* 1024)); % Q6.10
+	pack8 = uint8(2 * length(coeffs_q10) + 4);
+	rsvd1 = 0;


rsvd1 is assigned but never used in pack_selector_config(). This looks like leftover from the earlier reserved-field approach; either remove it or use it (e.g., if you intend to optionally zero out the channel-config bytes for single-profile blobs).

Suggested change

rsvd1 = 0;

Copilot · 2026-03-11T17:30:40Z

src/audio/selector/tune/sof_selector_blobs.m

+	pack8 = uint8(2 * length(coeffs_q10) + 4);
+	rsvd1 = 0;

-	blob8 = sof_selector_build_blob(sel);
-	tplg2_fn = sprintf("%s/%s.conf", sof_tplg_selector, blobname);
-	sof_check_create_dir(tplg2_fn);
-	sof_tplg2_write(tplg2_fn, blob8, str_config, str_exported, str_howto);
-end
+	% header
+	j = 1;
+	pack8(j:j + 1) = uint8(sel.ch_count);
+	j = j + 2;
+	pack8(j:j + 1) = uint8(sel.ch_config);
+	j = j + 2;


pack8 = uint8(2 * length(coeffs_q10) + 4); creates a scalar and then grows the array via indexed assignments. Preallocating pack8 to the final size (e.g., a zeroed uint8 row vector) would avoid dynamic resizing and makes the intended packed layout clearer.

Copilot · 2026-03-11T17:30:40Z

src/audio/selector/selector.c

 	cd = mod_zalloc(mod, sizeof(*cd));
 	if (!cd)
 		return -ENOMEM;

 	cd->sel_ipc4_cfg.init_payload_fmt = payload_fmt;
 	md->private = cd;

+	/* Allocate space for max number of configurations. */
+	cd->multi_coeffs_config_size =
+		SEL_MAX_NUM_CONFIGS * sizeof(struct ipc4_selector_coeffs_config);
+	cd->multi_coeffs_config = mod_zalloc(mod, cd->multi_coeffs_config_size);
+	if (!cd->multi_coeffs_config) {
+		mod_free(mod, cd);
+		return -ENOMEM;
+	}


md->private is assigned before multi_coeffs_config allocation succeeds; on allocation failure you mod_free(mod, cd) but leave md->private pointing to freed memory, which can cause use-after-free / double-free if the framework touches md->private on init failure. Set md->private only after all allocations succeed (or reset it to NULL before returning on failure).

Copilot · 2026-03-11T17:30:40Z

src/audio/selector/selector.c

 static int selector_set_config(struct processing_module *mod, uint32_t config_id,
 			       enum module_cfg_fragment_position pos, uint32_t data_offset_size,
 			       const uint8_t *fragment, size_t fragment_size, uint8_t *response,
 			       size_t response_size)
 {
 	struct comp_data *cd = module_get_private_data(mod);
+	int n;

 	if (config_id == IPC4_SELECTOR_COEFFS_CONFIG_ID) {
-		if (data_offset_size != sizeof(cd->coeffs_config))
+		if (data_offset_size > cd->multi_coeffs_config_size) {
+			comp_err(mod->dev, "The configuration blob is too large");
 			return -EINVAL;
+		}

-		memcpy_s(&cd->coeffs_config, sizeof(cd->coeffs_config), fragment, data_offset_size);
+		/* The size must be N times the coefficient vectors size of one channels
+		 * up/down mix profile.
+		 */
+		n = data_offset_size / sizeof(struct ipc4_selector_coeffs_config);
+		if (data_offset_size != n * sizeof(struct ipc4_selector_coeffs_config)) {
+			comp_err(mod->dev, "Invalid configuration size.");
+			return -EINVAL;
+		}


selector_set_config() accepts a 0-byte config (and doesn't enforce single-fragment delivery). With data_offset_size == 0, n becomes 0 and cd->num_configs is set to 0, which can silently leave stale/default coefficients in use. Consider requiring pos to indicate a single fragment and validating data_offset_size >= sizeof(struct ipc4_selector_coeffs_config) (and n >= 1).

Copilot · 2026-03-11T17:30:40Z

src/audio/selector/selector.c

+		memcpy_s(cd->multi_coeffs_config, cd->multi_coeffs_config_size,
+			 fragment, data_offset_size);
+		cd->num_configs = n;


The memcpy_s() return value is ignored here, and there is no check that fragment_size actually covers data_offset_size. If fragment_size < data_offset_size (or overlap is detected), memcpy_s() will fail and the selector will continue using old coefficients without an error. Check fragment_size/pos and handle a non-zero memcpy_s() return (log + return -EINVAL).

lyakh reviewed Mar 11, 2026

View reviewed changes

singalsu force-pushed the selector_multiblob_support branch from 7dcf377 to b2d33be Compare March 11, 2026 15:47

singalsu marked this pull request as ready for review March 11, 2026 15:51

singalsu requested a review from ranj063 as a code owner March 11, 2026 15:51

Copilot AI review requested due to automatic review settings March 11, 2026 15:51

singalsu requested review from dbaluta, jsarha, kv2019i, lbetlej, lgirdwood, mmaka1, plbossart and tlissows as code owners March 11, 2026 15:51

Copilot started reviewing on behalf of singalsu March 11, 2026 15:52 View session

Copilot AI reviewed Mar 11, 2026

View reviewed changes

singalsu added 3 commits March 11, 2026 19:13

singalsu force-pushed the selector_multiblob_support branch from b2d33be to c7c5e41 Compare March 11, 2026 17:19

singalsu requested a review from Copilot March 11, 2026 17:22

Copilot started reviewing on behalf of singalsu March 11, 2026 17:25 View session

Copilot AI reviewed Mar 11, 2026

View reviewed changes

Conversation

singalsu commented Mar 10, 2026

Uh oh!

singalsu commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lyakh Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

singalsu Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

singalsu commented Mar 10, 2026 •

edited

Loading

singalsu Mar 11, 2026 •

edited

Loading