audio: kpb: Fix overruns in kpb module #10067
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The ibs/obs should be equal between components. However, some modules (like kpb) produce different data sizes on output pins. Unfortunately, only one ibs/obs can be specified in the modules configuration. To avoid creating too small buffer choose the maximum value between ibs and obs. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
softwarecki
requested review from
abonislawski,
Copilot,
jxstelter,
kv2019i,
marcinszkudlinski,
serhiy-katsyuba-intel,
tmleman and
wszypelt
softwarecki
requested review from
dbaluta,
fkwasowi,
iganakov,
lbetlej,
lgirdwood,
mmaka1 and
plbossart
as code owners
There was a problem hiding this comment.
Pull Request Overview
This PR fixes buffer overrun issues in the kpb module by discarding input data when the kd module is not active and adjusting buffer size calculations to use the maximum value between ibs and obs. It also cleans up log messages by removing redundant function names and updates format specifiers for correct error code display.
- Discard incoming samples when the kd module isn't active.
- Use MAX(ibs, obs) for buffer size calculation and update log messages.
- Adjust log format specifiers for signed integers.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/ipc/ipc4/helper.c | Revised buffer size calculation using MAX(ibs, obs) |
| src/audio/kpb.c | Updated log messages and added check to discard data when sink inactive |
| src/audio/dai-zephyr.c | Changed log format specifiers to use signed formats |
Comments suppressed due to low confidence (4)
src/ipc/ipc4/helper.c:575
- Consider adding a brief inline comment to explain the rationale for using MAX(ibs, obs) when calculating the buffer size, which helps future maintainers understand the intent.
buf_size = MAX(ibs, obs) * 2;
src/audio/kpb.c:1218
- Add a comment clarifying why discarding all available bytes when the sink is inactive is considered safe and appropriate in this context.
/* Discard data if sink is not active */
src/audio/kpb.c:1525
- [nitpick] Verify that the updated log message, now without the function name prefix, still provides sufficient context for debugging unsupported commands.
comp_err(dev, "unsupported command");
src/audio/dai-zephyr.c:1675
- Using %d to display the signed error code 'ret' is correct; ensure this change is applied consistently across all similar log messages.
ret);
lyakh
approved these changes
Jun 25, 2025
Collaborator
lyakh
left a comment
lyakh
left a comment
There was a problem hiding this comment.
just a minor nitpick, not even worth a new version, only if an update is needed for another reason
Until the pipeline containing the kd module is started, discard incoming samples to avoid overrun on dai and not to pass a signal containing a glitch to kd. The kd module is placed on a separate pipeline, which can be started after the pipeline containing kpb. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
Use signed format when displaying an int error code. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
Use unsigned format when displaying an uint32_t size. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
Remove unnecessary function names from log messages. Zephyr automatically adds function names to log entries. In some places, after changing the function name, log messages were not corrected. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
Collaborator
Author
|
@lyakh I fixed a typo in the commit message, so I took your suggestion into account :) |
lgirdwood
approved these changes
Jun 25, 2025
tmleman
approved these changes
Jun 25, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Discard input data if kd module isn't active. Until the pipeline containing the kd module is started, discard incoming samples to avoid overrun on dai and not to pass a signal containing a glitch to kd. The kd module is placed on a separate pipeline, which can be started after the pipeline containing pkb.
Use
ibsin components buffer size calculation. Theibs/obsshould be equal between components. However, some modules (like kpb) produce different data sizes on output pins. Unfortunately, only oneibs/obscan be specified in the modules configuration. To avoid creating too small a buffer choose the maximum value betweenibsandobs.Extras: