libaudioclient: force onAudioDeviceUpdate on registration

This CL forces the onAudioDeviceUpdate on register event.
It also fixes the loss of callback on AudioTrack or AudioRecord
client if received before ioHandle is assigned.

Test: audio smoke tests
Test: CTS for AudioTrack and AudioRouting

Change-Id: I119b5c407da68a5b55162550bea5fa7e724165d1
Signed-off-by: Francois Gaffie <francois.gaffie@renault.com>

media/libaudioclient/AudioRecord.cpp

@@ -355,7 +355,10 @@ status_t AudioRecord::set(
     }
 
     // create the IAudioRecord
-    status = createRecord_l(0 /*epoch*/, mOpPackageName);
+    {
+        AutoMutex lock(mLock);
+        status = createRecord_l(0 /*epoch*/, mOpPackageName);
+    }
 
     ALOGV("%s(%d): status %d", __func__, mPortId, status);
 
@@ -1358,12 +1361,14 @@ status_t AudioRecord::removeAudioDeviceCallback(
         ALOGW("%s(%d): removing NULL callback!", __func__, mPortId);
         return BAD_VALUE;
     }
-    AutoMutex lock(mLock);
-    if (mDeviceCallback.unsafe_get() != callback.get()) {
-        ALOGW("%s(%d): removing different callback!", __func__, mPortId);
-        return INVALID_OPERATION;
+    {
+        AutoMutex lock(mLock);
+        if (mDeviceCallback.unsafe_get() != callback.get()) {
+            ALOGW("%s(%d): removing different callback!", __func__, mPortId);
+            return INVALID_OPERATION;
+        }
+        mDeviceCallback.clear();
     }
-    mDeviceCallback.clear();
     if (mInput != AUDIO_IO_HANDLE_NONE) {
         AudioSystem::removeAudioDeviceCallback(this, mInput);
     }

media/libaudioclient/AudioSystem.cpp

@@ -522,8 +522,9 @@ void AudioSystem::AudioFlingerClient::ioConfigChanged(audio_io_config_event even
     if (ioDesc == 0 || ioDesc->mIoHandle == AUDIO_IO_HANDLE_NONE) return;
 
     audio_port_handle_t deviceId = AUDIO_PORT_HANDLE_NONE;
-    Vector < wp<AudioDeviceCallback> > callbacks;
-
+    AudioDeviceCallbacks callbacks;
+    bool deviceValidOrChanged = false;
+    Mutex::Autolock _l(mCallbacksLock);
     {
         Mutex::Autolock _l(mLock);
 
@@ -546,6 +547,13 @@ void AudioSystem::AudioFlingerClient::ioConfigChanged(audio_io_config_event even
                     ssize_t ioIndex = mAudioDeviceCallbacks.indexOfKey(ioDesc->mIoHandle);
                     if (ioIndex >= 0) {
                         callbacks = mAudioDeviceCallbacks.valueAt(ioIndex);
+                        deviceValidOrChanged = true;
+                    }
+                }
+                if (event == AUDIO_OUTPUT_REGISTERED || event == AUDIO_INPUT_REGISTERED) {
+                    ssize_t ioIndex = mAudioDeviceCallbacks.indexOfKey(ioDesc->mIoHandle);
+                    if ((ioIndex >= 0) && !mAudioDeviceCallbacks.valueAt(ioIndex).notifiedOnce()) {
+                        callbacks = mAudioDeviceCallbacks.valueAt(ioIndex);
                     }
                 }
             }
@@ -584,6 +592,7 @@ void AudioSystem::AudioFlingerClient::ioConfigChanged(audio_io_config_event even
             mIoDescriptors.replaceValueFor(ioDesc->mIoHandle, ioDesc);
 
             if (deviceId != ioDesc->getDeviceId()) {
+                deviceValidOrChanged = true;
                 deviceId = ioDesc->getDeviceId();
                 ssize_t ioIndex = mAudioDeviceCallbacks.indexOfKey(ioDesc->mIoHandle);
                 if (ioIndex >= 0) {
@@ -600,22 +609,28 @@ void AudioSystem::AudioFlingerClient::ioConfigChanged(audio_io_config_event even
         } break;
         }
     }
-    bool callbackRemoved = false;
     // callbacks.size() != 0 =>  ioDesc->mIoHandle and deviceId are valid
-    for (size_t i = 0; i < callbacks.size(); ) {
-        sp<AudioDeviceCallback> callback = callbacks[i].promote();
-        if (callback.get() != nullptr) {
-            callback->onAudioDeviceUpdate(ioDesc->mIoHandle, deviceId);
-            i++;
-        } else {
-            callbacks.removeAt(i);
-            callbackRemoved = true;
+    if (callbacks.size() != 0) {
+        for (size_t i = 0; i < callbacks.size(); ) {
+            sp<AudioDeviceCallback> callback = callbacks[i].promote();
+            if (callback.get() != nullptr) {
+                // Call the callback only if the device actually changed, the input or output was
+                // opened or closed or the client was newly registered and the callback was never
+                // called
+                if (!callback->notifiedOnce() || deviceValidOrChanged) {
+                    // Must be called without mLock held. May lead to dead lock if calling for
+                    // example getRoutedDevice that updates the device and tries to acquire mLock.
+                    callback->onAudioDeviceUpdate(ioDesc->mIoHandle, deviceId);
+                    callback->setNotifiedOnce();
+                }
+                i++;
+            } else {
+                callbacks.removeAt(i);
+            }
         }
-    }
-    // clean up callback list while we are here if some clients have disappeared without
-    // unregistering their callback
-    if (callbackRemoved) {
-        Mutex::Autolock _l(mLock);
+        callbacks.setNotifiedOnce();
+        // clean up callback list while we are here if some clients have disappeared without
+        // unregistering their callback, or if cb was served for the first time since registered
         mAudioDeviceCallbacks.replaceValueFor(ioDesc->mIoHandle, callbacks);
     }
 }
@@ -671,8 +686,8 @@ sp<AudioIoDescriptor> AudioSystem::AudioFlingerClient::getIoDescriptor(audio_io_
 status_t AudioSystem::AudioFlingerClient::addAudioDeviceCallback(
         const wp<AudioDeviceCallback>& callback, audio_io_handle_t audioIo)
 {
-    Mutex::Autolock _l(mLock);
-    Vector < wp<AudioDeviceCallback> > callbacks;
+    Mutex::Autolock _l(mCallbacksLock);
+    AudioDeviceCallbacks callbacks;
     ssize_t ioIndex = mAudioDeviceCallbacks.indexOfKey(audioIo);
     if (ioIndex >= 0) {
         callbacks = mAudioDeviceCallbacks.valueAt(ioIndex);
@@ -684,7 +699,7 @@ status_t AudioSystem::AudioFlingerClient::addAudioDeviceCallback(
         }
     }
     callbacks.add(callback);
-
+    callbacks.resetNotifiedOnce();
     mAudioDeviceCallbacks.replaceValueFor(audioIo, callbacks);
     return NO_ERROR;
 }
@@ -692,12 +707,12 @@ status_t AudioSystem::AudioFlingerClient::addAudioDeviceCallback(
 status_t AudioSystem::AudioFlingerClient::removeAudioDeviceCallback(
         const wp<AudioDeviceCallback>& callback, audio_io_handle_t audioIo)
 {
-    Mutex::Autolock _l(mLock);
+    Mutex::Autolock _l(mCallbacksLock);
     ssize_t ioIndex = mAudioDeviceCallbacks.indexOfKey(audioIo);
     if (ioIndex < 0) {
         return INVALID_OPERATION;
     }
-    Vector < wp<AudioDeviceCallback> > callbacks = mAudioDeviceCallbacks.valueAt(ioIndex);
+    AudioDeviceCallbacks callbacks = mAudioDeviceCallbacks.valueAt(ioIndex);
 
     size_t cbIndex;
     for (cbIndex = 0; cbIndex < callbacks.size(); cbIndex++) {

media/libaudioclient/AudioTrack.cpp

@@ -621,8 +621,10 @@ status_t AudioTrack::set(
     }
 
     // create the IAudioTrack
-    status = createTrack_l();
-
+    {
+        AutoMutex lock(mLock);
+        status = createTrack_l();
+    }
     if (status != NO_ERROR) {
         if (mAudioTrackThread != 0) {
             mAudioTrackThread->requestExit();   // see comment in AudioTrack.h
@@ -2957,12 +2959,14 @@ status_t AudioTrack::removeAudioDeviceCallback(
         ALOGW("%s(%d): removing NULL callback!", __func__, mPortId);
         return BAD_VALUE;
     }
-    AutoMutex lock(mLock);
-    if (mDeviceCallback.unsafe_get() != callback.get()) {
-        ALOGW("%s(%d): removing different callback!", __func__, mPortId);
-        return INVALID_OPERATION;
+    {
+        AutoMutex lock(mLock);
+        if (mDeviceCallback.unsafe_get() != callback.get()) {
+            ALOGW("%s removing different callback!", __FUNCTION__);
+            return INVALID_OPERATION;
+        }
+        mDeviceCallback.clear();
     }
-    mDeviceCallback.clear();
     if (mOutput != AUDIO_IO_HANDLE_NONE) {
         AudioSystem::removeAudioDeviceCallback(this, mOutput);
     }

media/libaudioclient/include/media/AudioRecord.h

@@ -677,7 +677,7 @@ private:
     sp<IMemory>             mCblkMemory;
     audio_track_cblk_t*     mCblk;              // re-load after mLock.unlock()
     sp<IMemory>             mBufferMemory;
-    audio_io_handle_t       mInput;             // returned by AudioSystem::getInput()
+    audio_io_handle_t       mInput = AUDIO_IO_HANDLE_NONE; // from AudioSystem::getInputforAttr()
 
     int                     mPreviousPriority;  // before start()
     SchedPolicy             mPreviousSchedulingGroup;

media/libaudioclient/include/media/AudioSystem.h

@@ -398,6 +398,15 @@ public:
 
         virtual void onAudioDeviceUpdate(audio_io_handle_t audioIo,
                                          audio_port_handle_t deviceId) = 0;
+                bool notifiedOnce() const { return mNotifiedOnce; }
+                void setNotifiedOnce() { mNotifiedOnce = true; }
+    private:
+                /**
+                 * @brief mNotifiedOnce it forces the callback to be called at least once when
+                 * registered with a VALID AudioDevice, and allows not to flood other listeners
+                 * on this iohandle that already know the valid device.
+                 */
+                bool mNotifiedOnce = false;
     };
 
     static status_t addAudioDeviceCallback(const wp<AudioDeviceCallback>& callback,
@@ -443,8 +452,27 @@ private:
     private:
         Mutex                               mLock;
         DefaultKeyedVector<audio_io_handle_t, sp<AudioIoDescriptor> >   mIoDescriptors;
-        DefaultKeyedVector<audio_io_handle_t, Vector < wp<AudioDeviceCallback> > >
-                                                                        mAudioDeviceCallbacks;
+
+        class AudioDeviceCallbacks : public Vector<wp<AudioDeviceCallback>>
+        {
+        public:
+            /**
+             * @brief notifiedOnce ensures that if a client adds a callback, it must at least be
+             * called once with the device on which it will be routed to.
+             * @return true if already notified or nobody waits for a callback, false otherwise.
+             */
+            bool notifiedOnce() const { return (size() == 0) || mNotifiedOnce; }
+            void setNotifiedOnce() { mNotifiedOnce = true; }
+            void resetNotifiedOnce() { mNotifiedOnce = false; }
+        private:
+            /**
+             * @brief mNotifiedOnce it forces each callback to be called at least once when
+             * registered with a VALID AudioDevice
+             */
+            bool mNotifiedOnce = false;
+        };
+        Mutex                               mCallbacksLock; // prevents race on Callbacks
+        DefaultKeyedVector<audio_io_handle_t, AudioDeviceCallbacks> mAudioDeviceCallbacks;
         // cached values for recording getInputBufferSize() queries
         size_t                              mInBuffSize;    // zero indicates cache is invalid
         uint32_t                            mInSamplingRate;

media/libaudioclient/include/media/AudioTrack.h

@@ -1021,7 +1021,7 @@ public:
     sp<IAudioTrack>         mAudioTrack;
     sp<IMemory>             mCblkMemory;
     audio_track_cblk_t*     mCblk;                  // re-load after mLock.unlock()
-    audio_io_handle_t       mOutput;                // returned by AudioSystem::getOutputForAttr()
+    audio_io_handle_t       mOutput = AUDIO_IO_HANDLE_NONE; // from AudioSystem::getOutputForAttr()
 
     sp<AudioTrackThread>    mAudioTrackThread;
     bool                    mThreadCanCallJava;