Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f5e868ba authored by Eric Laurent's avatar Eric Laurent
Browse files

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.
parent cfc7010e
Loading
Loading
Loading
Loading
+69 −54
Original line number Original line Diff line number Diff line
@@ -62,8 +62,6 @@ static const char* kDeadlockedString = "AudioFlinger may be deadlocked\n";
static const char* kHardwareLockedString = "Hardware lock is taken\n";
static const char* kHardwareLockedString = "Hardware lock is taken\n";


//static const nsecs_t kStandbyTimeInNsecs = seconds(3);
//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;
static const float MAX_GAIN = 4096.0f;


// retry counts for buffer fill timeout
// retry counts for buffer fill timeout
@@ -1070,10 +1068,10 @@ status_t AudioFlinger::PlaybackThread::addTrack_l(const sp<Track>& track)
    // in both cases "unstop" the track
    // in both cases "unstop" the track
    if (track->isPaused()) {
    if (track->isPaused()) {
        track->mState = TrackBase::RESUMING;
        track->mState = TrackBase::RESUMING;
        LOGV("PAUSED => RESUMING (%d)", track->name());
        LOGV("PAUSED => RESUMING (%d) on thread %p", track->name(), this);
    } else {
    } else {
        track->mState = TrackBase::ACTIVE;
        track->mState = TrackBase::ACTIVE;
        LOGV("? => ACTIVE (%d)", track->name());
        LOGV("? => ACTIVE (%d) on thread %p", track->name(), this);
    }
    }
    // set retry count for buffer fill
    // set retry count for buffer fill
    track->mRetryCount = kMaxTrackStartupRetries;
    track->mRetryCount = kMaxTrackStartupRetries;
@@ -1175,7 +1173,8 @@ AudioFlinger::MixerThread::~MixerThread()


bool AudioFlinger::MixerThread::threadLoop()
bool AudioFlinger::MixerThread::threadLoop()
{
{
    unsigned long sleepTime = 0;
    uint32_t sleepTime = 0;
    uint32_t maxBufferRecoveryInUsecs = getMaxBufferRecoveryInUsecs();
    int16_t* curBuf = mMixBuffer;
    int16_t* curBuf = mMixBuffer;
    Vector< sp<Track> > tracksToRemove;
    Vector< sp<Track> > tracksToRemove;
    size_t enabledTracks = 0;
    size_t enabledTracks = 0;
@@ -1200,6 +1199,7 @@ bool AudioFlinger::MixerThread::threadLoop()
                // FIXME: Relaxed timing because of a certain device that can't meet latency
                // 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
                // Should be reduced to 2x after the vendor fixes the driver issue
                maxPeriod = seconds(mFrameCount) / mSampleRate * 3;
                maxPeriod = seconds(mFrameCount) / mSampleRate * 3;
                maxBufferRecoveryInUsecs = getMaxBufferRecoveryInUsecs();
            }
            }


            const SortedVector< wp<Track> >& activeTracks = mActiveTracks;
            const SortedVector< wp<Track> >& activeTracks = mActiveTracks;
@@ -1235,7 +1235,6 @@ bool AudioFlinger::MixerThread::threadLoop()
                    }
                    }


                    standbyTime = systemTime() + kStandbyTimeInNsecs;
                    standbyTime = systemTime() + kStandbyTimeInNsecs;
                    sleepTime = 0;
                    continue;
                    continue;
                }
                }
            }
            }
