RFC: Preparation to dynamically setup codec_id for codec_adapter by dbaluta · Pull Request #5245 · thesofproject/sof

dbaluta · 2022-01-21T15:31:59Z

Each family of codecs (e.g Cadence, DTS, WAVES) could support multiple processing algorithms. Best illustration for this is Cadence which suppports up to 14 algorithms (e.g DAP, MP3, AAC, DAB).

This PR wants to introduce a way to select the codec_id at runtime. The codec_id will be sent by the Linux kernel running on Host.

First we need to make sure that we resolve the codec_id before any API function is called because that would be impossible not knowing the codec_id.

First patches of this PR addresses the cadence codecs. It moves any API call later when codec_id will be available. For this we introduce cadence_codec_postinit function which encapsulates the api calls at init and moves them intro prepare function.

codec_adapter component has a .set_params callback but there is no equivalent for this in module_interface API.

Cc: @ranj063

This new function encapsulates all Cadence API calls at init time. We introduce this new function because the init API calls need to be done after we figure out cadence codec api id. Right now cadence codec api id is hardcoded to DEFAULT_CODEC_ID but later it will be needed to be determined dyncamically at runtime. It will be sent from host via sof_ipc_stream_params. Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>

Separate code that finds and assigns API function given an codec_id into a function. This makes code cleaner and helps us move finding the api id in another part of the code. Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>

Move cadence_codec_resolve_api to cadence_codec_post_init function. We need this because later the entire cadence_codec_post_init function will be called later then init. At init we do not know yet the codec id. Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>

We need to call cadence_codec_post_init later at a point where the codec_id is know. Best possible location is cadence_codec_prepare. By this time the sof_ipc_stream_params should have already been sent to the DSP by the Host. Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>

With this we can select a given codec from a family of codecs. E.g Cadence supports (DAP, MP3, AAC) and codec_id will help us identify at runtime which codec api function to use. Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>

ranj063 · 2022-01-21T16:07:46Z

 	uint16_t no_stream_position; /**< 1 means don't send stream position */

-	uint16_t reserved[3];
+	uint8_t codec_id;


this needs a minor ABI update

@plbossart what do you think of using the PCM_PARAMS IPC to send the API ID for compressed codecs? This field be ignored for PCM data.

it's natural to think of this, set_params and hw_params are similar in nature, but the former takes extra information into account.

Your proposal to use the codec_id is too limiting in general. For example for AAC you will need to send the format information for ADTS or ADIF. It gets even worse if you ever want to support encoders.

struct snd_codec { __u32 id; __u32 ch_in; __u32 ch_out; __u32 sample_rate; __u32 bit_rate; __u32 rate_control; __u32 profile; __u32 level; __u32 ch_mode; __u32 format; __u32 align; union snd_codec_options options; __u32 reserved[3]; } __attribute__((packed, aligned(4)));

@plbossart we have the capability to send the format information through the kcontrol data. Is that enough?

it will of course mean 2 IPC's instead of 1

And in that case, maybe we don't even need to modify the pcm_params ipc structure as snd_codec already has the api_id right?

Yes, but we need to modify pcm_params_ipc to add snd_codec inside it.

But we don't have enough reserved bytes left to embed this whole thing. Maybe what we can do instead is send the snd_codec as the data with comp.SET_DATA. Then there won't be compatibility issues as.well.

This could work in hw_params / set_params we could sent the normal SOF_IPC_STREAM_PCM_PARAMS IPC and then also call snd_sof_ipc_set_get_comp_data with SET_DATA containing the snd_codec.

But is it possible to call snd_sof_ipc_set_get_comp_data from set_params/hw_params? For PCM case we can just use the default codec id (and maybe fix it later if Intel decides to add more codecs).

but why? we can send it in the data part of the COMP SET_DATA IPC isnt it which will precede the PCM_PARAMS

But is it possible to call snd_sof_ipc_set_get_comp_data from set_params/hw_params? For PCM case we can just use the default codec id (and maybe fix it later if Intel decides to add more codecs).

