libaudiohal@aidl: Fix callbacks handling
- Ensure that 'StreamOutHalInterface' slice pointer is used
by all parties.
- Handle state changes in StreamHalAidl.
- Pace stream state queries by the buffer duration (for sync
modes) or by a fixed amount (for async) and allow it in
any state and from any thread.
Bug: 300416194
Bug: 331300943
Test: mp3 and ogg offloaded playback (bug repro steps)
Change-Id: I953cfc8862aa466d17f7992df052b7b68fb97e0a
diff --git a/media/libaudiohal/impl/StreamHalAidl.cpp b/media/libaudiohal/impl/StreamHalAidl.cpp
index 2a8ebc6..6138ceb 100644
--- a/media/libaudiohal/impl/StreamHalAidl.cpp
+++ b/media/libaudiohal/impl/StreamHalAidl.cpp
@@ -82,7 +82,12 @@
mConfig(configToBase(config)),
mContext(std::move(context)),
mStream(stream),
- mVendorExt(vext) {
+ mVendorExt(vext),
+ mLastReplyLifeTimeNs(
+ std::min(static_cast<size_t>(100),
+ 2 * mContext.getBufferDurationMs(mConfig.sample_rate))
+ * NANOS_PER_MILLISECOND)
+{
ALOGD("%p %s::%s", this, getClassName().c_str(), __func__);
{
std::lock_guard l(mLock);
@@ -285,8 +290,7 @@
ALOGV("%p %s::%s", this, getClassName().c_str(), __func__);
if (!mStream) return NO_INIT;
StreamDescriptor::Reply reply;
- // TODO: switch to updateCountersIfNeeded once we sort out mWorkerTid initialization
- RETURN_STATUS_IF_ERROR(sendCommand(makeHalCommand<HalCommand::Tag::getStatus>(), &reply, true));
+ RETURN_STATUS_IF_ERROR(updateCountersIfNeeded(&reply));
*frames = std::max<int64_t>(0, reply.hardware.frames);
*timestamp = std::max<int64_t>(0, reply.hardware.timeNs);
return OK;
@@ -409,6 +413,37 @@
return statusTFromBinderStatus(mStream->prepareToClose());
}
+void StreamHalAidl::onAsyncTransferReady() {
+ if (auto state = getState(); state == StreamDescriptor::State::TRANSFERRING) {
+ // Retrieve the current state together with position counters.
+ updateCountersIfNeeded();
+ } else {
+ ALOGW("%s: unexpected onTransferReady in the state %s", __func__, toString(state).c_str());
+ }
+}
+
+void StreamHalAidl::onAsyncDrainReady() {
+ if (auto state = getState(); state == StreamDescriptor::State::DRAINING) {
+ // Retrieve the current state together with position counters.
+ updateCountersIfNeeded();
+ } else {
+ ALOGW("%s: unexpected onDrainReady in the state %s", __func__, toString(state).c_str());
+ }
+}
+
+void StreamHalAidl::onAsyncError() {
+ std::lock_guard l(mLock);
+ if (mLastReply.state == StreamDescriptor::State::IDLE ||
+ mLastReply.state == StreamDescriptor::State::DRAINING ||
+ mLastReply.state == StreamDescriptor::State::TRANSFERRING) {
+ mLastReply.state = StreamDescriptor::State::ERROR;
+ ALOGW("%s: onError received", __func__);
+ } else {
+ ALOGW("%s: unexpected onError in the state %s", __func__,
+ toString(mLastReply.state).c_str());
+ }
+}
+
status_t StreamHalAidl::createMmapBuffer(int32_t minSizeFrames __unused,
struct audio_mmap_buffer_info *info) {
ALOGD("%p %s::%s", this, getClassName().c_str(), __func__);
@@ -487,6 +522,7 @@
reply->latencyMs = mLastReply.latencyMs;
}
mLastReply = *reply;
+ mLastReplyExpirationNs = uptimeNanos() + mLastReplyLifeTimeNs;
}
switch (reply->status) {
case STATUS_OK: return OK;
@@ -502,14 +538,17 @@
status_t StreamHalAidl::updateCountersIfNeeded(
::aidl::android::hardware::audio::core::StreamDescriptor::Reply* reply) {
- if (mWorkerTid.load(std::memory_order_acquire) == gettid()) {
- if (const auto state = getState(); state != StreamDescriptor::State::ACTIVE &&
- state != StreamDescriptor::State::DRAINING &&
- state != StreamDescriptor::State::TRANSFERRING) {
- return sendCommand(makeHalCommand<HalCommand::Tag::getStatus>(), reply);
- }
+ bool doUpdate = false;
+ {
+ std::lock_guard l(mLock);
+ doUpdate = uptimeNanos() > mLastReplyExpirationNs;
}
- if (reply != nullptr) {
+ if (doUpdate) {
+ // Since updates are paced, it is OK to perform them from any thread, they should
+ // not interfere with I/O operations of the worker.
+ return sendCommand(makeHalCommand<HalCommand::Tag::getStatus>(),
+ reply, true /*safeFromNonWorkerThread */);
+ } else if (reply != nullptr) { // provide cached reply
std::lock_guard l(mLock);
*reply = mLastReply;
}
@@ -545,7 +584,7 @@
StreamOutHalAidl::~StreamOutHalAidl() {
if (auto broker = mCallbackBroker.promote(); broker != nullptr) {
- broker->clearCallbacks(this);
+ broker->clearCallbacks(static_cast<StreamOutHalInterface*>(this));
}
}
@@ -602,21 +641,14 @@
}
status_t StreamOutHalAidl::setCallback(wp<StreamOutHalInterfaceCallback> callback) {
+ ALOGD("%p %s", this, __func__);
TIME_CHECK();
if (!mStream) return NO_INIT;
if (!mContext.isAsynchronous()) {
ALOGE("%s: the callback is intended for asynchronous streams only", __func__);
return INVALID_OPERATION;
}
- if (auto broker = mCallbackBroker.promote(); broker != nullptr) {
- if (auto cb = callback.promote(); cb != nullptr) {
- broker->setStreamOutCallback(this, cb);
- } else {
- // It is expected that the framework never passes a null pointer.
- // In the AIDL model callbacks can't be "unregistered".
- LOG_ALWAYS_FATAL("%s: received an expired or null callback pointer", __func__);
- }
- }
+ mClientCallback = callback;
return OK;
}
@@ -739,7 +771,7 @@
TIME_CHECK();
if (!mStream) return NO_INIT;
if (auto broker = mCallbackBroker.promote(); broker != nullptr) {
- broker->setStreamOutEventCallback(this, callback);
+ broker->setStreamOutEventCallback(static_cast<StreamOutHalInterface*>(this), callback);
}
return OK;
}
@@ -773,7 +805,8 @@
TIME_CHECK();
if (!mStream) return NO_INIT;
if (auto broker = mCallbackBroker.promote(); broker != nullptr) {
- broker->setStreamOutLatencyModeCallback(this, callback);
+ broker->setStreamOutLatencyModeCallback(
+ static_cast<StreamOutHalInterface*>(this), callback);
}
return OK;
};
@@ -782,6 +815,27 @@
return StreamHalAidl::exit();
}
+void StreamOutHalAidl::onWriteReady() {
+ onAsyncTransferReady();
+ if (auto clientCb = mClientCallback.load().promote(); clientCb != nullptr) {
+ clientCb->onWriteReady();
+ }
+}
+
+void StreamOutHalAidl::onDrainReady() {
+ onAsyncDrainReady();
+ if (auto clientCb = mClientCallback.load().promote(); clientCb != nullptr) {
+ clientCb->onDrainReady();
+ }
+}
+
+void StreamOutHalAidl::onError() {
+ onAsyncError();
+ if (auto clientCb = mClientCallback.load().promote(); clientCb != nullptr) {
+ clientCb->onError();
+ }
+}
+
status_t StreamOutHalAidl::filterAndUpdateOffloadMetadata(AudioParameter ¶meters) {
TIME_CHECK();
bool updateMetadata = false;