Skip to content
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

Tools: Testbench: Convert file component to module adapter #8811

Closed
wants to merge 2 commits into from

Conversation

singalsu
Copy link
Collaborator

This is done as preparation for testbench IPC4 support. The update to IPC4 is simpler for a module adapter component.

This is done as preparation for testbench IPC4 support. The
update to IPC4 is simpler for a module adapter component.

Signed-off-by: Seppo Ingalsuo <[email protected]>
The file module does not provide a dai_get_init_delay_ms()
operation so it is skipped for CONFIG_LIBRARY build.

Signed-off-by: Seppo Ingalsuo <[email protected]>
Copy link
Contributor

@btian1 btian1 left a comment

Choose a reason for hiding this comment

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

one question: will testbench support IPC3 and IPC4 both or after change, testbench only support IPC4?

uint32_t type; /**< sof_ipc_process_type */

/* reserved for future use */
uint32_t reserved[7];
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest put reserved to the bottom of struct definition.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did it this way since currently module adapter defaults to process type IPC unless I add exception handling to module adapter. The extra IPC data for file component is passed in the end of this struct.

I think I will try to another approach with CONFIG_LIBRARY exceptions added for SOF_COMP_FILEREAD and SOF_COMP_FILEWRITE. Not sure which is better.

Also this will be bit different with IPC4. I'm keeping this draft until I know better.

#include <rtos/wait.h>
#include <sof/audio/pipeline.h>
#include <sof/audio/component_ext.h>
#include <tplg_parser/topology.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. maybe it is a good time to check whether all the headers are all must to put it here.
  2. suggest a sequence for header include, from long to short.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm following a rule for alphabetic sort for 4 levels, 3 levels, 2 levels, standard C headers last. I got such suggestion some time ago in review. Though these could go to other cosmetic changes patch.

file_uuid->d[4] = 0x08;
file_uuid->d[5] = 0xa6;
file_uuid->d[6] = 0x98;
file_uuid->d[7] = 0xc2;
Copy link
Contributor

Choose a reason for hiding this comment

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

how about a strcpy to replace above?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, could be better, or at least better readable.

@singalsu
Copy link
Collaborator Author

one question: will testbench support IPC3 and IPC4 both or after change, testbench only support IPC4?

I think there need to be separate executable build for IPC versions. The SOF headers are constructed in such way that I can't have both in the same time.

@singalsu
Copy link
Collaborator Author

Closing, this change is part of #9025

@singalsu singalsu closed this May 30, 2024
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.

2 participants