-
Notifications
You must be signed in to change notification settings - Fork 349
Add multi-microphone beamformer component #2925
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
eeb46f0 to
935a490
Compare
|
@cujomalainey @sebcarlucci any review comments ? |
|
@perexg I just accidentally added you as reviewer but you are welcome if you have time. |
lgirdwood
left a comment
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.
LGTM, need to sort out the ABI and comp type though.
src/include/kernel/abi.h
Outdated
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.
Not sure, this could be 16 - just depends on when kernel PR is approved.
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.
Not sure if ABI needs bump (MINOR is now 17, would be to 18). There's the change to the user header fir.h.
|
I should create soon documentation for this. It may be hard to figure this out without. The improvement in capture SNR is the larger the more microphones, 3 or 4 or even more preferable. With only 2 microphones and steer angle 0 (broadside) it reduces to trivial sum of microphones with only 3 dB SNR improve. Though it might be still audible, I'll do tests here and make some demo recordings. The plots from the tool help to understand more what it is and what it isn't. Here's a 4 mic example steered to 30 deg azimuth, superdirective design criteria (minimize diffuse noise): |
|
Also as noted currently the beam is fixed and defined by blob coefficients. The blob does not currently contain multiple presets. A coefficients presets system would allow some IPC or internal direction of arrival tracker to change the beam direction. Since as it can be seen the beams with lower mic counts are not narrow, a few of them could cover 180 or 360 degree space. I wonder if I should add it. Though multiple pre-defined beams need plenty of .bss section size. With symmetrical 2D arrays it's possible to also simply rotate the mic inputs for changed beam direction. |
|
@lgirdwood @sebcarlucci finished his internship. @chiang831 and @dgreid for thoughts. |
|
@singalsu to confirm your comment, the filter does not spatial tracking, it takes in a target direction and sticks with that? Also, could the beam be disabled at runtime? |
Yep, the single beam or multiple beams are fixed for the azimuth and elevation angle in design phase with the Matlab/Octave tool. So, in current form it works only for use cases like notebook video call where the desired audio capture source is very likely in camera direction. Out of beam audio sources have especially their higher frequencies attenuated. The multiple simultaneous beams are done by designing one beam at time for the mic array and then merging them into the same blob with different output channels mixing (I made such simple stereo image enhance beamformer example as 2mic in -> 2ch out). I've done a Matlab concept for sound source tracking and it seems to work pretty well. In a future version the FW could be let to change the beam direction freely within designed presets. Though there's a risk that it locks to something undesired noisy in the room (fan?). Therefore fixed or user space controlled beam angle has benefits if the use case is known.
That would be useful. Currently it is supported in runtime/idle only via user space (sof-ctl) sent new pass-through blob. The update is identical to IIR and FIR. Maybe the beam on/off control could fit ALSA switch control type. @juimonen @kv2019i what do you think? I think currently the process type component implementation supports only binary control but adding more control types should be possible. Also I wonder if some other ALSA control type (enum?) could cover 360 degree azimuth angle with some granularity. What options would there be? Beam disable/enable is straightforward for case where number of input channels and output channels is equal. E.g. for 2 microphones when beamformer is "on" the output would be typically double mono and with beamformer "off" the mic channels as such. If the beamformer configuration is e.g. 4 microphones to 2 channels (double mono) there should be in blob a control for which of 4 pass into 2 outputs. Maybe there should be in the blob always a beams "off" preset included. I think I should add to this design some beam presets control capability. |
|
Due to recent PR #3194 merge, please rebase for TGL Multicore tests to Pass on Internal CI. |
lgirdwood
left a comment
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.
@singalsu just blocking on the UUID now. LGTM, will approve once UUID is merged and UUID changes is added here.
620f847 to
8ba8f7f
Compare
tools/topology/CMakeLists.txt
Outdated
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.
@juimonen Tested that this topology is more silent than a plain sof-hda-generic. I noticed in topology for UP2 a -6 dB level change vs. normal topology. I'll check the filters that there's no extra 6 dB attenuation in place.
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.
Done, there was a FIR shift value polarity issue in hifi3 version.
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 these changes to dmic-generic and sof-apl-512 be in separate, possible extra commit, before this commit?
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 probably should. I made one commit that contains the topology preparations. There should be a clear no-difference to any built topologies but with all needed preparations step.
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.
This is now fixed. I moved the non-TDFB related improvemets to the previous commit.
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.
Done!
tools/topology/m4/tdfb_coef_flat.m4
Outdated
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.
There's quite many of these and more to come later. I think I should rename these to m4/tdfb/coef_flat.m4 etc. to avoid clutter.
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.
This move is now done.
8ba8f7f to
5507aaa
Compare
|
@lgirdwood I'm still searching where/why the gain drop happens, so this is not yet OK to merge. |
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.
This should be tdfb/coef_line2_pass.m4. The file has been renamed.
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.
Done.
This patch enables FIR filter core usage independently from FIR equalizer component. The inline of FIR core is removed to reduce the code size when FIR is used from several components. Each component also typically used inline FIR version for each supported PCM format that increased further the size. Most of the changes are due to rename and directory move of some FIR data structures and macros. The code functionality is not changed. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
tools/topology/CMakeLists.txt
Outdated
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 tdfb-volume pipeline does not filter out the DC thump in the beginning. I'll change this to tdfb-eq-iir-volume.
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.
Done.
This patch adds the multi-microphone beamforming component. It enhances microphone capture via spatial noise suppression. The component is a quite generic FIR time domain filter bank and the fixed filter band needs to be programmed with super directive or other beamformer criteria filter coefficients. The coefficients are fixed but they can be re-programmed during run-time. The component reuses the FIR filter core but has different inputs selection and outputs mixing features than FIR EQ so it is made a separate new processing component. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
This patch adds the tool for creating beamformer configurations. The microphone array geometry and beam angle (azimuth, elevation) need to be specified. See the example scripts and sample array helper functions. The FIR blob quantize function needed a minor change to prevent strip of trailing zero coefficients. The beamformer filter bank needs to use equal length filters. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
This patch adds support to define from CMakeLists.txt or a higher level platform topology file the definition of DMICPROC and DMIC16KPROC to select desired capture processing algorithms pipeline from pipe-x-capture.m4 and pipe-x-capture-16khz.m4 macros instead of hard coded processing eq-iir-volume. It is preparation to add support for beamformer processing for microphones. The impacted platforms are sof-hda-generic, sof-cml-rt5682, and sof-apl-pcm512x. This patch does not change built topologies. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
These are data files created by the example scripts in tools/tune/tdfb. The generation is time consuming and requires Octave or Matlab. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
This patch adds the playback (for test only) and capture pipelines with Time Domain Fixed Beamformer (TDFB) component. Topologies variants to test capture with beamformer are built for sof-hda-generic and sof-apl-pcm512x platforms. The beam direction is +/- 10 degrees as compromise between notebook camera and stereo capture. The dual beams preserve the stereo characteristic. The beamformers are added to both 48 kHz and 16 kHz DMIC capture pipelines. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
The patch adds the playback and capture test pipelines. The configuration is set to `tdfb_coef_line2_50mm_pm90deg_16khz.m4'. A mistake in PIPELINE_FILTERx macro defining is fixed for IIR and FIR. The pipeline macros expect it to contain an include file or no macro defined at all. Defining it for empty string caused fail in topologies build when PIPELINE_FILTER1 is used for TDFB. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
The script adds tdfb_test.m to check the TDFB beam pattern versus theoretical. It also measures the noise suppression capability of the test beamformer in simulated diffuse and random noise field. As simple quick test this patch adds TDFB to cell array of accepted components for process test. Note: There tests can't be used until load of UUID based non-legacy components is added to testbench. The scripts were used with earlier legacy mode version of the component. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
This patch adds the needed information to testbench to load the TDFB component. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
The if test needs to be done for comp_type. The index does not refer to comp types but items in lib_table that is not correct. As result testbench loads crossover for all UUID based components. The load of beamformer works correctly with this change. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
5507aaa to
47ea071
Compare
|
@lgirdwood This version is at my best knowledge OK to merge based on my own build tests and tests with UP2. I'll continue testing with a suitable hda-generic device that I just got on Monday. @juimonen has tested an earlier version of this PR with his. |
|
@singalsu I'm going to merge this now so validation has enough time to check before the window closes for v1.6 and since it's a large PR. It will also be marked "initial" in the release notes. |
|
Thanks @lgirdwood ! Initial makes sense. This version does not have the control for processing on/off except the sof-ctl based re-configuring. Also need to work with pipelines framework to allow different channels count in source vs. sink. |


Please find the descriptions from commit messages. Known limitation: