Skip to content

Rename codec_adapter to module_adapter #5593

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.

Sign up for GitHub

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

Jump to bottom
Merged
lgirdwood merged 18 commits into from
May 4, 2022
Merged

Rename codec_adapter to module_adapter #5593

lgirdwood merged 18 commits into from
May 4, 2022

Conversation

@ranj063
Copy link
Collaborator

@ranj063 ranj063 commented Mar 24, 2022

Just a whole lot of renames. No functional changes.

@ranj063
Copy link
Collaborator Author

ranj063 commented Mar 24, 2022

@dbaluta @kkarask Please note that the kconfig menu has changes to COMP_MODULE_ADAPTER with this PR.

@ranj063 ranj063 changed the title Rename codec_adapter to module_adapter. Rename codec_adapter to module_adapter Mar 24, 2022
@keqiaozhang
Copy link
Collaborator

keqiaozhang commented Mar 24, 2022

SOFCI TEST

kv2019i
kv2019i approved these changes Mar 24, 2022
View reviewed changes
Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

That has been some work to split this into small patches. But easy to review this way. I'd have been happy with one big patch as well.

@kkarask
Copy link

kkarask commented Mar 24, 2022

There is a need to change build file. We should sync to adjust this change to CI process.

dbaluta
dbaluta requested changes Mar 24, 2022
View reviewed changes
Copy link
Collaborator

@dbaluta dbaluta left a comment

Choose a reason for hiding this comment

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

This looks good to me, but I prefer to have the renaming done after my dynamic codec patches are in.

See:
#5596

@ranj063 ranj063 force-pushed the fix/codec_bytes_to_process branch from fb8fb3c to 1076f2a Compare April 13, 2022 16:18
@ranj063
Copy link
Collaborator Author

ranj063 commented Apr 13, 2022

There is a need to change build file. We should sync to adjust this change to CI process.

@kkarask can you please change the QB CI to use the new config CONFIG_MODULE_ADAPTER instead of CONFIG_CODEC_ADAPTER. It will be needed for this PR to pass.

@kkarask
Copy link

kkarask commented Apr 14, 2022

I will be ready today - is it ok?

@ranj063
Copy link
Collaborator Author

ranj063 commented Apr 14, 2022

I will be ready today - is it ok?

@kkarask sure, let me know if you have any other issues with this PR

@lgirdwood
Copy link
Member

lgirdwood commented Apr 14, 2022

@dbaluta does this mean you are OK to merge whilst part 2 of your coded work is testing ? Just clarifying.

@sys-pt1s
Copy link

sys-pt1s commented Apr 15, 2022

Can one of the admins verify this patch?

@wszypelt
Copy link

wszypelt commented Apr 15, 2022 •
edited
Loading

@ranj063 @sys-pt1s @lgirdwood @dbaluta
We ran the first tests, unfortunately the problem is probably still here:
image

Because we have errors with the file not found

@ranj063
Copy link
Collaborator Author

ranj063 commented Apr 15, 2022

Because we have errors with the file not found

@wszypelt what is the error you're seeing? Can you please paste the error log ? Is it during build?

@dbaluta
Copy link
Collaborator

dbaluta commented Apr 18, 2022

@dbaluta does this mean you are OK to merge whilst part 2 of your coded work is testing ? Just clarifying.

@lgirdwood lets wait a little bit more time until we merge this PR. part 2 of my codec work has a problem with Intel internal tests and I will try to debug it but it will take a while.

Also, since @ranj063 is not in the office this week I don't think would be a problem to wait a little bit until we merge this current PR.

@wszypelt
Copy link

wszypelt commented Apr 19, 2022 •
edited
Loading

@ranj063 yes, the error occurs while building the codec FW

13:46:15,096 INFO  - In file included from /quickbuild/workspace1/24733/SOF_FW/src/audio/module_adapter/module_adapter.c:16:
13:46:15,096 INFO  - /quickbuild/workspace1/24733/SOF_FW/src/include/sof/audio/module_adapter/module_adapter.h:11:52: error: sof/audio/codec_adapter/module/generic.h: No such file or directory
13:46:15,100 INFO  - CMakeFiles/sof.dir/build.make:995: recipe for target 'CMakeFiles/sof.dir/src/audio/module_adapter/module_adapter.c.o' failed
13:46:15,100 INFO  - make[3]: *** [CMakeFiles/sof.dir/src/audio/module_adapter/module_adapter.c.o] Error 2

@ranj063 ranj063 force-pushed the fix/codec_bytes_to_process branch from 1076f2a to b886a9a Compare April 25, 2022 17:20
ranj063 added 5 commits April 27, 2022 08:26
@ranj063
codec_adapter: rename codec_adapter_trigger()
2a1b7fe 
to module_adapter_trigger(). No functional changes.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
@ranj063
codec_adapter: rename codec_adapter_reset()
b5fabd9 
to module_adapter_reset(). No functional changes.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
@ranj063
codec_adapter: rename codec_adapter_free()
f150310 
to module_adapter_free(). No functional changes.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
@ranj063
codec_adapter: rename codec_adapter.h
ffc5d0c 
To module_adapter.h. No functional changes.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
@ranj063
codec_adapter: rename references to codec_adapter
8dc8aae 
to module_adapter. No functional changes.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
@ranj063 ranj063 force-pushed the fix/codec_bytes_to_process branch from b886a9a to f23fe0a Compare April 27, 2022 15:35
@ranj063
Copy link
Collaborator Author

