-
Notifications
You must be signed in to change notification settings - Fork 349
Audio: Selector: Updates to use selector/micsel component for multi-channel audio up/downmix in SOF topologies #10039
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
With this change the component is passed the required extended base config. It was missing from rimage for TGL and TGL-H platforms. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
The mix coefficients are Q6.10 with max amplification of +32 dB, so the result need to be checked and saturated by 16 and 32 bit saturation functions. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
This patch separates processing functions sel_s32le() and sel_s24le(). The common S32_LE and S24_LE format processing is wrong because of different saturation need for the samples. The 32-bit saturation would cause samples with overflow in S24_LE format. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
This allows to test the component basics in testbench. The micsel16/24/32 are stereo in/out topologies. The micsel_multich32 topology is stereo in the DAI copier side and 1-8ch in the host copier side. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
This patch adds script Octave tune/sof_selector_blobs.m. The script execute is also added to scripts/sof-rebuild-processing-comp-blobs.sh. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
d1368ea to
8b278b4
Compare
| cd "$SOF_WORKSPACE"/sof/src/audio/eq_iir/tune; $OCTAVE sof_example_spk_eq.m | ||
| cd "$SOF_WORKSPACE"/sof/src/audio/multiband_drc/tune; $OCTAVE sof_example_multiband_drc.m | ||
| cd "$SOF_WORKSPACE"/sof/src/audio/tdfb/tune; ./sof_example_all.sh | ||
| cd "$SOF_WORKSPACE"/sof/src/audio/selector/tune; $OCTAVE ./sof_selector_blobs.m |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should add micsel test run to scripts/host-testbench.sh to get it tested regularly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also can we add to nocodec topology ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it would be currently useful as the only way to validate in a real device more complex channels changes. With HDA and SDW we are limited with DAI side to 2ch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the audio selector component to support multi-channel upmix/downmix for SOF topologies by incorporating new micsel blob configurations generated by the updated selector script, enhancing mixing routines, and updating associated benchmark and topology files.
- Updated configuration files for various downmix/upmix scenarios using the new selector blobs.
- Enhanced mixing routines in the selector component for S16_LE, S24_LE, and S32_LE formats with proper saturation.
- Updated build and benchmark files to integrate the new micsel component.
Reviewed Changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tools/topology/topology2/include/components/micsel/*.conf | New configuration files for different downmix/upmix paths generated via the selector script. |
| tools/topology/topology2/include/bench/*.conf | Added and modified benchmark configurations to include micsel components. |
| tools/topology/topology2/development/tplg-targets-bench.cmake | Updated to include micsel component and its parameters for benchmark topology builds. |
| tools/rimage/config/tgl*.toml | Added new configuration parameter (init_config) to support updated initialization. |
| src/audio/selector/tune/sof_selector_blobs.m | Updated blob generation for selector with new downmix/upmix configurations. |
| src/audio/selector/selector_generic.c | Revised mixing routines to include saturation functions and split 24-bit processing. |
| scripts/sof-rebuild-processing-comp-blobs.sh | Updated to rebuild selector blobs using the new script. |
| int n_chan_source = MIN(SEL_SOURCE_CHANNELS_MAX, audio_stream_get_channels(source)); | ||
| int n_chan_sink = MIN(SEL_SINK_CHANNELS_MAX, audio_stream_get_channels(sink)); | ||
|
|
||
| while (processed < frames) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like a simple for (processed = 0; processed < frames; processed += n)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like that syntax for this usage, n varies every iteration. Packs two lines but same instructions but with ambiguity for random code reader of when the increment is evaluated. I copied this from similar s32 function. Also this will change when the component is converted to source sink API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The for loop is easier to vectorize for the CC as we can increment by a constant, not an issue for this PR, but long term we need to factor this in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least we can remove the divisions from hot code in next improvements with source sink API. The convert to the new API would be in 2H tasks I think.
| int n_chan_source = MIN(SEL_SOURCE_CHANNELS_MAX, audio_stream_get_channels(source)); | ||
| int n_chan_sink = MIN(SEL_SINK_CHANNELS_MAX, audio_stream_get_channels(sink)); | ||
|
|
||
| while (processed < frames) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The for loop is easier to vectorize for the CC as we can increment by a constant, not an issue for this PR, but long term we need to factor this in.
| cd "$SOF_WORKSPACE"/sof/src/audio/eq_iir/tune; $OCTAVE sof_example_spk_eq.m | ||
| cd "$SOF_WORKSPACE"/sof/src/audio/multiband_drc/tune; $OCTAVE sof_example_multiband_drc.m | ||
| cd "$SOF_WORKSPACE"/sof/src/audio/tdfb/tune; ./sof_example_all.sh | ||
| cd "$SOF_WORKSPACE"/sof/src/audio/selector/tune; $OCTAVE ./sof_selector_blobs.m |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also can we add to nocodec topology ?
|
@lgirdwood I prefer to add to nocodec in separate PR, especially if the next milestone deliverables validation is with nocodec. This should be good to merge? |
See commit messages for the changes.