@@ -1249,28 +1248,23 @@ bool AudioFlinger::MixerThread::threadLoop()
            sleepTime = 0;
            sleepTime = 0;
            standbyTime = systemTime() + kStandbyTimeInNsecs;
            standbyTime = systemTime() + kStandbyTimeInNsecs;
        } else {
        } else {
            sleepTime += kBufferRecoveryInUsecs;
            // If no tracks are ready, sleep once for the duration of an output
            if (sleepTime > kMaxBufferRecoveryInUsecs) {
            // buffer size, then write 0s to the output
                sleepTime = kMaxBufferRecoveryInUsecs;
            if (sleepTime == 0) {
            }
                sleepTime = maxBufferRecoveryInUsecs;
            // There was nothing to mix this round, which means all
            } else if (mBytesWritten != 0) {
            // 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);
                memset (curBuf, 0, mixBufferSize);
                sleepTime = 0;
                sleepTime = 0;
            }
            }
        }
        }


        if (mSuspended) {
        if (mSuspended) {
            sleepTime = kMaxBufferRecoveryInUsecs;
            sleepTime = maxBufferRecoveryInUsecs;
        }
        }
        // sleepTime == 0 means we must write to audio hardware
        // sleepTime == 0 means we must write to audio hardware
        if (sleepTime == 0) {
        if (sleepTime == 0) {
            mLastWriteTime = systemTime();
            mLastWriteTime = systemTime();
            mInWrite = true;
            mInWrite = true;
            LOGV("mOutput->write() thread %p frames %d", this, mFrameCount);
            int bytesWritten = (int)mOutput->write(curBuf, mixBufferSize);
            int bytesWritten = (int)mOutput->write(curBuf, mixBufferSize);
            if (bytesWritten > 0) mBytesWritten += bytesWritten;
            if (bytesWritten > 0) mBytesWritten += bytesWritten;
            mNumWrites++;
            mNumWrites++;
@@ -1393,7 +1387,7 @@ size_t AudioFlinger::MixerThread::prepareTracks_l(const SortedVector< wp<Track>
                // No buffers for this track. Give it a few chances to
                // No buffers for this track. Give it a few chances to
                // fill a buffer, then remove it from active list.
                // fill a buffer, then remove it from active list.
                if (--(track->mRetryCount) <= 0) {
                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);
                    tracksToRemove->add(track);
                }
                }
                // For tracks using static shared memory buffer, make sure that we have
                // For tracks using static shared memory buffer, make sure that we have
@@ -1583,6 +1577,16 @@ status_t AudioFlinger::MixerThread::dumpInternals(int fd, const Vector<String16>
    return NO_ERROR;
    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)
AudioFlinger::DirectOutputThread::DirectOutputThread(const sp<AudioFlinger>& audioFlinger, AudioStreamOut* output)
    :   PlaybackThread(audioFlinger, output),
    :   PlaybackThread(audioFlinger, output),
@@ -1598,7 +1602,8 @@ AudioFlinger::DirectOutputThread::~DirectOutputThread()


bool AudioFlinger::DirectOutputThread::threadLoop()
bool AudioFlinger::DirectOutputThread::threadLoop()
{
{
    unsigned long sleepTime = 0;
    uint32_t sleepTime = 0;
    uint32_t maxBufferRecoveryInUsecs = getMaxBufferRecoveryInUsecs();
    sp<Track> trackToRemove;
    sp<Track> trackToRemove;
    sp<Track> activeTrack;
    sp<Track> activeTrack;
    nsecs_t standbyTime = systemTime();
    nsecs_t standbyTime = systemTime();
@@ -1615,6 +1620,7 @@ bool AudioFlinger::DirectOutputThread::threadLoop()


            if (checkForNewParameters_l()) {
            if (checkForNewParameters_l()) {
                mixBufferSize = mFrameCount*mFrameSize;
                mixBufferSize = mFrameCount*mFrameSize;
                maxBufferRecoveryInUsecs = getMaxBufferRecoveryInUsecs();
            }
            }


            // put audio hardware into standby after short delay
            // put audio hardware into standby after short delay
@@ -1648,7 +1654,6 @@ bool AudioFlinger::DirectOutputThread::threadLoop()
                    }
                    }


                    standbyTime = systemTime() + kStandbyTimeInNsecs;
                    standbyTime = systemTime() + kStandbyTimeInNsecs;
                    sleepTime = 0;
                    continue;
                    continue;
                }
                }
            }
            }
