Conversation
2151ccb to
28a11a0
Compare
28a11a0 to
2674e49
Compare
2674e49 to
c036f90
Compare
There was a problem hiding this comment.
🟡 Changes recommended
There are correctness issues that will break integration (notably the KPB UUID mismatch with the UUID registry) and should be fixed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR introduces a new microWakeWord (MWW) keyword-spotting component and integrates it into SOF’s IPC4/Zephyr-based build, including new topology2 pipelines (KPB-based Wake-on-Voice branches), rimage manifests, and supporting LLEXT/AMS/DP-scheduler plumbing needed to run TFLite Micro–based C++ extensions.
Changes:
- Add the
microwakewordaudio component (C/C++ + Kconfig/CMake + rimage TOML) and offline training/tuning scripts. - Add topology2 widget/pipeline templates and platform overlays to enable MWW+KPB Wake-on-Voice capture branches on HDA and SDW.
- Extend DP-scheduler integration (optional DP→DP binding), AMS payload handling, and LLEXT build/export behavior for C++ multi-TU libraries.
File summaries
| File | Description |
|---|---|
| zephyr/Kconfig | Broadens IPC4 pipeline2.0 + DP scheduler defaults to ACE/CAVS; adds DP→DP bind Kconfig. |
| zephyr/CMakeLists.txt | Adds -Wl,-Bsymbolic-functions for LLEXT shared builds to avoid unresolved local cross-TU calls. |
| uuid-registry.txt | Registers UUID for new mww module. |
| tools/topology/topology2/sof-hda-generic.conf | Adds optional HDA mic MWW/KPB capture include and required class includes. |
| tools/topology/topology2/platform/intel/sdw-jack-mww-kpb.conf | New SDW jack MWW/KPB WoV capture branch overlay topology. |
| tools/topology/topology2/platform/intel/sdw-dmic-mww-kpb.conf | New SDW DMIC MWW/KPB WoV capture branch overlay topology. |
| tools/topology/topology2/platform/intel/hda-mic-mww-kpb.conf | New HDA analog MWW/KPB WoV capture branch overlay topology. |
| tools/topology/topology2/platform/intel/dmic1-mfcc.conf | Removes redundant MFCC include (comment-only change). |
| tools/topology/topology2/include/pipelines/cavs/src-kpb-be.conf | New reusable SRC→KPB backend pipeline class for WoV branching. |
| tools/topology/topology2/include/pipelines/cavs/host-gateway-src-mfcc-mww-capture.conf | Adds a detection pipeline template combining SRC+MFCC+MWW. |
| tools/topology/topology2/include/pipelines/cavs/host-gateway-micsel-mfcc-mww-capture.conf | Adds a stereo-compatible detection pipeline template using micsel+MFCC+MWW. |
| tools/topology/topology2/include/components/mww.conf | Adds topology2 widget class definition for mww. |
| tools/topology/topology2/include/components/mfcc/mel80.conf | Updates exported MFCC config date header. |
| tools/topology/topology2/include/components/mfcc/mel80_compress.conf | Updates exported MFCC config date header. |
| tools/topology/topology2/include/components/mfcc/mel80_compress_dtx.conf | Updates exported MFCC config date header. |
| tools/topology/topology2/include/components/mfcc/mel40.conf | Adds new 40-bin MFCC config blob. |
| tools/topology/topology2/include/components/mfcc/mel40_compress.conf | Adds new compressed-output 40-bin MFCC config blob. |
| tools/topology/topology2/include/components/mfcc/mel40_10ms.conf | Adds new 40-bin, 10ms-hop MFCC config blob for MWW. |
| tools/topology/topology2/include/components/mfcc/mel40_10ms_compress.conf | Adds compressed-output 40-bin, 10ms-hop MFCC config blob. |
| tools/topology/topology2/include/components/mfcc/default.conf | Updates exported MFCC config date header. |
| tools/topology/topology2/include/components/mfcc/ceps13_compress_dtx.conf | Updates exported MFCC config date header. |
| tools/topology/topology2/include/components/kpb.conf | Updates KPB UUID (currently inconsistent with uuid registry; see comment). |
| tools/topology/topology2/include/common/common_definitions.conf | Adds feature flags for enabling MWW/KPB overlays. |
| tools/topology/topology2/include/common/abi.conf | Adds ABI manifest blob definition. |
| tools/topology/topology2/include/bench/mfccmel40_10ms_s32.conf | Adds MFCC mel40_10ms benchmark include (S32). |
| tools/topology/topology2/include/bench/mfccmel40_10ms_s24.conf | Adds MFCC mel40_10ms benchmark include (S24). |
| tools/topology/topology2/include/bench/mfccmel40_10ms_s16.conf | Adds MFCC mel40_10ms benchmark include (S16). |
| tools/topology/topology2/include/bench/mfcc_controls_playback.conf | Adds bench parameter key for mel40_10ms MFCC blob. |
| tools/topology/topology2/include/bench/mfcc_controls_capture.conf | Adds bench parameter key for mel40_10ms MFCC blob. |
| tools/topology/topology2/development/tplg-targets.cmake | Adds development topology targets enabling MWW/KPB branches for HDA + SDW. |
| tools/topology/topology2/development/tplg-targets-bench.cmake | Adds bench target/params for mel40_10ms MFCC benchmark. |
| tools/topology/topology2/cavs-sdw.conf | Adds optional includes and keys for SDW MWW/KPB overlays + required class includes. |
| tools/topology/topology2/cavs-benchmark-hda.conf | Adds benchmark configs for mfccmel40_10ms variants. |
| tools/rimage/config/wcl.toml.h | Includes MWW module TOML when enabled. |
| tools/rimage/config/tgl.toml.h | Includes MWW module TOML when enabled. |
| tools/rimage/config/tgl-h.toml.h | Includes MWW module TOML when enabled. |
| tools/rimage/config/ptl.toml.h | Includes MWW module TOML when enabled. |
| tools/rimage/config/mtl.toml.h | Includes MWW module TOML when enabled. |
| tools/rimage/config/lnl.toml.h | Includes MWW module TOML when enabled. |
| src/platform/intel/cavs/platform.c | Initializes DP scheduler when enabled. |
| src/library_manager/llext_manager_dram.c | Ensures VMA cleanup on restore failure. |
| src/lib/cpp_new_export.cpp | Exports C++ allocation/runtime symbols for LLEXT modules. |
| src/lib/CMakeLists.txt | Builds the new C++ symbol export shim when CONFIG_CPP is enabled. |
| src/lib/ams.c | Adds inline payload data copy into AMS slots; exports ams_send(). |
| src/ipc/ipc4/helper.c | Adds optional DP→DP binding support and DP ring buffer attachment changes. |
| src/ipc/ipc4/ams_helpers.c | Exports AMS helper functions for external users/modules. |
| src/include/sof/lib_manager.h | Extends lib manager module struct with export segment + VMA tracking. |
| src/include/sof/audio/mfcc/mfcc_vad.h | Tunes MFCC VAD constants (noise rise alpha + threshold). |
| src/audio/module_adapter/module_adapter.c | Propagates DP domain from extended init; minor sync constant fix. |
| src/audio/microwakeword/tune/sof_mww_verify.py | Adds streaming verification script for quantized MWW models. |
| src/audio/microwakeword/tune/sof_mww_train_pipeline.sh | Adds end-to-end dataset→features→train→verify pipeline runner. |
| src/audio/microwakeword/tune/sof_mww_prepare_silence_unknown.sh | Adds script to prepare silence/unknown classes from Speech Commands v2. |
| src/audio/microwakeword/tune/sof_mww_plot_mtrace.py | Adds mtrace visualization tool for MWW diagnostics. |
| src/audio/microwakeword/tune/sof_mww_generate_keyword_dataset.sh | Adds multi-speaker synthetic keyword dataset generator (has duplicated helper; see comment). |
| src/audio/microwakeword/tune/sof_mww_generate_keyword_dataset_from_dir.sh | Adds real-speech ingestion + augmentation dataset builder. |
| src/audio/microwakeword/tune/sof_mww_dataset.py | Adds feature loader + augmentation utilities for training. |
| src/audio/microwakeword/tune/sof_mfcc_extract_features.sh | Adds batch feature extraction via sof-testbench4 + topology2 bench tplg. |
| src/audio/microwakeword/tune/README.md | Documents offline MWW training/tuning toolchain and workflows. |
| src/audio/microwakeword/README.md | Documents MWW component architecture, dataflow, and deployment notes. |
| src/audio/microwakeword/mww.toml | Adds rimage module manifest entry for MWW. |
| src/audio/microwakeword/mww_model.h | Adds C API surface for the MWW TFLM inference wrapper. |
| src/audio/microwakeword/mww_model.cc | Implements TFLM interpreter setup + streaming inference wrapper. |
| src/audio/microwakeword/mww_model_data.h | Adds generated model-data header declaration. |
| src/audio/microwakeword/llext/llext.toml.h | Adds LLEXT-specific TOML wrapper for MWW module. |
| src/audio/microwakeword/llext/CMakeLists.txt | Adds LLEXT build for the MWW module and its private TFLM library. |
| src/audio/microwakeword/llext-wrap.c | Adds LLEXT portability stubs and PIC-safe math overrides. |
| src/audio/microwakeword/Kconfig | Adds Kconfig options for MWW component and debug/model-loading modes. |
| src/audio/microwakeword/CMakeLists.txt | Adds static + LLEXT build logic for MWW and its private deps. |
| src/audio/mfcc/tune/setup_mfcc.m | Adds MFCC export profiles for mel40 (20ms) and mel40_10ms (+ compress variants). |
| src/audio/mfcc/mfcc_common.c | Clarifies VAD input comment. |
| src/audio/Kconfig | Includes microwakeword Kconfig in audio menu. |
| src/audio/CMakeLists.txt | Adds microwakeword subdir to audio build when enabled. |
| src/audio/buffers/ring_buffer.c | Adds vregion refcount release for DP→DP bind case. |
| src/audio/buffers/audio_buffer.c | Adds DP→DP dual-secondary-buffer sync support and updates DP→DP commentary. |
| scripts/xtensa-build-zephyr.py | Installs symlinks by UUID name for library binaries. |
| scripts/tensorflow-clone.sh | Refactors dependency clone script to fetch + checkout pinned commits under workspace parent dir. |
| scripts/llext_offset_calc.py | Handles ELF with no allocated sections by returning size 0. |
| app/llext_relocatable.conf | Enables export-by-SLID for LLEXT relocatable builds. |
| app/boards/intel_adsp/Kconfig.defconfig | Enables DP→DP bind by default. |
| app/boards/intel_adsp_cavs25.conf | Enables MWW (static), increases heap sizes, and adds related settings for cavs2.5. |
| app/boards/intel_adsp_ace30_ptl.conf | Enables MWW as LLEXT, adjusts library base address, heap sizing, and LLEXT heap. |
Review details
- Files reviewed: 86/88 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
c036f90 to
56a1ad9
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The review found concrete correctness issues in new/changed runtime code paths (heap free mismatch in ring buffer cleanup, unsafe AMS memcpy_s handling, and incorrect syscall stub signatures/errno semantics) that should be fixed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 85/87 changed files
- Comments generated: 4
- Review effort level: Lite
56a1ad9 to
f518867
Compare
There was a problem hiding this comment.
🟡 Changes recommended
It introduces an unsafe free path in ring_buffer_free() that can corrupt module allocation lifetime (use-after-free/double-free risk).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
src/audio/buffers/ring_buffer.c:113
- ring_buffer_free() must not free the module allocation context.
allocis owned by the module (allocated with sof_heap_alloc() in module_adapter_mem_alloc() and freed in module_adapter_mem_free()). Freeing it here via rfree() can cause use-after-free/double-free when the module later tears down, andrfree()is also not the matching deallocator for sof_heap_alloc().
if (alloc && alloc->vreg) {
if (!vregion_put(alloc->vreg))
rfree(alloc);
}
- Files reviewed: 85/87 changed files
- Comments generated: 1
- Review effort level: Lite
f518867 to
35f972b
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The DP-to-DP ring buffer teardown path currently risks incorrect/freeing of shared allocation context and AMS slot message pointer rebinding relies on an invalid cross-core pointer value.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
src/audio/buffers/ring_buffer.c:113
- ring_buffer_free() drops the vregion refcount and then frees the shared mod_alloc_ctx with rfree(). This ctx is allocated with sof_heap_alloc() (see module_adapter_mem_alloc()) and is also freed via sof_heap_free() in other vregion_put() call sites (e.g. comp_buffer_free()), so using rfree() (and freeing it here at all) risks allocator mismatch or double-free when DP-to-DP bindings are torn down.
if (alloc && alloc->vreg) {
if (!vregion_put(alloc->vreg))
rfree(alloc);
}
- Files reviewed: 86/87 changed files
- Comments generated: 1
- Review effort level: Lite
| msg = shared_c->slots[slot].u.msg; | ||
| if (msg.message && msg.message_length > 0) { | ||
| if (msg.message_length <= sizeof(msg_buf)) { | ||
| if (memcpy_s(msg_buf, sizeof(msg_buf), | ||
| (__sparse_force void *)(shared_c->slots[slot].u.msg_raw + sizeof(msg)), | ||
| msg.message_length) != 0) { | ||
| ams_release(shared_c); | ||
| return -EINVAL; | ||
| } | ||
| msg.message = msg_buf; | ||
| } else { | ||
| msg.message = (__sparse_force uint8_t *)(shared_c->slots[slot].u.msg_raw + sizeof(msg)); | ||
| } | ||
| } |
a6eed3b to
d36213c
Compare
d36213c to
e1b3bf6
Compare
module_adapter_calculate_dp_period() divides by sink_get_frame_bytes() * sink_get_rate() for every sink of the module. Phrase-detect / event modules such as microWakeWord expose sinks that carry no audio data: their rate and frame size are zero, which crashes the DP scheduler with a divide-by-zero. Skip any sink whose frame_bytes or rate is zero. Modules with such sinks are expected to set dev->period themselves (as MFCC now does based on its FFT hop cadence). Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
sink_get_free_frames() unconditionally divided by sink_get_frame_bytes(), assuming the format had been fully propagated by the time a module queried the sink. That does not hold for component-to-component sinks whose format is set only after the upstream component finishes its own prepare (e.g. SRC->KPB before KPB publishes its input buffer format), leading to a divide-by-zero. Return 0 when frame_bytes is zero, mirroring the guard that already exists in source_get_data_frames_available(). Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
vregion.c uses EXPORT_SYMBOL() but only picked the macro up indirectly through other headers. When those headers stop pulling llext/symbol.h in (e.g. depending on Kconfig knobs) the build fails with an implicit declaration. Include the header directly. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
When a pipeline is being stopped or unbound, the component unbind handler may clear pipeline->source_comp before the low-latency copy task finishes its final execution tick. Attempting to access p->source_comp->direction without checking for NULL triggers a synchronous PIF exception (DSP panic). Add NULL checks for p->source_comp and start before proceeding with the pipeline graph copy walk, returning 0 cleanly if the pipeline endpoints are no longer valid. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
The module_adapter DP period helper derives its scheduling period from the sink's rate and free space. For MFCC the sink is a phrase-detect / feature stream whose rate is not yet propagated at prepare time, so the derived period would be bogus (or zero) and the DP thread would be scheduled at the wrong cadence. Compute dev->period from the FFT hop size and source rate directly in mfcc_prepare(). This gives module_adapter a valid override before it inspects the sinks, and matches the natural cadence at which the MFCC component produces feature frames. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
e1b3bf6 to
7a81efa
Compare
Add PCAN (Per-Channel AGC Normalization) fixed-point math library for normalizing mel filterbank energies. PCAN estimates per-channel noise background, applies non-linear wide-dynamic-range AGC compression with fixed-point LUT gain lookup, and delivers scaled features for streaming keyword-spotting networks. Include ztest test suite with MATLAB reference models covering gain lookup, noise estimation, dynamic shrinking, stream processing, and corner cases. Also add tflite-micro dependency to west.yml. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com> Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
Add 8-bit PCAN-normalized Mel feature extraction to the MFCC module. When CONFIG_COMP_MFCC_PCAN is enabled, linear Mel filterbank energies are processed through the PCAN AGC pipeline, quantized to 8-bit int8 values, and prepended with the standard mfcc_data_header. Ensure dev->frames is at least frame_shift in DP domain, calibrate linear Mel magnitudes against microfrontend scaling, and tune VAD noise tracking and speech thresholding for the scaled mel-log spectrum. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com> Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
Add PCAN export configuration to setup_mfcc.m and generate the MFCC profile blobs used by microWakeWord, including 40-bin mel profiles with 10 ms hops and 8-bit PCAN output. Disable VAD control updates in mel40_10ms_compress because its encoder-hosted MFCC widget cannot be mapped by the kernel for module notifications, avoiding unmatched-notification mailbox timeouts. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com> Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
Add the microWakeWord keyword-spotting processing component. MWW runs a TensorFlow Lite Micro streaming network (based on the microWakeWord project) on MFCC features in the Data Processing domain. The component supports consuming 8-bit normalized Mel features in PCAN mode or 32-bit features with AGC. Include the built-in 'Hi Intel' and 'Hey Jarvis' model headers, selected through Kconfig, while retaining runtime bytes-control model loading as an alternative. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com> Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
Include audio/microwakeword/mww.toml when CONFIG_COMP_MWW is enabled across platform rimage manifest headers (tgl, tgl-h, mtl, lnl, ptl, wcl) so the MWW module UUID and entry are registered in base firmware images and loadable on target devices. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com> Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
7a81efa to
f4282c5
Compare
…ner toolchain Add tooling for microWakeWord training, dataset preparation, and verification. Includes scripts for generating synthetic speech datasets via Piper TTS, ingesting multi-speaker genuine recordings, training PCAN streaming MixConv networks with noise and transient negatives, and exporting built-in model headers and sof-ctl blobs. The end-to-end pipeline can synthesize competing wake phrases with Piper TTS and append them to unknown/ as hard negatives before feature extraction. Also include sof_run_mww.py and mtrace visualization tooling. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
Add topology definitions, pipelines, and target configurations for microWakeWord (MWW) keyword spotting and Keyphrase Buffer (KPB) Wake-on-Voice capture across Intel platforms (SoundWire, PCH DMIC, HDA analog capture, and lean SSP0 nocodec). Include 8-bit PCAN feature extraction support, micsel downmix and upmix profiles, benchmark topologies, and targets for PTL laptop and ADL max98357a-rt5682. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
Add sof-ctl text blobs for the 'Hi Intel' and 'Hey Jarvis' microWakeWord models. They are used to load a model over the bytes control when CONFIG_COMP_MWW_MODEL_FROM_CONTROL is enabled. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
Call scheduler_dp_init() in platform_init() on cAVS platforms when CONFIG_ZEPHYR_DP_SCHEDULER is enabled so that DP tasks (such as MFCC and MWW in the Data Processing domain) can bind to the DP scheduler without failing with -ENODEV (-19). Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
Binding two DP (Data Processing) scheduled components was previously rejected with IPC4_INVALID_REQUEST because both sides required a secondary ring buffer. This patch adds support for DP-to-DP component binding under a new CONFIG_DP_TO_DP_BIND Kconfig option. In a DP-to-DP connection, a single shared ring buffer is created and attached as a secondary buffer on both the source and sink sides of the intermediate comp_buffer. The upstream DP module writes directly to the ring buffer sink API, and the downstream DP module reads directly from its source API. No copying or intermediate synchronization is required during low-latency (LL) scheduling cycles. The DP module virtual memory region backing the ring buffer is refcounted so that it remains valid across component lifetimes, and audio buffer reset and free operations ensure the shared secondary buffer is not reset or freed twice. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
The AMS/IDC path used by MWW to notify KPB about a detected keyword is not reliable when the two components live on different cores on upstream Zephyr: the wake path goes through p4wq and can crash under DP context. Add a small dcache-managed slot (kpb_notify_slot) written by the detector via kpb_notify_request_drain() and polled by kpb_copy() while KPB is in RUN state. When a pending drain request is picked up, KPB kicks off draining with a synthetic client descriptor. Make kpb_copy() defensive on IPC4 for two states where sinks can go away asynchronously. In RUN, sel_sink may still be NULL because kpb_bind() has not connected the detector pipeline yet; buffer input into history and stop the copy chain instead of returning an error. In HOST_COPY, host_sink can disappear when the host copier tears down mid-drain (arecord stopped, xrun recovery); fall back to RUN and return 0 so the kernel can pause/reset the pipeline cleanly instead of failing the free with a stuck ACTIVE state. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
Replace the AMS/IDC-based mww_notify_kpb() implementation with a call to kpb_notify_request_drain(). The AMS path went through p4wq wake-up across cores, which is unsafe from DP context on upstream Zephyr and had been observed to fault when the detector and KPB run on different cores. The new polling notify slot handles the cross-core case with a plain dcache flush + poll in kpb_copy(), so mww only has to publish the requested drain time and return. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
This patch enables the modules needed for keyword detect in a static build: KPB, MFCC, MWW, Tensorflow, etc. The impacted platforms are cavs25 and ace30. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
Keep keyword detection active while suppressing MFCC feature traffic to the host when D0i3 entry is allowed. Resume the host output when power gating is prevented again. When a keyword is detected in D0i3, latch the MWW host output on until D0i0 so the next MFCC hop wakes the host through PCM103. Mark both the WoV capture and detector PCMs as D0i3-compatible, and avoid dereferencing an absent KPB host sink when detection runs without the WoV capture stream. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Allow MWW detection modules to run without a firmware output sink. The kernel ignores routes involving virtual widgets, so the detector must be source-only while the virtual route connects the DAPM graph to the KPB host copier. Update all MWW topology variants to use virtual.detect_sink and remove the dedicated detect PCM. Keep the alternate SRC pipeline consistent with the same source-only behavior. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
Use the DAI-side DMIC micsel output directly as the KPB and MWW input for the ADL MWW topology. Remove the redundant detector-side micsel and configure the detector for mono MFCC frames. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
f4282c5 to
c87661e
Compare
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
One or more issues must be addressed before approval.
Review effort: Lite
Findings: 13
Open (15)
Remove unconditional Clang flag from global build options · New Require selector sink readiness before draining · New Prevent PCAN from bypassing normal MFCC DCT output · New Add IPC4 and module-adapter dependencies for MWW · New Include Zephyr kernel API for debug timing calls · New Align PCAN hop layout with topology selection · New Set MWW module capacity to one sink · New Match PCAN log scale shift with training configuration · New Enable PCAN for the cavs25 MWW topology · New Declare one output pin for routed MWW sink · New Declare the routed MWW output pin · New Match MWW formats to configured channel count · New ams_process_slot() decides whether to rebind msg.message based on the saved pointer value… Preserve legacy Mel spectrogram log defaults · New Quantize PCAN features using model parameters · New
Resolved since last review (1)
| if(CMAKE_C_COMPILER_ID MATCHES "Clang") | ||
| add_compile_options(-mllvm --text-section-literals=false) | ||
| endif() |
| #if CONFIG_COMP_MFCC_PCAN | ||
| if (state->pcan.enable_pcan) { | ||
| int8_t *out8 = (int8_t *)state->out_stage; | ||
|
|
||
| for (k = 0; k < num_ceps; k++) { | ||
| /* Map PCAN log-scaled output (~0..666) to int8 [-128..127] */ | ||
| int32_t val = (int32_t)state->mel_linear[k]; |
| depends on SOF_STAGING | ||
| depends on CPP | ||
| depends on STD_CPP17 |
|
|
||
| #include <stdarg.h> | ||
| #include <stdio.h> | ||
| #include <zephyr/sys/printk.h> |
| stack_bytes_requirement 8192 | ||
| heap_bytes_requirement "$[(24 * 1024)]" | ||
| num_input_audio_formats 1 | ||
| num_output_pins 0 |
| num_input_pins 1 | ||
| num_output_pins 0 | ||
| num_input_audio_formats 1 | ||
| num_output_audio_formats 1 |
| Object.Base.input_audio_format [ | ||
| { | ||
| in_channels 2 | ||
| in_bit_depth 32 | ||
| in_valid_bit_depth 32 | ||
| in_rate 16000 | ||
| ibs $MWW_INPUT_BYTES |
| cfg.vtln_warp = 1.0; | ||
| cfg.window_type = 'hann'; | ||
| cfg.mel_log = 'log10'; | ||
| cfg.mel_log = 'log'; % Set to 'db' for librosa, set to 'log10' for matlab |
| #if CONFIG_COMP_MWW_PCAN | ||
| /* PCAN mode: MFCC has already normalized and quantized Mel energies | ||
| * to int8_t (Q1.7). Copy directly into the feature buffer slice. | ||
| */ | ||
| memcpy(slice, hop_src + sizeof(struct mfcc_data_header), MWW_FEATURE_SIZE); |
Consolidate and refactor all microWakeWord (MWW) and Key Phrase Buffer (KPB) Wake-on-Voice topology configurations to use a shared mono 16 kHz pipeline architecture across SoundWire DMIC, SoundWire jack, PCH DMIC, and CAVS NoCodec capture paths. Changes: - Add mww-kpb-fe.conf: shared frontend containing the host-gateway-capture pipeline (100), mww-detect pipeline (103), WoV PCM (102), and common drain routes. - Add micsel-src-kpb-be.conf: pipeline class that downmixes stereo DAI capture to mono via micsel before SRC and KPB buffering. - Add sdw-mww-kpb.conf: SoundWire MWW KPB backend template including micsel-src-kpb-be and mww-kpb-fe; used by both DMIC and jack paths. - Refactor sdw-dmic-mww-kpb.conf and sdw-jack-mww-kpb.conf as thin wrappers that define only the endpoint module-copier ID and stream name. - Refactor dmic-mww-kpb.conf to define only its DAI-side backend route and include mww-kpb-fe.conf. - Refactor cavs-nocodec-mww-kpb.conf to use micsel-src-kpb-be and mww-kpb-fe, removing redundant pipeline/route/PCM definitions. - Move micsel to between the DAI module-copier and SRC on SoundWire and NoCodec paths (was before the MFCC detection pipeline). Configure KPB, host copier, and WoV PCM for single-channel mono operation at 16 kHz. Connect KPB directly to MFCC with a 640-byte hop buffer. HDA topologies are left unchanged as mono capture is not supported on HDA. - Rename the TGL NoCodec MWW topology target from sof-tgl-nocodec-mww to sof-tgl-nocodec-mww-pcan-kpb for consistency with other PCAN KPB topologies; set MWW_PCAN=true explicitly in the target arguments. - Remove sof-arl-cs42l43-l0-cs35l56-l23-mww-kpb and sof-mtl-rt713-l0-rt1316-l12-mww-kpb from development topology targets; both platforms lack sufficient DSP RAM for the MWW and KPB pipelines. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
When connecting a pipeline with CONFIG_ZEPHYR_DP_SCHEDULER enabled, ipc4_comp_connect() checks if either source or sink is in the DP domain (src_is_dp || sink_is_dp). Previously, it unconditionally called comp_mod() on both source and sink to retrieve module private data (mpd.in_buff_size and mpd.out_buff_size) for ring_buffer_create(). However, components that are not module adapters (such as SOF_COMP_KPB) do not have dev->mod set, returning NULL from comp_mod(). When binding KPB directly to a DP module (such as MFCC), dereferencing src_module_data->mpd resulted in an EXCCAUSE 0x0000000d (LoadStorePIFDataErrorCause) synchronous PIF data error DSP panic at helper.c:956. Only retrieve module private buffer sizes when the corresponding endpoint is both in the DP domain and is a module adapter. Otherwise, fall back safely to ibs / obs. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
A pipeline containing only DP-domain modules (e.g. mww-detect with mfcc + mww) never had pipeline_comp_ll_task_init() called during pipeline_comp_prepare(), because the call was guarded by: if (current->ipc_config.proc_domain == COMP_PROCESSING_DOMAIN_LL) As a result, p->pipe_task and p->trigger_task remained NULL. When the host triggered such a pipeline to RUNNING, pipeline_schedule_triggered() called schedule_task(p->trigger_task, 0, 0) on a NULL pointer (producing a 'failed to schedule trigger task' log), then called pipeline_schedule_copy() which dereferenced p->pipe_task via task_is_active(), causing a LoadStorePIFDataErrorCause DSP panic. Fix by removing the LL-only guard in pipeline_comp_prepare() so that pipeline_comp_ll_task_init() is called unconditionally for every pipeline component. Every pipeline needs the LL trigger task to handle IPC trigger commands and the pipe task to drive inter-component ring buffer synchronisation, regardless of whether its modules run in LL or DP domain. Add defensive guards to make the code robust against future regressions: - task_is_active(): return false for NULL task pointer. - pipeline_schedule_copy(): early return with an error log if pipe_task is NULL to prevent a silent crash. - pipeline_schedule_triggered(): check trigger_task != NULL before calling schedule_task() in all IPC4 branches. - pipeline_task_init(): guard p->sched_comp != NULL before dereferencing it when computing task->registrable. - pipeline_comp_ll_task_free(): NULL out trigger_task and pipe_task after freeing to prevent use-after-free. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
Address valid concerns raised by the GitHub Copilot review bot on PR thesofproject#11135. Items already fixed in later commits (HTTP->HTTPS, memcpy_s return check, ams.c message pointer, llext-wrap.c ssize_t stubs, shell script globs and duplicate function) are not repeated here. CMakeLists.txt: Remove the global Clang --text-section-literals=false flag which was applied to every Clang build, including targets unrelated to MWW. Older Clang versions reject this option, breaking unrelated targets. The MWW component's own CMakeLists.txt already guards it with check_cxx_compiler_flag() scoped to the MWW target only. src/audio/microwakeword/Kconfig: Add 'depends on IPC_MAJOR_4'. mww.c calls ipc4_d0ix_is_allowed() which is an IPC4-only API; selecting COMP_MWW for an IPC3 build results in an unresolved symbol at link time. src/audio/microwakeword/mww.c: Add #include <zephyr/kernel.h> under the CONFIG_COMP_MWW_DEBUG_TRACE guard. The debug path calls k_cycle_get_32() and k_cyc_to_us_near32() which are declared in that header; without it the build fails when DEBUG_TRACE is enabled. Add a compile-time #error guard that enforces CONFIG_COMP_MWW_PCAN and CONFIG_COMP_MFCC_PCAN are always set together. If MFCC produces int32 Q9.23 hops (PCAN off) while MWW expects int8 hops (PCAN on), every subsequent hop header and feature frame is desynchronized and inference results are garbage. The topology MWW_PCAN flag must also match this Kconfig pair. src/audio/mfcc/mfcc_common.c: In PCAN mode mfcc_prepare_output() indexes mel_linear[] by k < num_ceps, but the array has only melfb.mel_bins entries. If num_ceps > mel_bins (misconfiguration) the loop reads out of bounds. Add an early return guard using the correct field name melfb.mel_bins. src/audio/kpb.c: kpb_init_draining() already guards host_sink, but the draining path later unconditionally dereferences kpb->sel_sink when pausing the selector widget. With IPC4 late-binding sel_sink can still be NULL when a drain request arrives on another core. Add sel_sink to the is_sink_ready check so draining is deferred until both sinks are present and active. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
Update the NHLT configuration and DMIC DAI copier input formats for the Alder Lake microWakeWord (MWW) PCAN KPB topology. Specifically: - In dai-copier-micsel-eqiir-kpb-be, restrict the DMIC DAI copier input audio formats to 32-bit (16 kHz, 4 channels), removing 16-bit and 24-in-32 formats to match the NHLT configuration. - In tplg-targets.cmake, update NHLT_BIN to nhlt-sof-adl-max98357a-rt5682-mww-pcan-kpb.bin for the sof-adl-max98357a-rt5682-mww-pcan-kpb target. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
Copy the static build keyword detect Kconfig configuration from ACE 3.0 (intel_adsp_ace30_ptl) to ACE 4.0 (intel_adsp_ace40_nvl and nvls). This enables the static modules needed for keyword detect and feature processing (MFCC, PCAN, MWW, AMS, DRC, MULTIBAND_DRC, S24_3LE), disables loadable modules, adjusts EDF stack and heap/malloc arena sizes, sets IDC timeout, enables telemetry, disables UAOL, and retains platform-specific settings for Nova Lake. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
9972bbf to
d8efb6c
Compare
Address valid concerns raised by the GitHub Copilot review bot on PR thesofproject#11135. Items already fixed in later commits (HTTP->HTTPS, memcpy_s return check, ams.c message pointer, llext-wrap.c ssize_t stubs, shell script globs and duplicate function) are not repeated here. CMakeLists.txt: Remove the global Clang --text-section-literals=false flag which was applied to every Clang build, including targets unrelated to MWW. Older Clang versions reject this option, breaking unrelated targets. The MWW component's own CMakeLists.txt already guards it with check_cxx_compiler_flag() scoped to the MWW target only. src/audio/microwakeword/Kconfig: Add 'depends on IPC_MAJOR_4'. mww.c calls ipc4_d0ix_is_allowed() which is an IPC4-only API; selecting COMP_MWW for an IPC3 build results in an unresolved symbol at link time. src/audio/microwakeword/mww.c: Add #include <zephyr/kernel.h> under the CONFIG_COMP_MWW_DEBUG_TRACE guard. The debug path calls k_cycle_get_32() and k_cyc_to_us_near32() which are declared in that header; without it the build fails when DEBUG_TRACE is enabled. Add a compile-time #error guard that enforces CONFIG_COMP_MWW_PCAN and CONFIG_COMP_MFCC_PCAN are always set together. If MFCC produces int32 Q9.23 hops (PCAN off) while MWW expects int8 hops (PCAN on), every subsequent hop header and feature frame is desynchronized and inference results are garbage. The topology MWW_PCAN flag must also match this Kconfig pair. src/audio/mfcc/mfcc_common.c: In PCAN mode mfcc_prepare_output() indexes mel_linear[] by k < num_ceps, but the array has only melfb.mel_bins entries. If num_ceps > mel_bins (misconfiguration) the loop reads out of bounds. Add an early return guard using the correct field name melfb.mel_bins. src/audio/kpb.c: kpb_init_draining() already guards host_sink, but the draining path later unconditionally dereferences kpb->sel_sink when pausing the selector widget. With IPC4 late-binding sel_sink can still be NULL when a drain request arrives on another core. Add sel_sink to the is_sink_ready check so draining is deferred until both sinks are present and active. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>


No description provided.