Skip to content

Commit e843f37

Browse files
committed
TimedAudio: Track of the number of pending frames.
Keep track of the number of frames pending in the timed audio queue so we can implement framesReady in O(1) time instead of O(N). This change partially addresses bug 6020970; the bug will be completely addressed once this change has been up-integrated into master. Change-Id: I599eb15ea1f6d715b97b30e65214fb6fadd169df Signed-off-by: John Grossman <johngro@google.com>
1 parent 14d3b80 commit e843f37

File tree

2 files changed

+92
-34
lines changed

2 files changed

+92
-34
lines changed

services/audioflinger/AudioFlinger.cpp

Lines changed: 77 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,18 @@
6363

6464
// ----------------------------------------------------------------------------
6565

66+
// Note: the following macro is used for extremely verbose logging message. In
67+
// order to run with LOG_ASSERT turned on, we need to have LOG_NDEBUG set to 0;
68+
// but one side effect of this is to turn all LOGV's as well. Some messages are
69+
// so verbose that we want to suppress them even when we have LOG_ASSERT turned
70+
// on. Do not uncomment the #def below unless you really know what you are
71+
// doing and want to see all of the extremely verbose messages.
72+
//#define VERY_VERY_VERBOSE_LOGGING
73+
#ifdef VERY_VERY_VERBOSE_LOGGING
74+
#define LOGVV LOGV
75+
#else
76+
#define LOGVV(a...) do { } while(0)
77+
#endif
6678

6779
namespace android {
6880

@@ -3843,6 +3855,7 @@ AudioFlinger::PlaybackThread::TimedTrack::TimedTrack(
38433855
frameCount, sharedBuffer, sessionId),
38443856
mQueueHeadInFlight(false),
38453857
mTrimQueueHeadOnRelease(false),
3858+
mFramesPendingInQueue(0),
38463859
mTimedSilenceBuffer(NULL),
38473860
mTimedSilenceBufferSize(0),
38483861
mTimedAudioOutputOnTime(false),
@@ -3921,9 +3934,9 @@ void AudioFlinger::PlaybackThread::TimedTrack::trimTimedBufferQueue_l() {
39213934
}
39223935
}
39233936

