Commit: c4e90131d8cdb2bef34bc1edea784efe36bd3c86 - frameworks-av (git)

frameworks/av

Commit MetaInfo

• Revisão: c4e90131d8cdb2bef34bc1edea784efe36bd3c86 (tree)
• Hora: 2019-11-12 02:10:42
• Autor: Kevin Rocard <krocard@goog...>
• Commiter: Eric Laurent

Mensagem de Log

Fix secondary output under&over run

There were three issues with the implementation
of Audio Playback Capture patches on secondary outputs.

1) Patch tracks were always forced to be ready, even if there were not
enough audio to be played. That led to playing partial buffers if the
source output had smaller (time wise) buffer than the secondary output.

This is fixed by avoiding setting CBLK_FORCEREADY on every buffer push
from the primary output thread.

2) After 1 is fixed, the patch tracks now behave like regular (non fast)
tracks. This means that the track only starts when it is full.
That leads to overrun on startup as the primary and secondary outputs
usually do not have the same write period.

What makes the issue worst is that Remote submix has a fixed buffer in bytes,
so its write period changes depending on sample rate contrary to the
primary HAL which pulls with fixed time periods (usually 20/40ms).

This patch solution is to introduce a ready threshold. The track will be
considered ready to be active if there are more than frames in its buffer.

To avoid changing previous latency behaviour except for APC,
legacy tracks have this threshold set to their buffer size and
non APC patch tracks have it set to 1, similar to setting
CBLK_FORCEREADY.

3) The patch track buffer size calculation of the patch track did not take
into account primary and secondary output could be of different sampling
rate.
This mean that the patch track buffer was usually too small as the
secondary output usually had smaller sampling rate (Live caption
requests 16kHz) than the track (music usually plays in 48kHz).
This made the two problems even worst.

This was easily fixed by scaling the buffer size with each side sample rate.

Bug: 136691300
Test: play audio

adb shell audiorecorder --target /data/file1.raw
# Check that the recorded file has no underrun

Change-Id: Ib2846b2827afd1b953d4a25acb18c7cadf57cd3e
Signed-off-by: Kevin Rocard <[e-mail]>
(cherry picked from commit 01c7d9ee1b170a516e20b9c7aad3f9d1db982c55)

Mudança Sumário

• modified: services/audioflinger/AudioFlinger.cpp
• modified: services/audioflinger/PlaybackTracks.h
• modified: services/audioflinger/Tracks.cpp

Diff

--- a/services/audioflinger/AudioFlinger.cpp
+++ b/services/audioflinger/AudioFlinger.cpp
@@ -810,7 +810,33 @@ sp<IAudioTrack> AudioFlinger::createTrack(const CreateTrackInput& input,
                     continue;
                 }
 
-                size_t frameCount = std::lcm(thread->frameCount(), secondaryThread->frameCount());
+                size_t sourceFrameCount = thread->frameCount() * output.sampleRate
+                                          / thread->sampleRate();
+                size_t sinkFrameCount = secondaryThread->frameCount() * output.sampleRate
+                                          / secondaryThread->sampleRate();
+                // If the secondary output has just been opened, the first secondaryThread write
+                // will not block as it will fill the empty startup buffer of the HAL,
+                // so a second sink buffer needs to be ready for the immediate next blocking write.
+                // Additionally, have a margin of one main thread buffer as the scheduling jitter
+                // can reorder the writes (eg if thread A&B have the same write intervale,
+                // the scheduler could schedule AB...BA)
+                size_t frameCountToBeReady = 2 * sinkFrameCount + sourceFrameCount;
+                // Total secondary output buffer must be at least as the read frames plus
+                // the margin of a few buffers on both sides in case the
+                // threads scheduling has some jitter.
+                // That value should not impact latency as the secondary track is started before
+                // its buffer is full, see frameCountToBeReady.
+                size_t frameCount = frameCountToBeReady + 2 * (sourceFrameCount + sinkFrameCount);
+                // The frameCount should also not be smaller than the secondary thread min frame
+                // count
+                size_t minFrameCount = AudioSystem::calculateMinFrameCount(
+                            [&] { Mutex::Autolock _l(secondaryThread->mLock);
+                                  return secondaryThread->latency_l(); }(),
+                            secondaryThread->mNormalFrameCount,
+                            secondaryThread->mSampleRate,
+                            output.sampleRate,
+                            input.speed);
+                frameCount = std::max(frameCount, minFrameCount);
 
                 using namespace std::chrono_literals;
                 auto inChannelMask = audio_channel_mask_out_to_in(input.config.channel_mask);
@@ -843,7 +869,8 @@ sp<IAudioTrack> AudioFlinger::createTrack(const CreateTrackInput& input,
                                                                patchRecord->buffer(),
                                                                patchRecord->bufferSize(),
                                                                outputFlags,
-                                                               0ns /* timeout */);
+                                                               0ns /* timeout */,
+                                                               frameCountToBeReady);
                 status = patchTrack->initCheck();
                 if (status != NO_ERROR) {
                     ALOGE("Secondary output patchTrack init failed: %d", status);

--- a/services/audioflinger/PlaybackTracks.h
+++ b/services/audioflinger/PlaybackTracks.h
@@ -74,7 +74,10 @@ public:
                                 uid_t uid,
                                 audio_output_flags_t flags,
                                 track_type type,
-                                audio_port_handle_t portId = AUDIO_PORT_HANDLE_NONE);
+                                audio_port_handle_t portId = AUDIO_PORT_HANDLE_NONE,
+                                /** default behaviour is to start when there are as many frames
+                                  * ready as possible (aka. Buffer is full). */
+                                size_t frameCountToBeReady = SIZE_MAX);
     virtual             ~Track();
     virtual status_t    initCheck() const;
 
