audio: codec_adapter: add DAX audio processing module#10241
audio: codec_adapter: add DAX audio processing module#10241kv2019i merged 1 commit intothesofproject:mainfrom
Conversation
checkupup
commented
Sep 16, 2025
- Add DAX audio processing module
- Add topology support for MTL platform
- Add testbench topology support
|
Can one of the admins verify this patch?
|
|
@blin-dolby Hello, bin, This commit is waiting for CI checking. Is there any suggestion for reviewers of SOF? |
|
Let me check with Google team. |
singalsu
left a comment
There was a problem hiding this comment.
Looks good. I added some non-critical suggestions, and maybe spit this to multiple commits per topic.
If you'd like SOF CI to regularly test this component with testbench run, add run command to https://github.com/thesofproject/sof/blob/main/scripts/host-testbench.sh.
tools/topology/topology2/include/bench/dolby-dax_hda_route.conf
Outdated
Show resolved
Hide resolved
|
I added @andyross and myself as reviewers from Google since this will support Google projects. |
johnylin76
left a comment
There was a problem hiding this comment.
if commit "audio: codec_adapter: update code based on review results" is to address comments, you should merge it into the original one.
|
@checkupup @blin-dolby @johnylin76 I'm going to fork stable v2.14 tomorrow, but I'm open to delaying release a little if we can get the review comments resolved and this PR merged soon. |
d5f9206 to
6b2fa98
Compare
6b2fa98 to
b0bc86d
Compare
|
test this please |
@checkupup just to avoid misunderstanding - this wasn't directed to you or any other human, this was directed to our CI |
* Add DAX audio processing module * Add topology support for MTL platform * Add testbench topology support Signed-off-by: Jun Lai <jun.lai@dolby.com>
b0bc86d to
07acece
Compare
lyakh
left a comment
There was a problem hiding this comment.
just some ideas maybe for a follow-up, not critical for the initial merge
| uint32_t free; /* Free bytes for writing */ | ||
| }; | ||
|
|
||
| struct sof_dax { |
There was a problem hiding this comment.
Not worth an update, but if you do for some reason have to update or maybe do this as an incremental patch - maybe you could consider using standard C types in this and other structures, which aren't parts of an ABI:
|
|
||
| struct sof_dax { | ||
| /* SOF module parameters */ | ||
| uint32_t sof_period_bytes; |
There was a problem hiding this comment.
struct comp_dev *dev = mod->dev;
dax_ctx->sof_period_bytes = dev->frames *
dax_ctx->output_media_format.num_channels *
dax_ctx->output_media_format.bytes_per_sample;sof_period_bytes uses the same type as dev->frames, and the remaining uint32_t type also serves the same purpose.
| uint32_t sof_period_bytes; | ||
|
|
||
| /* DAX state parameters */ | ||
| uint32_t period_bytes; |
There was a problem hiding this comment.
same reason as above, period_bytes keep same type with sof_period_bytes.
|
|
||
| /* DAX state parameters */ | ||
| uint32_t period_bytes; | ||
| uint32_t period_us; |
There was a problem hiding this comment.
this depends on how it's used, where you get it from, maybe unsigned long?
There was a problem hiding this comment.
from:
dax_ctx->period_us = 1000000 * dax_ctx->period_bytes /
(dax_ctx->output_media_format.bytes_per_sample *
dax_ctx->output_media_format.num_channels *
dax_ctx->output_media_format.sampling_rate);period_us uses same types as period_bytes.
| struct dax_media_fmt output_media_format; | ||
|
|
||
| /* DAX control parameters */ | ||
| int32_t enable; |
There was a problem hiding this comment.
if this is an on-off flag, then bool?
There was a problem hiding this comment.
Each *_enable variables in sof_dax is an on-off flag. The bool type may encount a compiation error when using rebuild_testbench.sh -p ${platform} based on xtensa toolchains, so I avoid using bool type for compatibility.
There was a problem hiding this comment.
@checkupup sorry, this is strange. SOF has bool in a lot of places, which compilations break with it? You might need to include a suitable header for it, but I'd be surprised if there was a build option currently that didn't come across a single bool use.
There was a problem hiding this comment.
Could we can pick up this after merging this request if it is not critical for this merge? It's been quite a while since I last encountered this issue. I will try to reproduce bool error later.
|
@johnylin76 hello, could you help to merge this request if it is ok? |
|
SOFCI TEST |
|
@checkupup This looks good to go now. Let me rerun the one set of CI jobs that seems to be stuck. We should get results today and can then merge. |