-
Notifications
You must be signed in to change notification settings - Fork 349
audio: buffer: use add BUFFER_USAGE_ flags to improve readability #10230
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
Conversation
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.
Pull Request Overview
This PR introduces named constants BUFFER_USAGE_SHARED and BUFFER_USAGE_NOT_SHARED to replace boolean literals in buffer allocation functions, improving code readability and maintainability.
- Adds buffer usage flag definitions to make code more self-documenting
- Replaces
falseliterals withBUFFER_USAGE_NOT_SHAREDin buffer allocation calls - Updates multiple audio component files to use the new constants
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/include/sof/audio/buffer.h | Defines BUFFER_USAGE_SHARED and BUFFER_USAGE_NOT_SHARED constants |
| src/audio/module_adapter/module_adapter.c | Updates buffer allocation to use BUFFER_USAGE_NOT_SHARED |
| src/audio/host-zephyr.c | Updates buffer allocation to use BUFFER_USAGE_NOT_SHARED |
| src/audio/host-legacy.c | Updates buffer allocation to use BUFFER_USAGE_NOT_SHARED |
| src/audio/dai-zephyr.c | Updates buffer allocation to use BUFFER_USAGE_NOT_SHARED |
| src/audio/dai-legacy.c | Updates buffer allocation to use BUFFER_USAGE_NOT_SHARED |
| src/audio/chain_dma.c | Updates buffer allocation to use BUFFER_USAGE_NOT_SHARED |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| #define BUFFER_USAGE_SHARED true /* buffer used by multiple DSP core and/or HW blocks */ | ||
| #define BUFFER_USAGE_NOT_SHARED false /* buffer only used by one HW block */ |
Copilot
AI
Sep 11, 2025
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.
[nitpick] Consider using enum values instead of boolean literals for buffer usage flags. This would provide better type safety and prevent accidental mixing of boolean values with these constants.
| #define BUFFER_USAGE_SHARED true /* buffer used by multiple DSP core and/or HW blocks */ | |
| #define BUFFER_USAGE_NOT_SHARED false /* buffer only used by one HW block */ | |
| enum buffer_usage { | |
| BUFFER_USAGE_NOT_SHARED = 0, /* buffer only used by one HW block */ | |
| BUFFER_USAGE_SHARED = 1 /* buffer used by multiple DSP core and/or HW blocks */ | |
| }; |
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.
There are still places in code that have logic to calculate a boolean "is_shared" and I see no value to change these and make this an explicit enum. The define is fine to improve readability and matches existing coding style in SOF.
lyakh
left a comment
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.
could add buffer_new() here as well
Passing boolean parameters to key functions results in hard to read code. Add BUFFER_USAGE_SHARED and BUFFER_USAGE_NOT_SHARED definitions to make code allocating buffers easier to read and maintain. No functional change. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
9d48798 to
c2ce371
Compare
|
V2:
|
|
@kv2019i can you check CI. Thanks ! |
|
Sorry, I didn't realize this was failing. I was monitoring the previous quickbuild run for this PR and it passes (14737859 id -> all pass). Now another quickbuild was run (14738805) and it filed to chain-dma xruns on MTL: This seems like a real issue, but given this PR (at least should be) a no-op patch with no functional impact, I suspect this is not related to this PR. @lrudyX have been seen similar failures in other PRs? |
Today we have some hardware problems with CI. We have quite often problems with this one test. I will rerun this PR as soon as possible. |
Passing boolean parameters to key functions results in hard to read code. Add BUFFER_USAGE_SHARED and BUFFER_USAGE_NOT_SHARED definitions to make code allocating buffers easier to read and maintain.
No functional change.