Fix issue 2139634: DTMF tones on Sholes popping, hissing (audio latency too high).
This change is a complement to the main fix in kernel driver for the same issue (partner change #1250).
It removes clicks sometimes heard after the end of the tones while audio flinger is sending 0s to the audio output stream.
The problem was that the sleep time between two writes was more than the duration of one audio output stream buffer which could cause some underrun.
Also fixed a recent regression in ToneGenerator that made that the end of previous tone was repeated at the beginning of current one under certain timing circumstances when the maximum tone duration was specified.
diff --git a/include/media/ToneGenerator.h b/include/media/ToneGenerator.h
index e8df08e..c884c2c 100644
--- a/include/media/ToneGenerator.h
+++ b/include/media/ToneGenerator.h
@@ -248,6 +248,7 @@
// only if tone duration is less than about 27 Hours(@44100Hz sampling rate). If this time is exceeded,
// no crash will occur but tone sequence will show a glitch.
unsigned int mMaxSmp; // Maximum number of audio samples played (maximun tone duration)
+ int mDurationMs; // Maximum tone duration in ms
unsigned short mCurSegment; // Current segment index in ToneDescriptor segments[]
unsigned short mCurCount; // Current sequence repeat count
diff --git a/libs/audioflinger/AudioFlinger.cpp b/libs/audioflinger/AudioFlinger.cpp
index 8e967fb..ba8b322 100644
--- a/libs/audioflinger/AudioFlinger.cpp
+++ b/libs/audioflinger/AudioFlinger.cpp
@@ -62,8 +62,6 @@
static const char* kHardwareLockedString = "Hardware lock is taken\n";
//static const nsecs_t kStandbyTimeInNsecs = seconds(3);
-static const unsigned long kBufferRecoveryInUsecs = 2000;
-static const unsigned long kMaxBufferRecoveryInUsecs = 20000;
static const float MAX_GAIN = 4096.0f;
// retry counts for buffer fill timeout
@@ -1070,10 +1068,10 @@
// in both cases "unstop" the track
if (track->isPaused()) {
track->mState = TrackBase::RESUMING;
- LOGV("PAUSED => RESUMING (%d)", track->name());
+ LOGV("PAUSED => RESUMING (%d) on thread %p", track->name(), this);
} else {
track->mState = TrackBase::ACTIVE;
- LOGV("? => ACTIVE (%d)", track->name());
+ LOGV("? => ACTIVE (%d) on thread %p", track->name(), this);
}
// set retry count for buffer fill
track->mRetryCount = kMaxTrackStartupRetries;
@@ -1175,7 +1173,8 @@
bool AudioFlinger::MixerThread::threadLoop()
{
- unsigned long sleepTime = 0;
+ uint32_t sleepTime = 0;
+ uint32_t maxBufferRecoveryInUsecs = getMaxBufferRecoveryInUsecs();
int16_t* curBuf = mMixBuffer;
Vector< sp<Track> > tracksToRemove;
size_t enabledTracks = 0;
@@ -1200,6 +1199,7 @@
// FIXME: Relaxed timing because of a certain device that can't meet latency
// Should be reduced to 2x after the vendor fixes the driver issue
maxPeriod = seconds(mFrameCount) / mSampleRate * 3;
+ maxBufferRecoveryInUsecs = getMaxBufferRecoveryInUsecs();
}
const SortedVector< wp<Track> >& activeTracks = mActiveTracks;
@@ -1235,7 +1235,6 @@
}
standbyTime = systemTime() + kStandbyTimeInNsecs;
- sleepTime = 0;
continue;
}
}
@@ -1249,28 +1248,23 @@
sleepTime = 0;
standbyTime = systemTime() + kStandbyTimeInNsecs;
} else {
- sleepTime += kBufferRecoveryInUsecs;
- if (sleepTime > kMaxBufferRecoveryInUsecs) {
- sleepTime = kMaxBufferRecoveryInUsecs;
- }
- // There was nothing to mix this round, which means all
- // active tracks were late. Sleep a little bit to give
- // them another chance. If we're too late, write 0s to audio
- // hardware to avoid underrun.
- if (mBytesWritten != 0 && sleepTime >= kMaxBufferRecoveryInUsecs) {
+ // If no tracks are ready, sleep once for the duration of an output
+ // buffer size, then write 0s to the output
+ if (sleepTime == 0) {
+ sleepTime = maxBufferRecoveryInUsecs;
+ } else if (mBytesWritten != 0) {
memset (curBuf, 0, mixBufferSize);
sleepTime = 0;
}
}
if (mSuspended) {
- sleepTime = kMaxBufferRecoveryInUsecs;
+ sleepTime = maxBufferRecoveryInUsecs;
}
// sleepTime == 0 means we must write to audio hardware
if (sleepTime == 0) {
mLastWriteTime = systemTime();
mInWrite = true;
- LOGV("mOutput->write() thread %p frames %d", this, mFrameCount);
int bytesWritten = (int)mOutput->write(curBuf, mixBufferSize);
if (bytesWritten > 0) mBytesWritten += bytesWritten;
mNumWrites++;
@@ -1393,7 +1387,7 @@
// No buffers for this track. Give it a few chances to
// fill a buffer, then remove it from active list.
if (--(track->mRetryCount) <= 0) {
- LOGV("BUFFER TIMEOUT: remove(%d) from active list", track->name());
+ LOGV("BUFFER TIMEOUT: remove(%d) from active list on thread %p", track->name(), this);
tracksToRemove->add(track);
}
// For tracks using static shared memory buffer, make sure that we have
@@ -1583,6 +1577,16 @@
return NO_ERROR;
}
+uint32_t AudioFlinger::MixerThread::getMaxBufferRecoveryInUsecs()
+{
+ uint32_t time = ((mFrameCount * 1000) / mSampleRate) * 1000;
+ // Add some margin with regard to scheduling precision
+ if (time > 10000) {
+ time -= 10000;
+ }
+ return time;
+}
+
// ----------------------------------------------------------------------------
AudioFlinger::DirectOutputThread::DirectOutputThread(const sp<AudioFlinger>& audioFlinger, AudioStreamOut* output)
: PlaybackThread(audioFlinger, output),
@@ -1598,7 +1602,8 @@
bool AudioFlinger::DirectOutputThread::threadLoop()
{
- unsigned long sleepTime = 0;
+ uint32_t sleepTime = 0;
+ uint32_t maxBufferRecoveryInUsecs = getMaxBufferRecoveryInUsecs();
sp<Track> trackToRemove;
sp<Track> activeTrack;
nsecs_t standbyTime = systemTime();
@@ -1615,6 +1620,7 @@
if (checkForNewParameters_l()) {
mixBufferSize = mFrameCount*mFrameSize;
+ maxBufferRecoveryInUsecs = getMaxBufferRecoveryInUsecs();
}
// put audio hardware into standby after short delay
@@ -1648,7 +1654,6 @@
}
standbyTime = systemTime() + kStandbyTimeInNsecs;
- sleepTime = 0;
continue;
}
}
@@ -1761,23 +1766,16 @@
sleepTime = 0;
standbyTime = systemTime() + kStandbyTimeInNsecs;
} else {
- sleepTime += kBufferRecoveryInUsecs;
- if (sleepTime > kMaxBufferRecoveryInUsecs) {
- sleepTime = kMaxBufferRecoveryInUsecs;
- }
- // There was nothing to mix this round, which means all
- // active tracks were late. Sleep a little bit to give
- // them another chance. If we're too late, write 0s to audio
- // hardware to avoid underrun.
- if (mBytesWritten != 0 && sleepTime >= kMaxBufferRecoveryInUsecs &&
- AudioSystem::isLinearPCM(mFormat)) {
+ if (sleepTime == 0) {
+ sleepTime = maxBufferRecoveryInUsecs;
+ } else if (mBytesWritten != 0 && AudioSystem::isLinearPCM(mFormat)) {
memset (mMixBuffer, 0, mFrameCount * mFrameSize);
sleepTime = 0;
}
}
if (mSuspended) {
- sleepTime = kMaxBufferRecoveryInUsecs;
+ sleepTime = maxBufferRecoveryInUsecs;
}
// sleepTime == 0 means we must write to audio hardware
if (sleepTime == 0) {
@@ -1861,6 +1859,21 @@
return reconfig;
}
+uint32_t AudioFlinger::DirectOutputThread::getMaxBufferRecoveryInUsecs()
+{
+ uint32_t time;
+ if (AudioSystem::isLinearPCM(mFormat)) {
+ time = ((mFrameCount * 1000) / mSampleRate) * 1000;
+ // Add some margin with regard to scheduling precision
+ if (time > 10000) {
+ time -= 10000;
+ }
+ } else {
+ time = 10000;
+ }
+ return time;
+}
+
// ----------------------------------------------------------------------------
AudioFlinger::DuplicatingThread::DuplicatingThread(const sp<AudioFlinger>& audioFlinger, AudioFlinger::MixerThread* mainThread)
@@ -1877,13 +1890,15 @@
bool AudioFlinger::DuplicatingThread::threadLoop()
{
- unsigned long sleepTime = kBufferRecoveryInUsecs;
+ uint32_t sleepTime = 0;
+ uint32_t maxBufferRecoveryInUsecs = getMaxBufferRecoveryInUsecs();
int16_t* curBuf = mMixBuffer;
Vector< sp<Track> > tracksToRemove;
size_t enabledTracks = 0;
nsecs_t standbyTime = systemTime();
size_t mixBufferSize = mFrameCount*mFrameSize;
SortedVector< sp<OutputTrack> > outputTracks;
+ uint32_t writeFrames = 0;
while (!exitPending())
{
@@ -1896,6 +1911,7 @@
if (checkForNewParameters_l()) {
mixBufferSize = mFrameCount*mFrameSize;
+ maxBufferRecoveryInUsecs = getMaxBufferRecoveryInUsecs();
}
const SortedVector< wp<Track> >& activeTracks = mActiveTracks;
@@ -1935,7 +1951,6 @@
}
standbyTime = systemTime() + kStandbyTimeInNsecs;
- sleepTime = kBufferRecoveryInUsecs;
continue;
}
}
@@ -1947,29 +1962,30 @@
// mix buffers...
mAudioMixer->process(curBuf);
sleepTime = 0;
- standbyTime = systemTime() + kStandbyTimeInNsecs;
+ writeFrames = mFrameCount;
} else {
- sleepTime += kBufferRecoveryInUsecs;
- if (sleepTime > kMaxBufferRecoveryInUsecs) {
- sleepTime = kMaxBufferRecoveryInUsecs;
- }
- // There was nothing to mix this round, which means all
- // active tracks were late. Sleep a little bit to give
- // them another chance. If we're too late, write 0s to audio
- // hardware to avoid underrun.
- if (mBytesWritten != 0 && sleepTime >= kMaxBufferRecoveryInUsecs) {
- memset (curBuf, 0, mixBufferSize);
- sleepTime = 0;
+ if (sleepTime == 0) {
+ sleepTime = maxBufferRecoveryInUsecs;
+ } else if (mBytesWritten != 0) {
+ // flush remaining overflow buffers in output tracks
+ for (size_t i = 0; i < outputTracks.size(); i++) {
+ if (outputTracks[i]->isActive()) {
+ sleepTime = 0;
+ writeFrames = 0;
+ break;
+ }
+ }
}
}
if (mSuspended) {
- sleepTime = kMaxBufferRecoveryInUsecs;
+ sleepTime = maxBufferRecoveryInUsecs;
}
// sleepTime == 0 means we must write to audio hardware
if (sleepTime == 0) {
+ standbyTime = systemTime() + kStandbyTimeInNsecs;
for (size_t i = 0; i < outputTracks.size(); i++) {
- outputTracks[i]->write(curBuf, mFrameCount);
+ outputTracks[i]->write(curBuf, writeFrames);
}
mStandby = false;
mBytesWritten += mixBufferSize;
@@ -2026,7 +2042,6 @@
LOGV("removeOutputTrack(): unkonwn thread: %p", thread);
}
-
// ----------------------------------------------------------------------------
// TrackBase constructor must be called with AudioFlinger::mLock held
@@ -2300,7 +2315,7 @@
getNextBuffer_exit:
buffer->raw = 0;
buffer->frameCount = 0;
- LOGV("getNextBuffer() no more data");
+ LOGV("getNextBuffer() no more data for track %d on thread %p", mName, mThread.unsafe_get());
return NOT_ENOUGH_DATA;
}
@@ -2341,7 +2356,7 @@
if (playbackThread->mActiveTracks.indexOf(this) < 0) {
reset();
}
- LOGV("(> STOPPED) => STOPPED (%d)", mName);
+ LOGV("(> STOPPED) => STOPPED (%d) on thread %p", mName, playbackThread);
}
}
}
@@ -2354,7 +2369,7 @@
Mutex::Autolock _l(thread->mLock);
if (mState == ACTIVE || mState == RESUMING) {
mState = PAUSING;
- LOGV("ACTIVE/RESUMING => PAUSING (%d)", mName);
+ LOGV("ACTIVE/RESUMING => PAUSING (%d) on thread %p", mName, thread.get());
}
}
}
@@ -2566,7 +2581,7 @@
uint32_t waitTimeLeftMs = mWaitTimeMs;
- if (!mActive) {
+ if (!mActive && frames != 0) {
start();
sp<ThreadBase> thread = mThread.promote();
if (thread != 0) {
@@ -2608,7 +2623,7 @@
break;
}
uint32_t waitTimeMs = (uint32_t)ns2ms(systemTime() - startTime);
-// LOGV("OutputTrack::write() waitTimeMs %d waitTimeLeftMs %d", waitTimeMs, waitTimeLeftMs)
+ LOGV("OutputTrack::write() to thread %p waitTimeMs %d waitTimeLeftMs %d", mThread.unsafe_get(), waitTimeMs, waitTimeLeftMs);
if (waitTimeLeftMs >= waitTimeMs) {
waitTimeLeftMs -= waitTimeMs;
} else {
@@ -2663,7 +2678,7 @@
pInBuffer->i16 = pInBuffer->mBuffer;
memset(pInBuffer->raw, 0, frames * channels * sizeof(int16_t));
mBufferQueue.add(pInBuffer);
- } else {
+ } else if (mActive) {
stop();
}
}
diff --git a/libs/audioflinger/AudioFlinger.h b/libs/audioflinger/AudioFlinger.h
index 3699019..4bf73ce 100644
--- a/libs/audioflinger/AudioFlinger.h
+++ b/libs/audioflinger/AudioFlinger.h
@@ -524,6 +524,10 @@
bool mMasterMute;
SortedVector< wp<Track> > mActiveTracks;
+ virtual int getTrackName_l() = 0;
+ virtual void deleteTrackName_l(int name) = 0;
+ virtual uint32_t getMaxBufferRecoveryInUsecs() = 0;
+
private:
friend class AudioFlinger;
@@ -539,8 +543,7 @@
status_t addTrack_l(const sp<Track>& track);
void destroyTrack_l(const sp<Track>& track);
- virtual int getTrackName_l() = 0;
- virtual void deleteTrackName_l(int name) = 0;
+
void readOutputParameters();
virtual status_t dumpInternals(int fd, const Vector<String16>& args);
@@ -571,13 +574,14 @@
int streamType);
void putTracks(SortedVector < sp<Track> >& tracks,
SortedVector < wp<Track> >& activeTracks);
- virtual int getTrackName_l();
- virtual void deleteTrackName_l(int name);
virtual bool checkForNewParameters_l();
virtual status_t dumpInternals(int fd, const Vector<String16>& args);
protected:
size_t prepareTracks_l(const SortedVector< wp<Track> >& activeTracks, Vector< sp<Track> > *tracksToRemove);
+ virtual int getTrackName_l();
+ virtual void deleteTrackName_l(int name);
+ virtual uint32_t getMaxBufferRecoveryInUsecs();
AudioMixer* mAudioMixer;
};
@@ -591,9 +595,12 @@
// Thread virtuals
virtual bool threadLoop();
+ virtual bool checkForNewParameters_l();
+
+ protected:
virtual int getTrackName_l();
virtual void deleteTrackName_l(int name);
- virtual bool checkForNewParameters_l();
+ virtual uint32_t getMaxBufferRecoveryInUsecs();
private:
float mLeftVolume;
diff --git a/media/libmedia/ToneGenerator.cpp b/media/libmedia/ToneGenerator.cpp
index d36bec9..3729eb3 100644
--- a/media/libmedia/ToneGenerator.cpp
+++ b/media/libmedia/ToneGenerator.cpp
@@ -879,6 +879,7 @@
////////////////////////////////////////////////////////////////////////////////
bool ToneGenerator::startTone(int toneType, int durationMs) {
bool lResult = false;
+ status_t lStatus;
if ((toneType < 0) || (toneType >= NUM_TONES))
return lResult;
@@ -898,15 +899,16 @@
toneType = getToneForRegion(toneType);
mpNewToneDesc = &sToneDescriptors[toneType];
- if (durationMs == -1) {
- mMaxSmp = TONEGEN_INF;
- } else {
- if (durationMs > (int)(TONEGEN_INF / mSamplingRate)) {
- mMaxSmp = (durationMs / 1000) * mSamplingRate;
- } else {
- mMaxSmp = (durationMs * mSamplingRate) / 1000;
+ mDurationMs = durationMs;
+
+ if (mState == TONE_STOPPED) {
+ LOGV("Start waiting for previous tone to stop");
+ lStatus = mWaitCbkCond.waitRelative(mLock, seconds(1));
+ if (lStatus != NO_ERROR) {
+ LOGE("--- start wait for stop timed out, status %d", lStatus);
+ mState = TONE_IDLE;
+ return lResult;
}
- LOGV("startTone, duration limited to %d ms", durationMs);
}
if (mState == TONE_INIT) {
@@ -919,7 +921,7 @@
mLock.lock();
if (mState == TONE_STARTING) {
LOGV("Wait for start callback");
- status_t lStatus = mWaitCbkCond.waitRelative(mLock, seconds(1));
+ lStatus = mWaitCbkCond.waitRelative(mLock, seconds(1));
if (lStatus != NO_ERROR) {
LOGE("--- Immediate start timed out, status %d", lStatus);
mState = TONE_IDLE;
@@ -931,9 +933,8 @@
}
} else {
LOGV("Delayed start\n");
-
mState = TONE_RESTARTING;
- status_t lStatus = mWaitCbkCond.waitRelative(mLock, seconds(1));
+ lStatus = mWaitCbkCond.waitRelative(mLock, seconds(1));
if (lStatus == NO_ERROR) {
if (mState != TONE_IDLE) {
lResult = true;
@@ -1316,6 +1317,17 @@
mpToneDesc = mpNewToneDesc;
+ if (mDurationMs == -1) {
+ mMaxSmp = TONEGEN_INF;
+ } else {
+ if (mDurationMs > (int)(TONEGEN_INF / mSamplingRate)) {
+ mMaxSmp = (mDurationMs / 1000) * mSamplingRate;
+ } else {
+ mMaxSmp = (mDurationMs * mSamplingRate) / 1000;
+ }
+ LOGV("prepareWave, duration limited to %d ms", mDurationMs);
+ }
+
while (mpToneDesc->segments[segmentIdx].duration) {
// Get total number of sine waves: needed to adapt sine wave gain.
unsigned int lNumWaves = numWaves(segmentIdx);