1890467 - Reduce oscillations in audio `DriftController` output rate amplified by hysteresis

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, defect)

Description

[Karl Tomlinson (:karlt)]

The hysteresis band is interfering with the PID controller's wishes to follow exponential curves to the desired buffering and desired output rate.

Fluctuations in the output rate seem to make the AEC's filters ineffective until the AEC locks in on the echo again. Large enough fluctuations, of the size seen here, are sufficient to interfere with the AEC's ability to cancel echo.

Comment 1

[Karl Tomlinson (:karlt)]

Attachment: plots

The attached plots are from a system with pulseaudio. The DriftController sat between a graph running at 48kHz and a secondary audio output (from setSinkId()) running at 44.1kHz. The "media.cubeb.force_sample_rate" pref can be useful to configure the graph for a rate different to the preferred rate of an output device. The difference in rates causes variability in the number of frames buffered in the AudioResampler (on top of any variability in system callback times).

Until about 22 seconds, the "Actual buffering" averaged more than "Desired buffering" but was within the Hysteresis Threshold and so I accumulated without any change in the "Configured out sample rate". At about 22 seconds, the actual buffering briefly left the hysteresis band, so mDurationWithinHysteresis was reset and mIntegralCenterForCap was then set to about -4700.

At about 25 seconds, the actual buffering went 30% past the center line and so the output rate was corrected.

From about 25 to about 58 seconds, the actual buffering averaged more than desired but remained within the hysteresis threshold and so I accumulated again until about -12000. This would have been approximately the maximum additional accumulation beyond mIntegralCenterForCap.

From 58 seconds, the output sample rate was reduced over about three seconds. The change in rate was so much that the buffering was averaging less than desired within about another four seconds. The hysteresis threshold again prevented correction of the output rate until the buffering was lower than the hysteresis band at about 70 seconds. The integral term was still negative, so this time the correction was slower.

At about 82 seconds, the actual buffering had returned to the hysteresis band, and continued correcting at the same rate until it was 30% past on the other side of the center line at about 105 seconds. The output rate then set an uncorrected trajectory for buffering to return to lower than the hysteresis band.

Comment 2

[Karl Tomlinson (:karlt)]

I'd like to try replacing the hysteresis band in the buffered frame count with a deadband in output rate changes, so that the output rate is changed when and only when sufficient evidence implies that the change is sufficiently large that another resampler initialization is worthwhile.

Comment 3

[Karl Tomlinson (:karlt)]

https://treeherder.mozilla.org/jobs?repo=try&revision=132bda07d3a7b2a3b355fd19d12541dbcf2b0de1

Comment 4

[Karl Tomlinson (:karlt)]

Attachment: Bug 1890467 Use knowledge of resampling rate correction effects to determine acceptable correction rates for hysteresis r?pehrsons

The PD controller is replaced by a controller with a similar proportional
term, but expressed in expected time to converged to the desired buffering
level.

The hysteresis decisions based on proximity to the desired buffering level are
replaced with decisions based on the speed at which the current resampling
ratio will converge. This means that overshoot and consequent oscillations
are avoided.

The derivative term is dropped because it is not necessary, and can trigger
oscillations when the corrected rate is close to steady state and rounds to a
different integer values: even small changes in the derivative term can be
enough to cause the corrected rate to oscillate back to the previous integer
value. The derivative term did provide a more rapid initial response to
changes in the desired buffer level, but significant changes in resampling
rates for such rapid responses can degrade echo canceller performance.

Because some hysteresis is now applied even when average buffering is far from
desired, if the existing correction is making sufficient progress,
TestDriftController.LargeStepResponse takes a little longer to converge, but
converges to a stable rate and buffer level.

For similar reasons TestDriftController.VerySmallBufferedFrames takes longer
to reach its end point. Hysteresis means the corrected rate is not always
changing and the change in rate is capped when it does change.

The buffer level is higher before the missing input packet in
TestAudioDriftCorrection.DriftStepResponseUnderrunHighLatencyInput so the
underrun occurs a little later.

TestAudioDriftCorrection.DynamicInputBufferSizeChanges is adjusted for less
hysteresis influence in response to the first missing packet and more
influence in response to reductions in desired buffering.

Depends on D215487

Comment 5

[Karl Tomlinson (:karlt)]

Attachment: Bug 1890467 consider buffering level variation in more measurements when deciding whether to reduce desired buffering r?pehrsons

Instead of once a second, buffering level measurements are performed after
each output packet for which an input packet has recently arrived.

AudioDriftCorrection::RequestFrames() targets desired buffering at
13/10 * MeasuredSourceLatency() if actual buffering stays near desired.
20% less than this is 104/100 * MeasuredSourceLatency(). When input packets
are much larger than output packets, then when this much is buffered after an
input packet arrives, then the buffer was close to underruning before the
packet arrived.

Depends on D215488

Comment 6

[Karl Tomlinson (:karlt)]

Attachment: TestDriftController.SmallStepResponse before

TestDriftController.SmallStepResponse is added in https://phabricator.services.mozilla.com/D215488, but this plot is generated from a build without the code changes in that patch. It includes patches for bug 1897328.
The existing hysteresis decisions based on proximity to the desired buffering level prevent convergence to a stable state.

Comment 7

[Karl Tomlinson (:karlt)]

Attachment: TestDriftController.SmallStepResponse with new hysteresis decisions of D215488

Comment 8

[Karl Tomlinson (:karlt)]

Attachment: Pulseaudio 48->44.1 with new hysteresis decisions of D215488

for comparison with attachment 9410791 [details].

Comment 9

[Karl Tomlinson (:karlt)]

The improvements here are more obvious in configurations with less variability than in attachment 9410802 [details], such as equal input and output rates or Pipewire. TestDriftController.SmallStepResponse is reflective of the improvements there.

Comment 10

[Andreas Pehrson [:pehrsons]]

Attachment: DriftStepResponse_before.html

Comment 11

[Andreas Pehrson [:pehrsons]]

Attachment: DriftStepResponse_after_1890467.html

Comment 12

[Andreas Pehrson [:pehrsons]]

Attachment: DriftStepResponseOverrun_before.html

Comment 13

[Andreas Pehrson [:pehrsons]]

Attachment: DriftStepResponseOverrun_after_1890467.html

Comment 14

[Andreas Pehrson [:pehrsons]]

Attachment: DriftStepResponseUnderrun_before.html

Comment 15

[Andreas Pehrson [:pehrsons]]

Attachment: DriftStepResponseUnderrun_after_1890467.html

Comment 16

[Andreas Pehrson [:pehrsons]]

Attachment: DriftStepResponseUnderrunHighLatencyInput_before.html

Comment 17

[Andreas Pehrson [:pehrsons]]

Attachment: DriftStepResponseUnderrunHighLatencyInput_after_1890467.html

Comment 18

[Andreas Pehrson [:pehrsons]]

The attached documents are the differences on all prior step response tests in tree.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7ebd6e87ee50
Use knowledge of resampling rate correction effects to determine acceptable correction rates for hysteresis r=pehrsons

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8fa8ade86151
consider buffering level variation in more measurements when deciding whether to reduce desired buffering r=pehrsons

Comment 21

[tszentpeteri]

https://hg.mozilla.org/mozilla-central/rev/7ebd6e87ee50
https://hg.mozilla.org/mozilla-central/rev/8fa8ade86151