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.

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

Audio: Simplify volume ramp functions #6636

Merged
merged 1 commit into from
Mar 9, 2023
Merged

Audio: Simplify volume ramp functions #6636

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
180 changes: 81 additions & 99 deletions src/audio/module_adapter/module/volume/volume.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,10 +232,8 @@ static const struct comp_zc_func_map zc_func_map[] = {
* \param[in] ramp_time Time spent since ramp start as milliseconds Q29.3
* \param[in] channel Current channel to update
*/
static int32_t volume_linear_ramp(struct processing_module *mod, int32_t ramp_time, int channel)
static inline int32_t volume_linear_ramp(struct vol_data *cd, int32_t ramp_time, int channel)
lgirdwood marked this conversation as resolved.
Show resolved Hide resolved
{
struct vol_data *cd = module_get_private_data(mod);

return cd->rvolume[channel] + ramp_time * cd->ramp_coef[channel];
}
#endif
Expand All @@ -248,10 +246,8 @@ static int32_t volume_linear_ramp(struct processing_module *mod, int32_t ramp_ti
* \param[in] channel Current channel to update
*/

static int32_t volume_windows_fade_ramp(struct processing_module *mod, int32_t ramp_time,
int channel)
static inline int32_t volume_windows_fade_ramp(struct vol_data *cd, int32_t ramp_time, int channel)
{
struct vol_data *cd = module_get_private_data(mod);
int32_t time_ratio; /* Q2.30 */
int32_t pow_value; /* Q2.30 */
int32_t volume_delta = cd->tvolume[channel] - cd->rvolume[channel]; /* Q16.16 */
Expand All @@ -264,86 +260,92 @@ static int32_t volume_windows_fade_ramp(struct processing_module *mod, int32_t r

/**
* \brief Ramps volume changes over time.
* \param[in,out] mod Volume processing module handle

* \param[in,out] vol_data Volume component data
*/
static void volume_ramp(struct processing_module *mod)
singalsu marked this conversation as resolved.
Show resolved Hide resolved

/* Note: Using inline saves 0.4 MCPS */
static inline void volume_ramp(struct vol_data *cd)
Copy link
Collaborator

Choose a reason for hiding this comment

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

...and inline shouldn't be needed here either, the compiler decides by itself when to inline static functions

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The load increased by 0.4 MCPS when I removed the inline from volume_ramp(), volume_linear_ramp(), and volume_windows_fade_ramp(). Seems it's beneficial with xt-xcc version RG-2017.8-linux that I use for TGL.

Copy link
Collaborator

Choose a reason for hiding this comment

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

hm, interesting, I'd literary expect the compiler to do that for us automatically. Since the gain isn't huge, to confirm that there's really a difference - could you perhaps just build the firmware with and without inline and check that the resulting image changed (in size)? At least volume_windows_fade_ramp() really shouldn't need inline since you take its address:

	cd->ramp_func =  &volume_windows_fade_ramp;

Copy link
Member

Choose a reason for hiding this comment

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

ok, so we dont have time to figure out why we get the XCC speed improvement with inline here, lets just mark this with an inline comment so that its obvious.

{
struct vol_data *cd = module_get_private_data(mod);
struct comp_dev *dev = mod->dev;
int32_t vol;
int32_t ramp_time;
int32_t new_vol;
int32_t tvolume;
int32_t volume;
int i;
bool ramp_finished = true;

cd->ramp_finished = true;
cd->copy_gain = true;
/* No need to ramp in idle state, jump volume to request. */
if (dev->state == COMP_STATE_READY) {
for (i = 0; i < PLATFORM_MAX_CHANNELS; i++)
cd->volume[i] = cd->tvolume[i];

cd->ramp_finished = true;
return;
}

/* The first is set and cleared to indicate ongoing ramp, the
* latter is set once to enable self launched ramp only once
* in stream start.
*/
cd->vol_ramp_active = true;

/* Current ramp time in Q29.3 milliseconds. Note that max. ramp length
* can be 1.3s at 192 kHz rate and 5.5s at 48 kHz rate without
* exceeding int32_t range. Value 8000 is 1000 for converting to
* milliseconds times 8 (2 ^ 3) for fraction.
* exceeding int32_t range. Inverse of sample rate is 1000/sample_rate
* for milliseconds.
btian1 marked this conversation as resolved.
Show resolved Hide resolved
*/
ramp_time = cd->vol_ramp_elapsed_frames * 8000 / cd->sample_rate;
#if defined CONFIG_COMP_VOLUME_WINDOWS_FADE || defined CONFIG_COMP_VOLUME_LINEAR_RAMP
int32_t ramp_time = Q_MULTSR_32X32((int64_t)cd->vol_ramp_elapsed_frames,
cd->sample_rate_inv, 0, 31, 3);
#endif

/* Update each volume if it's not at target for active channels */
for (i = 0; i < cd->channels; i++) {
/* skip if target reached */
if (cd->volume[i] == cd->tvolume[i])
volume = cd->volume[i];
tvolume = cd->tvolume[i];
if (volume == tvolume)
continue;

/* Update volume gain with ramp. The ramp gain value is
* calculated from previous gain and ramp time. The slope
* coefficient is calculated in volume_set_chan().
*/
vol = cd->ramp_func(mod, ramp_time, i);
if (cd->volume[i] < cd->tvolume[i]) {
#if defined CONFIG_COMP_VOLUME_WINDOWS_FADE && defined CONFIG_COMP_VOLUME_LINEAR_RAMP
if (cd->ramp_type == SOF_VOLUME_WINDOWS_FADE)
new_vol = volume_windows_fade_ramp(cd, ramp_time, i);
else
new_vol = volume_linear_ramp(cd, ramp_time, i);
#elif defined CONFIG_COMP_VOLUME_WINDOWS_FADE
new_vol = volume_windows_fade_ramp(cd, ramp_time, i);
#elif defined CONFIG_COMP_VOLUME_LINEAR_RAMP
new_vol = volume_linear_ramp(cd, ramp_time, i);
#else
new_vol = tvolume;
#endif
if (volume < tvolume) {
/* ramp up, check if ramp completed */
if (vol >= cd->tvolume[i] || vol >= cd->vol_max) {
cd->ramp_coef[i] = 0;
cd->volume[i] = cd->tvolume[i];
} else {
ramp_finished = false;
cd->volume[i] = vol;
}
if (new_vol < tvolume)
cd->ramp_finished = false;
else
new_vol = tvolume;
} else {
/* ramp down */
if (vol <= 0) {
/* cannot ramp down below 0 */
cd->volume[i] = cd->tvolume[i];
cd->ramp_coef[i] = 0;
} else {
/* ramp completed ? */
if (vol <= cd->tvolume[i] ||
vol <= cd->vol_min) {
cd->ramp_coef[i] = 0;
cd->volume[i] = cd->tvolume[i];
} else {
ramp_finished = false;
cd->volume[i] = vol;
}
}
if (new_vol > tvolume)
cd->ramp_finished = false;
else
new_vol = tvolume;
}
cd->volume[i] = new_vol;
}
}

/**
* \brief Ramps volume changes over time.
* \param[in,out] mod Volume processing module handle
*/
#if CONFIG_IPC_MAJOR_3
static void volume_ramp_check(struct processing_module *mod)
{
struct vol_data *cd = module_get_private_data(mod);
struct comp_dev *dev = mod->dev;
int i;

/* No need to ramp in idle state, jump volume to request. */
cd->ramp_finished = false;
if (dev->state == COMP_STATE_READY) {
for (i = 0; i < PLATFORM_MAX_CHANNELS; i++)
cd->volume[i] = cd->tvolume[i];

if (ramp_finished) {
cd->ramp_finished = true;
cd->vol_ramp_active = false;
}
}
#endif

/**
* \brief Reset state except controls.
Expand All @@ -359,10 +361,9 @@ static void reset_state(struct vol_data *cd)

cd->channels = 0;
cd->ramp_finished = true;
cd->vol_ramp_active = false;
cd->vol_ramp_frames = 0;
cd->vol_ramp_elapsed_frames = 0;
cd->sample_rate = 0;
cd->sample_rate_inv = 0;
cd->copy_gain = true;
}

Expand Down Expand Up @@ -437,21 +438,17 @@ static int volume_init(struct processing_module *mod)
cd->muted[i] = false;
}

cd->ramp_type = vol->ramp;
cd->initial_ramp = vol->initial_ramp;

switch (cd->ramp_type) {
#if CONFIG_COMP_VOLUME_LINEAR_RAMP
case SOF_VOLUME_LINEAR:
case SOF_VOLUME_LINEAR_ZC:
cd->ramp_func = &volume_linear_ramp;
break;
#endif
#if CONFIG_COMP_VOLUME_WINDOWS_FADE
case SOF_VOLUME_WINDOWS_FADE:
cd->ramp_func = &volume_windows_fade_ramp;
break;
#endif
cd->ramp_type = vol->ramp;
cd->initial_ramp = vol->initial_ramp;
break;
default:
comp_err(dev, "volume_new(): invalid ramp type %d", vol->ramp);
rfree(cd);
Expand Down Expand Up @@ -486,12 +483,10 @@ static int set_volume_ipc4(struct vol_data *cd, uint32_t const channel,
cd->muted[channel] = false;
#if CONFIG_COMP_VOLUME_WINDOWS_FADE
/* ATM there is support for the same ramp for all channels */
if (curve_type == IPC4_AUDIO_CURVE_TYPE_WINDOWS_FADE) {
if (curve_type == IPC4_AUDIO_CURVE_TYPE_WINDOWS_FADE)
cd->ramp_type = SOF_VOLUME_WINDOWS_FADE;
cd->ramp_func = &volume_windows_fade_ramp;
} else {
else
cd->ramp_type = SOF_VOLUME_WINDOWS_NO_FADE;
}
#endif
return 0;
}
Expand Down Expand Up @@ -696,18 +691,18 @@ static inline int volume_set_chan(struct processing_module *mod, int chan,
* multiplication overflow with the 32 bit value. Non-zero MIN option
* can be useful to prevent totally muted small volume gain.
*/
if (v < VOL_MIN) {
if (v < cd->vol_min) {
/* No need to fail, just trace the event. */
comp_warn(mod->dev, "volume_set_chan: Limited request %d to min. %d",
v, VOL_MIN);
v = VOL_MIN;
v, cd->vol_min);
v = cd->vol_min;
btian1 marked this conversation as resolved.
Show resolved Hide resolved
}

if (v > VOL_MAX) {
if (v > cd->vol_max) {
/* No need to fail, just trace the event. */
comp_warn(mod->dev, "volume_set_chan: Limited request %d to max. %d",
v, VOL_MAX);
v = VOL_MAX;
v, cd->vol_max);
v = cd->vol_max;
}

cd->tvolume[chan] = v;
Expand Down Expand Up @@ -844,10 +839,7 @@ static int volume_set_config(struct processing_module *mod, uint32_t config_id,
}
}

if (!cd->vol_ramp_active) {
cd->ramp_finished = false;
volume_ramp(mod);
}
volume_ramp_check(mod);
break;

case SOF_CTRL_CMD_SWITCH:
Expand All @@ -870,10 +862,7 @@ static int volume_set_config(struct processing_module *mod, uint32_t config_id,
volume_set_chan_mute(mod, ch);
}

if (!cd->vol_ramp_active) {
cd->ramp_finished = false;
volume_ramp(mod);
}
volume_ramp_check(mod);
break;

default:
Expand Down Expand Up @@ -1172,14 +1161,13 @@ static int volume_process(struct processing_module *mod,
frames = cd->vol_ramp_frames;
}

/* copy and scale volume */
cd->scale_vol(mod, &input_buffers[0], &output_buffers[0], frames, cd->attenuation);

if (cd->vol_ramp_active)
if (!cd->ramp_finished) {
volume_ramp(cd);
cd->vol_ramp_elapsed_frames += frames;
}

if (!cd->ramp_finished)
volume_ramp(mod);
/* copy and scale volume */
cd->scale_vol(mod, &input_buffers[0], &output_buffers[0], frames, cd->attenuation);

avail_frames -= frames;
}
Expand Down Expand Up @@ -1305,21 +1293,15 @@ static int volume_prepare(struct processing_module *mod)
goto err;
}

if (cd->initial_ramp && !cd->ramp_func) {
comp_err(dev, "volume_prepare(): invalid cd->ramp_func");
ret = -EINVAL;
goto err;
}

/* Set current volume to min to ensure ramp starts from minimum
* to previous volume request. Copy() checks for ramp finished
* and executes it if it has not yet finished as result of
* driver commands. Ramp is not constant rate to ensure it lasts
* for entire topology specified time.
*/
cd->ramp_finished = true;
cd->ramp_finished = false;
cd->channels = sink_c->stream.channels;
cd->sample_rate = sink_c->stream.rate;
cd->sample_rate_inv = (int32_t)(1000LL * INT32_MAX / sink_c->stream.rate);

buffer_release(sink_c);

Expand Down
6 changes: 1 addition & 5 deletions src/include/sof/audio/volume.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,6 @@ typedef uint32_t (*vol_zc_func)(const struct audio_stream __sparse_cache *source
* \brief Function for volume ramp shape function
*/

typedef int32_t (*vol_ramp_func)(struct processing_module *mod, int32_t ramp_time, int channel);

struct vol_data {
#if CONFIG_IPC_MAJOR_4
uint32_t mailbox_offset; /**< store peak volume in mailbox */
Expand All @@ -151,14 +149,12 @@ struct vol_data {
/**< max number of frames to process per ramp transition */
uint32_t vol_ramp_frames;
uint32_t vol_ramp_elapsed_frames; /**< frames since transition */
uint32_t sample_rate; /**< stream sample rate in Hz */
int32_t sample_rate_inv; /**< 1000x inverse of sample rate as Q1.31 */
unsigned int channels; /**< current channels count */
bool muted[SOF_IPC_MAX_CHANNELS]; /**< set if channel is muted */
bool vol_ramp_active; /**< set if volume is ramped */
bool ramp_finished; /**< control ramp launch */
vol_scale_func scale_vol; /**< volume processing function */
vol_zc_func zc_get; /**< function getting nearest zero crossing frame */
vol_ramp_func ramp_func; /**< function for ramp shape */
bool copy_gain; /**< control copy gain or not */
uint32_t attenuation; /**< peakmeter adjustment in range [0 - 31] */
};
Expand Down