[haiku-commits] Change in haiku[master]: multi_audio: Fix use of uninitialized entry to calculate drift.

  • From: Gerrit <review@xxxxxxxxxxxxxxxxxxx>
  • To: waddlesplash <waddlesplash@xxxxxxxxx>, haiku-commits@xxxxxxxxxxxxx
  • Date: Sun, 26 Jul 2020 00:00:39 +0000

From Michael Lotz <mmlr@xxxxxxxx>:

Michael Lotz has uploaded this change for review. ( 
https://review.haiku-os.org/c/haiku/+/3092 ;)


Change subject: multi_audio: Fix use of uninitialized entry to calculate drift.
......................................................................

multi_audio: Fix use of uninitialized entry to calculate drift.

Two indices into the fEntries ring buffer are used in TimeComputer,
fLastEntry which points to the most recent sample and fFirstEntry which
points to the oldest one. The drift calculation then uses the oldest
values pointed at by fFirstEntry to produce smoother values.

Both indices are initialized to 0, then fLastEntry is incremented prior
to writing the first sample, resulting in the first sample to be written
to fEntries[1]. When drift is calculated on subsequent samples, they
were using the uninitialized data from fEntries[0] due to fFirstEntry
still pointing there, until enough samples had been added for
fFirstEntry to be moved forward. The resulting incorrect drift values
would depend on the entry content and ratio between real and performance
time and generally end up being much too small.

The drift values are published on the time source and used to correct
the conversion between performance and real time in
BTimeSource::RealTimeFor(). Given the incorrectly small drift values,
this conversion produced real time values far in the future, causing
increased delays in processes that wait for such computed real times.

One likely victim was the startup of the AudioMixer, as it generally
starts soon after the driver and its MultiAudioNode. If it started right
in the time window where the drift was broken, it would end up delaying
its initial buffer processing by a factor generally depending on the
difference between real and performance time start points, so usually
the difference of the start of the boot process to the start of the
audio driver.

The symptom was then noticable by initial silence after boot, as the
mixer was not producing any buffers until the wrongly calculated real
time had been reached. After that, lots of queued up events were
processed at once, resulting in a CPU spike and glitchy audio for a
couple of seconds after which audio finally normalized. As the problem
was in the MultiAudioNode all mutli audio drivers were affected, HDA
being the most prominent.

As the erroneous delay depended on the initial entry content and the
boot time, it was highly variable. Whether or not the mixer was affected
at all also depended on it starting reasonably early after the driver,
which made the symptom of the bug appear and disappear depending on
slight timing changes, making unrelated code changes look like they were
affecting the bug.

The fFirstEntry index is now updated to fLastEntry after the initial
entry is added, so that the drift calculation always uses valid data.

Fixes #16222, may also affect #15472 and possibly other "no audio"
tickets in case they are really "delayed" audio tickets.
---
M src/add-ons/media/media-add-ons/multi_audio/TimeComputer.cpp
1 file changed, 1 insertion(+), 0 deletions(-)



  git pull ssh://git.haiku-os.org:22/haiku refs/changes/92/3092/1

diff --git a/src/add-ons/media/media-add-ons/multi_audio/TimeComputer.cpp 
b/src/add-ons/media/media-add-ons/multi_audio/TimeComputer.cpp
index e0d2c24..74366c3 100644
--- a/src/add-ons/media/media-add-ons/multi_audio/TimeComputer.cpp
+++ b/src/add-ons/media/media-add-ons/multi_audio/TimeComputer.cpp
@@ -62,6 +62,7 @@
                fFrameBase = frames;
                fResetTimeBase = false;
                _AddEntry(fRealTime, fPerformanceTime);
+               fFirstEntry = fLastEntry;
                return;
        }


--
To view, visit https://review.haiku-os.org/c/haiku/+/3092
To unsubscribe, or for help writing mail filters, visit 
https://review.haiku-os.org/settings

Gerrit-Project: haiku
Gerrit-Branch: master
Gerrit-Change-Id: I61f4a67e8b8bf14d2f311cb41b24a1c385aebc19
Gerrit-Change-Number: 3092
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Lotz <mmlr@xxxxxxxx>
Gerrit-MessageType: newchange

Other related posts:

  • » [haiku-commits] Change in haiku[master]: multi_audio: Fix use of uninitialized entry to calculate drift. - Gerrit