From 778f7953372ef3368023b22f46463434995d8567 Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo Date: Tue, 23 Jun 2020 19:53:45 +0300 Subject: [PATCH 1/3] Tools: Topology: Change playback volume ramp duration to 20 ms This patch shortens the ramp duration to 20 ms from 250 ms in all topologies with volume playback. In addition the zero crossings detector mode is enabled to mitigate zipper noise. The change includes for the not yet updated pipeline macros a rename of macro for tokens data with pipeline number suffix to ensure no undesired volume instances are impacted by this change. Signed-off-by: Seppo Ingalsuo --- tools/topology/sof/pipe-asrc-volume-playback.m4 | 4 ++-- .../topology/sof/pipe-dcblock-volume-playback.m4 | 4 ++-- tools/topology/sof/pipe-eq-fir-volume-playback.m4 | 4 ++-- .../sof/pipe-eq-iir-eq-fir-volume-playback.m4 | 4 ++-- tools/topology/sof/pipe-eq-iir-volume-playback.m4 | 4 ++-- tools/topology/sof/pipe-src-volume-playback.m4 | 14 +++++++++----- tools/topology/sof/pipe-volume-playback.m4 | 15 ++++++++++----- 7 files changed, 29 insertions(+), 20 deletions(-) diff --git a/tools/topology/sof/pipe-asrc-volume-playback.m4 b/tools/topology/sof/pipe-asrc-volume-playback.m4 index 13247fdbc802..5007db5c1f0e 100644 --- a/tools/topology/sof/pipe-asrc-volume-playback.m4 +++ b/tools/topology/sof/pipe-asrc-volume-playback.m4 @@ -57,8 +57,8 @@ W_ASRC(0, PIPELINE_FORMAT, 2, 2, MY_ASRC_CONF) define(MY_PGA_TOKENS, concat(`pga_tokens_', PIPELINE_ID)) define(MY_PGA_CONF, concat(`pga_conf_', PIPELINE_ID)) W_VENDORTUPLES(MY_PGA_TOKENS, sof_volume_tokens, -LIST(` ', `SOF_TKN_VOLUME_RAMP_STEP_TYPE "0"' - ` ', `SOF_TKN_VOLUME_RAMP_STEP_MS "250"')) +LIST(` ', `SOF_TKN_VOLUME_RAMP_STEP_TYPE "2"' + ` ', `SOF_TKN_VOLUME_RAMP_STEP_MS "20"')) W_DATA(MY_PGA_CONF, MY_PGA_TOKENS) # "Volume" has x sink and 2 source periods diff --git a/tools/topology/sof/pipe-dcblock-volume-playback.m4 b/tools/topology/sof/pipe-dcblock-volume-playback.m4 index 2a6fabea52e7..decd275a8e4b 100644 --- a/tools/topology/sof/pipe-dcblock-volume-playback.m4 +++ b/tools/topology/sof/pipe-dcblock-volume-playback.m4 @@ -33,8 +33,8 @@ C_CONTROLMIXER(Master Playback Volume, PIPELINE_ID, define(MY_PGA_TOKENS, concat(`pga_tokens_', PIPELINE_ID)) define(MY_PGA_CONF, concat(`pga_conf_', PIPELINE_ID)) W_VENDORTUPLES(MY_PGA_TOKENS, sof_volume_tokens, -LIST(` ', `SOF_TKN_VOLUME_RAMP_STEP_TYPE "0"' - ` ', `SOF_TKN_VOLUME_RAMP_STEP_MS "250"')) +LIST(` ', `SOF_TKN_VOLUME_RAMP_STEP_TYPE "2"' + ` ', `SOF_TKN_VOLUME_RAMP_STEP_MS "20"')) W_DATA(MY_PGA_CONF, MY_PGA_TOKENS) diff --git a/tools/topology/sof/pipe-eq-fir-volume-playback.m4 b/tools/topology/sof/pipe-eq-fir-volume-playback.m4 index 3dc2ff4f7b41..236ff605cfb9 100644 --- a/tools/topology/sof/pipe-eq-fir-volume-playback.m4 +++ b/tools/topology/sof/pipe-eq-fir-volume-playback.m4 @@ -35,8 +35,8 @@ define(DEF_PGA_TOKENS, concat(`pga_tokens_', PIPELINE_ID)) define(DEF_PGA_CONF, concat(`pga_conf_', PIPELINE_ID)) W_VENDORTUPLES(DEF_PGA_TOKENS, sof_volume_tokens, -LIST(` ', `SOF_TKN_VOLUME_RAMP_STEP_TYPE "0"' - ` ', `SOF_TKN_VOLUME_RAMP_STEP_MS "250"')) +LIST(` ', `SOF_TKN_VOLUME_RAMP_STEP_TYPE "2"' + ` ', `SOF_TKN_VOLUME_RAMP_STEP_MS "20"')) W_DATA(DEF_PGA_CONF, DEF_PGA_TOKENS) diff --git a/tools/topology/sof/pipe-eq-iir-eq-fir-volume-playback.m4 b/tools/topology/sof/pipe-eq-iir-eq-fir-volume-playback.m4 index 1efc2e3c3a23..4c7a32a5c261 100644 --- a/tools/topology/sof/pipe-eq-iir-eq-fir-volume-playback.m4 +++ b/tools/topology/sof/pipe-eq-iir-eq-fir-volume-playback.m4 @@ -37,8 +37,8 @@ define(DEF_PGA_TOKENS, concat(`pga_tokens_', PIPELINE_ID)) define(DEF_PGA_CONF, concat(`pga_conf_', PIPELINE_ID)) W_VENDORTUPLES(DEF_PGA_TOKENS, sof_volume_tokens, -LIST(` ', `SOF_TKN_VOLUME_RAMP_STEP_TYPE "0"' - ` ', `SOF_TKN_VOLUME_RAMP_STEP_MS "250"')) +LIST(` ', `SOF_TKN_VOLUME_RAMP_STEP_TYPE "2"' + ` ', `SOF_TKN_VOLUME_RAMP_STEP_MS "20"')) W_DATA(DEF_PGA_CONF, DEF_PGA_TOKENS) diff --git a/tools/topology/sof/pipe-eq-iir-volume-playback.m4 b/tools/topology/sof/pipe-eq-iir-volume-playback.m4 index ee7bfd4e3750..482e21df0e12 100644 --- a/tools/topology/sof/pipe-eq-iir-volume-playback.m4 +++ b/tools/topology/sof/pipe-eq-iir-volume-playback.m4 @@ -35,8 +35,8 @@ define(DEF_PGA_TOKENS, concat(`pga_tokens_', PIPELINE_ID)) define(DEF_PGA_CONF, concat(`pga_conf_', PIPELINE_ID)) W_VENDORTUPLES(DEF_PGA_TOKENS, sof_volume_tokens, -LIST(` ', `SOF_TKN_VOLUME_RAMP_STEP_TYPE "0"' - ` ', `SOF_TKN_VOLUME_RAMP_STEP_MS "250"')) +LIST(` ', `SOF_TKN_VOLUME_RAMP_STEP_TYPE "2"' + ` ', `SOF_TKN_VOLUME_RAMP_STEP_MS "20"')) W_DATA(DEF_PGA_CONF, DEF_PGA_TOKENS) diff --git a/tools/topology/sof/pipe-src-volume-playback.m4 b/tools/topology/sof/pipe-src-volume-playback.m4 index 73c048ac4e80..c6deebc57658 100644 --- a/tools/topology/sof/pipe-src-volume-playback.m4 +++ b/tools/topology/sof/pipe-src-volume-playback.m4 @@ -49,15 +49,17 @@ W_SRC(0, PIPELINE_FORMAT, 3, 3, media_src_conf) # Volume Configuration # -W_VENDORTUPLES(playback_pga_tokens, sof_volume_tokens, -LIST(` ', `SOF_TKN_VOLUME_RAMP_STEP_TYPE "0"' - ` ', `SOF_TKN_VOLUME_RAMP_STEP_MS "250"')) +define(DEF_PGA_TOKENS, concat(`pga_tokens_', PIPELINE_ID)) +define(DEF_PGA_CONF, concat(`pga_conf_', PIPELINE_ID)) +W_VENDORTUPLES(DEF_PGA_TOKENS, sof_volume_tokens, +LIST(` ', `SOF_TKN_VOLUME_RAMP_STEP_TYPE "2"' + ` ', `SOF_TKN_VOLUME_RAMP_STEP_MS "20"')) -W_DATA(playback_pga_conf, playback_pga_tokens) +W_DATA(DEF_PGA_CONF, DEF_PGA_TOKENS) # "Volume" has 3 source and x sink periods -W_PGA(0, PIPELINE_FORMAT, DAI_PERIODS, 3, playback_pga_conf, SCHEDULE_CORE, +W_PGA(0, PIPELINE_FORMAT, DAI_PERIODS, 3, DEF_PGA_CONF, SCHEDULE_CORE, LIST(` ', "PIPELINE_ID Master Playback Volume")) # Playback Buffers @@ -96,3 +98,5 @@ indir(`define', concat(`PIPELINE_PCM_', PIPELINE_ID), SRC Playback PCM_ID) PCM_CAPABILITIES(SRC Playback PCM_ID, `S32_LE,S24_LE,S16_LE', 8000, 192000, 2, PIPELINE_CHANNELS, 2, 16, 192, 16384, 65536, 65536) +undefine(`DEF_PGA_TOKENS') +undefine(`DEF_PGA_CONF') diff --git a/tools/topology/sof/pipe-volume-playback.m4 b/tools/topology/sof/pipe-volume-playback.m4 index 61d0d61d75ba..244d38c4fdb6 100644 --- a/tools/topology/sof/pipe-volume-playback.m4 +++ b/tools/topology/sof/pipe-volume-playback.m4 @@ -29,11 +29,14 @@ C_CONTROLMIXER(Master Playback Volume, PIPELINE_ID, # Volume configuration # -W_VENDORTUPLES(playback_pga_tokens, sof_volume_tokens, -LIST(` ', `SOF_TKN_VOLUME_RAMP_STEP_TYPE "0"' - ` ', `SOF_TKN_VOLUME_RAMP_STEP_MS "250"')) +define(DEF_PGA_TOKENS, concat(`pga_tokens_', PIPELINE_ID)) +define(DEF_PGA_CONF, concat(`pga_conf_', PIPELINE_ID)) -W_DATA(playback_pga_conf, playback_pga_tokens) +W_VENDORTUPLES(DEF_PGA_TOKENS, sof_volume_tokens, +LIST(` ', `SOF_TKN_VOLUME_RAMP_STEP_TYPE "2"' + ` ', `SOF_TKN_VOLUME_RAMP_STEP_MS "20"')) + +W_DATA(DEF_PGA_CONF, DEF_PGA_TOKENS) # # Components and Buffers @@ -44,7 +47,7 @@ W_DATA(playback_pga_conf, playback_pga_tokens) W_PCM_PLAYBACK(PCM_ID, Passthrough Playback, 2, 0, SCHEDULE_CORE) # "Volume" has 2 source and x sink periods -W_PGA(0, PIPELINE_FORMAT, DAI_PERIODS, 2, playback_pga_conf, SCHEDULE_CORE, +W_PGA(0, PIPELINE_FORMAT, DAI_PERIODS, 2, DEF_PGA_CONF, SCHEDULE_CORE, LIST(` ', "PIPELINE_ID Master Playback Volume")) # Playback Buffers @@ -79,3 +82,5 @@ indir(`define', concat(`PIPELINE_PCM_', PIPELINE_ID), Passthrough Playback PCM_I # PCM_CAPABILITIES(Passthrough Playback PCM_ID, `S32_LE,S24_LE,S16_LE', PCM_MIN_RATE, PCM_MAX_RATE, 2, PIPELINE_CHANNELS, 2, 16, 192, 16384, 65536, 65536) +undefine(`DEF_PGA_TOKENS') +undefine(`DEF_PGA_CONF') From 84244b4702bc03d8ec3c6855c2ca0731273ba8b3 Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo Date: Wed, 24 Jun 2020 20:14:00 +0300 Subject: [PATCH 2/3] Audio: Improve volume ramp time calculation precision This patch changes the ramp time (from ramp start) calculation unit from millisecond to Q29.3 milliseconds. It changes the smallest time difference and therefore ramp slop point value down to 125 us so finer than 1000 us volume update rate can be used. E.g. update rates 125/250/500 us are good choices for high update rate. The higher ramp update rates can be defined in volume.h if the volume ramp need to be much shorter than currently while avoiding audible zipper noise. Signed-off-by: Seppo Ingalsuo --- src/audio/volume/volume.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/audio/volume/volume.c b/src/audio/volume/volume.c index a742009ad88f..4dfc282eac39 100644 --- a/src/audio/volume/volume.c +++ b/src/audio/volume/volume.c @@ -233,8 +233,13 @@ static void volume_ramp(struct comp_dev *dev) */ cd->vol_ramp_active = true; - /* Current ramp time in milliseconds */ - ramp_time = cd->vol_ramp_elapsed_frames * 1000 / cd->sample_rate; + /* 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. + */ + ramp_time = cd->vol_ramp_elapsed_frames * 8000 / cd->sample_rate; + /* Update each volume if it's not at target for active channels */ for (i = 0; i < cd->channels; i++) { @@ -462,7 +467,12 @@ static inline int volume_set_chan(struct comp_dev *dev, int chan, coef = delta_abs; } - /* Ensure ramp coefficient is at least min. non-zero Q16.16 */ + /* Scale coefficient by 1/8, round */ + coef = ((coef >> 2) + 1) >> 1; + + /* Ensure ramp coefficient is at least min. non-zero + * fractional value. + */ coef = MAX(coef, 1); /* Invert sign for volume down ramp step */ From 09faf2a1824358793edd061401ca7bd66cfe537b Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo Date: Thu, 25 Jun 2020 14:30:30 +0300 Subject: [PATCH 3/3] Audio: Make volume gain ramp update rate variable This patch adds to volume component capability to use one of four ramp update rate presets 125 to 1000 us depending on topology defined ramp length that is typically between 20 to 250 ms. The rate needs to be higher for faster ramps to avoid zipper noise. Signed-off-by: Seppo Ingalsuo --- src/audio/volume/volume.c | 20 ++++++++++++++++++-- src/include/sof/audio/volume.h | 14 ++++++++++++-- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/audio/volume/volume.c b/src/audio/volume/volume.c index 4dfc282eac39..2bf398964549 100644 --- a/src/audio/volume/volume.c +++ b/src/audio/volume/volume.c @@ -789,9 +789,11 @@ static int volume_prepare(struct comp_dev *dev) struct comp_data *cd = comp_get_drvdata(dev); struct comp_buffer *sinkb; struct sof_ipc_comp_config *config = dev_comp_config(dev); + struct sof_ipc_comp_volume *pga; uint32_t sink_period_bytes; - int i; + int ramp_update_us; int ret; + int i; comp_dbg(dev, "volume_prepare()"); @@ -840,7 +842,6 @@ static int volume_prepare(struct comp_dev *dev) * for entire topology specified time. */ cd->ramp_finished = true; - cd->vol_ramp_frames = dev->frames / (dev->period / VOL_RAMP_UPDATE_US); cd->channels = sinkb->stream.channels; cd->sample_rate = sinkb->stream.rate; for (i = 0; i < cd->channels; i++) { @@ -850,6 +851,21 @@ static int volume_prepare(struct comp_dev *dev) cd->ramp_finished = false; } + /* Determine ramp update rate depending on requested ramp length. To + * ensure evenly updated gain envelope with limited fraction resolution + * four presets are used. + */ + pga = COMP_GET_IPC(dev, sof_ipc_comp_volume); + if (pga->initial_ramp < VOL_RAMP_UPDATE_THRESHOLD_FASTEST_MS) + ramp_update_us = VOL_RAMP_UPDATE_FASTEST_US; + else if (pga->initial_ramp < VOL_RAMP_UPDATE_THRESHOLD_FAST_MS) + ramp_update_us = VOL_RAMP_UPDATE_FAST_US; + else if (pga->initial_ramp < VOL_RAMP_UPDATE_THRESHOLD_SLOW_MS) + ramp_update_us = VOL_RAMP_UPDATE_SLOW_US; + else + ramp_update_us = VOL_RAMP_UPDATE_SLOWEST_US; + + cd->vol_ramp_frames = dev->frames / (dev->period / ramp_update_us); return 0; err: diff --git a/src/include/sof/audio/volume.h b/src/include/sof/audio/volume.h index b92133fd0e7b..8b1b656506c6 100644 --- a/src/include/sof/audio/volume.h +++ b/src/include/sof/audio/volume.h @@ -47,9 +47,19 @@ struct sof_ipc_ctrl_value_chan; /** * \brief Volume ramp update rate in microseconds. - * Update volume gain value every 1 ms. + * Update volume gain value every 125 to 1000 us. The faster gain ramps need + * higher update rate to avoid annoying zipper noise sound. The below + * values were tested subjectively for constraint of 125 microseconds + * multiple gain update rate. */ -#define VOL_RAMP_UPDATE_US 1000 +#define VOL_RAMP_UPDATE_SLOWEST_US 1000 +#define VOL_RAMP_UPDATE_SLOW_US 500 +#define VOL_RAMP_UPDATE_FAST_US 250 +#define VOL_RAMP_UPDATE_FASTEST_US 125 + +#define VOL_RAMP_UPDATE_THRESHOLD_SLOW_MS 128 +#define VOL_RAMP_UPDATE_THRESHOLD_FAST_MS 64 +#define VOL_RAMP_UPDATE_THRESHOLD_FASTEST_MS 32 /** * \brief Volume maximum value.