Fix issue 2553359: Pandora does not work well with Passion deskdock / Cardock.

The problem is due to a too big difference between the buffer size used at the hardware interface and at the A2DP interface.
When no resampling occurs we don't notice problems but the timing is very tight. As soon as resampling is activated, the AudioTrack underruns.
This is because the AudioTrack buffers are not resized when moving the AudioTrack from hardware to A2DP output.
The AudioTrack buffers are calculated based on a hardware output buffer size of 3072 bytes. Which is much less than the A2DP output buffer size (10240).

The solution consists in creating new tracks with new buffers in AudioFlinger when the A2DP output is opened
instead of just transfering active tracks from hardware output mixer thread to the new A2DP output mixer thread.
To avoid synchronization issues between mixer threads and client processes, this is done by invalidating tracks
by setting a flag in their control block and having AudioTrack release the handle on this track (IAudioTrack)
and create a new IAudioTrack when this flag is detected next time obtainBuffer() or start() is executed.

AudioFlinger modifications:
- invalidate the tracks when setStreamOutput() is called
- make sure that notifications of output opening/closing and change of stream type to output mapping are sent synchronously to client process.
This is necessary so that AudioSystem has the new stream to output mapping when the AudioTrack detects the invalidate flag in the client process.
Previously their were sent when the corresponding thread loop was executed.

AudioTrack modifications:
- move frame count calculation and verification from set() to createTrack() so that is is updated every time a new IAudioTrack is created.
- detect track invalidate flag in obtainBuffer() and start() and create a new IAudioTrack.

AudioTrackShared modifications
- group all flags (out, flowControlFlag, forceReady...) into a single bit filed to save space.

Change-Id: I9ac26b6192230627d35084e1449640caaf7d56ee

diff --git a/libs/audioflinger/AudioFlinger.cpp b/libs/audioflinger/AudioFlinger.cpp
index 58eb590..3b38d83 100644 (file)
--- a/libs/audioflinger/AudioFlinger.cpp
+++ b/libs/audioflinger/AudioFlinger.cpp
@@ -873,11 +873,12 @@ void AudioFlinger::ThreadBase::processConfigEvents()
         LOGV("processConfigEvents() remaining events %d", mConfigEvents.size());
         ConfigEvent *configEvent = mConfigEvents[0];
         mConfigEvents.removeAt(0);
-        // release mLock because audioConfigChanged() will lock AudioFlinger mLock
-        // before calling Audioflinger::audioConfigChanged_l() thus creating
-        // potential cross deadlock between AudioFlinger::mLock and mLock
+        // release mLock before locking AudioFlinger mLock: lock order is always
+        // AudioFlinger then ThreadBase to avoid cross deadlock
         mLock.unlock();
-        audioConfigChanged(configEvent->mEvent, configEvent->mParam);
+        mAudioFlinger->mLock.lock();
+        audioConfigChanged_l(configEvent->mEvent, configEvent->mParam);
+        mAudioFlinger->mLock.unlock();
         delete configEvent;
         mLock.lock();
     }
@@ -953,8 +954,6 @@ AudioFlinger::PlaybackThread::PlaybackThread(const sp<AudioFlinger>& audioFlinge
         mStreamTypes[stream].volume = mAudioFlinger->streamVolumeInternal(stream);
         mStreamTypes[stream].mute = mAudioFlinger->streamMute(stream);
     }
-    // notify client processes that a new input has been opened
-    sendConfigEvent(AudioSystem::OUTPUT_OPENED);
 }
 
 AudioFlinger::PlaybackThread::~PlaybackThread()
@@ -1234,11 +1233,12 @@ String8 AudioFlinger::PlaybackThread::getParameters(const String8& keys)
     return mOutput->getParameters(keys);
 }
 