@@ -263,6 +266,8 @@ protected:
     };
     sp<AudioVibrationController> mAudioVibrationController;
     sp<os::ExternalVibration>    mExternalVibration;
+    /** How many frames should be in the buffer before the track is considered ready */
+    const size_t        mFrameCountToBeReady;
 
 private:
     void                interceptBuffer(const AudioBufferProvider::Buffer& buffer);
@@ -384,7 +389,11 @@ public:
                                    void *buffer,
                                    size_t bufferSize,
                                    audio_output_flags_t flags,
-                                   const Timeout& timeout = {});
+                                   const Timeout& timeout = {},
+                                   size_t frameCountToBeReady = 1 /** Default behaviour is to start
+                                                                    *  as soon as possible to have
+                                                                    *  the lowest possible latency
+                                                                    *  even if it might glitch. */);
     virtual             ~PatchTrack();
 
     virtual status_t    start(AudioSystem::sync_event_t event =
@@ -402,5 +411,4 @@ public:
 
 private:
             void restartIfDisabled();
-
 };  // end of PatchTrack

--- a/services/audioflinger/Tracks.cpp
+++ b/services/audioflinger/Tracks.cpp
@@ -511,7 +511,8 @@ AudioFlinger::PlaybackThread::Track::Track(
             uid_t uid,
             audio_output_flags_t flags,
             track_type type,
-            audio_port_handle_t portId)
+            audio_port_handle_t portId,
+            size_t frameCountToBeReady)
     :   TrackBase(thread, client, attr, sampleRate, format, channelMask, frameCount,
                   (sharedBuffer != 0) ? sharedBuffer->pointer() : buffer,
                   (sharedBuffer != 0) ? sharedBuffer->size() : bufferSize,
@@ -530,6 +531,7 @@ AudioFlinger::PlaybackThread::Track::Track(
     mVolumeHandler(new media::VolumeHandler(sampleRate)),
     mOpPlayAudioMonitor(OpPlayAudioMonitor::createIfNeeded(uid, attr, id(), streamType)),
     // mSinkTimestamp
+    mFrameCountToBeReady(frameCountToBeReady),
     mFastIndex(-1),
     mCachedVolume(1.0),
     /* The track might not play immediately after being active, similarly as if its volume was 0.
@@ -837,7 +839,7 @@ void AudioFlinger::PlaybackThread::Track::interceptBuffer(
     auto spent = ceil<std::chrono::microseconds>(std::chrono::steady_clock::now() - start);
     using namespace std::chrono_literals;
     // Average is ~20us per track, this should virtually never be logged (Logging takes >200us)
-    ALOGD_IF(spent > 200us, "%s: took %lldus to intercept %zu tracks", __func__,
+    ALOGD_IF(spent > 500us, "%s: took %lldus to intercept %zu tracks", __func__,
              spent.count(), mTeePatches.size());
 }
 
@@ -910,8 +912,12 @@ bool AudioFlinger::PlaybackThread::Track::isReady() const {
         return true;
     }
 
-    if (framesReady() >= mServerProxy->getBufferSizeInFrames() ||
-            (mCblk->mFlags & CBLK_FORCEREADY)) {
+    size_t bufferSizeInFrames = mServerProxy->getBufferSizeInFrames();
+    size_t framesToBeReady = std::min(mFrameCountToBeReady, bufferSizeInFrames);
+
+    if (framesReady() >= framesToBeReady || (mCblk->mFlags & CBLK_FORCEREADY)) {
+        ALOGV("%s(%d): consider track ready with %zu/%zu, target was %zu)",
+              __func__, mId, framesReady(), bufferSizeInFrames, framesToBeReady);
         mFillingUpStatus = FS_FILLED;
         android_atomic_and(~CBLK_FORCEREADY, &mCblk->mFlags);
         return true;
@@ -1413,6 +1419,7 @@ void AudioFlinger::PlaybackThread::Track::invalidate()
 
 void AudioFlinger::PlaybackThread::Track::disable()
 {
+    // TODO(b/142394888): the filling status should also be reset to filling
     signalClientFlag(CBLK_DISABLED);
 }
 
@@ -1790,12 +1797,14 @@ AudioFlinger::PlaybackThread::PatchTrack::PatchTrack(PlaybackThread *playbackThr
                                                      void *buffer,
                                                      size_t bufferSize,
                                                      audio_output_flags_t flags,
-                                                     const Timeout& timeout)
+                                                     const Timeout& timeout,
+                                                     size_t frameCountToBeReady)
     :   Track(playbackThread, NULL, streamType,
               audio_attributes_t{} /* currently unused for patch track */,
               sampleRate, format, channelMask, frameCount,
               buffer, bufferSize, nullptr /* sharedBuffer */,
-              AUDIO_SESSION_NONE, getpid(), AID_AUDIOSERVER, flags, TYPE_PATCH),
+              AUDIO_SESSION_NONE, getpid(), AID_AUDIOSERVER, flags, TYPE_PATCH,
+              AUDIO_PORT_HANDLE_NONE, frameCountToBeReady),
         PatchTrackBase(new ClientProxy(mCblk, mBuffer, frameCount, mFrameSize, true, true),
                        *playbackThread, timeout)
 {
@@ -1869,7 +1878,6 @@ void AudioFlinger::PlaybackThread::PatchTrack::releaseBuffer(Proxy::Buffer* buff
 {
     mProxy->releaseBuffer(buffer);
     restartIfDisabled();
-    android_atomic_or(CBLK_FORCEREADY, &mCblk->mFlags);
 }
 
 void AudioFlinger::PlaybackThread::PatchTrack::restartIfDisabled()