Yes, absolutely

we only need to do it from set_params() right? or do we need this in hw_params() too?

lyakh · 2022-01-24T07:19:05Z

+static int cadence_codec_resolve_api(struct comp_dev *dev, uint32_t api_id)
+{
+	struct module_data *codec = comp_get_module_data(dev);
+	uint32_t no_of_api = ARRAY_SIZE(cadence_api_table);


unsigned int should be fine for this. My personal current favourite for this kind of variables would be n_apis, not saying it's the absolute best, but no_of_<single_form_noun> seems even less clear to me :-)

lyakh · 2022-01-24T07:20:35Z

+		}
+	}
+
+	cd->api_id = api_id;


should this be assigned after the verification below or is this by design - even if the ID isn't found?

lgirdwood · 2022-01-24T12:50:13Z

@kkarask fyi - may need to sync CI update with this PR for merge.

dbaluta · 2022-03-17T20:00:44Z

@ranj063 i restarted work on this. So, it looks like the first challenge is how to find the component id for codec adapter so that i can retrieve the associated scontrol structure.

It looks like we have something like spcm->stream[cstream->direction].comp_id; but this is the ID of the host component.

Any idea?

lgirdwood · 2022-03-18T21:17:28Z

@ranj063 i restarted work on this. So, it looks like the first challenge is how to find the component id for codec adapter so that i can retrieve the associated scontrol structure.

It looks like we have something like spcm->stream[cstream->direction].comp_id; but this is the ID of the host component.

Any idea?

@mwasko @marcinszkudlinski any ideas here ?

ranj063 · 2022-03-18T21:26:53Z

@ranj063 i restarted work on this. So, it looks like the first challenge is how to find the component id for codec adapter so that i can retrieve the associated scontrol structure.

It looks like we have something like spcm->stream[cstream->direction].comp_id; but this is the ID of the host component.

Any idea?

@dbaluta I can look into this early next wek and come back with some ideas

dbaluta · 2022-03-21T19:37:12Z

@dbaluta I can look into this early next wek and come back with some ideas

One idea to figure out the component ID for the codec adapter it to iterate over all widgets and find the one with id snd_soc_dapm_effect. This could work.

Then I need to go through understanding how to fill in the control_data for scontrol with id equal with comp_id.

lgirdwood · 2022-03-30T13:02:38Z

@dbaluta @ranj063 any updates ? I guess this is folded into parts 1/2/3 of the codec API PRs ?

dbaluta · 2022-03-30T15:36:32Z

Yes, this was the initial PR which later emerged in part 1/2/3. I am closing this now.

dbaluta added 5 commits January 21, 2022 17:11

stream: Add codec_id field in ipc_params

67cf947

With this we can select a given codec from a family of codecs. E.g Cadence supports (DAP, MP3, AAC) and codec_id will help us identify at runtime which codec api function to use. Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>

dbaluta requested review from RanderWang, lbetlej, lgirdwood, marcinszkudlinski, mmaka1 and plbossart as code owners January 21, 2022 15:32

dbaluta mentioned this pull request Jan 21, 2022

codec_adapter: prepare for dynamic codec id #4572

Closed

ranj063 reviewed Jan 21, 2022

View reviewed changes

lyakh reviewed Jan 24, 2022

View reviewed changes

paulstelian97 mentioned this pull request Mar 17, 2022

[SOF CODEC]how can i use sof/cadence decoder or encoder? #5381

Open

dbaluta closed this Mar 30, 2022

Conversation

dbaluta commented Jan 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgirdwood commented Jan 24, 2022

Uh oh!

dbaluta commented Mar 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lgirdwood commented Mar 18, 2022

Uh oh!

ranj063 commented Mar 18, 2022

Uh oh!

dbaluta commented Mar 21, 2022

Uh oh!

lgirdwood commented Mar 30, 2022

Uh oh!

dbaluta commented Mar 30, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dbaluta commented Jan 21, 2022 •

edited

Loading

dbaluta commented Mar 17, 2022 •

edited

Loading