3924-
size_t trimIndex;
3925-
for (trimIndex = 0; trimIndex < mTimedBufferQueue.size(); trimIndex++) {
3926-
int64_t frameCount = mTimedBufferQueue[trimIndex].buffer()->size()
3937+
size_t trimEnd;
3938+
for (trimEnd = 0; trimEnd < mTimedBufferQueue.size(); trimEnd++) {
3939+
int64_t frameCount = mTimedBufferQueue[trimEnd].buffer()->size()
39273940
/ mCblk->frameSize;
39283941
int64_t bufEnd;
39293942

@@ -3936,25 +3949,63 @@ void AudioFlinger::PlaybackThread::TimedTrack::trimTimedBufferQueue_l() {
39363949
__PRETTY_FUNCTION__);
39373950
break;
39383951
}
3939-
bufEnd += mTimedBufferQueue[trimIndex].pts();
3952+
bufEnd += mTimedBufferQueue[trimEnd].pts();
39403953

39413954
if (bufEnd > mediaTimeNow)
39423955
break;
39433956

39443957
// Is the buffer we want to use in the middle of a mix operation right
39453958
// now? If so, don't actually trim it. Just wait for the releaseBuffer
39463959
// from the mixer which should be coming back shortly.
3947-
if (!trimIndex && mQueueHeadInFlight) {
3960+
if (!trimEnd && mQueueHeadInFlight) {
39483961
mTrimQueueHeadOnRelease = true;
39493962
}
39503963
}
39513964

39523965
size_t trimStart = mTrimQueueHeadOnRelease ? 1 : 0;
3953-
if (trimStart < trimIndex) {
3954-
mTimedBufferQueue.removeItemsAt(trimStart, trimIndex);
3966+
if (trimStart < trimEnd) {
3967+
// Update the bookkeeping for framesReady()
3968+
for (size_t i = trimStart; i < trimEnd; ++i) {
3969+
updateFramesPendingAfterTrim_l(mTimedBufferQueue[i], "trim");
3970+
}
3971+
3972+
// Now actually remove the buffers from the queue.
3973+
mTimedBufferQueue.removeItemsAt(trimStart, trimEnd);
39553974
}
39563975
}
39573976

3977+
void AudioFlinger::PlaybackThread::TimedTrack::trimTimedBufferQueueHead_l(
3978+
const char* logTag) {
3979+
LOG_ASSERT(mTimedBufferQueue.size() > 0,
3980+
"%s called (reason \"%s\"), but timed buffer queue has no"
3981+
" elements to trim.", __FUNCTION__, logTag);
3982+
3983+
updateFramesPendingAfterTrim_l(mTimedBufferQueue[0], logTag);
3984+
mTimedBufferQueue.removeAt(0);
3985+
}
3986+
3987+
void AudioFlinger::PlaybackThread::TimedTrack::updateFramesPendingAfterTrim_l(
3988+
const TimedBuffer& buf,
3989+
const char* logTag) {
3990+
uint32_t bufBytes = buf.buffer()->size();
3991+
uint32_t consumedAlready = buf.position();
3992+
3993+
LOG_ASSERT(consumedAlready <= bufFrames,
3994+
"Bad bookkeeping while updating frames pending. Timed buffer is"
3995+
" only %u bytes long, but claims to have consumed %u"
3996+
" bytes. (update reason: \"%s\")",
3997+
bufFrames, consumedAlready, logTag);
3998+
3999+
uint32_t bufFrames = (bufBytes - consumedAlready) / mCblk->frameSize;
4000+
LOG_ASSERT(mFramesPendingInQueue >= bufFrames,
4001+
"Bad bookkeeping while updating frames pending. Should have at"
4002+
" least %u queued frames, but we think we have only %u. (update"
4003+
" reason: \"%s\")",
4004+
bufFrames, mFramesPendingInQueue, logTag);
4005+
4006+
mFramesPendingInQueue -= bufFrames;
4007+
}
4008+
39584009
status_t AudioFlinger::PlaybackThread::TimedTrack::queueTimedBuffer(
39594010
const sp<IMemory>& buffer, int64_t pts) {
39604011

@@ -3966,6 +4017,8 @@ status_t AudioFlinger::PlaybackThread::TimedTrack::queueTimedBuffer(
39664017

39674018
Mutex::Autolock _l(mTimedBufferQueueLock);
39684019

4020+
uint32_t bufFrames = buffer->size() / mCblk->frameSize;
4021+
mFramesPendingInQueue += bufFrames;
39694022
mTimedBufferQueue.add(TimedBuffer(buffer, pts));
39704023

39714024
return NO_ERROR;
@@ -3974,7 +4027,7 @@ status_t AudioFlinger::PlaybackThread::TimedTrack::queueTimedBuffer(
39744027
status_t AudioFlinger::PlaybackThread::TimedTrack::setMediaTimeTransform(
39754028
const LinearTransform& xform, TimedAudioTrack::TargetTimeline target) {
39764029

3977-
LOGV("%s az=%lld bz=%lld n=%d d=%u tgt=%d", __PRETTY_FUNCTION__,
4030+
LOGVV("setMediaTimeTransform az=%lld bz=%lld n=%d d=%u tgt=%d",
39784031
xform.a_zero, xform.b_zero, xform.a_to_b_numer, xform.a_to_b_denom,
39794032
target);
39804033

@@ -4050,7 +4103,7 @@ status_t AudioFlinger::PlaybackThread::TimedTrack::getNextBuffer(
40504103
LOGW("timedGetNextBuffer transform failed");
40514104
buffer->raw = 0;
40524105
buffer->frameCount = 0;
4053-
mTimedBufferQueue.removeAt(0);
4106+
trimTimedBufferQueueHead_l("getNextBuffer; no transform");
40544107
return NO_ERROR;
40554108
}
40564109

@@ -4079,18 +4132,18 @@ status_t AudioFlinger::PlaybackThread::TimedTrack::getNextBuffer(
40794132
int64_t sampleDelta;
40804133
if (llabs(effectivePTS - pts) >= (static_cast<int64_t>(1) << 31)) {
40814134
LOGV("*** head buffer is too far from PTS: dropped buffer");
4082-
mTimedBufferQueue.removeAt(0);
4135+
trimTimedBufferQueueHead_l("getNextBuffer, buf pts too far from mix");
40834136
continue;
40844137
}
40854138
if (!mLocalTimeToSampleTransform.doForwardTransform(
40864139
(effectivePTS - pts) << 32, &sampleDelta)) {
40874140
LOGV("*** too late during sample rate transform: dropped buffer");
4088-
mTimedBufferQueue.removeAt(0);
4141+
trimTimedBufferQueueHead_l("getNextBuffer, bad local to sample");
40894142
continue;
40904143
}
40914144

4092-
LOGV("*** %s head.pts=%lld head.pos=%d pts=%lld sampleDelta=[%d.%08x]",
4093-
__PRETTY_FUNCTION__, head.pts(), head.position(), pts,
4145+
LOGVV("*** getNextBuffer head.pts=%lld head.pos=%d pts=%lld sampleDelta=[%d.%08x]",
4146+
head.pts(), head.position(), pts,
40944147
static_cast<int32_t>((sampleDelta >= 0 ? 0 : 1) + (sampleDelta >> 32)),
40954148
static_cast<uint32_t>(sampleDelta & 0xFFFFFFFF));
40964149

@@ -4110,7 +4163,7 @@ status_t AudioFlinger::PlaybackThread::TimedTrack::getNextBuffer(
41104163
// with the last output
41114164
timedYieldSamples_l(buffer);
41124165

4113-
LOGV("*** on time: head.pos=%d frameCount=%u", head.position(), buffer->frameCount);
4166+
LOGVV("*** on time: head.pos=%d frameCount=%u", head.position(), buffer->frameCount);
41144167
return NO_ERROR;
41154168
} else if (sampleDelta > 0) {
41164169
// the gap between the current output position and the proper start of
@@ -4130,7 +4183,7 @@ status_t AudioFlinger::PlaybackThread::TimedTrack::getNextBuffer(
41304183
// all the remaining samples in the head are too late, so
41314184
// drop it and move on
41324185
LOGV("*** too late: dropped buffer");
4133-
mTimedBufferQueue.removeAt(0);
4186+
trimTimedBufferQueueHead_l("getNextBuffer, dropped late buffer");
41344187
continue;
41354188
} else {
41364189
// skip over the late samples
@@ -4226,9 +4279,16 @@ void AudioFlinger::PlaybackThread::TimedTrack::releaseBuffer(
42264279
(buffer->frameCount * mCblk->frameSize));
42274280
mQueueHeadInFlight = false;
42284281

4282+
LOG_ASSERT(mFramesPendingInQueue >= buffer->frameCount,
4283+
"Bad bookkeeping during releaseBuffer! Should have at"
4284+
" least %u queued frames, but we think we have only %u",
4285+
buffer->frameCount, mFramesPendingInQueue);
4286+
4287+
mFramesPendingInQueue -= buffer->frameCount;
4288+
42294289
if ((static_cast<size_t>(head.position()) >= head.buffer()->size())
42304290
|| mTrimQueueHeadOnRelease) {
4231-
mTimedBufferQueue.removeAt(0);
4291+
trimTimedBufferQueueHead_l("releaseBuffer");
42324292
mTrimQueueHeadOnRelease = false;
42334293
}
42344294
} else {
@@ -4243,14 +4303,7 @@ void AudioFlinger::PlaybackThread::TimedTrack::releaseBuffer(
42434303

42444304
uint32_t AudioFlinger::PlaybackThread::TimedTrack::framesReady() const {
42454305
Mutex::Autolock _l(mTimedBufferQueueLock);
4246-
4247-
uint32_t frames = 0;
4248-
for (size_t i = 0; i < mTimedBufferQueue.size(); i++) {
4249-
const TimedBuffer& tb = mTimedBufferQueue[i];
4250-
frames += (tb.buffer()->size() - tb.position()) / mCblk->frameSize;
4251-
}
4252-
4253-
return frames;
4306+
return mFramesPendingInQueue;
42544307
}
42554308

42564309
AudioFlinger::PlaybackThread::TimedTrack::TimedBuffer::TimedBuffer()

services/audioflinger/AudioFlinger.h

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -697,32 +697,28 @@ class AudioFlinger :
697697
TimedBuffer(const sp<IMemory>& buffer, int64_t pts);
698698
const sp<IMemory>& buffer() const { return mBuffer; }
699699
int64_t pts() const { return mPTS; }
700-
int position() const { return mPosition; }
701-
void setPosition(int pos) { mPosition = pos; }
700+
uint32_t position() const { return mPosition; }
701+
void setPosition(uint32_t pos) { mPosition = pos; }
702702
private:
703703
sp<IMemory> mBuffer;
704-
int64_t mPTS;
705-
int mPosition;
704+
int64_t mPTS;
705+
uint32_t mPosition;
706706
};
707707

708+
// Mixer facing methods.
708709
virtual bool isTimedTrack() const { return true; }
709-
710710
virtual uint32_t framesReady() const;
711-
712711
virtual status_t getNextBuffer(AudioBufferProvider::Buffer* buffer,
713712
int64_t pts);
714713
virtual void releaseBuffer(AudioBufferProvider::Buffer* buffer);
715-
void timedYieldSamples_l(AudioBufferProvider::Buffer* buffer);
716-
void timedYieldSilence_l(uint32_t numFrames,
717-
AudioBufferProvider::Buffer* buffer);
718714

715+
// Client/App facing methods.
719716
status_t allocateTimedBuffer(size_t size,
720717
sp<IMemory>* buffer);
721718
status_t queueTimedBuffer(const sp<IMemory>& buffer,
722719
int64_t pts);
723720
status_t setMediaTimeTransform(const LinearTransform& xform,
724721
TimedAudioTrack::TargetTimeline target);
725-
void trimTimedBufferQueue_l();
726722

727723
private:
728724
TimedTrack(const wp<ThreadBase>& thread,
@@ -735,6 +731,14 @@ class AudioFlinger :
735731
const sp<IMemory>& sharedBuffer,
736732
int sessionId);
737733

734+
void timedYieldSamples_l(AudioBufferProvider::Buffer* buffer);
735+
void timedYieldSilence_l(uint32_t numFrames,
736+
AudioBufferProvider::Buffer* buffer);
737+
void trimTimedBufferQueue_l();
738+
void trimTimedBufferQueueHead_l(const char* logTag);
739+
void updateFramesPendingAfterTrim_l(const TimedBuffer& buf,
740+
const char* logTag);
741+
738742
uint64_t mLocalTimeFreq;
739743
LinearTransform mLocalTimeToSampleTransform;
740744
LinearTransform mMediaTimeToSampleTransform;
@@ -743,6 +747,7 @@ class AudioFlinger :
743747
Vector<TimedBuffer> mTimedBufferQueue;
744748
bool mQueueHeadInFlight;
745749
bool mTrimQueueHeadOnRelease;
750+
uint32_t mFramesPendingInQueue;
746751

747752
uint8_t* mTimedSilenceBuffer;
748753
uint32_t mTimedSilenceBufferSize;

0 commit comments

Comments
 (0)