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>
diff --git a/media/libaudioclient/AudioRecord.cpp b/media/libaudioclient/AudioRecord.cpp
index 72a23e3..8afb1cc 100644
--- a/media/libaudioclient/AudioRecord.cpp
+++ b/media/libaudioclient/AudioRecord.cpp
@@ -355,7 +355,10 @@
}
// 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 @@
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);
}
diff --git a/media/libaudioclient/AudioSystem.cpp b/media/libaudioclient/AudioSystem.cpp
index b83a441e..33c2008 100644
--- a/media/libaudioclient/AudioSystem.cpp
+++ b/media/libaudioclient/AudioSystem.cpp
@@ -522,8 +522,9 @@
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 @@
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 @@
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 @@
} 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 @@
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 @@
}
}
callbacks.add(callback);
-
+ callbacks.resetNotifiedOnce();
mAudioDeviceCallbacks.replaceValueFor(audioIo, callbacks);
return NO_ERROR;
}
@@ -692,12 +707,12 @@
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++) {
diff --git a/media/libaudioclient/AudioTrack.cpp b/media/libaudioclient/AudioTrack.cpp
index 79abea0..b5a7ebe 100644
--- a/media/libaudioclient/AudioTrack.cpp
+++ b/media/libaudioclient/AudioTrack.cpp
@@ -621,8 +621,10 @@
}
// 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 @@
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);
}
diff --git a/media/libaudioclient/include/media/AudioRecord.h b/media/libaudioclient/include/media/AudioRecord.h
index 1f71844..4707c4a 100644
--- a/media/libaudioclient/include/media/AudioRecord.h
+++ b/media/libaudioclient/include/media/AudioRecord.h
@@ -677,7 +677,7 @@
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;
diff --git a/media/libaudioclient/include/media/AudioSystem.h b/media/libaudioclient/include/media/AudioSystem.h
index 87a9919..74b994e 100644
--- a/media/libaudioclient/include/media/AudioSystem.h
+++ b/media/libaudioclient/include/media/AudioSystem.h
@@ -398,6 +398,15 @@
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:
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;
diff --git a/media/libaudioclient/include/media/AudioTrack.h b/media/libaudioclient/include/media/AudioTrack.h
index cbb750f..12f5d71 100644
--- a/media/libaudioclient/include/media/AudioTrack.h
+++ b/media/libaudioclient/include/media/AudioTrack.h
@@ -1021,7 +1021,7 @@
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;