Repository navigation
[RFC] Audio: Shorten volume playback ramp length to 20 ms including other improvements - #3098
Conversation
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 <seppo.ingalsuo@linux.intel.com>
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 <seppo.ingalsuo@linux.intel.com>
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 <seppo.ingalsuo@linux.intel.com>
|
I made this RFC since there's some small processing load cost to this (peak load during first 20 ms). ZC mode helps a bit but this would work and sound decent also without. The issue that triggered this work is #3072 . |
|
@zrombel any update on CI, looks like it has timed out ? |
|
I added comments on #3072, maybe wait until we agree on how to deal with ramps consistently? |
|
Probably some QB issue, scheduled to build again. |
|
@singalsu @plbossart ack - I think this is probably something that could be done with a kcontrol - however what should the default ramp be. i.e. do we apply this PR as the new default and then allow userspace to tune it ? |
|
The existing controls allow pulseaudio to apply mute/unmute whenever to trigger a fade-in/out ramp. But the ramp duration is determined in topology load time. The playback ramp can be short, just enough to mitigate possible transient effects of processing components. The DAC should handle it's own start transients. Though we have seen that ADCs do not fully prevent start transients so we have there the long ramp. It seems it should be even longer or logarithmic type to conceal some issues. Capture to humans should have the capture ramp, while recording to automatic speech recognition can be without. Pulseaudio experts please chime in! |
|
@singalsu thanks, so can we set some defualt values for each direction that should be used for playback and capture unless overidden by topology ove later on via kcontrols. |
|
@lgirdwood In this PR I have set all volume playback pipelines to use the short ramp while capture pipelines use the longer ramp. I've been always fixing in topologies cases where tokens are not defined with unique container names so they can be set per instance. Actually I have not checked what the default is if topology does not define the tokens, likely no ramp since the Linux driver initializes IPC structs to zero (ramp type 0=linear, length 0=no ramp). @plbossart @lgirdwood Is it OK to leave the run-time ramp control to another PR later? I'm running out of time to complete it before my vacation. |
This pull request contains changes:
The shortening from 250 ms to 20 ms was made to get short notification tones better audible. The long ramp suppresses quite a lot the beginning. The two other changes are are improvements to volume component to sound better with fast ramp and optimize ramp rate for needed ramp time.