From 9fdcb0a9497ca290bcf364b10868587b6bde3a34 Mon Sep 17 00:00:00 2001 From: Glenn Kasten Date: Wed, 26 Jun 2013 16:11:36 -0700 Subject: Fix theoretical race using TrackBase::sampleRate() In two places we assumed that TrackBase::sampleRate() would return the same value when it is called twice in the same function. This is not guaranteed; sampleRate() reads from the control block so the return value could change. To fix this, only call sampleRate() once and cache the return value to get a consistent value. This was only a theoretical race. In MixerThread::prepareTracks_l() it would have no bad effect. In TimedTrack::getNextBuffer() it could cause a real problem, but we don't currently support dynamic sample rate ratios for timed tracks. Change-Id: I8e5c33f0121fc058d1e70c2ab5e9135397d3e0b7 --- services/audioflinger/Threads.cpp | 7 ++++--- services/audioflinger/Tracks.cpp | 6 ++++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp index 0773534..3b5727b 100644 --- a/services/audioflinger/Threads.cpp +++ b/services/audioflinger/Threads.cpp @@ -2427,7 +2427,7 @@ AudioFlinger::PlaybackThread::mixer_state AudioFlinger::MixerThread::prepareTrac } for (size_t i=0 ; i t = mActiveTracks[i].promote(); + const sp t = mActiveTracks[i].promote(); if (t == 0) { continue; } @@ -2597,11 +2597,12 @@ AudioFlinger::PlaybackThread::mixer_state AudioFlinger::MixerThread::prepareTrac // hence the test on (mMixerStatus == MIXER_TRACKS_READY) meaning the track was mixed // during last round size_t desiredFrames; - if (t->sampleRate() == mSampleRate) { + uint32_t sr = track->sampleRate(); + if (sr == mSampleRate) { desiredFrames = mNormalFrameCount; } else { // +1 for rounding and +1 for additional sample needed for interpolation - desiredFrames = (mNormalFrameCount * t->sampleRate()) / mSampleRate + 1 + 1; + desiredFrames = (mNormalFrameCount * sr) / mSampleRate + 1 + 1; // add frames already consumed but not yet released by the resampler // because cblk->framesReady() will include these frames desiredFrames += mAudioMixer->getUnreleasedFrames(track->name()); diff --git a/services/audioflinger/Tracks.cpp b/services/audioflinger/Tracks.cpp index bfc197c..6aca95f 100644 --- a/services/audioflinger/Tracks.cpp +++ b/services/audioflinger/Tracks.cpp @@ -1130,10 +1130,12 @@ status_t AudioFlinger::PlaybackThread::TimedTrack::getNextBuffer( } } + uint32_t sr = sampleRate(); + // adjust the head buffer's PTS to reflect the portion of the head buffer // that has already been consumed int64_t effectivePTS = headLocalPTS + - ((head.position() / mFrameSize) * mLocalTimeFreq / sampleRate()); + ((head.position() / mFrameSize) * mLocalTimeFreq / sr); // Calculate the delta in samples between the head of the input buffer // queue and the start of the next output buffer that will be written. @@ -1165,7 +1167,7 @@ status_t AudioFlinger::PlaybackThread::TimedTrack::getNextBuffer( // the current output position is within this threshold, then we will // concatenate the next input samples to the previous output const int64_t kSampleContinuityThreshold = - (static_cast(sampleRate()) << 32) / 250; + (static_cast(sr) << 32) / 250; // if this is the first buffer of audio that we're emitting from this track // then it should be almost exactly on time. -- cgit v1.1