@@ -1761,23 +1766,16 @@ bool AudioFlinger::DirectOutputThread::threadLoop()
            sleepTime = 0;
            sleepTime = 0;
            standbyTime = systemTime() + kStandbyTimeInNsecs;
            standbyTime = systemTime() + kStandbyTimeInNsecs;
        } else {
        } else {
            sleepTime += kBufferRecoveryInUsecs;
            if (sleepTime == 0) {
            if (sleepTime > kMaxBufferRecoveryInUsecs) {
                sleepTime = maxBufferRecoveryInUsecs;
                sleepTime = kMaxBufferRecoveryInUsecs;
            } else if (mBytesWritten != 0 && AudioSystem::isLinearPCM(mFormat)) {
            }
            // 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)) {
                memset (mMixBuffer, 0, mFrameCount * mFrameSize);
                memset (mMixBuffer, 0, mFrameCount * mFrameSize);
                sleepTime = 0;
                sleepTime = 0;
            }
            }
        }
        }


        if (mSuspended) {
        if (mSuspended) {
            sleepTime = kMaxBufferRecoveryInUsecs;
            sleepTime = maxBufferRecoveryInUsecs;
        }
        }
        // sleepTime == 0 means we must write to audio hardware
        // sleepTime == 0 means we must write to audio hardware
        if (sleepTime == 0) {
        if (sleepTime == 0) {
@@ -1861,6 +1859,21 @@ bool AudioFlinger::DirectOutputThread::checkForNewParameters_l()
    return reconfig;
    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)
AudioFlinger::DuplicatingThread::DuplicatingThread(const sp<AudioFlinger>& audioFlinger, AudioFlinger::MixerThread* mainThread)
@@ -1877,13 +1890,15 @@ AudioFlinger::DuplicatingThread::~DuplicatingThread()


bool AudioFlinger::DuplicatingThread::threadLoop()
bool AudioFlinger::DuplicatingThread::threadLoop()
{
{
    unsigned long sleepTime = kBufferRecoveryInUsecs;
    uint32_t sleepTime = 0;
    uint32_t maxBufferRecoveryInUsecs = getMaxBufferRecoveryInUsecs();
    int16_t* curBuf = mMixBuffer;
    int16_t* curBuf = mMixBuffer;
    Vector< sp<Track> > tracksToRemove;
    Vector< sp<Track> > tracksToRemove;
    size_t enabledTracks = 0;
    size_t enabledTracks = 0;
    nsecs_t standbyTime = systemTime();
    nsecs_t standbyTime = systemTime();
    size_t mixBufferSize = mFrameCount*mFrameSize;
    size_t mixBufferSize = mFrameCount*mFrameSize;
    SortedVector< sp<OutputTrack> > outputTracks;
    SortedVector< sp<OutputTrack> > outputTracks;
    uint32_t writeFrames = 0;


    while (!exitPending())
    while (!exitPending())
    {
    {
@@ -1896,6 +1911,7 @@ bool AudioFlinger::DuplicatingThread::threadLoop()


            if (checkForNewParameters_l()) {
            if (checkForNewParameters_l()) {
                mixBufferSize = mFrameCount*mFrameSize;
                mixBufferSize = mFrameCount*mFrameSize;
                maxBufferRecoveryInUsecs = getMaxBufferRecoveryInUsecs();
            }
            }


            const SortedVector< wp<Track> >& activeTracks = mActiveTracks;
            const SortedVector< wp<Track> >& activeTracks = mActiveTracks;
@@ -1935,7 +1951,6 @@ bool AudioFlinger::DuplicatingThread::threadLoop()
                    }
                    }


                    standbyTime = systemTime() + kStandbyTimeInNsecs;
                    standbyTime = systemTime() + kStandbyTimeInNsecs;
                    sleepTime = kBufferRecoveryInUsecs;
                    continue;
                    continue;
                }
                }
            }
            }