-void AudioFlinger::PlaybackThread::audioConfigChanged(int event, int param) {
+// destroyTrack_l() must be called with AudioFlinger::mLock held
+void AudioFlinger::PlaybackThread::audioConfigChanged_l(int event, int param) {
     AudioSystem::OutputDescriptor desc;
     void *param2 = 0;
 
-    LOGV("PlaybackThread::audioConfigChanged, thread %p, event %d, param %d", this, event, param);
+    LOGV("PlaybackThread::audioConfigChanged_l, thread %p, event %d, param %d", this, event, param);
 
     switch (event) {
     case AudioSystem::OUTPUT_OPENED:
@@ -1257,7 +1257,6 @@ void AudioFlinger::PlaybackThread::audioConfigChanged(int event, int param) {
     default:
         break;
     }
-    Mutex::Autolock _l(mAudioFlinger->mLock);
     mAudioFlinger->audioConfigChanged_l(event, mId, param2);
 }
 
@@ -1614,66 +1613,22 @@ uint32_t AudioFlinger::MixerThread::prepareTracks_l(const SortedVector< wp<Track
     return mixerStatus;
 }
 
-void AudioFlinger::MixerThread::getTracks(
-        SortedVector < sp<Track> >& tracks,
-        SortedVector < wp<Track> >& activeTracks,
-        int streamType)
+void AudioFlinger::MixerThread::invalidateTracks(int streamType)
 {
-    LOGV ("MixerThread::getTracks() mixer %p, mTracks.size %d, mActiveTracks.size %d", this,  mTracks.size(), mActiveTracks.size());
+    LOGV ("MixerThread::invalidateTracks() mixer %p, streamType %d, mTracks.size %d", this,  streamType, mTracks.size());
     Mutex::Autolock _l(mLock);
     size_t size = mTracks.size();
     for (size_t i = 0; i < size; i++) {
         sp<Track> t = mTracks[i];
         if (t->type() == streamType) {
-            tracks.add(t);
-            int j = mActiveTracks.indexOf(t);
-            if (j >= 0) {
-                t = mActiveTracks[j].promote();
-                if (t != NULL) {
-                    activeTracks.add(t);
+            t->mCblk->lock.lock();
+            t->mCblk->flags |= CBLK_INVALID_ON;
+            t->mCblk->cv.signal();
+            t->mCblk->lock.unlock();
                 }
             }
         }
-    }
-
-    size = activeTracks.size();
-    for (size_t i = 0; i < size; i++) {
-        mActiveTracks.remove(activeTracks[i]);
-    }
-
-    size = tracks.size();
-    for (size_t i = 0; i < size; i++) {
-        sp<Track> t = tracks[i];
-        mTracks.remove(t);
-        deleteTrackName_l(t->name());
-    }
-}
-
-void AudioFlinger::MixerThread::putTracks(
-        SortedVector < sp<Track> >& tracks,
-        SortedVector < wp<Track> >& activeTracks)
-{
-    LOGV ("MixerThread::putTracks() mixer %p, tracks.size %d, activeTracks.size %d", this,  tracks.size(), activeTracks.size());
-    Mutex::Autolock _l(mLock);
-    size_t size = tracks.size();
-    for (size_t i = 0; i < size ; i++) {
-        sp<Track> t = tracks[i];
-        int name = getTrackName_l();
-
-        if (name < 0) return;
 
-        t->mName = name;
-        t->mThread = this;
-        mTracks.add(t);
-
-        int j = activeTracks.indexOf(t);
-        if (j >= 0) {
-            mActiveTracks.add(t);
-            // force buffer refilling and no ramp volume when the track is mixed for the first time
-            t->mFillingUpStatus = Track::FS_FILLING;
-        }
-    }
-}
 
 // getTrackName_l() must be called with ThreadBase::mLock held
 int AudioFlinger::MixerThread::getTrackName_l()
@@ -2348,7 +2303,7 @@ AudioFlinger::ThreadBase::TrackBase::TrackBase(
                     memset(mBuffer, 0, frameCount*channelCount*sizeof(int16_t));
                     // Force underrun condition to avoid false underrun callback until first data is
                     // written to buffer
-                    mCblk->flowControlFlag = 1;
+                    mCblk->flags = CBLK_UNDERRUN_ON;
                 } else {
                     mBuffer = sharedBuffer->pointer();
                 }
@@ -2371,7 +2326,7 @@ AudioFlinger::ThreadBase::TrackBase::TrackBase(
            memset(mBuffer, 0, frameCount*channelCount*sizeof(int16_t));
            // Force underrun condition to avoid false underrun callback until first data is
            // written to buffer
-           mCblk->flowControlFlag = 1;
+           mCblk->flags = CBLK_UNDERRUN_ON;
            mBufferEnd = (uint8_t *)mBuffer + bufferSize;
        }
    }
@@ -2589,9 +2544,9 @@ bool AudioFlinger::PlaybackThread::Track::isReady() const {
     if (mFillingUpStatus != FS_FILLING) return true;
 
     if (mCblk->framesReady() >= mCblk->frameCount ||
-        mCblk->forceReady) {
+            (mCblk->flags & CBLK_FORCEREADY_MSK)) {
         mFillingUpStatus = FS_FILLED;
-        mCblk->forceReady = 0;
+        mCblk->flags &= ~CBLK_FORCEREADY_MSK;
         return true;
     }
     return false;
@@ -2706,8 +2661,8 @@ void AudioFlinger::PlaybackThread::Track::reset()
         TrackBase::reset();
         // Force underrun condition to avoid false underrun callback until first data is
         // written to buffer
-        mCblk->flowControlFlag = 1;
-        mCblk->forceReady = 0;
+        mCblk->flags |= CBLK_UNDERRUN_ON;
+        mCblk->flags &= ~CBLK_FORCEREADY_MSK;
         mFillingUpStatus = FS_FILLING;
         mResetDone = true;
     }
@@ -2818,7 +2773,7 @@ void AudioFlinger::RecordThread::RecordTrack::stop()
         TrackBase::reset();
         // Force overerrun condition to avoid false overrun callback until first data is
         // read from buffer
-        mCblk->flowControlFlag = 1;
+        mCblk->flags |= CBLK_UNDERRUN_ON;
     }
 }
 
@@ -2851,7 +2806,7 @@ AudioFlinger::PlaybackThread::OutputTrack::OutputTrack(
 
     PlaybackThread *playbackThread = (PlaybackThread *)thread.unsafe_get();
     if (mCblk != NULL) {
-        mCblk->out = 1;
+        mCblk->flags |= CBLK_DIRECTION_OUT;
         mCblk->buffers = (char*)mCblk + sizeof(audio_track_cblk_t);
         mCblk->volume[0] = mCblk->volume[1] = 0x1000;
         mOutBuffer.frameCount = 0;
@@ -3274,7 +3229,6 @@ AudioFlinger::RecordThread::RecordThread(const sp<AudioFlinger>& audioFlinger, A
     mReqChannelCount = AudioSystem::popCount(channels);
     mReqSampleRate = sampleRate;
     readInputParameters();
-    sendConfigEvent(AudioSystem::INPUT_OPENED);
 }
 
 
@@ -3689,7 +3643,7 @@ String8 AudioFlinger::RecordThread::getParameters(const String8& keys)
     return mInput->getParameters(keys);
 }
 
-void AudioFlinger::RecordThread::audioConfigChanged(int event, int param) {
+void AudioFlinger::RecordThread::audioConfigChanged_l(int event, int param) {
     AudioSystem::OutputDescriptor desc;
     void *param2 = 0;
 
@@ -3708,7 +3662,6 @@ void AudioFlinger::RecordThread::audioConfigChanged(int event, int param) {
     default:
         break;
     }
-    Mutex::Autolock _l(mAudioFlinger->mLock);
     mAudioFlinger->audioConfigChanged_l(event, mId, param2);
 }
 
@@ -3828,6 +3781,8 @@ int AudioFlinger::openOutput(uint32_t *pDevices,
         if (pChannels) *pChannels = channels;
         if (pLatencyMs) *pLatencyMs = thread->latency();
 
+        // notify client processes of the new output creation
+        thread->audioConfigChanged_l(AudioSystem::OUTPUT_OPENED);
         return mNextThreadId;
     }
 
@@ -3849,6 +3804,8 @@ int AudioFlinger::openDuplicateOutput(int output1, int output2)
     DuplicatingThread *thread = new DuplicatingThread(this, thread1, ++mNextThreadId);
     thread->addOutputTrack(thread2);
     mPlaybackThreads.add(mNextThreadId, thread);
+    // notify client processes of the new output creation
+    thread->audioConfigChanged_l(AudioSystem::OUTPUT_OPENED);
     return mNextThreadId;
 }
 
@@ -3978,6 +3935,8 @@ int AudioFlinger::openInput(uint32_t *pDevices,
 
         input->standby();
 
+        // notify client processes of the new input creation
+        thread->audioConfigChanged_l(AudioSystem::INPUT_OPENED);
         return mNextThreadId;
     }
 
@@ -4018,22 +3977,16 @@ status_t AudioFlinger::setStreamOutput(uint32_t stream, int output)
     }
 
     LOGV("setStreamOutput() stream %d to output %d", stream, output);
+    audioConfigChanged_l(AudioSystem::STREAM_CONFIG_CHANGED, output, &stream);
 
     for (size_t i = 0; i < mPlaybackThreads.size(); i++) {
         PlaybackThread *thread = mPlaybackThreads.valueAt(i).get();
         if (thread != dstThread &&
             thread->type() != PlaybackThread::DIRECT) {
             MixerThread *srcThread = (MixerThread *)thread;
-            SortedVector < sp<MixerThread::Track> > tracks;
-            SortedVector < wp<MixerThread::Track> > activeTracks;
-            srcThread->getTracks(tracks, activeTracks, stream);
-            if (tracks.size()) {
-                dstThread->putTracks(tracks, activeTracks);
+            srcThread->invalidateTracks(stream);
             }
         }
-    }
-
-    dstThread->sendConfigEvent(AudioSystem::STREAM_CONFIG_CHANGED, stream);
 
     return NO_ERROR;
 }

diff --git a/libs/audioflinger/AudioFlinger.h b/libs/audioflinger/AudioFlinger.h
index c4a5305..f35f38b 100644 (file)
--- a/libs/audioflinger/AudioFlinger.h
+++ b/libs/audioflinger/AudioFlinger.h
@@ -342,7 +342,7 @@ private:
         virtual     bool        checkForNewParameters_l() = 0;
         virtual     status_t    setParameters(const String8& keyValuePairs);
         virtual     String8     getParameters(const String8& keys) = 0;
-        virtual     void        audioConfigChanged(int event, int param = 0) = 0;
+        virtual     void        audioConfigChanged_l(int event, int param = 0) = 0;
                     void        sendConfigEvent(int event, int param = 0);
                     void        sendConfigEvent_l(int event, int param = 0);
                     void        processConfigEvents();
@@ -547,7 +547,7 @@ private:
                     void        restore() { if (mSuspended) mSuspended--; }
                     bool        isSuspended() { return (mSuspended != 0); }
         virtual     String8     getParameters(const String8& keys);
