audiohal: Fix incompatibility with legacy hal for writes
The combo call to stream_out->write + get_presentation_position
wasn't delivering the results of these calls entirely
correctly. Since the 'WriteResult' struct was lacking the field
for returning the status of the call to
'get_presentation_position', the client could erroneously update
the presentation position to bogus values if the call to the
legacy HAL had failed.
Updated IStreamOut.WriteStatus to include the missing field,
and updated the code to fill it out.
Also fixed logspam resulting from calling a stubbed
stream_in->get_capture_position.
Bug: 30222631
Test: Loopback RTT, media CTS
Change-Id: I38ac3b01beb095e176b54608e11e71ae5d5eafb6
diff --git a/audio/2.0/default/StreamIn.cpp b/audio/2.0/default/StreamIn.cpp
index 51c2cc7..de7bf13 100644
--- a/audio/2.0/default/StreamIn.cpp
+++ b/audio/2.0/default/StreamIn.cpp
@@ -330,7 +330,10 @@
int64_t halFrames, halTime;
retval = Stream::analyzeStatus(
"get_capture_position",
- mStream->get_capture_position(mStream, &halFrames, &halTime));
+ mStream->get_capture_position(mStream, &halFrames, &halTime),
+ // HAL may have a stub function, always returning ENOSYS, don't
+ // spam the log in this case.
+ ENOSYS);
if (retval == Result::OK) {
frames = halFrames;
time = halTime;
diff --git a/audio/2.0/default/StreamOut.cpp b/audio/2.0/default/StreamOut.cpp
index 4bb2274..ea6221e 100644
--- a/audio/2.0/default/StreamOut.cpp
+++ b/audio/2.0/default/StreamOut.cpp
@@ -88,24 +88,20 @@
}
const size_t availToRead = mDataMQ->availableToRead();
- Result retval = Result::OK;
- uint64_t written = 0;
+ IStreamOut::WriteStatus status;
+ status.writeRetval = Result::OK;
+ status.written = 0;
if (mDataMQ->read(&mBuffer[0], availToRead)) {
ssize_t writeResult = mStream->write(mStream, &mBuffer[0], availToRead);
if (writeResult >= 0) {
- written = writeResult;
+ status.written = writeResult;
} else {
- retval = Stream::analyzeStatus("write", writeResult);
+ status.writeRetval = Stream::analyzeStatus("write", writeResult);
}
}
- uint64_t frames = 0;
- struct timespec halTimeStamp = { 0, 0 };
- if (retval == Result::OK && mStream->get_presentation_position != NULL) {
- mStream->get_presentation_position(mStream, &frames, &halTimeStamp);
- }
- IStreamOut::WriteStatus status = { retval, written, frames,
- { static_cast<uint64_t>(halTimeStamp.tv_sec),
- static_cast<uint64_t>(halTimeStamp.tv_nsec) } };
+ status.presentationPositionRetval = status.writeRetval == Result::OK ?
+ StreamOut::getPresentationPositionImpl(mStream, &status.frames, &status.timeStamp) :
+ Result::OK;
if (!mStatusMQ->write(&status)) {
ALOGW("status message queue write failed");
}
@@ -399,23 +395,29 @@
Result::NOT_SUPPORTED;
}
-Return<void> StreamOut::getPresentationPosition(getPresentationPosition_cb _hidl_cb) {
+// static
+Result StreamOut::getPresentationPositionImpl(
+ audio_stream_out_t *stream, uint64_t *frames, TimeSpec *timeStamp) {
Result retval(Result::NOT_SUPPORTED);
+ if (stream->get_presentation_position == NULL) return retval;
+ struct timespec halTimeStamp;
+ retval = Stream::analyzeStatus(
+ "get_presentation_position",
+ stream->get_presentation_position(stream, frames, &halTimeStamp),
+ // Don't logspam on EINVAL--it's normal for get_presentation_position
+ // to return it sometimes.
+ EINVAL);
+ if (retval == Result::OK) {
+ timeStamp->tvSec = halTimeStamp.tv_sec;
+ timeStamp->tvNSec = halTimeStamp.tv_nsec;
+ }
+ return retval;
+}
+
+Return<void> StreamOut::getPresentationPosition(getPresentationPosition_cb _hidl_cb) {
uint64_t frames = 0;
TimeSpec timeStamp = { 0, 0 };
- if (mStream->get_presentation_position != NULL) {
- struct timespec halTimeStamp;
- retval = Stream::analyzeStatus(
- "get_presentation_position",
- mStream->get_presentation_position(mStream, &frames, &halTimeStamp),
- // Don't logspam on EINVAL--it's normal for get_presentation_position
- // to return it sometimes.
- EINVAL);
- if (retval == Result::OK) {
- timeStamp.tvSec = halTimeStamp.tv_sec;
- timeStamp.tvNSec = halTimeStamp.tv_nsec;
- }
- }
+ Result retval = getPresentationPositionImpl(mStream, &frames, &timeStamp);
_hidl_cb(retval, frames, timeStamp);
return Void();
}
diff --git a/audio/2.0/default/StreamOut.h b/audio/2.0/default/StreamOut.h
index 83f4447..754a0c0 100644
--- a/audio/2.0/default/StreamOut.h
+++ b/audio/2.0/default/StreamOut.h
@@ -108,6 +108,9 @@
Return<void> createMmapBuffer(int32_t minSizeFrames, createMmapBuffer_cb _hidl_cb) override;
Return<void> getMmapPosition(getMmapPosition_cb _hidl_cb) override;
+ static Result getPresentationPositionImpl(
+ audio_stream_out_t *stream, uint64_t *frames, TimeSpec *timeStamp);
+
private:
bool mIsClosed;
audio_hw_device_t *mDevice;