@@ -1947,29 +1962,30 @@ bool AudioFlinger::DuplicatingThread::threadLoop()
            // mix buffers...
            // mix buffers...
            mAudioMixer->process(curBuf);
            mAudioMixer->process(curBuf);
            sleepTime = 0;
            sleepTime = 0;
            standbyTime = systemTime() + kStandbyTimeInNsecs;
            writeFrames = mFrameCount;
        } else {
        } else {
            sleepTime += kBufferRecoveryInUsecs;
            if (sleepTime == 0) {
            if (sleepTime > kMaxBufferRecoveryInUsecs) {
                sleepTime = maxBufferRecoveryInUsecs;
                sleepTime = kMaxBufferRecoveryInUsecs;
            } else if (mBytesWritten != 0) {
            }
                // flush remaining overflow buffers in output tracks
            // There was nothing to mix this round, which means all
                for (size_t i = 0; i < outputTracks.size(); i++) {
            // active tracks were late. Sleep a little bit to give
                    if (outputTracks[i]->isActive()) {
            // 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;
                        sleepTime = 0;
                        writeFrames = 0;
                        break;
                    }
                }
            }
            }
        }
        }


        if (mSuspended) {
        if (mSuspended) {
            sleepTime = kMaxBufferRecoveryInUsecs;
            sleepTime = maxBufferRecoveryInUsecs;
        }
        }
        // sleepTime == 0 means we must write to audio hardware
        // sleepTime == 0 means we must write to audio hardware
        if (sleepTime == 0) {
        if (sleepTime == 0) {
            standbyTime = systemTime() + kStandbyTimeInNsecs;
            for (size_t i = 0; i < outputTracks.size(); i++) {
            for (size_t i = 0; i < outputTracks.size(); i++) {
                outputTracks[i]->write(curBuf, mFrameCount);
                outputTracks[i]->write(curBuf, writeFrames);
            }
            }
            mStandby = false;
            mStandby = false;
            mBytesWritten += mixBufferSize;
            mBytesWritten += mixBufferSize;
@@ -2026,7 +2042,6 @@ void AudioFlinger::DuplicatingThread::removeOutputTrack(MixerThread *thread)
    LOGV("removeOutputTrack(): unkonwn thread: %p", thread);
    LOGV("removeOutputTrack(): unkonwn thread: %p", thread);
}
}



// ----------------------------------------------------------------------------
// ----------------------------------------------------------------------------


// TrackBase constructor must be called with AudioFlinger::mLock held
// TrackBase constructor must be called with AudioFlinger::mLock held
@@ -2300,7 +2315,7 @@ status_t AudioFlinger::PlaybackThread::Track::getNextBuffer(AudioBufferProvider:
getNextBuffer_exit:
getNextBuffer_exit:
     buffer->raw = 0;
     buffer->raw = 0;
     buffer->frameCount = 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;
     return NOT_ENOUGH_DATA;
}
}