ranj063 commented Apr 27, 2022

@ranj063 part 3 now merged so this PR is next.

@lgirdwood conflicts fixed now. @wszypelt could you please update the config in QB and retry this one?

ranj063 added 5 commits April 27, 2022 09:07
@ranj063
codec_adapter: rename DECLARE_CODEC_ADAPTER and comp_driver
8398184 
to DECLARE_MODULE_ADAPTER and comp_driver to comp_module_adapter.
Also rename SOF_COMP_MODULE_ADAPTER to SOF_COM_MODULE_ADAPTER.
No functional changes.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
@ranj063
codec_adapter: move CODEC_GET_API_ID definition
a050b51 
to cadence.h. It isn't used elsewhere.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
@ranj063
codec_adapter: rename codec_adapter.c to module_adapter.c
30d685e 
No functional changes.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
@ranj063
codec_adapter: rename the codec folder to module
5383b7d 
and fix all the includes accordingly. No functional changes.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
@ranj063
codec_adapter: rename it to module_adapter and fix all includes
2d0dd91 
No functional changes.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
@ranj063 ranj063 force-pushed the fix/codec_bytes_to_process branch from f23fe0a to 2d0dd91 Compare April 27, 2022 16:08
@lgirdwood
Copy link
Member

lgirdwood commented Apr 27, 2022

@ranj063 can you check CI. Thanks !

@ranj063
Copy link
Collaborator Author

ranj063 commented Apr 27, 2022

@ranj063 can you check CI. Thanks !

Sure. I cant seem to load any of them but its also quite surprising the regular tests fail. This PR doesnt touch anything outside of the codec_adapter. @wszypelt any ideas?

@lgirdwood
Copy link
Member

lgirdwood commented Apr 27, 2022

@ranj063 can you check CI. Thanks !

Sure. I cant seem to load any of them but its also quite surprising the regular tests fail. This PR doesnt touch anything outside of the codec_adapter. @wszypelt any ideas?

Could be the DUT needing a reboot ? @wszypelt good to merge ?

@wszypelt
Copy link

wszypelt commented Apr 28, 2022

@lgirdwood @ranj063 Yes, one of the DUTs needed a reset, PR is under testing. there should be results soon.

@wszypelt
Copy link

wszypelt commented Apr 28, 2022

@ranj063 @lgirdwood I don't really understand what happened, do we no longer need a configuration file change to get the codec build to work? (COMP_MODULE_ADAPTER) Even if, as you can see there was a problem with ipc error -22 on codec tests.

@ranj063
Copy link
Collaborator Author

ranj063 commented Apr 28, 2022

@ranj063 @lgirdwood I don't really understand what happened, do we no longer need a configuration file change to get the codec build to work? (COMP_MODULE_ADAPTER) Even if, as you can see there was a problem with ipc error -22 on codec tests.

@wszypelt yes, you do need a configuration change when building to include COMP_MODULE_ADAPTER and CADENCE_CODEC selected. Could you please send me the config from your build?

@wszypelt
Copy link

wszypelt commented Apr 28, 2022

@ranj063 Unfortunately, the changes can be made together with the team from the internal service, additionally I'm now on ooo, back to work at Wednesday

@dbaluta
Copy link
Collaborator

dbaluta commented May 4, 2022

@ranj063 thanks! Looks good to me.

@lgirdwood
Copy link
Member

lgirdwood commented May 4, 2022

@wszypelt any ETA on CI updates ?

@wszypelt
Copy link

wszypelt commented May 4, 2022

@lgirdwood The case was reported and explained, I am waiting for a move from the internal service, I hope that everything will be done within an hour.

@wszypelt
Copy link

wszypelt commented May 4, 2022 •
edited
Loading

@lgirdwood @ranj063
After many tries, it finally started to work! Quick merge please. Then I will be able run the rest of the pull requests

@lgirdwood lgirdwood merged commit b4591ca into thesofproject:main May 4, 2022
@lgirdwood
Copy link
Member

lgirdwood commented May 4, 2022

@wszypelt merged

@wszypelt wszypelt mentioned this pull request May 4, 2022
Merged
@marc-hb
Copy link
Collaborator

marc-hb commented May 11, 2022

Fixed by

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dbaluta dbaluta dbaluta approved these changes

@paulstelian97 paulstelian97 paulstelian97 approved these changes

@kv2019i kv2019i kv2019i approved these changes

@mrajwa mrajwa Awaiting requested review from mrajwa

@RanderWang RanderWang Awaiting requested review from RanderWang RanderWang is a code owner

@marcinszkudlinski marcinszkudlinski Awaiting requested review from marcinszkudlinski marcinszkudlinski is a code owner

@lgirdwood lgirdwood Awaiting requested review from lgirdwood lgirdwood is a code owner

@plbossart plbossart Awaiting requested review from plbossart plbossart is a code owner

@mmaka1 mmaka1 Awaiting requested review from mmaka1 mmaka1 is a code owner

@lbetlej lbetlej Awaiting requested review from lbetlej lbetlej is a code owner

@aborisovich aborisovich Awaiting requested review from aborisovich

+1 more reviewer

@cujomalainey cujomalainey cujomalainey approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.