kpb: add multi-KPB WOV arbiter for 3-keyword DMIC capture with VAD - #11022
kpb: add multi-KPB WOV arbiter for 3-keyword DMIC capture with VAD#11022lgirdwood wants to merge 8 commits into
Conversation
49f3dc6 to
c3c7161
Compare
20edbb3 to
0f6f889
Compare
Update KPB component to retrieve downstream WOV detector widgets via comp_buffer_get_sink_component(sink) on sel_sink and trigger downstream detector pipelines directly during copy. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
…g copy Ensure all connected mixout components are automatically activated during mixin_copy() so capture stream audio is mixed concurrently to all active WOV detector pipelines. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Add VAD gate audio component to pre-filter background silence and gate downstream WOV keyphrase detector pipelines when no voice energy is present. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Add AMS message UUIDs, payload structures (wov_detect_payload, wov_ctrl_payload), and command codes for inter-component communication between WOV detectors and the multi-slot WOV arbiter. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Add multi-slot WOV arbiter audio component to manage keyword detection events across multiple detector pipelines, dispatch AMS slot activation and control messages, and broadcast pause/resume commands to un-triggered slots. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
…ication - Add multi-slot zero-crossing frequency estimation for Slot 0 (Male 120Hz), Slot 1 (Female 220Hz), and Slot 2 (Child 350Hz). - Require 30ms sustained frequency match (consec_match_count >= 3) and energy > 500 to eliminate transient background room noise triggers. - Map IPC4 pipeline IDs 101/102/103 to slot IDs 0/1/2. - Populate IPC4 phrase detection notifications with WOV detector module ID, instance ID, and slot ID. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Add topology manifests and configuration files supporting 3-slot WOV capture (Male, Female, Child) with multi-KPB buffer routes, mixin/mixout splitters, wov-arbiter widget bindings, and HDA Jack Mic / DMIC input bindings. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
…tion, and testing Add detailed documentation for multi-slot Wake-On-Voice (WOV) architecture, including Mermaid diagrams for audio dataflow and control messaging, Kconfig requirements, topology syntax, and test procedures on Tiger Lake (Spider/Moth). Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
0f6f889 to
3af37c0
Compare
lgirdwood
left a comment
There was a problem hiding this comment.
Needs more inline comments around code blocks and important statements can also do with more context in each commit message. I would put the README.md patch first to give reviewer context first before further review.
| return -ENOMEM; | ||
| } | ||
|
|
||
| struct comp_buffer *sink; |
There was a problem hiding this comment.
lets comment why this is needed, and what we are doing in these 2 new blocks. This does look like it would benefit a utility API.
| } | ||
| #endif /* CONFIG_IPC_MAJOR_4 */ | ||
|
|
||
| if (!kpb->sel_sink || !kpb->host_sink) { |
There was a problem hiding this comment.
lets comment what this block is doing and why.
| comp_err(dev, "could not find sink: sel_sink %p", | ||
| kpb->sel_sink); | ||
| ret = -EIO; | ||
| } else { |
| #UUID: D8218443-5FF3-4A4C-B388-6CFE07B9562E | ||
| uuid "43:84:21:d8:f3:5f:4c:4a:b3:88:6c:fe:07:b9:56:2e" | ||
| #UUID: a8a0cb32-4a77-4db1-85c753d7ee07bce6 (kpb4 static module) | ||
| uuid "32:cb:a0:a8:77:4a:b1:4d:85:c7:53:d7:ee:07:bc:e6" |
There was a problem hiding this comment.
why do we need a new UUID
| if (frames_to_copy + start_frame > mixout_data->mixed_frames) | ||
| mixout_data->mixed_frames = frames_to_copy + start_frame; | ||
|
|
||
| if (mixout_mod && mixout_mod->dev) |
There was a problem hiding this comment.
need comment for what we are checking and why.
| @@ -0,0 +1,455 @@ | |||
| // SPDX-License-Identifier: BSD-3-Clause | |||
| // | |||
| // Copyright(c) 2024 Intel Corporation. All rights reserved. | |||
| */ | ||
| uint8_t active_slot; | ||
|
|
||
| #if CONFIG_AMS |
There was a problem hiding this comment.
we should actually depend on AMS in teh Kconfig and then we can remove all these ifdefs
| if(CONFIG_COMP_VAD_GATE) | ||
| add_subdirectory(vad_gate) | ||
| endif() |
There was a problem hiding this comment.
this should be in the vad gate patch.
| e93326d8-0d14-4bf0-bcb9e063d3d80136 twb_sched | ||
| 42f8060c-832f-4dbf-b24751e961997b34 up_down_mixer | ||
| 6f6b6f4b-6f73-7466-20e1e62b9779f003 userspace_proxy | ||
| 8c7d6e5f-4a3b-2c1d-0e9f8a7b6c5d4e3f vad_gate |
There was a problem hiding this comment.
this should be in vad gate patch
| dai_type "DMIC" | ||
| copier_type "DMIC" | ||
| stream_name $DMIC1_NAME |
There was a problem hiding this comment.
isnt this passed in via build for the target ?
|
|
||
| if (sink_buf_id == buf_id) { | ||
| struct comp_dev *sc = comp_buffer_get_sink_component(sink); | ||
| comp_err(dev, "kpb_bind: buf_id=%d sink_comp=0x%x -> %s", |
There was a problem hiding this comment.
is this an error condition?
| comp_dev_for_each_consumer(dev, sink) { | ||
| struct comp_dev *sc = comp_buffer_get_sink_component(sink); | ||
| if (sc) { | ||
| comp_err(dev, "kpb consumer in bsink_list: comp_id=0x%x type=%d sink_buf=%p", |
There was a problem hiding this comment.
replace comp_err() everywhere in this commit / PR
| if (sc) { | ||
| comp_err(dev, "kpb consumer in bsink_list: comp_id=0x%x type=%d sink_buf=%p", | ||
| dev_comp_id(sc), sc->drv ? sc->drv->type : -1, sink); | ||
| if (dev_comp_id(sc) != 0x10) |
| kpb->sel_sink = sink; | ||
| else | ||
| kpb->host_sink = sink; | ||
| } |
There was a problem hiding this comment.
I suppose the loop should run exactly twice to only set each of these pointers once? Should we check or at least add a comment?
| if (!kpb->sel_sink) | ||
| kpb->sel_sink = sink; | ||
| else if (!kpb->host_sink) | ||
| kpb->host_sink = sink; |
There was a problem hiding this comment.
same here - shouldn't be overwriting, right?
| } | ||
|
|
||
| LOG_MODULE_REGISTER(kd_test, CONFIG_SOF_LOG_LEVEL); | ||
| LOG_MODULE_REGISTER(kd_test, LOG_LEVEL_INF); |
| struct comp_data *cd = comp_get_drvdata(dev); | ||
|
|
||
| comp_info(dev, "entry"); | ||
| comp_err(dev, "notify_host: WOV module_id=0x%x instance_id=0x%x slot_id=%u detected", |
| uint8_t slot_id = cd->wov_slot_id; | ||
| if (slot_id == WOV_SLOT_INVALID) { | ||
| if (dev->pipeline && dev->pipeline->pipeline_id >= 101 && dev->pipeline->pipeline_id <= 103) | ||
| slot_id = dev->pipeline->pipeline_id - 101; |
There was a problem hiding this comment.
what are these magic numbers?
| freq_match = (freq_hz >= 80 && freq_hz <= 170); | ||
| voice_type = "MALE"; | ||
| break; | ||
| case 1: /* Female Voice Range: 175 - 270 Hz */ |
There was a problem hiding this comment.
so 172Hz will be "nobody?" Or are frequencies detected with 5Hz granularity?
There was a problem hiding this comment.
typical human frequency bands from internet, so yes - nobody.
There was a problem hiding this comment.
@lgirdwood sorry, I cannot imagine this being right. Typical - yes. But I cannot imagine no man / woman / child being able to intersect frequencies. So, if someone can speak at 170Hz and somebody else can speak at 175Hz, I'm pretty sure it's humanly possible to speak at 172Hz too.
There was a problem hiding this comment.
Its taken from some internet references, I'm not an expert on the subject so will leave it as is. Its not that important for the test.
| else if (ppl_id == 102 || ppl_id == 3) | ||
| cd->wov_slot_id = 1; | ||
| else if (ppl_id == 103 || ppl_id == 4) | ||
| cd->wov_slot_id = 2; |
There was a problem hiding this comment.
these numbers look familiar
There was a problem hiding this comment.
Pull request overview
This PR introduces a multi-slot Wake-on-Voice (WoV) capture design that allows three concurrent detectors to share a single capture source, with a new firmware “arbiter” component selecting which KPB drain feeds a single host PCM stream. It also adds a VAD gate component intended to suppress downstream processing during silence, and extends the detect_test sample to coordinate with the arbiter via AMS.
Changes:
- Add new firmware components:
wov_arbiter(multi-input drain routing + AMS control) andvad_gate(energy-based gating). - Add/adjust Topology2 definitions for multi-slot WoV, plus wiring/includes updates.
- Extend
detect_testfor multi-slot behavior and AMS messaging; adjust rimage/module TOMLs for additional instances.
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| uuid-registry.txt | Registers new UUIDs for vad_gate and wov_arbiter. |
| tools/topology/topology2/sof-hda-generic.conf | Adds includes for WoV/VAD/KPB-related topology snippets. |
| tools/topology/topology2/sof-hda-generic-wov-manifest.conf | New manifest-style topology source including WoV/VAD/KPB assets. |
| tools/topology/topology2/sof-hda-generic-manifest.conf | New manifest-style topology source for HDA generic. |
| tools/topology/topology2/sof-hda-generic-4ch.conf | New manifest-style topology source variant. |
| tools/topology/topology2/platform/intel/dmic-wov-multi.conf | New multi-detector WoV topology definition (DMIC multi-slot). |
| tools/topology/topology2/platform/intel/dmic-generic.conf | Switches/extends DMIC includes to pull in the new multi-WoV topology. |
| tools/topology/topology2/include/pipelines/cavs/wov-detect.conf | Adjusts pipeline class formatting metadata. |
| tools/topology/topology2/include/pipelines/cavs/dai-kpb-be.conf | Modifies the DAI→KPB pipeline class defaults and routing source. |
| tools/topology/topology2/include/components/wov.conf | Changes WoV control naming to be index-qualified. |
| tools/topology/topology2/include/components/wov-arbiter.conf | New Topology2 widget class for the arbiter component. |
| tools/topology/topology2/include/components/vad-gate.conf | New Topology2 widget class for the VAD gate component. |
| tools/topology/topology2/include/components/kpb.conf | Switches KPB widget UUID to the kpb4 static module UUID. |
| tools/topology/topology2/dmic-wov-multi-manifest.conf | New manifest-style topology for the multi-slot WoV design. |
| tools/rimage/config/tgl.toml.h | Updates rimage TOML include set for new modules. |
| src/samples/audio/detect_test.toml | Increases instance count for the detector sample module. |
| src/samples/audio/detect_test.c | Extends detect_test with slot awareness, AMS integration, and new detection behavior. |
| src/include/sof/lib/ams_msg.h | Adds new AMS UUIDs and payload structs for arbiter↔detector control. |
| src/include/sof/audio/wov_arbiter.h | New public header for arbiter constants/IPC param IDs. |
| src/include/sof/audio/vad_gate.h | New public header for VAD gate runtime config/IPC param IDs. |
| src/include/sof/audio/kpb.h | Adds a new detector lookup prototype. |
| src/include/ipc4/detect_test.h | Adds IPC4 large-config param for setting WoV slot ID. |
| src/audio/wov_arbiter/wov_arbiter.toml | New module metadata for rimage/LLEXT. |
| src/audio/wov_arbiter/wov_arbiter.c | New arbiter component implementation. |
| src/audio/wov_arbiter/README.md | New architecture and testing documentation for multi-slot WoV. |
| src/audio/wov_arbiter/CMakeLists.txt | Builds the new arbiter component. |
| src/audio/vad_gate/vad_gate.toml | New module metadata for rimage/LLEXT. |
| src/audio/vad_gate/vad_gate.c | New VAD gate component implementation. |
| src/audio/vad_gate/Kconfig | Kconfig option for building the VAD gate component. |
| src/audio/vad_gate/CMakeLists.txt | Builds the new VAD gate component. |
| src/audio/mixin_mixout/mixin_mixout.c | Modifies scheduling/state/copy behavior in mixin/mixout processing. |
| src/audio/kpb.toml | Increases instance count for KPB module. |
| src/audio/kpb.c | Modifies KPB binding/prepare/copy behavior and downstream triggering. |
| src/audio/Kconfig | Adds Kconfig entry for the new arbiter component and sources VAD gate Kconfig. |
| src/audio/CMakeLists.txt | Adds build integration for vad_gate and wov_arbiter. |
| app/prj.conf | Enables samples / sample keyphrase in the app config. |
| app/debug_overlay.conf | Enables fatal breadcrumb recording in debug overlay. |
| app/boards/intel_adsp_cavs25.conf | Enables new components and sample config on the cavs25 board. |
Suppressed comments (2)
src/audio/mixin_mixout/mixin_mixout.c:506
- Calling comp_copy() on other components from inside mixin_process() is likely to cause re-entrancy/recursion and breaks the normal scheduler-driven copy order. This should be removed and left to the pipeline scheduler.
if (mixout_mod && mixout_mod->dev)
comp_copy(mixout_mod->dev);
src/audio/mixin_mixout/mixin_mixout.c:607
- mixout_process() manually triggers comp_copy() on the downstream sink component (e.g., KPB). This can cause recursive copy calls and violates the scheduler’s ownership of graph execution order.
comp_err(dev, "mixout_process: sink_buf=0x%x produced=%u bytes, triggering kpb=0x%x",
sink_buf, bytes_to_produce, dev_comp_id(sink_comp));
comp_copy(sink_comp);
| e93326d8-0d14-4bf0-bcb9e063d3d80136 twb_sched | ||
| 42f8060c-832f-4dbf-b24751e961997b34 up_down_mixer | ||
| 6f6b6f4b-6f73-7466-20e1e62b9779f003 userspace_proxy | ||
| 8c7d6e5f-4a3b-2c1d-0e9f8a7b6c5d4e3f vad_gate |
| 1028070e-04e8-46ab-8d8110a0116ce738 wait | ||
| d944281a-afe9-4695-a043d7f62b89538e waves | ||
| 13c8bc59-c4fa-4ad1-b93ace97cd30acc7 wdt | ||
| 4a5b6c7d-8e9f-4a1b-2c3d4e5f60718293 wov_arbiter |
| #endif | ||
|
|
||
| #ifdef CONFIG_SAMPLE_KEYPHRASE | ||
| #include <samples/audio/detect_test.toml> |
| <vendor-token.conf> | ||
| <tokens.conf> | ||
| <virtual.conf> | ||
| <vad-gate.conf> |
| KWD_TEST_UUID "1f:d5:a8:eb:27:78:b5:47:ee:82:de:6e:77:43:af:67" | ||
| WOV_UUID "1f:d5:a8:eb:27:78:b5:47:ee:82:de:6e:77:43:af:67" |
| if (mixout->state != COMP_STATE_ACTIVE) | ||
| mixout->state = COMP_STATE_ACTIVE; | ||
|
|
| struct comp_dev *wov_comp = sink ? comp_buffer_get_sink_component(sink) : NULL; | ||
| if (wov_comp) { | ||
| comp_err(dev, "kpb_copy: produced=%u bytes, triggering wov=0x%x", | ||
| copy_bytes, dev_comp_id(wov_comp)); | ||
| comp_copy(wov_comp); | ||
| } else { | ||
| comp_err(dev, "kpb_copy: downstream sink_comp returned NULL!"); | ||
| } |
| } | ||
|
|
||
| LOG_MODULE_REGISTER(kd_test, CONFIG_SOF_LOG_LEVEL); | ||
| LOG_MODULE_REGISTER(kd_test, LOG_LEVEL_INF); |
| comp_err(dev, "notify_host: WOV module_id=0x%x instance_id=0x%x slot_id=%u detected", | ||
| dev_comp_id(dev) >> 16, dev_comp_id(dev) & 0xffff, cd->wov_slot_id); |
| static const struct comp_driver comp_keyword; | ||
| static struct comp_dev *wov_detect_devs[256]; | ||
|
|
||
| struct comp_dev *get_wov_detector_comp(uint32_t ppl_id) | ||
| { | ||
| if (ppl_id < 256) | ||
| return wov_detect_devs[ppl_id]; | ||
| return NULL; | ||
| } |
EDIT now added test detection in each WoV module to wake on voice frequencies for male, female and children voice frequencies. i.e. each WoV will trigger on a different speaker type.
Adds a new WOV arbiter component and multi-KPB topology to support three simultaneous wake-word detectors sharing a single DMIC, with exclusive host PCM drain arbitration when any keyword fires.
New component: src/audio/wov_arbiter/
AMS additions (ams_msg.h):
detect_test extensions:
Topology: tools/topology/topology2/platform/intel/dmic-wov-multi.conf
Requires: CONFIG_COMP_WOV_ARBITER, CONFIG_COMP_KPB, CONFIG_COMP_MIXIN_MIXOUT,
CONFIG_AMS, CONFIG_COMP_KWD_DETECT