Skip to content

Simplifications for codec_adapter_set_params() #5160

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

Merged
dbaluta merged 5 commits into from
Jan 11, 2022
Merged

Simplifications for codec_adapter_set_params() #5160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ddc8a04
codec_adapter: drop the type parameter in codec_adapter_set_params()
ranj063 Jan 6, 2022
92d6d7a
codec_adapter: remove unnecessary goto's
ranj063 Jan 6, 2022
879e8fa
codec_adapter: remove unnecessary braces
ranj063 Jan 6, 2022
91bf064
codec_adapter: check if more fragments are expected first.
ranj063 Jan 6, 2022
2d39351
codec_adapter: remove load_setup_config()
ranj063 Jan 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/audio/codec_adapter/codec/generic.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,24 @@ int module_load_config(struct comp_dev *dev, void *cfg, size_t size, enum module
return -EINVAL;
}

/*
* Setup config comprises of two parts: 1. Essential data needed for initialization of
* codec_adapter and follows struct ca_config. 2: Module specific data needed to setup the
* module. Copy the codec_adapter data first.
*/
if (type == MODULE_CFG_SETUP) {
ret = memcpy_s(&mod->ca_config, sizeof(mod->ca_config), cfg,
sizeof(struct ca_config));
assert(!ret);

/* And then copy the module-specific data */
cfg = (char *)cfg + sizeof(struct ca_config);
size -= sizeof(struct ca_config);

if (!size)
return 0;
}

dst = (type == MODULE_CFG_SETUP) ? &md->s_cfg :
&md->r_cfg;

Expand Down
171 changes: 43 additions & 128 deletions src/audio/codec_adapter/codec_adapter.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
#include <sof/platform.h>
#include <sof/ut.h>

int load_setup_config(struct comp_dev *dev, void *cfg, uint32_t size);
int validate_setup_config(struct ca_config *cfg);

/**
Expand Down Expand Up @@ -69,7 +68,8 @@ struct comp_dev *codec_adapter_new(const struct comp_driver *drv,
comp_set_drvdata(dev, mod);

/* Copy setup config */
ret = load_setup_config(dev, ipc_codec_adapter->data, ipc_codec_adapter->size);
ret = module_load_config(dev, ipc_codec_adapter->data, ipc_codec_adapter->size,
MODULE_CFG_SETUP);
if (ret) {
comp_err(dev, "codec_adapter_new() error %d: config loading has failed.",
ret);
Expand Down Expand Up @@ -99,66 +99,6 @@ int validate_setup_config(struct ca_config *cfg)
return 0;
}

/**
* \brief Load setup config for both codec adapter and codec library.
* \param[in] dev - codec adapter component device pointer.
* \param[in] cfg - pointer to the configuration data.
* \param[in] size - size of config.
*
* The setup config comprises of two parts - one contains essential data
* for the initialization of codec_adapter and follows struct ca_config.
* Second contains codec specific data needed to setup the codec itself.
* The latter is send in a TLV format organized by struct module_param.
*
* \return integer representing either:
* 0 -> success
* negative value -> failure.
*/
int load_setup_config(struct comp_dev *dev, void *cfg, uint32_t size)
{
int ret;
void *lib_cfg;
size_t lib_cfg_size;
struct processing_module *mod = comp_get_drvdata(dev);

comp_dbg(dev, "load_setup_config() start.");

if (!cfg || !size) {
comp_err(dev, "load_setup_config(): no config available cfg: %x, size: %d",
(uintptr_t)cfg, size);
ret = -EINVAL;
goto end;
} else if (size < sizeof(struct ca_config)) {
comp_err(dev, "load_setup_config(): no codec config available, size %d", size);
ret = -EIO;
goto end;
}
/* Copy codec_adapter part */
ret = memcpy_s(&mod->ca_config, sizeof(mod->ca_config), cfg,
sizeof(struct ca_config));
assert(!ret);
ret = validate_setup_config(&mod->ca_config);
if (ret) {
comp_err(dev, "load_setup_config(): validation of setup config for codec_adapter failed.");
goto end;
}
/* Copy codec specific part */
lib_cfg_size = size - sizeof(struct ca_config);
if (lib_cfg_size) {
lib_cfg = (char *)cfg + sizeof(struct ca_config);
ret = module_load_config(dev, lib_cfg, lib_cfg_size, MODULE_CFG_SETUP);
if (ret) {
comp_err(dev, "load_setup_config(): %d: failed to load setup config for codec id %x",
ret, mod->ca_config.module_id);
goto end;
}
}

comp_dbg(dev, "load_setup_config() done.");
end:
return ret;
}

/*
* \brief Prepare a codec adapter component.
* \param[in] dev - component device pointer.
Expand Down Expand Up @@ -461,15 +401,15 @@ int codec_adapter_copy(struct comp_dev *dev)
return ret;
}

static int codec_adapter_set_params(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdata,
enum module_cfg_type type)
static int codec_adapter_set_params(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdata)
{
int ret;
char *dst, *src;
static uint32_t size;
uint32_t offset;
struct processing_module *mod = comp_get_drvdata(dev);
struct module_data *md = &mod->priv;
enum module_cfg_type type = cdata->data->type;

comp_dbg(dev, "codec_adapter_set_params(): start: num_of_elem %d, elem remain %d msg_index %u",
cdata->num_elems, cdata->elems_remaining, cdata->msg_index);
Expand All @@ -480,38 +420,38 @@ static int codec_adapter_set_params(struct comp_dev *dev, struct sof_ipc_ctrl_da
/* Check that there is no work-in-progress on previous request */
if (md->runtime_params) {
comp_err(dev, "codec_adapter_set_params() error: busy with previous request");
ret = -EBUSY;
goto end;
} else if (!size) {
return -EBUSY;
}

