Clean up HAL render position reporting
Since AIDL HAL reports 64-bit position values, move the logic
for proper expansion of 32-bit positions from AudioStreamOut
into StreamOutHidl. Add synchronization which did not exist
in AudioStreamOut. This is because the calling code
does not always use same mutexes when calling into relevant
methods, thus data races are possible.
Also, clean up StreamOutHalInterface and AudioStreamOut
interface by removing obsolete methods.
Bug: 338557486
Test: atest CtsMediaAudioTestCases
(cherry picked from https://android-review.googlesource.com/q/commit:0ea58fe84db5c97a905dfbd0be718eef325b049e)
Merged-In: I30701af589f3721b3d8bf7886acc251ceefadc46
Change-Id: I30701af589f3721b3d8bf7886acc251ceefadc46
diff --git a/media/libaudiohal/impl/StreamHalAidl.cpp b/media/libaudiohal/impl/StreamHalAidl.cpp
index ecb47e0..9d32be6 100644
--- a/media/libaudiohal/impl/StreamHalAidl.cpp
+++ b/media/libaudiohal/impl/StreamHalAidl.cpp
@@ -654,21 +654,16 @@
return transfer(const_cast<void*>(buffer), bytes, written);
}
-status_t StreamOutHalAidl::getRenderPosition(uint32_t *dspFrames) {
+status_t StreamOutHalAidl::getRenderPosition(uint64_t *dspFrames) {
if (dspFrames == nullptr) {
return BAD_VALUE;
}
int64_t aidlFrames = 0, aidlTimestamp = 0;
RETURN_STATUS_IF_ERROR(getObservablePosition(&aidlFrames, &aidlTimestamp));
- *dspFrames = static_cast<uint32_t>(aidlFrames);
+ *dspFrames = aidlFrames;
return OK;
}
-status_t StreamOutHalAidl::getNextWriteTimestamp(int64_t *timestamp __unused) {
- // Obsolete, use getPresentationPosition.
- return INVALID_OPERATION;
-}
-
status_t StreamOutHalAidl::setCallback(wp<StreamOutHalInterfaceCallback> callback) {
ALOGD("%p %s", this, __func__);
TIME_CHECK();
@@ -729,6 +724,11 @@
return OK;
}
+status_t StreamOutHalAidl::presentationComplete() {
+ ALOGD("%p %s::%s", this, getClassName().c_str(), __func__);
+ return OK;
+}
+
status_t StreamOutHalAidl::updateSourceMetadata(
const StreamOutHalInterface::SourceMetadata& sourceMetadata) {
TIME_CHECK();
diff --git a/media/libaudiohal/impl/StreamHalAidl.h b/media/libaudiohal/impl/StreamHalAidl.h
index b20eb00..8a398d8 100644
--- a/media/libaudiohal/impl/StreamHalAidl.h
+++ b/media/libaudiohal/impl/StreamHalAidl.h
@@ -308,10 +308,7 @@
// Return the number of audio frames written by the audio dsp to DAC since
// the output has exited standby.
- status_t getRenderPosition(uint32_t *dspFrames) override;
-
- // Get the local time at which the next write to the audio driver will be presented.
- status_t getNextWriteTimestamp(int64_t *timestamp) override;
+ status_t getRenderPosition(uint64_t *dspFrames) override;
// Set the callback for notifying completion of non-blocking write and drain.
status_t setCallback(wp<StreamOutHalInterfaceCallback> callback) override;
@@ -337,6 +334,9 @@
// Return a recent count of the number of audio frames presented to an external observer.
status_t getPresentationPosition(uint64_t *frames, struct timespec *timestamp) override;
+ // Notifies the HAL layer that the framework considers the current playback as completed.
+ status_t presentationComplete() override;
+
// Called when the metadata of the stream's source has been changed.
status_t updateSourceMetadata(const SourceMetadata& sourceMetadata) override;
diff --git a/media/libaudiohal/impl/StreamHalHidl.cpp b/media/libaudiohal/impl/StreamHalHidl.cpp
index 77c75db..9e22700 100644
--- a/media/libaudiohal/impl/StreamHalHidl.cpp
+++ b/media/libaudiohal/impl/StreamHalHidl.cpp
@@ -17,6 +17,8 @@
#define LOG_TAG "StreamHalHidl"
//#define LOG_NDEBUG 0
+#include <cinttypes>
+
#include <android/hidl/manager/1.0/IServiceManager.h>
#include <hwbinder/IPCThreadState.h>
#include <media/AudioParameter.h>
@@ -589,32 +591,39 @@
return OK;
}
-status_t StreamOutHalHidl::getRenderPosition(uint32_t *dspFrames) {
+status_t StreamOutHalHidl::getRenderPosition(uint64_t *dspFrames) {
// TIME_CHECK(); // TODO(b/243839867) reenable only when optimized.
if (mStream == 0) return NO_INIT;
Result retval;
+ uint32_t halPosition = 0;
Return<void> ret = mStream->getRenderPosition(
[&](Result r, uint32_t d) {
retval = r;
if (retval == Result::OK) {
- *dspFrames = d;
+ halPosition = d;
}
});
- return processReturn("getRenderPosition", ret, retval);
-}
+ status_t status = processReturn("getRenderPosition", ret, retval);
+ if (status != OK) {
+ return status;
+ }
+ // Maintain a 64-bit render position using the 32-bit result from the HAL.
+ // This delta calculation relies on the arithmetic overflow behavior
+ // of integers. For example (100 - 0xFFFFFFF0) = 116.
+ std::lock_guard l(mPositionMutex);
+ const auto truncatedPosition = (uint32_t)mRenderPosition;
+ int32_t deltaHalPosition; // initialization not needed, overwitten by __builtin_sub_overflow()
+ (void) __builtin_sub_overflow(halPosition, truncatedPosition, &deltaHalPosition);
-status_t StreamOutHalHidl::getNextWriteTimestamp(int64_t *timestamp) {
- TIME_CHECK();
- if (mStream == 0) return NO_INIT;
- Result retval;
- Return<void> ret = mStream->getNextWriteTimestamp(
- [&](Result r, int64_t t) {
- retval = r;
- if (retval == Result::OK) {
- *timestamp = t;
- }
- });
- return processReturn("getRenderPosition", ret, retval);
+ if (deltaHalPosition >= 0) {
+ mRenderPosition += deltaHalPosition;
+ } else if (mExpectRetrograde) {
+ mExpectRetrograde = false;
+ mRenderPosition -= static_cast<uint64_t>(-deltaHalPosition);
+ ALOGW("Retrograde motion of %" PRId32 " frames", -deltaHalPosition);
+ }
+ *dspFrames = mRenderPosition;
+ return OK;
}
status_t StreamOutHalHidl::setCallback(wp<StreamOutHalInterfaceCallback> callback) {
@@ -667,9 +676,23 @@
status_t StreamOutHalHidl::flush() {
TIME_CHECK();
if (mStream == 0) return NO_INIT;
+ {
+ std::lock_guard l(mPositionMutex);
+ mRenderPosition = 0;
+ mExpectRetrograde = false;
+ }
return processReturn("pause", mStream->flush());
}
+status_t StreamOutHalHidl::standby() {
+ {
+ std::lock_guard l(mPositionMutex);
+ mRenderPosition = 0;
+ mExpectRetrograde = false;
+ }
+ return StreamHalHidl::standby();
+}
+
status_t StreamOutHalHidl::getPresentationPosition(uint64_t *frames, struct timespec *timestamp) {
// TIME_CHECK(); // TODO(b/243839867) reenable only when optimized.
if (mStream == 0) return NO_INIT;
@@ -696,6 +719,16 @@
}
}
+status_t StreamOutHalHidl::presentationComplete() {
+ // Avoid suppressing retrograde motion in mRenderPosition for gapless offload/direct when
+ // transitioning between tracks.
+ // The HAL resets the frame position without flush/stop being called, but calls back prior to
+ // this event. So, on the next occurrence of retrograde motion, we permit backwards movement of
+ // mRenderPosition.
+ mExpectRetrograde = true;
+ return OK;
+}
+
#if MAJOR_VERSION == 2
status_t StreamOutHalHidl::updateSourceMetadata(
const StreamOutHalInterface::SourceMetadata& /* sourceMetadata */) {
diff --git a/media/libaudiohal/impl/StreamHalHidl.h b/media/libaudiohal/impl/StreamHalHidl.h
index 48da633..80379d0 100644
--- a/media/libaudiohal/impl/StreamHalHidl.h
+++ b/media/libaudiohal/impl/StreamHalHidl.h
@@ -18,10 +18,12 @@
#define ANDROID_HARDWARE_STREAM_HAL_HIDL_H
#include <atomic>
+#include <mutex>
#include PATH(android/hardware/audio/CORE_TYPES_FILE_VERSION/IStream.h)
#include PATH(android/hardware/audio/CORE_TYPES_FILE_VERSION/IStreamIn.h)
#include PATH(android/hardware/audio/FILE_VERSION/IStreamOut.h)
+#include <android-base/thread_annotations.h>
#include <fmq/EventFlag.h>
#include <fmq/MessageQueue.h>
#include <media/audiohal/EffectHalInterface.h>
@@ -119,6 +121,9 @@
class StreamOutHalHidl : public StreamOutHalInterface, public StreamHalHidl {
public:
+ // Put the audio hardware input/output into standby mode (from StreamHalInterface).
+ status_t standby() override;
+
// Return the frame size (number of bytes per sample) of a stream.
virtual status_t getFrameSize(size_t *size);
@@ -136,10 +141,7 @@
// Return the number of audio frames written by the audio dsp to DAC since
// the output has exited standby.
- virtual status_t getRenderPosition(uint32_t *dspFrames);
-
- // Get the local time at which the next write to the audio driver will be presented.
- virtual status_t getNextWriteTimestamp(int64_t *timestamp);
+ virtual status_t getRenderPosition(uint64_t *dspFrames);
// Set the callback for notifying completion of non-blocking write and drain.
virtual status_t setCallback(wp<StreamOutHalInterfaceCallback> callback);
@@ -165,6 +167,9 @@
// Return a recent count of the number of audio frames presented to an external observer.
virtual status_t getPresentationPosition(uint64_t *frames, struct timespec *timestamp);
+ // Notifies the HAL layer that the framework considers the current playback as completed.
+ status_t presentationComplete() override;
+
// Called when the metadata of the stream's source has been changed.
status_t updateSourceMetadata(const SourceMetadata& sourceMetadata) override;
@@ -221,6 +226,10 @@
std::unique_ptr<StatusMQ> mStatusMQ;
std::atomic<pid_t> mWriterClient;
EventFlag* mEfGroup;
+ std::mutex mPositionMutex;
+ // Used to expand correctly the 32-bit position from the HAL.
+ uint64_t mRenderPosition GUARDED_BY(mPositionMutex) = 0;
+ bool mExpectRetrograde GUARDED_BY(mPositionMutex) = false; // See 'presentationComplete'.
// Can not be constructed directly by clients.
StreamOutHalHidl(const sp<::android::hardware::audio::CPP_VERSION::IStreamOut>& stream);
diff --git a/media/libaudiohal/include/media/audiohal/StreamHalInterface.h b/media/libaudiohal/include/media/audiohal/StreamHalInterface.h
index 37615af..eb14f6b 100644
--- a/media/libaudiohal/include/media/audiohal/StreamHalInterface.h
+++ b/media/libaudiohal/include/media/audiohal/StreamHalInterface.h
@@ -151,10 +151,7 @@
// Return the number of audio frames written by the audio dsp to DAC since
// the output has exited standby.
- virtual status_t getRenderPosition(uint32_t *dspFrames) = 0;
-
- // Get the local time at which the next write to the audio driver will be presented.
- virtual status_t getNextWriteTimestamp(int64_t *timestamp) = 0;
+ virtual status_t getRenderPosition(uint64_t *dspFrames) = 0;
// Set the callback for notifying completion of non-blocking write and drain.
// The callback must be owned by someone else. The output stream does not own it
@@ -182,6 +179,9 @@
// Return a recent count of the number of audio frames presented to an external observer.
virtual status_t getPresentationPosition(uint64_t *frames, struct timespec *timestamp) = 0;
+ // Notifies the HAL layer that the framework considers the current playback as completed.
+ virtual status_t presentationComplete() = 0;
+
struct SourceMetadata {
std::vector<playback_track_metadata_v7_t> tracks;
};
diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp
index 41914e3..a0485ce 100644
--- a/services/audioflinger/Threads.cpp
+++ b/services/audioflinger/Threads.cpp
@@ -3398,9 +3398,9 @@
return NO_ERROR;
} else {
status_t status;
- uint32_t frames;
+ uint64_t frames = 0;
status = mOutput->getRenderPosition(&frames);
- *dspFrames = (size_t)frames;
+ *dspFrames = (uint32_t)frames;
return status;
}
}
diff --git a/services/audioflinger/datapath/AudioStreamIn.cpp b/services/audioflinger/datapath/AudioStreamIn.cpp
index 76618f4..165ac25 100644
--- a/services/audioflinger/datapath/AudioStreamIn.cpp
+++ b/services/audioflinger/datapath/AudioStreamIn.cpp
@@ -58,7 +58,7 @@
if (mHalFormatHasProportionalFrames &&
(flags & AUDIO_INPUT_FLAG_DIRECT) == AUDIO_INPUT_FLAG_DIRECT) {
- // For DirectRecord reset timestamp to 0 on standby.
+ // For DirectRecord reset position to 0 on standby.
const uint64_t adjustedPosition = (halPosition <= mFramesReadAtStandby) ?
0 : (halPosition - mFramesReadAtStandby);
// Scale from HAL sample rate to application rate.
diff --git a/services/audioflinger/datapath/AudioStreamOut.cpp b/services/audioflinger/datapath/AudioStreamOut.cpp
index aad538f..a686ff6 100644
--- a/services/audioflinger/datapath/AudioStreamOut.cpp
+++ b/services/audioflinger/datapath/AudioStreamOut.cpp
@@ -51,42 +51,17 @@
return NO_INIT;
}
- uint32_t halPosition = 0;
+ uint64_t halPosition = 0;
const status_t status = stream->getRenderPosition(&halPosition);
if (status != NO_ERROR) {
return status;
}
-
- // Maintain a 64-bit render position using the 32-bit result from the HAL.
- // This delta calculation relies on the arithmetic overflow behavior
- // of integers. For example (100 - 0xFFFFFFF0) = 116.
- const auto truncatedPosition = (uint32_t)mRenderPosition;
- int32_t deltaHalPosition; // initialization not needed, overwitten by __builtin_sub_overflow()
- (void) __builtin_sub_overflow(halPosition, truncatedPosition, &deltaHalPosition);
-
- if (deltaHalPosition >= 0) {
- mRenderPosition += deltaHalPosition;
- } else if (mExpectRetrograde) {
- mExpectRetrograde = false;
- mRenderPosition -= static_cast<uint64_t>(-deltaHalPosition);
- }
// Scale from HAL sample rate to application rate.
- *frames = mRenderPosition / mRateMultiplier;
+ *frames = halPosition / mRateMultiplier;
return status;
}
-// return bottom 32-bits of the render position
-status_t AudioStreamOut::getRenderPosition(uint32_t *frames)
-{
- uint64_t position64 = 0;
- const status_t status = getRenderPosition(&position64);
- if (status == NO_ERROR) {
- *frames = (uint32_t)position64;
- }
- return status;
-}
-
status_t AudioStreamOut::getPresentationPosition(uint64_t *frames, struct timespec *timestamp)
{
if (stream == nullptr) {
@@ -101,7 +76,7 @@
if (mHalFormatHasProportionalFrames &&
(flags & AUDIO_OUTPUT_FLAG_DIRECT) == AUDIO_OUTPUT_FLAG_DIRECT) {
- // For DirectTrack reset timestamp to 0 on standby.
+ // For DirectTrack reset position to 0 on standby.
const uint64_t adjustedPosition = (halPosition <= mFramesWrittenAtStandby) ?
0 : (halPosition - mFramesWrittenAtStandby);
// Scale from HAL sample rate to application rate.
@@ -179,8 +154,6 @@
int AudioStreamOut::flush()
{
- mRenderPosition = 0;
- mExpectRetrograde = false;
mFramesWritten = 0;
mFramesWrittenAtStandby = 0;
const status_t result = stream->flush();
@@ -189,12 +162,14 @@
int AudioStreamOut::standby()
{
- mRenderPosition = 0;
- mExpectRetrograde = false;
mFramesWrittenAtStandby = mFramesWritten;
return stream->standby();
}
+void AudioStreamOut::presentationComplete() {
+ stream->presentationComplete();
+}
+
ssize_t AudioStreamOut::write(const void *buffer, size_t numBytes)
{
size_t bytesWritten;
diff --git a/services/audioflinger/datapath/AudioStreamOut.h b/services/audioflinger/datapath/AudioStreamOut.h
index ea41bba..2c9fb3e 100644
--- a/services/audioflinger/datapath/AudioStreamOut.h
+++ b/services/audioflinger/datapath/AudioStreamOut.h
@@ -51,9 +51,6 @@
virtual ~AudioStreamOut();
- // Get the bottom 32-bits of the 64-bit render position.
- status_t getRenderPosition(uint32_t *frames);
-
virtual status_t getRenderPosition(uint64_t *frames);
virtual status_t getPresentationPosition(uint64_t *frames, struct timespec *timestamp);
@@ -91,21 +88,14 @@
virtual status_t flush();
virtual status_t standby();
- // Avoid suppressing retrograde motion in mRenderPosition for gapless offload/direct when
- // transitioning between tracks.
- // The HAL resets the frame position without flush/stop being called, but calls back prior to
- // this event. So, on the next occurrence of retrograde motion, we permit backwards movement of
- // mRenderPosition.
- virtual void presentationComplete() { mExpectRetrograde = true; }
+ virtual void presentationComplete();
protected:
uint64_t mFramesWritten = 0; // reset by flush
uint64_t mFramesWrittenAtStandby = 0;
- uint64_t mRenderPosition = 0; // reset by flush, standby, or presentation complete
int mRateMultiplier = 1;
bool mHalFormatHasProportionalFrames = false;
size_t mHalFrameSize = 0;
- bool mExpectRetrograde = false; // see presentationComplete
};
} // namespace android