Skip to content

audio: kpb: Fix overruns in kpb module #10067

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

Merged
merged 5 commits into from
Jun 25, 2025
Merged

Conversation

softwarecki
Copy link
Collaborator

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 ibs in components buffer size calculation. 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 a buffer choose the maximum value between ibs and obs.

Extras:

  • 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.
  • Use unsigned format when displaying an uint32_t size.
  • Use signed format when displaying an int error code.

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 <[email protected]>
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);

@softwarecki softwarecki requested a review from lyakh June 24, 2025 23:00
Copy link
Collaborator

@lyakh lyakh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 <[email protected]>
Use signed format when displaying an int error code.

Signed-off-by: Adrian Warecki <[email protected]>
Use unsigned format when displaying an uint32_t size.

Signed-off-by: Adrian Warecki <[email protected]>
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 <[email protected]>
@softwarecki
Copy link
Collaborator Author

@lyakh I fixed a typo in the commit message, so I took your suggestion into account :)

@lgirdwood lgirdwood merged commit efc75b5 into thesofproject:main Jun 25, 2025
24 of 48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants