Skip to content

Conversation

@checkupup
Copy link
Contributor

  • Add DAX audio processing module
  • Add topology support for MTL platform
  • Add testbench topology support

@sofci
Copy link
Collaborator

sofci commented Sep 16, 2025

Can one of the admins verify this patch?

reply test this please to run this test once

@checkupup
Copy link
Contributor Author

@blin-dolby Hello, bin, This commit is waiting for CI checking. Is there any suggestion for reviewers of SOF?

@blin-dolby
Copy link

Let me check with Google team.

Copy link
Collaborator

@singalsu singalsu left a comment

Choose a reason for hiding this comment

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

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.

@johnylin76
Copy link
Contributor

I added @andyross and myself as reviewers from Google since this will support Google projects.

Copy link
Contributor

@johnylin76 johnylin76 left a comment

Choose a reason for hiding this comment

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

if commit "audio: codec_adapter: update code based on review results" is to address comments, you should merge it into the original one.

@lgirdwood
Copy link
Member

@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.

@checkupup checkupup force-pushed the dax_dev_main branch 2 times, most recently from d5f9206 to 6b2fa98 Compare September 22, 2025 10:00
@lyakh
Copy link
Collaborator

lyakh commented Sep 26, 2025

test this please

@lyakh
Copy link
Collaborator

lyakh commented Sep 26, 2025

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>
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 some ideas maybe for a follow-up, not critical for the initial merge

uint32_t free; /* Free bytes for writing */
};

struct sof_dax {
Copy link
Collaborator

Choose a reason for hiding this comment

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

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;
Copy link
Collaborator

Choose a reason for hiding this comment

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

this could become size_t?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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;
Copy link
Collaborator

Choose a reason for hiding this comment

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

and this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same reason as above, period_bytes keep same type with sof_period_bytes.


/* DAX state parameters */
uint32_t period_bytes;
uint32_t period_us;
Copy link
Collaborator

Choose a reason for hiding this comment

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

this depends on how it's used, where you get it from, maybe unsigned long?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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;
Copy link
Collaborator

Choose a reason for hiding this comment

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

if this is an on-off flag, then bool?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

@checkupup
Copy link
Contributor Author

@johnylin76 hello, could you help to merge this request if it is ok?

@kv2019i
Copy link
Collaborator

kv2019i commented Oct 16, 2025

SOFCI TEST

@kv2019i
Copy link
Collaborator

kv2019i commented Oct 16, 2025

@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.

@kv2019i kv2019i merged commit 38a7e63 into thesofproject:main Oct 16, 2025
38 of 47 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.

9 participants