@@ -2341,7 +2356,7 @@ void AudioFlinger::PlaybackThread::Track::stop()
            if (playbackThread->mActiveTracks.indexOf(this) < 0) {
            if (playbackThread->mActiveTracks.indexOf(this) < 0) {
                reset();
                reset();
            }
            }
            LOGV("(> STOPPED) => STOPPED (%d)", mName);
            LOGV("(> STOPPED) => STOPPED (%d) on thread %p", mName, playbackThread);
        }
        }
    }
    }
}
}
@@ -2354,7 +2369,7 @@ void AudioFlinger::PlaybackThread::Track::pause()
        Mutex::Autolock _l(thread->mLock);
        Mutex::Autolock _l(thread->mLock);
        if (mState == ACTIVE || mState == RESUMING) {
        if (mState == ACTIVE || mState == RESUMING) {
            mState = PAUSING;
            mState = PAUSING;
            LOGV("ACTIVE/RESUMING => PAUSING (%d)", mName);
            LOGV("ACTIVE/RESUMING => PAUSING (%d) on thread %p", mName, thread.get());
        }
        }
    }
    }
}
}
@@ -2566,7 +2581,7 @@ bool AudioFlinger::PlaybackThread::OutputTrack::write(int16_t* data, uint32_t fr


    uint32_t waitTimeLeftMs = mWaitTimeMs;
    uint32_t waitTimeLeftMs = mWaitTimeMs;


    if (!mActive) {
    if (!mActive && frames != 0) {
        start();
        start();
        sp<ThreadBase> thread = mThread.promote();
        sp<ThreadBase> thread = mThread.promote();
        if (thread != 0) {
        if (thread != 0) {
@@ -2608,7 +2623,7 @@ bool AudioFlinger::PlaybackThread::OutputTrack::write(int16_t* data, uint32_t fr
                break;
                break;
            }
            }
            uint32_t waitTimeMs = (uint32_t)ns2ms(systemTime() - startTime);
            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) {
            if (waitTimeLeftMs >= waitTimeMs) {
                waitTimeLeftMs -= waitTimeMs;
                waitTimeLeftMs -= waitTimeMs;
            } else {
            } else {
@@ -2663,7 +2678,7 @@ bool AudioFlinger::PlaybackThread::OutputTrack::write(int16_t* data, uint32_t fr
            pInBuffer->i16 = pInBuffer->mBuffer;
            pInBuffer->i16 = pInBuffer->mBuffer;
            memset(pInBuffer->raw, 0, frames * channels * sizeof(int16_t));
            memset(pInBuffer->raw, 0, frames * channels * sizeof(int16_t));
            mBufferQueue.add(pInBuffer);
            mBufferQueue.add(pInBuffer);
        } else {
        } else if (mActive) {
            stop();
            stop();
        }
        }
    }
    }
+12 −5
Original line number Original line Diff line number Diff line
@@ -524,6 +524,10 @@ private:
        bool                            mMasterMute;
        bool                            mMasterMute;
        SortedVector< wp<Track> >       mActiveTracks;
        SortedVector< wp<Track> >       mActiveTracks;


        virtual int             getTrackName_l() = 0;
        virtual void            deleteTrackName_l(int name) = 0;
        virtual uint32_t        getMaxBufferRecoveryInUsecs() = 0;

    private:
    private:


        friend class AudioFlinger;
        friend class AudioFlinger;
@@ -539,8 +543,7 @@ private:


        status_t    addTrack_l(const sp<Track>& track);
        status_t    addTrack_l(const sp<Track>& track);
        void        destroyTrack_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();
        void        readOutputParameters();


        virtual status_t    dumpInternals(int fd, const Vector<String16>& args);
        virtual status_t    dumpInternals(int fd, const Vector<String16>& args);
@@ -571,13 +574,14 @@ private:
                                      int streamType);
                                      int streamType);
                    void        putTracks(SortedVector < sp<Track> >& tracks,
                    void        putTracks(SortedVector < sp<Track> >& tracks,
                                      SortedVector < wp<Track> >& activeTracks);
                                      SortedVector < wp<Track> >& activeTracks);
        virtual     int         getTrackName_l();
        virtual     void        deleteTrackName_l(int name);
        virtual     bool        checkForNewParameters_l();
        virtual     bool        checkForNewParameters_l();
        virtual     status_t    dumpInternals(int fd, const Vector<String16>& args);
        virtual     status_t    dumpInternals(int fd, const Vector<String16>& args);


    protected:
    protected:
        size_t prepareTracks_l(const SortedVector< wp<Track> >& activeTracks, Vector< sp<Track> > *tracksToRemove);
        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;
        AudioMixer*                     mAudioMixer;
    };
    };
@@ -591,9 +595,12 @@ private:
        // Thread virtuals
        // Thread virtuals
        virtual     bool        threadLoop();
        virtual     bool        threadLoop();


        virtual     bool        checkForNewParameters_l();

    protected:
        virtual     int         getTrackName_l();
        virtual     int         getTrackName_l();
        virtual     void        deleteTrackName_l(int name);
        virtual     void        deleteTrackName_l(int name);
        virtual     bool        checkForNewParameters_l();
        virtual     uint32_t    getMaxBufferRecoveryInUsecs();


    private:
    private:
        float mLeftVolume;
        float mLeftVolume;