-        virtual     void        audioConfigChanged(int event, int param = 0);
+        virtual     void        audioConfigChanged_l(int event, int param = 0);
         virtual     status_t    getRenderPosition(uint32_t *halFrames, uint32_t *dspFrames);
 
         struct  stream_type_t {
@@ -613,11 +613,7 @@ private:
         // Thread virtuals
         virtual     bool        threadLoop();
 
-                    void        getTracks(SortedVector < sp<Track> >& tracks,
-                                      SortedVector < wp<Track> >& activeTracks,
-                                      int streamType);
-                    void        putTracks(SortedVector < sp<Track> >& tracks,
-                                      SortedVector < wp<Track> >& activeTracks);
+                    void        invalidateTracks(int streamType);
         virtual     bool        checkForNewParameters_l();
         virtual     status_t    dumpInternals(int fd, const Vector<String16>& args);
 
@@ -764,7 +760,7 @@ private:
         virtual void        releaseBuffer(AudioBufferProvider::Buffer* buffer);
         virtual bool        checkForNewParameters_l();
         virtual String8     getParameters(const String8& keys);
-        virtual void        audioConfigChanged(int event, int param = 0);
+        virtual void        audioConfigChanged_l(int event, int param = 0);
                 void        readInputParameters();
         virtual unsigned int  getInputFramesLost();
 

diff --git a/libs/audioflinger/AudioPolicyManagerBase.cpp b/libs/audioflinger/AudioPolicyManagerBase.cpp
index c8b3f48..381a958 100644 (file)
--- a/libs/audioflinger/AudioPolicyManagerBase.cpp
+++ b/libs/audioflinger/AudioPolicyManagerBase.cpp
@@ -1249,6 +1249,17 @@ void AudioPolicyManagerBase::closeA2dpOutputs()
     LOGV("setDeviceConnectionState() closing A2DP and duplicated output!");
 
     if (mDuplicatedOutput != 0) {
+        AudioOutputDescriptor *dupOutputDesc = mOutputs.valueFor(mDuplicatedOutput);
+        AudioOutputDescriptor *hwOutputDesc = mOutputs.valueFor(mHardwareOutput);
+        // As all active tracks on duplicated output will be deleted,
+        // and as they were also referenced on hardware output, the reference
+        // count for their stream type must be adjusted accordingly on
+        // hardware output.
+        for (int i = 0; i < (int)AudioSystem::NUM_STREAM_TYPES; i++) {
+            int refCount = dupOutputDesc->mRefCount[i];
+            hwOutputDesc->changeRefCount((AudioSystem::stream_type)i,-refCount);
+        }
+
         mpClientInterface->closeOutput(mDuplicatedOutput);
         delete mOutputs.valueFor(mDuplicatedOutput);
         mOutputs.removeItem(mDuplicatedOutput);
@@ -1288,11 +1299,6 @@ void AudioPolicyManagerBase::checkOutputForStrategy(routing_strategy strategy, u
         for (int i = 0; i < (int)AudioSystem::NUM_STREAM_TYPES; i++) {
             if (getStrategy((AudioSystem::stream_type)i) == strategy) {
                 mpClientInterface->setStreamOutput((AudioSystem::stream_type)i, mHardwareOutput);
-                int refCount = a2dpOutputDesc->mRefCount[i];
-                // in the case of duplicated output, the ref count is first incremented
-                // and then decremented on hardware output tus keeping its value
-                hwOutputDesc->changeRefCount((AudioSystem::stream_type)i, refCount);
-                a2dpOutputDesc->changeRefCount((AudioSystem::stream_type)i,-refCount);
             }
         }
         // do not change newDevice if it was already set before this call by a previous call to
@@ -1318,11 +1324,6 @@ void AudioPolicyManagerBase::checkOutputForStrategy(routing_strategy strategy, u
         for (int i = 0; i < (int)AudioSystem::NUM_STREAM_TYPES; i++) {
             if (getStrategy((AudioSystem::stream_type)i) == strategy) {
                 mpClientInterface->setStreamOutput((AudioSystem::stream_type)i, a2dpOutput);
-                int refCount = hwOutputDesc->mRefCount[i];
-                // in the case of duplicated output, the ref count is first incremented
-                // and then decremented on hardware output tus keeping its value
-                a2dpOutputDesc->changeRefCount((AudioSystem::stream_type)i, refCount);
-                hwOutputDesc->changeRefCount((AudioSystem::stream_type)i,-refCount);
             }
         }
     }