Audio: Add enum controls to beamformer #3780
Conversation
lgirdwood
left a comment
lgirdwood
left a comment
There was a problem hiding this comment.
LGTM, but needs some more comments and how to ID the kcontrols better.
src/audio/tdfb/tdfb.c
Outdated
There was a problem hiding this comment.
Can we not assign an ID to each control as using index is easily broken ? @juimonen ?
There was a problem hiding this comment.
I thought this was the standard? Given we have id index.
There was a problem hiding this comment.
I will need to remove enum controls add from topology to make this work with current kernel. I'd like to leave this to FW though. It's best that @juimonen proposes how to do, e.g. is this approach seen in this PR OK.
There was a problem hiding this comment.
What is the kernel limitation?
src/audio/tdfb/tdfb.c
Outdated
There was a problem hiding this comment.
I thought this was the standard? Given we have id index.
src/audio/tdfb/tdfb.c
Outdated
There was a problem hiding this comment.
if setup failed do we fall back to the old values or enter a broken state?
There was a problem hiding this comment.
The return value can be -EINVAL or -ENOMEM. The pipeline should stop the copy() scheduring if a component returns error. I haven't tested it though. I'll check it happens.
singalsu
force-pushed
the
tdfb_add_enum_control_newtry
branch
from
c85a0ca to
6b43600
Compare
singalsu
requested review from
a team,
dbaluta,
lbetlej,
mmaka1,
plbossart,
ranj063 and
xiulipan
as code owners
|
SOFCI TEST |
|
@xiulipan Can SOFCI TEST still be used to re-launch the tests? The Travis CI build has been in progress for 2 days already. |
|
@singalsu please ignore Travis atm, were moving away to GH actions - I should make this a non requirement now. |
singalsu
force-pushed
the
tdfb_add_enum_control_newtry
branch
from
6b43600 to
88d64f0
Compare
|
I just updated Matlab scripts. Thanks to @ShriramShastry for testing and reporting issues! |
singalsu
force-pushed
the
tdfb_add_enum_control_newtry
branch
from
88d64f0 to
aaef4d1
Compare
|
I'm debugging an issue with 4ch array, 2 of 4 filters are not what they should be. I hope to be able to spot and fix it soon, apparently blob packing or FW code issue. Edit: After debugging and testing I found that it was fortunately a mistake in my test. |
This patch adds the enums set/get controls. The configuration blob is updated to pass filters settings for multiple angles, a bypass coefficients set, and microphones (x, y, z) coordinates for direction enum get (implemented later). Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
This patch prevents topology parse failure when there are multiple controls, e.g. enum controls. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
This patch improves the beamformer performance (vs. tap count) by shifting the extract window location for IFFT output by mean of all impulse responses center. Previously the first filter was centered and other filters were more towards start or end of impulse response. The change helps to cope with smaller filter length with larger size arrays with steer angles those spread the impulse response further away time-wise from each other. Signed-off-by: singalsu@linux.intel.com <seppo.ingalsuo@linux.intel.com>
The plots help to understand beamformer channel filters characteristic. Frequency response is usually all-pass type and group delays match the delays need to time-align the waveforms from steer angle. Signed-off-by: singalsu@linux.intel.com <seppo.ingalsuo@linux.intel.com>
This patch fixes the incorrect delaying of simulation input signal to microphones. Instead of delaying the old code advanced the input the more the larger the input is. E.g. It caused the tests for line arrays to show a mirrored beam pattern. The num_filters is replaced by mic_n (microphones count). It is the correct count to use here though usually filters count is the same. Signed-off-by: singalsu@linux.intel.com <seppo.ingalsuo@linux.intel.com>
This patch essentially adds support for steer_az/steer_el to be vector instead of scalar. The beam off preset is added by default automatically for N-channel in N-channel out configurations. Microphone (x,y,z) coordinates are passed in to blob for direction of arrival reporting. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
Matlab does not support isfile(), so another command is used to check if a file exists. The new way works both in Octave and Matlab. The bf_defaults.m is fixed to properly initialize empty strings to cell arrays. It's needed with minimal command like usage as shown in SOF Docs examples. In bf_design.m the num_filters determination is moved after array helpers because other than line and circle do not use mic_n parameter. The bug resulted to zero num_filters. In bf_merge.m the unused bf3 and bf4 are removed. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
The delay-and-sum beamformer has highest white noise gain (WNG) performance. It's good id suppressing microphone self noise. The beam is steered with pure delay adjust in the all-pass filter bank. Default type is still super directive. Use DSB or SDB as beamformer type to select. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
This patch contains updates to add beam on/off and direction controls. The common m4 tasks to add controls are placed to include files tdfb_controls.m4, tdfb_defines.m4, tdfb_undefines.m4. The single beam examples are replaced by blobs with 0, +/-30, and +/-90 degree angled beams. The Arrays are 50m, 68mm spaced for typical notebook microphones. The 28mm example is for four microphones. A four microphones for notebook with microphones at 0, 36, 146, and 182mm line locations. The add of enum controls is currently commented out from tdfb.m4. Please uncomment to test with multiple controls capable kernel. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
singalsu
force-pushed
the
tdfb_add_enum_control_newtry
branch
from
aaef4d1 to
74afbc6
Compare
|
Please resubmit with "main" as PR base branch. |
4 participants
I created a new PR to replace #3685 that I had created incorrectly from a sof repository branch.
I still need to revert the enum control from topologies to be able to pass CI and make the topologies compatible with current Linux kernel. Linux needs a separate patch for multiple controls.