libaudiohal@aidl: Fix position and latency reporting
For latency, use the last returned value if the current
command did not provide latency (not every command does).
This avoids "Suspicious latency value reported by HAL..."
warnings.
Make sure that position and timestamp values are non-negative
(HAL may left them initialized to the default value `-1`).
Replace `if (status = ...; status != OK) return status` with
`RETURN_STATUS_IF_ERROR`. This also fixes `getRenderPosition`
which was returning `OK`.
Bug: 302132812
Bug: 302519087
Test: atest CtsMediaAudioTestCases
Change-Id: I5780eafd558bca85e966aa8374573df6a4133b9a
diff --git a/media/libaudiohal/impl/StreamHalAidl.cpp b/media/libaudiohal/impl/StreamHalAidl.cpp
index c058386..e74fc16 100644
--- a/media/libaudiohal/impl/StreamHalAidl.cpp
+++ b/media/libaudiohal/impl/StreamHalAidl.cpp
@@ -193,7 +193,7 @@
StreamDescriptor::Reply reply;
switch (state) {
case StreamDescriptor::State::ACTIVE:
- if (status_t status = pause(&reply); status != OK) return status;
+ RETURN_STATUS_IF_ERROR(pause(&reply));
if (reply.state != StreamDescriptor::State::PAUSED) {
ALOGE("%s: unexpected stream state: %s (expected PAUSED)",
__func__, toString(reply.state).c_str());
@@ -203,7 +203,7 @@
case StreamDescriptor::State::PAUSED:
case StreamDescriptor::State::DRAIN_PAUSED:
if (mIsInput) return flush();
- if (status_t status = flush(&reply); status != OK) return status;
+ RETURN_STATUS_IF_ERROR(flush(&reply));
if (reply.state != StreamDescriptor::State::IDLE) {
ALOGE("%s: unexpected stream state: %s (expected IDLE)",
__func__, toString(reply.state).c_str());
@@ -211,10 +211,8 @@
}
FALLTHROUGH_INTENDED;
case StreamDescriptor::State::IDLE:
- if (status_t status = sendCommand(makeHalCommand<HalCommand::Tag::standby>(),
- &reply, true /*safeFromNonWorkerThread*/); status != OK) {
- return status;
- }
+ RETURN_STATUS_IF_ERROR(sendCommand(makeHalCommand<HalCommand::Tag::standby>(),
+ &reply, true /*safeFromNonWorkerThread*/));
if (reply.state != StreamDescriptor::State::STANDBY) {
ALOGE("%s: unexpected stream state: %s (expected STANDBY)",
__func__, toString(reply.state).c_str());
@@ -244,10 +242,7 @@
const auto state = getState();
StreamDescriptor::Reply reply;
if (state == StreamDescriptor::State::STANDBY) {
- if (status_t status = sendCommand(makeHalCommand<HalCommand::Tag::start>(), &reply, true);
- status != OK) {
- return status;
- }
+ RETURN_STATUS_IF_ERROR(sendCommand(makeHalCommand<HalCommand::Tag::start>(), &reply, true));
return sendCommand(makeHalCommand<HalCommand::Tag::burst>(0), &reply, true);
}
@@ -264,10 +259,7 @@
ALOGV("%p %s::%s", this, getClassName().c_str(), __func__);
if (!mStream) return NO_INIT;
StreamDescriptor::Reply reply;
- if (status_t status = updateCountersIfNeeded(&reply); status != OK) {
- return status;
- }
-
+ RETURN_STATUS_IF_ERROR(updateCountersIfNeeded(&reply));
*latency = std::clamp(std::max<int32_t>(0, reply.latencyMs), 1, 3000);
ALOGW_IF(reply.latencyMs != static_cast<int32_t>(*latency),
"Suspicious latency value reported by HAL: %d, clamped to %u", reply.latencyMs,
@@ -279,11 +271,9 @@
ALOGV("%p %s::%s", this, getClassName().c_str(), __func__);
if (!mStream) return NO_INIT;
StreamDescriptor::Reply reply;
- if (status_t status = updateCountersIfNeeded(&reply); status != OK) {
- return status;
- }
- *frames = reply.observable.frames;
- *timestamp = reply.observable.timeNs;
+ RETURN_STATUS_IF_ERROR(updateCountersIfNeeded(&reply));
+ *frames = std::max<int64_t>(0, reply.observable.frames);
+ *timestamp = std::max<int64_t>(0, reply.observable.timeNs);
return OK;
}
@@ -292,12 +282,9 @@
if (!mStream) return NO_INIT;
StreamDescriptor::Reply reply;
// TODO: switch to updateCountersIfNeeded once we sort out mWorkerTid initialization
- if (status_t status = sendCommand(makeHalCommand<HalCommand::Tag::getStatus>(), &reply, true);
- status != OK) {
- return status;
- }
- *frames = reply.hardware.frames;
- *timestamp = reply.hardware.timeNs;
+ RETURN_STATUS_IF_ERROR(sendCommand(makeHalCommand<HalCommand::Tag::getStatus>(), &reply, true));
+ *frames = std::max<int64_t>(0, reply.hardware.frames);
+ *timestamp = std::max<int64_t>(0, reply.hardware.timeNs);
return OK;
}
@@ -305,10 +292,8 @@
ALOGV("%p %s::%s", this, getClassName().c_str(), __func__);
if (!mStream) return NO_INIT;
StreamDescriptor::Reply reply;
- if (status_t status = updateCountersIfNeeded(&reply); status != OK) {
- return status;
- }
- *frames = reply.xrunFrames;
+ RETURN_STATUS_IF_ERROR(updateCountersIfNeeded(&reply));
+ *frames = std::max<int32_t>(0, reply.xrunFrames);
return OK;
}
@@ -323,10 +308,7 @@
// stream state), however this scenario wasn't supported by the HIDL HAL.
if (getState() == StreamDescriptor::State::STANDBY) {
StreamDescriptor::Reply reply;
- if (status_t status = sendCommand(makeHalCommand<HalCommand::Tag::start>(), &reply);
- status != OK) {
- return status;
- }
+ RETURN_STATUS_IF_ERROR(sendCommand(makeHalCommand<HalCommand::Tag::start>(), &reply));
if (reply.state != StreamDescriptor::State::IDLE) {
ALOGE("%s: failed to get the stream out of standby, actual state: %s",
__func__, toString(reply.state).c_str());
@@ -345,9 +327,7 @@
}
}
StreamDescriptor::Reply reply;
- if (status_t status = sendCommand(burst, &reply); status != OK) {
- return status;
- }
+ RETURN_STATUS_IF_ERROR(sendCommand(burst, &reply));
*transferred = reply.fmqByteCount;
if (mIsInput) {
LOG_ALWAYS_FATAL_IF(*transferred > bytes,
@@ -385,11 +365,8 @@
if (state == StreamDescriptor::State::IDLE) {
StreamDescriptor::Reply localReply{};
StreamDescriptor::Reply* innerReply = reply ?: &localReply;
- if (status_t status =
- sendCommand(makeHalCommand<HalCommand::Tag::burst>(0), innerReply);
- status != OK) {
- return status;
- }
+ RETURN_STATUS_IF_ERROR(
+ sendCommand(makeHalCommand<HalCommand::Tag::burst>(0), innerReply));
if (innerReply->state != StreamDescriptor::State::ACTIVE) {
ALOGE("%s: unexpected stream state: %s (expected ACTIVE)",
__func__, toString(innerReply->state).c_str());
@@ -452,10 +429,7 @@
return BAD_VALUE;
}
int64_t aidlPosition = 0, aidlTimestamp = 0;
- if (status_t status = getHardwarePosition(&aidlPosition, &aidlTimestamp); status != OK) {
- return status;
- }
-
+ RETURN_STATUS_IF_ERROR(getHardwarePosition(&aidlPosition, &aidlTimestamp));
position->time_nanoseconds = aidlTimestamp;
position->position_frames = static_cast<int32_t>(aidlPosition);
return OK;
@@ -503,6 +477,11 @@
}
{
std::lock_guard l(mLock);
+ // Not every command replies with 'latencyMs' field filled out, substitute the last
+ // returned value in that case.
+ if (reply->latencyMs <= 0) {
+ reply->latencyMs = mLastReply.latencyMs;
+ }
mLastReply = *reply;
}
switch (reply->status) {
@@ -608,10 +587,8 @@
return BAD_VALUE;
}
int64_t aidlFrames = 0, aidlTimestamp = 0;
- if (status_t status = getObservablePosition(&aidlFrames, &aidlTimestamp); status != OK) {
- return OK;
- }
- *dspFrames = std::clamp<int64_t>(aidlFrames, 0, UINT32_MAX);
+ RETURN_STATUS_IF_ERROR(getObservablePosition(&aidlFrames, &aidlTimestamp));
+ *dspFrames = static_cast<uint32_t>(aidlFrames);
return OK;
}
@@ -680,10 +657,8 @@
return BAD_VALUE;
}
int64_t aidlFrames = 0, aidlTimestamp = 0;
- if (status_t status = getObservablePosition(&aidlFrames, &aidlTimestamp); status != OK) {
- return status;
- }
- *frames = std::max<int64_t>(0, aidlFrames);
+ RETURN_STATUS_IF_ERROR(getObservablePosition(&aidlFrames, &aidlTimestamp));
+ *frames = aidlFrames;
timestamp->tv_sec = aidlTimestamp / NANOS_PER_SECOND;
timestamp->tv_nsec = aidlTimestamp - timestamp->tv_sec * NANOS_PER_SECOND;
return OK;
@@ -901,9 +876,7 @@
return BAD_VALUE;
}
int32_t aidlXruns = 0;
- if (status_t status = getXruns(&aidlXruns); status != OK) {
- return status;
- }
+ RETURN_STATUS_IF_ERROR(getXruns(&aidlXruns));
*framesLost = std::max<int32_t>(0, aidlXruns);
return OK;
}