if (!size) {
comp_err(dev, "codec_adapter_set_params() error: no configuration size %d",
size);
/* TODO: return -EINVAL. This is temporary until driver fixes its issue */
ret = 0;
goto end;
} else if (size > MAX_BLOB_SIZE) {
return 0;
}

if (size > MAX_BLOB_SIZE) {
comp_err(dev, "codec_adapter_set_params() error: blob size is too big cfg size %d, allowed %d",
size, MAX_BLOB_SIZE);
ret = -EINVAL;
goto end;
} else if (type != MODULE_CFG_SETUP && type != MODULE_CFG_RUNTIME) {
return -EINVAL;
}

if (type != MODULE_CFG_SETUP && type != MODULE_CFG_RUNTIME) {
comp_err(dev, "codec_adapter_set_params() error: unknown config type");
ret = -EINVAL;
goto end;
return -EINVAL;
}

/* Allocate buffer for new params */
md->runtime_params = rballoc(0, SOF_MEM_CAPS_RAM, size);
if (!md->runtime_params) {
comp_err(dev, "codec_adapter_set_params(): space allocation for new params failed");
ret = -ENOMEM;
goto end;
return -ENOMEM;
}

memset(md->runtime_params, 0, size);
} else if (!md->runtime_params) {
comp_err(dev, "codec_adapter_set_params() error: no memory available for runtime params in consecutive load");
ret = -EIO;
goto end;
return -EIO;
}

offset = size - (cdata->num_elems + cdata->elems_remaining);
Expand All @@ -521,61 +461,36 @@ static int codec_adapter_set_params(struct comp_dev *dev, struct sof_ipc_ctrl_da
ret = memcpy_s(dst, size - offset, src, cdata->num_elems);
assert(!ret);

/* Config has been copied now we can load & apply it depending on
* codec state.
*/
if (!cdata->elems_remaining) {
Comment thread
ranj063 marked this conversation as resolved.
Outdated
switch (type) {
case MODULE_CFG_SETUP:
ret = load_setup_config(dev, md->runtime_params, size);
if (ret) {
comp_err(dev, "codec_adapter_set_params(): error %d: load of setup config failed.",
ret);
} else {
comp_dbg(dev, "codec_adapter_set_params() load of setup config done.");
}

break;
case MODULE_CFG_RUNTIME:
ret = module_load_config(dev, md->runtime_params, size, MODULE_CFG_RUNTIME);
if (ret) {
comp_err(dev, "codec_adapter_set_params() error %d: load of runtime config failed.",
/* return as more fragments of config data expected */
if (cdata->elems_remaining)
return 0;
Comment thread
ranj063 marked this conversation as resolved.
Outdated

/* config fully copied, now load it */
ret = module_load_config(dev, md->runtime_params, size, type);
if (ret)
goto end;

comp_dbg(dev, "codec_adapter_set_params(): config load successful");

/* And apply runtime config right away if codec is already prepared */
if (type == MODULE_CFG_RUNTIME) {
if (md->state >= MODULE_INITIALIZED) {
ret = module_apply_runtime_config(dev);
if (ret)
comp_err(dev, "codec_adapter_set_params() error %x: codec runtime config apply failed",
ret);
goto done;
} else {
comp_dbg(dev, "codec_adapter_set_params() load of runtime config done.");
}

if (md->state >= MODULE_INITIALIZED) {
/* We are already prepared so we can apply runtime
* config right away.
*/
ret = module_apply_runtime_config(dev);
if (ret) {
comp_err(dev, "codec_adapter_set_params() error %x: codec runtime config apply failed",
ret);
} else {
comp_dbg(dev, "codec_adapter_set_params() apply of runtime config done.");
}
} else {
mod->priv.r_cfg.avail = true;
}

break;
default:
comp_err(dev, "codec_adapter_set_params(): error: unknown config type.");
break;
else
comp_dbg(dev, "codec_adapter_set_params() apply of runtime config done.");
} else {
mod->priv.r_cfg.avail = true;
}
} else
goto end;
}

done:
end:
if (md->runtime_params)
rfree(md->runtime_params);
md->runtime_params = NULL;
return ret;
end:
return ret;
}

static int ca_set_binary_data(struct comp_dev *dev,
Expand All @@ -588,7 +503,7 @@ static int ca_set_binary_data(struct comp_dev *dev,
switch (cdata->data->type) {
case MODULE_CFG_SETUP:
case MODULE_CFG_RUNTIME:
ret = codec_adapter_set_params(dev, cdata, cdata->data->type);
ret = codec_adapter_set_params(dev, cdata);
break;
default:
comp_err(dev, "ca_set_binary_data() error: unknown binary data type");
Expand Down
4 changes: 0 additions & 4 deletions src/include/sof/audio/codec_adapter/codec_adapter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,5 @@
#include <sof/audio/codec_adapter/codec/generic.h>

#define MAX_BLOB_SIZE 8192
/*****************************************************************************/
/* Codec adapter internal functions */
/*****************************************************************************/
static int load_setup_config(struct comp_dev *dev, void *cfg, uint32_t size);

#endif /* __SOF_AUDIO_CODEC_ADAPTER__ */