Audio: Volume: Fix ongoing gain ramp stop and wrong final gain #8950
Conversation
|
Draft - there's also other bug to fix with no ramp happening at times. |
singalsu
force-pushed
the
fix_gain_channels_fail
branch
from
5687768
to
72b9006
Compare
src/audio/volume/volume_ipc4.c
Outdated
| } | ||
|
|
||
| for (i = 0; i < channels_count; i++) { | ||
| if (cd->volume[i] != cd->tvolume[i]) |
There was a problem hiding this comment.
"In IPC4 the individual channel gains are passed in separate messages."
Is this really something we want to support?
I don't really see how this might work if someone wants to e.g. implement a pan effect by moving a sound from the left to the right in a continuous motion?
Isn't it a classic requirement that such changes need to be handled in an 'atomic' way where all parameters are updated at the same time?
Maybe my perception is twisted by attending SDCA meetings where such volume/EQ changes are handled with a 'commit register' with all parameters flipped at once. Or I must be missing something...
There was a problem hiding this comment.
We could have a new "linux volume" component with different UUID to overcome this limitation while "gain" can be kept as-is an IPC4 compatible. The peakvolume/gain also has overhead from unused features in Linux.
There is already support for mute switch (that Windows is not using) that can control any number of channels atomically. The same kind of ALSA mixercontrol (vs. gain/peakvolume set large config) could control the volume with no delays from channel to channel. But this would need a new kernel.
singalsu
force-pushed
the
fix_gain_channels_fail
branch
from
72b9006
to
d9dda73
Compare
|
Still, checking but looks like MCPS is not impacted. But need more tests to be sure. Here's switch control before and after: controls were Here's volume control before and after: controls were |
singalsu
force-pushed
the
fix_gain_channels_fail
branch
2 times, most recently
from
38b9400
to
4211121
Compare
singalsu
requested review from
lgirdwood,
mmaka1,
lbetlej,
dbaluta and
kv2019i
as code owners
singalsu
force-pushed
the
fix_gain_channels_fail
branch
from
4211121
to
70bd2e3
Compare
In IPC4 the individual channel gains are passed in separate messages. One channel may have started to ramp to a new gain while a new channel gain arrives for other channel. If the gain is same as the previous control value, the ongoing ramp is stopped and the gain remains at a transition value. The incorrect code is not fixed but instead the volume_set_volume() function is simplified. When a volume control is received, it is assumed that pass-through mode is disabled and ramp is prepared. If the control is received but gains are not changed, the ramp mode is finished and pass-through is restored in ramp function. The volume_set_switch() function is updated similarly to ensure similar operation. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
The linear slope coefficient calculation is moved to a separate function. Two functional changes are done. - The ramp_coef for channel is set to zero if there is no transition for the channel. The ensure of non-zero coefficient is only needed if there is a transition that is so slow that the slope coefficient would round to zero. If this function is called for equal volume and tvolume for channel, the ramp_coef remains zero, and not smallest non-zero value. - The handling of ramp disable with zero initial_ramp is changed to similar as for windows fade. The set of coefficient to volume delta (a large value for large volume jump) is not correct even if it appeared to work. It is a remain from old code where ramp was not a function of time but a constant step added. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
The "is_same_volume=true" was returned if target volumes for all channels are the same. The check omitted the fact that start volumes for ramp can be different, e.g. one channel is attenuated while others are 0 dB. This change fixes the random ignore of volume ramp and smooth transition when a volume control is changed. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
This ensures that volume for a channel changes immediately after receiving the control if ramp duration is zero or if type is no fade. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
The volume process function for pass-through can be changed when all ramps are completed. This change that avoids processing function lookup in worst case every 128 us is measured to save in TGL platform about 1 MCPS from CPU_PEAK(MAX). Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
singalsu
force-pushed
the
fix_gain_channels_fail
branch
from
70bd2e3
to
ba8af58
Compare
|
No updates, just rebase to restart CI. |
|
@wszypelt The quildbuild check seems to be stuck, can you check? |
|
@kv2019i Sure, I'm already looking into it |
|
@singalsu Hmm, SDW MTL config has failed in SOF driver CI runs and when I look back, the same SDW target failed in previous PR testing (build ids 39443 and 39061 ). At the same time, SDW MTL passes for other PRs at least today. so this is somewhat suspicious. Let me trigger the SOF CI once more. |
|
SOFCI TEST |
It shows "NotTested" for SDW. Strange if it runs for other PRs! |
|
Now passing, proceeding with merge. |
In IPC4 the individual channel gains are passed in separate messages. One channel may have started to ramp to a new gain while a new channel gain arrives for other channel. If the gain is same as the previous control value, the ongoing ramp is stopped and the gain remains at a transition value.