Merge "Fix thread safety in libaudioclient tests" into main am: d14f1ad882 (04dd1165) · Commits · e / os / android_frameworks_av

media/libaudioclient/tests/Android.bp

+3 −0

Original line number	Original line	Diff line number	Diff line
	@@ -134,6 +134,9 @@ cc_defaults {
	"libaudiomanager",		"libaudiomanager",
	"libaudiopolicy",		"libaudiopolicy",
	],		],
			cflags: [
			"-Wthread-safety",
			],
	data: ["bbb*.raw"],		data: ["bbb*.raw"],
	srcs: [		srcs: [
	"audio_test_utils.cpp",		"audio_test_utils.cpp",

media/libaudioclient/tests/audio_test_utils.cpp

+48 −19

Original line number	Original line	Diff line number	Diff line
	@@ -28,25 +28,35 @@

	void OnAudioDeviceUpdateNotifier::onAudioDeviceUpdate(audio_io_handle_t audioIo,		void OnAudioDeviceUpdateNotifier::onAudioDeviceUpdate(audio_io_handle_t audioIo,
	audio_port_handle_t deviceId) {		audio_port_handle_t deviceId) {
	std::unique_lock<std::mutex> lock{mMutex};
	ALOGI("%s: audioIo=%d deviceId=%d", __func__, audioIo, deviceId);		ALOGI("%s: audioIo=%d deviceId=%d", __func__, audioIo, deviceId);
			{
			std::lock_guard lock(mMutex);
	mAudioIo = audioIo;		mAudioIo = audioIo;
	mDeviceId = deviceId;		mDeviceId = deviceId;
			}
	mCondition.notify_all();		mCondition.notify_all();
	}		}

	status_t OnAudioDeviceUpdateNotifier::waitForAudioDeviceCb(audio_port_handle_t expDeviceId) {		status_t OnAudioDeviceUpdateNotifier::waitForAudioDeviceCb(audio_port_handle_t expDeviceId) {
	std::unique_lock<std::mutex> lock{mMutex};		std::unique_lock lock(mMutex);
			android::base::ScopedLockAssertion lock_assertion(mMutex);
	if (mAudioIo == AUDIO_IO_HANDLE_NONE \|\|		if (mAudioIo == AUDIO_IO_HANDLE_NONE \|\|
	(expDeviceId != AUDIO_PORT_HANDLE_NONE && expDeviceId != mDeviceId)) {		(expDeviceId != AUDIO_PORT_HANDLE_NONE && expDeviceId != mDeviceId)) {
	mCondition.wait_for(lock, std::chrono::milliseconds(500));		mCondition.wait_for(lock, std::chrono::milliseconds(500));
	if (mAudioIo == AUDIO_IO_HANDLE_NONE \|\|		if (mAudioIo == AUDIO_IO_HANDLE_NONE \|\|
	(expDeviceId != AUDIO_PORT_HANDLE_NONE && expDeviceId != mDeviceId))		(expDeviceId != AUDIO_PORT_HANDLE_NONE && expDeviceId != mDeviceId)) {
	return TIMED_OUT;		return TIMED_OUT;
	}		}
			}
	return OK;		return OK;
	}		}

			std::pair<audio_io_handle_t, audio_port_handle_t>
			OnAudioDeviceUpdateNotifier::getLastPortAndDevice() const {
			std::lock_guard lock(mMutex);
			return {mAudioIo, mDeviceId};
			}

	AudioPlayback::AudioPlayback(uint32_t sampleRate, audio_format_t format,		AudioPlayback::AudioPlayback(uint32_t sampleRate, audio_format_t format,
	audio_channel_mask_t channelMask, audio_output_flags_t flags,		audio_channel_mask_t channelMask, audio_output_flags_t flags,
	audio_session_t sessionId, AudioTrack::transfer_type transferType,		audio_session_t sessionId, AudioTrack::transfer_type transferType,
	@@ -147,9 +157,8 @@ status_t AudioPlayback::start() {
	}		}

	void AudioPlayback::onBufferEnd() {		void AudioPlayback::onBufferEnd() {
	std::unique_lock<std::mutex> lock{mMutex};		std::lock_guard lock(mMutex);
	mStopPlaying = true;		mStopPlaying = true;
	mCondition.notify_all();
	}		}

	status_t AudioPlayback::fillBuffer() {		status_t AudioPlayback::fillBuffer() {
	@@ -187,7 +196,12 @@ status_t AudioPlayback::waitForConsumption(bool testSeek) {
	const int maxTries = MAX_WAIT_TIME_MS / WAIT_PERIOD_MS;		const int maxTries = MAX_WAIT_TIME_MS / WAIT_PERIOD_MS;
	int counter = 0;		int counter = 0;
	size_t totalFrameCount = mMemCapacity / mTrack->frameSize();		size_t totalFrameCount = mMemCapacity / mTrack->frameSize();
	while (!mStopPlaying && counter < maxTries) {		bool stopPlaying;
			{
			std::lock_guard lock(mMutex);
			stopPlaying = mStopPlaying;
			}
			while (!stopPlaying && counter < maxTries) {
	uint32_t currPosition;		uint32_t currPosition;
	mTrack->getPosition(&currPosition);		mTrack->getPosition(&currPosition);
	if (currPosition >= totalFrameCount) counter++;		if (currPosition >= totalFrameCount) counter++;
	@@ -213,7 +227,10 @@ status_t AudioPlayback::waitForConsumption(bool testSeek) {
	mTrack->start();		mTrack->start();
	}		}
	std::this_thread::sleep_for(std::chrono::milliseconds(WAIT_PERIOD_MS));		std::this_thread::sleep_for(std::chrono::milliseconds(WAIT_PERIOD_MS));
			std::lock_guard lock(mMutex);
			stopPlaying = mStopPlaying;
	}		}
			std::lock_guard lock(mMutex);
	if (!mStopPlaying && counter == maxTries) return TIMED_OUT;		if (!mStopPlaying && counter == maxTries) return TIMED_OUT;
	return OK;		return OK;
	}		}
	@@ -228,8 +245,10 @@ status_t AudioPlayback::onProcess(bool testSeek) {
	}		}

	void AudioPlayback::stop() {		void AudioPlayback::stop() {
	std::unique_lock<std::mutex> lock{mMutex};		{
			std::lock_guard lock(mMutex);
	mStopPlaying = true;		mStopPlaying = true;
			}
	if (mState != PLAY_STOPPED && mState != PLAY_NO_INIT) {		if (mState != PLAY_STOPPED && mState != PLAY_NO_INIT) {
	int32_t msec = 0;		int32_t msec = 0;
	(void)mTrack->pendingDuration(&msec);		(void)mTrack->pendingDuration(&msec);
	@@ -257,11 +276,14 @@ size_t AudioCapture::onMoreData(const AudioRecord::Buffer& buffer) {
	return 0;		return 0;
	}		}

			{
			std::lock_guard l(mMutex);
	// no more frames to read		// no more frames to read
	if (mNumFramesReceived >= mNumFramesToRecord \|\| mStopRecording) {		if (mNumFramesReceived >= mNumFramesToRecord \|\| mStopRecording) {
	mStopRecording = true;		mStopRecording = true;
	return 0;		return 0;
	}		}
			}

	int64_t timeUs = 0, position = 0, timeNs = 0;		int64_t timeUs = 0, position = 0, timeNs = 0;
	ExtendedTimestamp ts;		ExtendedTimestamp ts;
	@@ -324,9 +346,12 @@ size_t AudioCapture::onMoreData(const AudioRecord::Buffer& buffer) {
	}		}

	if (tmpQueue.size() > 0) {		if (tmpQueue.size() > 0) {
	std::unique_lock<std::mutex> lock{mMutex};		{
	for (auto it = tmpQueue.begin(); it != tmpQueue.end(); it++)		std::lock_guard lock(mMutex);
	mBuffersReceived.push_back(std::move(*it));		mBuffersReceived.insert(mBuffersReceived.end(),
			std::make_move_iterator(tmpQueue.begin()),
			std::make_move_iterator(tmpQueue.end()));
			}
	mCondition.notify_all();		mCondition.notify_all();
	}		}
	return buffer.size();		return buffer.size();
	@@ -484,7 +509,10 @@ status_t AudioCapture::start(AudioSystem::sync_event_t event, audio_session_t tr

	status_t AudioCapture::stop() {		status_t AudioCapture::stop() {
	status_t status = OK;		status_t status = OK;
			{
			std::lock_guard l(mMutex);
	mStopRecording = true;		mStopRecording = true;
			}
	if (mState != REC_STOPPED && mState != REC_NO_INIT) {		if (mState != REC_STOPPED && mState != REC_NO_INIT) {
	if (mInputSource != AUDIO_SOURCE_DEFAULT) {		if (mInputSource != AUDIO_SOURCE_DEFAULT) {
	bool state = false;		bool state = false;
	@@ -530,7 +558,8 @@ status_t AudioCapture::obtainBufferCb(RawBuffer& buffer) {
	if (REC_STARTED != mState) return INVALID_OPERATION;		if (REC_STARTED != mState) return INVALID_OPERATION;
	const int maxTries = MAX_WAIT_TIME_MS / WAIT_PERIOD_MS;		const int maxTries = MAX_WAIT_TIME_MS / WAIT_PERIOD_MS;
	int counter = 0;		int counter = 0;
	std::unique_lock<std::mutex> lock{mMutex};		std::unique_lock lock(mMutex);
			android::base::ScopedLockAssertion lock_assertion(mMutex);
	while (mBuffersReceived.empty() && !mStopRecording && counter < maxTries) {		while (mBuffersReceived.empty() && !mStopRecording && counter < maxTries) {
	mCondition.wait_for(lock, std::chrono::milliseconds(WAIT_PERIOD_MS));		mCondition.wait_for(lock, std::chrono::milliseconds(WAIT_PERIOD_MS));
	counter++;		counter++;

media/libaudioclient/tests/audio_test_utils.h

+21 −21

Original line number	Original line	Diff line number	Diff line
	@@ -19,14 +19,13 @@

	#include <sys/stat.h>		#include <sys/stat.h>
	#include <unistd.h>		#include <unistd.h>
	#include <atomic>
	#include <chrono>
	#include <cinttypes>
	#include <deque>		#include <deque>
	#include <memory>		#include <memory>
	#include <mutex>		#include <mutex>
	#include <thread>		#include <thread>
			#include <utility>

			#include <android-base/thread_annotations.h>
	#include <binder/MemoryDealer.h>		#include <binder/MemoryDealer.h>
	#include <media/AidlConversion.h>		#include <media/AidlConversion.h>
	#include <media/AudioRecord.h>		#include <media/AudioRecord.h>
	@@ -63,13 +62,15 @@ std::string dumpPatch(const audio_patch& patch);

	class OnAudioDeviceUpdateNotifier : public AudioSystem::AudioDeviceCallback {		class OnAudioDeviceUpdateNotifier : public AudioSystem::AudioDeviceCallback {
	public:		public:
	audio_io_handle_t mAudioIo = AUDIO_IO_HANDLE_NONE;		void onAudioDeviceUpdate(audio_io_handle_t audioIo, audio_port_handle_t deviceId) override;
	audio_port_handle_t mDeviceId = AUDIO_PORT_HANDLE_NONE;
	std::mutex mMutex;
	std::condition_variable mCondition;

	void onAudioDeviceUpdate(audio_io_handle_t audioIo, audio_port_handle_t deviceId);
	status_t waitForAudioDeviceCb(audio_port_handle_t expDeviceId = AUDIO_PORT_HANDLE_NONE);		status_t waitForAudioDeviceCb(audio_port_handle_t expDeviceId = AUDIO_PORT_HANDLE_NONE);
			std::pair<audio_io_handle_t, audio_port_handle_t> getLastPortAndDevice() const;

			private:
			audio_io_handle_t mAudioIo GUARDED_BY(mMutex) = AUDIO_IO_HANDLE_NONE;
			audio_port_handle_t mDeviceId GUARDED_BY(mMutex) = AUDIO_PORT_HANDLE_NONE;
			mutable std::mutex mMutex;
			std::condition_variable mCondition;
	};		};

	// Simple AudioPlayback class.		// Simple AudioPlayback class.
	@@ -86,15 +87,14 @@ class AudioPlayback : public AudioTrack::IAudioTrackCallback {
	status_t create();		status_t create();
	sp<AudioTrack> getAudioTrackHandle();		sp<AudioTrack> getAudioTrackHandle();
	status_t start();		status_t start();
	status_t waitForConsumption(bool testSeek = false);		status_t waitForConsumption(bool testSeek = false) EXCLUDES(mMutex);
	status_t fillBuffer();		status_t fillBuffer();
	status_t onProcess(bool testSeek = false);		status_t onProcess(bool testSeek = false);
	virtual void onBufferEnd() override;		void onBufferEnd() override EXCLUDES(mMutex);
	void stop();		void stop() EXCLUDES(mMutex);

	bool mStopPlaying;		bool mStopPlaying GUARDED_BY(mMutex);
	std::mutex mMutex;		mutable std::mutex mMutex;
	std::condition_variable mCondition;

	enum State {		enum State {
	PLAY_NO_INIT,		PLAY_NO_INIT,
	@@ -144,7 +144,7 @@ class AudioCapture : public AudioRecord::IAudioRecordCallback {
	AudioRecord::transfer_type transferType = AudioRecord::TRANSFER_CALLBACK,		AudioRecord::transfer_type transferType = AudioRecord::TRANSFER_CALLBACK,
	const audio_attributes_t* attributes = nullptr);		const audio_attributes_t* attributes = nullptr);
	~AudioCapture();		~AudioCapture();
	size_t onMoreData(const AudioRecord::Buffer& buffer) override;		size_t onMoreData(const AudioRecord::Buffer& buffer) override EXCLUDES(mMutex);
	void onOverrun() override;		void onOverrun() override;
	void onMarker(uint32_t markerPosition) override;		void onMarker(uint32_t markerPosition) override;
	void onNewPos(uint32_t newPos) override;		void onNewPos(uint32_t newPos) override;
	@@ -156,10 +156,10 @@ class AudioCapture : public AudioRecord::IAudioRecordCallback {
	sp<AudioRecord> getAudioRecordHandle();		sp<AudioRecord> getAudioRecordHandle();
	status_t start(AudioSystem::sync_event_t event = AudioSystem::SYNC_EVENT_NONE,		status_t start(AudioSystem::sync_event_t event = AudioSystem::SYNC_EVENT_NONE,
	audio_session_t triggerSession = AUDIO_SESSION_NONE);		audio_session_t triggerSession = AUDIO_SESSION_NONE);
	status_t obtainBufferCb(RawBuffer& buffer);		status_t obtainBufferCb(RawBuffer& buffer) EXCLUDES(mMutex);
	status_t obtainBuffer(RawBuffer& buffer);		status_t obtainBuffer(RawBuffer& buffer);
	status_t audioProcess();		status_t audioProcess();
	status_t stop();		status_t stop() EXCLUDES(mMutex);

	uint32_t mFrameCount;		uint32_t mFrameCount;
	uint32_t mNotificationFrames;		uint32_t mNotificationFrames;
	@@ -192,13 +192,13 @@ class AudioCapture : public AudioRecord::IAudioRecordCallback {
	size_t mMaxBytesPerCallback = 2048;		size_t mMaxBytesPerCallback = 2048;
	sp<AudioRecord> mRecord;		sp<AudioRecord> mRecord;
	State mState;		State mState;
	bool mStopRecording;		bool mStopRecording GUARDED_BY(mMutex);
	std::string mFileName;		std::string mFileName;
	int mOutFileFd = -1;		int mOutFileFd = -1;

	std::mutex mMutex;		mutable std::mutex mMutex;
	std::condition_variable mCondition;		std::condition_variable mCondition;
	std::deque<RawBuffer> mBuffersReceived;		std::deque<RawBuffer> mBuffersReceived GUARDED_BY(mMutex);
	};		};

	#endif // AUDIO_TEST_UTILS_H_		#endif // AUDIO_TEST_UTILS_H_

media/libaudioclient/tests/audiorecord_tests.cpp

+8 −4

media/libaudioclient/tests/audiorouting_tests.cpp

+4 −3

Original line number	Original line	Diff line number	Diff line
	@@ -120,10 +120,12 @@ TEST_F(AudioRecordTest, TestAudioCbNotifier) {
	EXPECT_EQ(OK, mAC->getAudioRecordHandle()->addAudioDeviceCallback(cb));		EXPECT_EQ(OK, mAC->getAudioRecordHandle()->addAudioDeviceCallback(cb));
	EXPECT_EQ(OK, mAC->start()) << "record creation failed";		EXPECT_EQ(OK, mAC->start()) << "record creation failed";
	EXPECT_EQ(OK, cb->waitForAudioDeviceCb());		EXPECT_EQ(OK, cb->waitForAudioDeviceCb());
	EXPECT_EQ(AUDIO_IO_HANDLE_NONE, cbOld->mAudioIo);		const auto [oldAudioIo, oldDeviceId] = cbOld->getLastPortAndDevice();
	EXPECT_EQ(AUDIO_PORT_HANDLE_NONE, cbOld->mDeviceId);		EXPECT_EQ(AUDIO_IO_HANDLE_NONE, oldAudioIo);
	EXPECT_NE(AUDIO_IO_HANDLE_NONE, cb->mAudioIo);		EXPECT_EQ(AUDIO_PORT_HANDLE_NONE, oldDeviceId);
	EXPECT_NE(AUDIO_PORT_HANDLE_NONE, cb->mDeviceId);		const auto [audioIo, deviceId] = cb->getLastPortAndDevice();
			EXPECT_NE(AUDIO_IO_HANDLE_NONE, audioIo);
			EXPECT_NE(AUDIO_PORT_HANDLE_NONE, deviceId);
	EXPECT_EQ(BAD_VALUE, mAC->getAudioRecordHandle()->removeAudioDeviceCallback(nullptr));		EXPECT_EQ(BAD_VALUE, mAC->getAudioRecordHandle()->removeAudioDeviceCallback(nullptr));
	EXPECT_EQ(INVALID_OPERATION, mAC->getAudioRecordHandle()->removeAudioDeviceCallback(cbOld));		EXPECT_EQ(INVALID_OPERATION, mAC->getAudioRecordHandle()->removeAudioDeviceCallback(cbOld));
	EXPECT_EQ(OK, mAC->getAudioRecordHandle()->removeAudioDeviceCallback(cb));		EXPECT_EQ(OK, mAC->getAudioRecordHandle()->removeAudioDeviceCallback(cb));
	@@ -174,6 +176,7 @@ TEST_F(AudioRecordTest, TestGetSetMarker) {
	<< "getMarkerPosition() failed";		<< "getMarkerPosition() failed";
	EXPECT_EQ(OK, mAC->start()) << "start recording failed";		EXPECT_EQ(OK, mAC->start()) << "start recording failed";
	EXPECT_EQ(OK, mAC->audioProcess()) << "audioProcess failed";		EXPECT_EQ(OK, mAC->audioProcess()) << "audioProcess failed";
			// TODO(b/348658586): Properly synchronize callback updates with the test thread.
	EXPECT_EQ(marker, mAC->mMarkerPosition)		EXPECT_EQ(marker, mAC->mMarkerPosition)
	<< "configured marker and received marker are different";		<< "configured marker and received marker are different";
	EXPECT_EQ(mAC->mReceivedCbMarkerAtPosition, mAC->mMarkerPosition)		EXPECT_EQ(mAC->mReceivedCbMarkerAtPosition, mAC->mMarkerPosition)
	@@ -189,6 +192,7 @@ TEST_F(AudioRecordTest, TestGetSetMarkerPeriodical) {
	<< "getPositionUpdatePeriod() failed";		<< "getPositionUpdatePeriod() failed";
	EXPECT_EQ(OK, mAC->start()) << "start recording failed";		EXPECT_EQ(OK, mAC->start()) << "start recording failed";
	EXPECT_EQ(OK, mAC->audioProcess()) << "audioProcess failed";		EXPECT_EQ(OK, mAC->audioProcess()) << "audioProcess failed";
			// TODO(b/348658586): Properly synchronize callback updates with the test thread.
	EXPECT_EQ(marker, mAC->mMarkerPeriod) << "configured marker and received marker are different";		EXPECT_EQ(marker, mAC->mMarkerPeriod) << "configured marker and received marker are different";
	EXPECT_EQ(mAC->mReceivedCbMarkerCount, mAC->mNumFramesToRecord / mAC->mMarkerPeriod)		EXPECT_EQ(mAC->mReceivedCbMarkerCount, mAC->mNumFramesToRecord / mAC->mMarkerPeriod)
	<< "configured marker and received cb marker are different";		<< "configured marker and received cb marker are different";

Original line number	Original line	Diff line number	Diff line
	@@ -64,16 +64,17 @@ TEST(AudioTrackTest, TestPerformanceMode) {
	EXPECT_EQ(OK, ap->start()) << "audio track start failed";		EXPECT_EQ(OK, ap->start()) << "audio track start failed";
	EXPECT_EQ(OK, ap->onProcess());		EXPECT_EQ(OK, ap->onProcess());
	EXPECT_EQ(OK, cb->waitForAudioDeviceCb());		EXPECT_EQ(OK, cb->waitForAudioDeviceCb());
	EXPECT_TRUE(checkPatchPlayback(cb->mAudioIo, cb->mDeviceId));		const auto [audioIo, deviceId] = cb->getLastPortAndDevice();
			EXPECT_TRUE(checkPatchPlayback(audioIo, deviceId));
	EXPECT_NE(0, ap->getAudioTrackHandle()->getFlags() & output_flags[i]);		EXPECT_NE(0, ap->getAudioTrackHandle()->getFlags() & output_flags[i]);
	audio_patch patch;		audio_patch patch;
	EXPECT_EQ(OK, getPatchForOutputMix(cb->mAudioIo, patch));		EXPECT_EQ(OK, getPatchForOutputMix(audioIo, patch));
	if (output_flags[i] != AUDIO_OUTPUT_FLAG_FAST) {		if (output_flags[i] != AUDIO_OUTPUT_FLAG_FAST) {
	// A "normal" output can still have a FastMixer, depending on the buffer size.		// A "normal" output can still have a FastMixer, depending on the buffer size.
	// Thus, a fast track can be created on a mix port which does not have the FAST flag.		// Thus, a fast track can be created on a mix port which does not have the FAST flag.
	for (auto j = 0; j < patch.num_sources; j++) {		for (auto j = 0; j < patch.num_sources; j++) {
	if (patch.sources[j].type == AUDIO_PORT_TYPE_MIX &&		if (patch.sources[j].type == AUDIO_PORT_TYPE_MIX &&
	patch.sources[j].ext.mix.handle == cb->mAudioIo) {		patch.sources[j].ext.mix.handle == audioIo) {
	SCOPED_TRACE(dumpPortConfig(patch.sources[j]));		SCOPED_TRACE(dumpPortConfig(patch.sources[j]));
	EXPECT_NE(0, patch.sources[j].flags.output & output_flags[i])		EXPECT_NE(0, patch.sources[j].flags.output & output_flags[i])
	<< "expected output flag "		<< "expected output flag "