-
Notifications
You must be signed in to change notification settings - Fork 367
kpb: add multi-KPB WOV arbiter for 3-keyword DMIC capture with VAD #11022
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
base: main
Are you sure you want to change the base?
Changes from all commits
827a95d
4a9c79a
d478129
7a75506
899e126
ee08587
54447ff
74aaaf1
9458dbd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -104,9 +104,15 @@ if(NOT CONFIG_COMP_MODULE_SHARED_LIBRARY_BUILD) | |
| if(CONFIG_COMP_UP_DOWN_MIXER) | ||
| add_subdirectory(up_down_mixer) | ||
| endif() | ||
| if(CONFIG_COMP_VAD_GATE) | ||
| add_subdirectory(vad_gate) | ||
| endif() | ||
|
Comment on lines
+107
to
+109
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should be in the vad gate patch. |
||
| if(CONFIG_COMP_VOLUME) | ||
| add_subdirectory(volume) | ||
| endif() | ||
| if(CONFIG_COMP_WOV_ARBITER) | ||
| add_subdirectory(wov_arbiter) | ||
| endif() | ||
| if(CONFIG_DTS_CODEC) | ||
| add_subdirectory(codec) | ||
| endif() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,10 +18,14 @@ | |
| #include <sof/audio/component_ext.h> | ||
| #include <sof/audio/pipeline.h> | ||
| #include <sof/audio/kpb.h> | ||
| #define SOF_MODULE_API_PRIVATE | ||
| #include <sof/audio/module_adapter/module/generic.h> | ||
| #include <module/module/base.h> | ||
| #include <sof/audio/ipc-config.h> | ||
| #include <sof/common.h> | ||
| #include <rtos/panic.h> | ||
| #include <sof/ipc/msg.h> | ||
| #include <sof/ipc/topology.h> | ||
| #include <rtos/timer.h> | ||
| #include <rtos/alloc.h> | ||
| #include <rtos/clk.h> | ||
|
|
@@ -369,6 +373,9 @@ static int kpb_bind(struct comp_dev *dev, struct bind_info *bind_data) | |
| sink_buf_id = buf_get_id(sink); | ||
|
|
||
| if (sink_buf_id == buf_id) { | ||
| struct comp_dev *sc = comp_buffer_get_sink_component(sink); | ||
| comp_dbg(dev, "kpb_bind: buf_id=%d sink_comp=0x%x -> %s", | ||
| buf_id, sc ? dev_comp_id(sc) : 0, sink_buf_id == 0 ? "sel_sink" : "host_sink"); | ||
| if (sink_buf_id == 0) | ||
| kpb->sel_sink = sink; | ||
| else | ||
|
|
@@ -887,6 +894,43 @@ static int kpb_prepare(struct comp_dev *dev) | |
| return -ENOMEM; | ||
| } | ||
|
|
||
| struct comp_buffer *sink; | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| /* Output pin 0 connects to the WOV detector (sel_sink), | ||
| * output pin 1 connects to the WOV arbiter (host_sink). */ | ||
| comp_dev_for_each_consumer(dev, sink) { | ||
| if (buf_get_id(sink) == 0) | ||
| kpb->sel_sink = sink; | ||
| else | ||
| kpb->host_sink = sink; | ||
| } | ||
| comp_dbg(dev, "kpb_params: sel_sink=%p host_sink=%p", | ||
| kpb->sel_sink, kpb->host_sink); | ||
|
|
||
| /* Cross-pipeline prepare: the WOV detector (detect_test) lives on a | ||
| * separate IPC4 pipeline and won't be prepared by the normal IPC4 walk, | ||
| * so KPB bootstraps it here during its own prepare phase. | ||
| */ | ||
| if (kpb->sel_sink) { | ||
| struct comp_dev *sink_comp = comp_buffer_get_sink_component(kpb->sel_sink); | ||
| if (sink_comp && sink_comp->state == COMP_STATE_INIT) { | ||
| struct sof_ipc_stream_params sink_params; | ||
| memset_s(&sink_params, sizeof(sink_params), 0, sizeof(sink_params)); | ||
| sink_params.channels = kpb->config.channels ? kpb->config.channels : 2; | ||
| sink_params.rate = kpb->config.sampling_freq ? kpb->config.sampling_freq : 16000; | ||
| sink_params.sample_container_bytes = 4; | ||
| sink_params.sample_valid_bytes = 4; | ||
| sink_params.frame_fmt = SOF_IPC_FRAME_S32_LE; | ||
| comp_params(sink_comp, &sink_params); | ||
| ret = comp_prepare(sink_comp); | ||
| if (ret < 0) { | ||
| comp_err(dev, "kpb_prepare: cross-pipeline prepare of wov detector failed: %d", ret); | ||
| return ret; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| kpb_change_state(kpb, KPB_STATE_RUN); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. kpb enters run too early ? another run L1012 |
||
|
|
||
| #ifndef CONFIG_IPC_MAJOR_4 | ||
| /* Search for KPB related sinks. | ||
| * NOTE! We assume here that channel selector component device | ||
|
|
@@ -936,10 +980,40 @@ static int kpb_prepare(struct comp_dev *dev) | |
| } | ||
| #endif /* CONFIG_IPC_MAJOR_4 */ | ||
|
|
||
| if (!kpb->sel_sink && !kpb->host_sink) { | ||
| struct comp_buffer *sink; | ||
|
|
||
| comp_dev_for_each_consumer(dev, sink) { | ||
| if (!kpb->sel_sink) | ||
| kpb->sel_sink = sink; | ||
| else if (!kpb->host_sink) | ||
| kpb->host_sink = sink; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here - shouldn't be overwriting, right? |
||
| } | ||
| } | ||
|
|
||
| if (!kpb->sel_sink) { | ||
| comp_err(dev, "could not find sink: sel_sink %p", | ||
| kpb->sel_sink); | ||
| ret = -EIO; | ||
| } else { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto re comments. |
||
| struct comp_dev *sink_comp = comp_buffer_get_sink_component(kpb->sel_sink); | ||
| if (sink_comp && sink_comp->state == COMP_STATE_INIT) { | ||
| struct sof_ipc_stream_params sink_params; | ||
| memset_s(&sink_params, sizeof(sink_params), 0, sizeof(sink_params)); | ||
| sink_params.channels = kpb->config.channels ? kpb->config.channels : 2; | ||
| sink_params.rate = kpb->config.sampling_freq ? kpb->config.sampling_freq : 16000; | ||
| sink_params.sample_container_bytes = 4; | ||
| sink_params.sample_valid_bytes = 4; | ||
| sink_params.frame_fmt = SOF_IPC_FRAME_S32_LE; | ||
|
Comment on lines
+1005
to
+1007
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. these should come from the topology for kpb object |
||
| comp_params(sink_comp, &sink_params); | ||
| ret = comp_prepare(sink_comp); | ||
| comp_info(dev, "kpb_prepare: prepared downstream sink_comp %d in state %d", | ||
| dev_comp_id(sink_comp), sink_comp->state); | ||
| if (ret < 0) { | ||
| comp_err(dev, "kpb_prepare: cross-pipeline prepare failed: %d", ret); | ||
| return ret; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| kpb->sync_draining_mode = true; | ||
|
|
@@ -981,11 +1055,30 @@ static int kpb_reset(struct comp_dev *dev) | |
| switch (kpb->state) { | ||
| case KPB_STATE_BUFFERING: | ||
| case KPB_STATE_DRAINING: | ||
| /* KPB is performing some task now, | ||
| * terminate it gently. | ||
| /* If a host drain is in progress, terminate gently and let | ||
| * kpb_copy complete the reset once scheduled. When there is | ||
| * no host_sink (WOV-only path) the scheduler has already | ||
| * stopped by the time RESET arrives, so reset immediately. | ||
| */ | ||
| kpb_change_state(kpb, KPB_STATE_RESETTING); | ||
| ret = -EBUSY; | ||
| if (kpb->host_sink) { | ||
| kpb_change_state(kpb, KPB_STATE_RESETTING); | ||
| ret = -EBUSY; | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we inline comment when reset will be done since we break here. |
||
| break; | ||
| } | ||
| /* host_sink == NULL: immediate full reset (same as default) */ | ||
| kpb->hd.buffered = 0; | ||
| kpb->sel_sink = NULL; | ||
| kpb->host_sink = NULL; | ||
| kpb->host_buffer_size = 0; | ||
| kpb->host_period_size = 0; | ||
| for (i = 0; i < KPB_MAX_NO_OF_CLIENTS; i++) { | ||
| kpb->clients[i].state = KPB_CLIENT_UNREGISTERED; | ||
| kpb->clients[i].r_ptr = NULL; | ||
| } | ||
| if (kpb->hd.c_hb) | ||
| kpb_reset_history_buffer(kpb->hd.c_hb); | ||
| kpb_change_state(kpb, KPB_STATE_PREPARING); | ||
| ret = comp_set_state(dev, COMP_TRIGGER_RESET); | ||
| break; | ||
| case KPB_STATE_DISABLED: | ||
| case KPB_STATE_CREATED: | ||
|
|
@@ -1234,19 +1327,16 @@ static int kpb_copy(struct comp_dev *dev) | |
| sink = kpb->sel_sink; | ||
| ret = PPL_STATUS_PATH_STOP; | ||
|
|
||
| comp_dbg(dev, "kpb_copy: source_buf=%p sel_sink=%p avail=%u", | ||
| source, sink, audio_stream_get_avail_bytes(&source->stream)); | ||
|
|
||
| if (!sink) { | ||
| comp_err(dev, "no sink."); | ||
| ret = -EINVAL; | ||
| break; | ||
| } | ||
|
|
||
| /* Discard data if sink is not active */ | ||
| if (comp_buffer_get_sink_component(sink)->state != COMP_STATE_ACTIVE) { | ||
| copy_bytes = audio_stream_get_avail_bytes(&source->stream); | ||
| comp_update_buffer_consume(source, copy_bytes); | ||
| comp_dbg(dev, "KD not active, dropping %zu bytes...", copy_bytes); | ||
| break; | ||
| } | ||
|
Comment on lines
-1243
to
-1249
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do we need to drop this, if downstream is not active then this seems like the right thing todo. |
||
| /* Allow downstream WOV detector copy regardless of state */ | ||
|
|
||
| /* Validate sink */ | ||
| if (!audio_stream_get_wptr(&sink->stream)) { | ||
|
|
@@ -1313,6 +1403,15 @@ static int kpb_copy(struct comp_dev *dev) | |
| else | ||
| comp_update_buffer_produce(sink, produced_bytes); | ||
|
|
||
| struct comp_dev *wov_comp = sink ? comp_buffer_get_sink_component(sink) : NULL; | ||
| if (wov_comp) { | ||
| comp_dbg(dev, "kpb_copy: produced=%u bytes, triggering wov=0x%x", | ||
| copy_bytes, dev_comp_id(wov_comp)); | ||
| comp_copy(wov_comp); | ||
| } else { | ||
| comp_warn(dev, "kpb_copy: downstream sink_comp returned NULL!"); | ||
| } | ||
|
|
||
| comp_update_buffer_consume(source, copy_bytes); | ||
|
|
||
| break; | ||
|
|
@@ -1607,6 +1706,28 @@ static int kpb_register_client(struct comp_data *kpb, struct kpb_client *cli) | |
| static void kpb_init_draining(struct comp_dev *dev, struct kpb_client *cli) | ||
| { | ||
| struct comp_data *kpb = comp_get_drvdata(dev); | ||
|
|
||
| if (!kpb->host_sink) { | ||
| if (!kpb->sel_sink) { | ||
| comp_warn(dev, "kpb_init_draining: no drain path, skipping"); | ||
| return; | ||
| } | ||
| /* WOV-only path: no dedicated host PCM sink. Route history drain | ||
| * through sel_sink so wov passthrough delivers it to the arbiter. | ||
| * Set host_period_size to one real-time period so sync_draining_mode | ||
| * throttles the EDF drain task to match the LL pipeline rate. | ||
| */ | ||
| comp_warn(dev, "kpb_init_draining: no host_sink, draining via sel_sink"); | ||
| kpb->host_sink = kpb->sel_sink; | ||
| if (!kpb->host_period_size) { | ||
| size_t bpm = (size_t)KPB_SAMPLES_PER_MS * | ||
| (KPB_SAMPLE_CONTAINER_SIZE(kpb->config.sampling_width) / 8) * | ||
| kpb->config.channels; | ||
| kpb->host_period_size = bpm; | ||
| kpb->host_buffer_size = audio_stream_get_size(&kpb->sel_sink->stream); | ||
| } | ||
| } | ||
|
|
||
| bool is_sink_ready = (comp_buffer_get_sink_state(kpb->host_sink) == COMP_STATE_ACTIVE); | ||
| size_t sample_width = kpb->config.sampling_width; | ||
| size_t drain_req = (size_t)cli->drain_req * kpb->config.channels * | ||
|
|
@@ -1633,14 +1754,16 @@ static void kpb_init_draining(struct comp_dev *dev, struct kpb_client *cli) | |
| /* TODO: check also if client is registered */ | ||
| } else if (!is_sink_ready) { | ||
| comp_err(dev, "sink not ready for draining"); | ||
| } else if (kpb->hd.buffered < drain_req || | ||
| cli->drain_req > KPB_MAX_DRAINING_REQ) { | ||
| comp_cl_err(&comp_kpb, "not enough data in history buffer"); | ||
| } else if (cli->drain_req > KPB_MAX_DRAINING_REQ) { | ||
| comp_cl_err(&comp_kpb, "drain request exceeds max"); | ||
| } else { | ||
| /* Draining accepted, find proper buffer to start reading | ||
| * At this point we are guaranteed that there is enough data | ||
| * in the history buffer. All we have to do now is to calculate | ||
| * read pointer from which we will start draining. | ||
| if (kpb->hd.buffered < drain_req) { | ||
| comp_cl_warn(&comp_kpb, "partial pre-roll: capping drain to buffered"); | ||
| drain_req = kpb->hd.buffered; | ||
| } | ||
| /* Draining accepted, find proper buffer to start reading. | ||
| * If less history than requested is buffered, drain_req is | ||
| * capped above so we drain whatever is available. | ||
| */ | ||
| kpb_lock(kpb); | ||
|
|
||
|
|
@@ -1750,8 +1873,11 @@ static void kpb_init_draining(struct comp_dev *dev, struct kpb_client *cli) | |
| comp_set_attribute(comp_buffer_get_sink_component(kpb->host_sink), | ||
| COMP_ATTR_COPY_TYPE, &kpb->force_copy_type); | ||
|
|
||
| /* Pause selector copy. */ | ||
| comp_buffer_get_sink_component(kpb->sel_sink)->state = COMP_STATE_PAUSED; | ||
| /* Pause selector copy to stop detection on stale drain data. | ||
| * Skip when sel_sink IS the drain path (wov passthrough needed). | ||
| */ | ||
| if (kpb->host_sink != kpb->sel_sink) | ||
| comp_buffer_get_sink_component(kpb->sel_sink)->state = COMP_STATE_PAUSED; | ||
|
|
||
| if (!pm_runtime_is_active(PM_RUNTIME_DSP, PLATFORM_PRIMARY_CORE_ID)) | ||
| pm_runtime_disable(PM_RUNTIME_DSP, PLATFORM_PRIMARY_CORE_ID); | ||
|
|
@@ -2368,7 +2494,7 @@ static void kpb_reset_history_buffer(struct history_buffer *buff) | |
| if (!buff) | ||
| return; | ||
|
|
||
| kpb_clear_history_buffer(buff); | ||
|
|
||
|
Comment on lines
-2371
to
+2497
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do we no longer need to clear ? |
||
|
|
||
| do { | ||
| buff->w_ptr = buff->start_addr; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| # SPDX-License-Identifier: BSD-3-Clause | ||
| add_local_sources(sof vad_gate.c) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| # SPDX-License-Identifier: BSD-3-Clause | ||
|
|
||
| config COMP_VAD_GATE | ||
| bool "VAD gate component" | ||
| depends on IPC_MAJOR_4 | ||
| help | ||
| Select to build the VAD (Voice Activity Detection) gate component. | ||
| The gate sits between the DMIC copier and the downstream Mixin in | ||
| a WOV capture pipeline. During silence it drains the DMIC input | ||
| and returns PPL_STATUS_PATH_STOP so KPB and WOV detectors idle. | ||
| When voice onset is detected audio flows through and the downstream | ||
| chain wakes and begins buffering. |
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 be =m