From 422afc131a003d9958ac306bc9360da3e3569157 Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Wed, 20 Nov 2019 16:58:12 -0800 Subject: [PATCH] audio: Factor out IStream operations into a helper class Bug: 114451103 Test: atest VtsHalAudioV5_0TargetTest atest VtsHalAudioV6_0TargetTest Change-Id: I7c1b16df2b52fd5279ff1f431bfc7269fb9cd8f6 --- .../4.0/AudioPrimaryHidlHalTest.cpp | 28 ++--- .../vts/functional/AudioPrimaryHidlHalTest.h | 104 +++++++++++------- 2 files changed, 71 insertions(+), 61 deletions(-) diff --git a/audio/core/all-versions/vts/functional/4.0/AudioPrimaryHidlHalTest.cpp b/audio/core/all-versions/vts/functional/4.0/AudioPrimaryHidlHalTest.cpp index 81f963d34d..709b7cd369 100644 --- a/audio/core/all-versions/vts/functional/4.0/AudioPrimaryHidlHalTest.cpp +++ b/audio/core/all-versions/vts/functional/4.0/AudioPrimaryHidlHalTest.cpp @@ -52,7 +52,6 @@ TEST_P(AudioHidlDeviceTest, GetMicrophonesTest) { doc::test( "Make sure getMicrophones always succeeds" "and getActiveMicrophones always succeeds when recording from these microphones."); - AudioIoHandle ioHandle = (AudioIoHandle)AudioHandleConsts::AUDIO_IO_HANDLE_NONE; AudioConfig config{}; config.channelMask = mkEnumBitfield(AudioChannelMask::IN_MONO); config.sampleRateHz = 8000; @@ -65,18 +64,14 @@ TEST_P(AudioHidlDeviceTest, GetMicrophonesTest) { continue; } sp stream; + StreamHelper helper(stream); AudioConfig suggestedConfig{}; - ASSERT_OK(getDevice()->openInputStream(ioHandle, microphone.deviceAddress, config, - flags, initMetadata, - returnIn(res, stream, suggestedConfig))); - if (res != Result::OK) { - ASSERT_TRUE(stream == nullptr); - AudioConfig suggestedConfigRetry{}; - ASSERT_OK(getDevice()->openInputStream( - ioHandle, microphone.deviceAddress, suggestedConfig, flags, initMetadata, - returnIn(res, stream, suggestedConfigRetry))); - } - ASSERT_OK(res); + ASSERT_NO_FATAL_FAILURE(helper.open( + [&](AudioIoHandle handle, AudioConfig config, auto cb) { + return getDevice()->openInputStream(handle, microphone.deviceAddress, + config, flags, initMetadata, cb); + }, + config, &res, &suggestedConfig)); hidl_vec activeMicrophones; Result readRes; typedef MessageQueue CommandMQ; @@ -110,13 +105,8 @@ TEST_P(AudioHidlDeviceTest, GetMicrophonesTest) { ASSERT_OK(res); ASSERT_NE(0U, activeMicrophones.size()); } - stream->close(); -#if MAJOR_VERSION <= 5 - // Workaround for b/139329877. Ensures the stream gets closed on the audio hal side. - stream.clear(); - IPCThreadState::self()->flushCommands(); - usleep(1000); -#endif + helper.close(true /*clear*/, &res); + ASSERT_OK(res); if (efGroup) { EventFlag::deleteEventFlag(&efGroup); } diff --git a/audio/core/all-versions/vts/functional/AudioPrimaryHidlHalTest.h b/audio/core/all-versions/vts/functional/AudioPrimaryHidlHalTest.h index 38a06c4740..d0d39e8683 100644 --- a/audio/core/all-versions/vts/functional/AudioPrimaryHidlHalTest.h +++ b/audio/core/all-versions/vts/functional/AudioPrimaryHidlHalTest.h @@ -809,62 +809,84 @@ TEST_P(AudioHidlDeviceTest, DebugDumpInvalidArguments) { ////////////////////////// open{Output,Input}Stream ////////////////////////// ////////////////////////////////////////////////////////////////////////////// +// This class is also used by some device tests. template -class OpenStreamTest : public AudioHidlTestWithDeviceConfigParameter { - protected: +class StreamHelper { + public: + // StreamHelper doesn't own the stream, this is for simpler stream lifetime management. + explicit StreamHelper(sp& stream) : mStream(stream) {} template - void testOpen(Open openStream, const AudioConfig& config) { + void open(Open openStream, const AudioConfig& config, Result* res, + AudioConfig* suggestedConfigPtr) { // FIXME: Open a stream without an IOHandle // This is not required to be accepted by hal implementations AudioIoHandle ioHandle = (AudioIoHandle)AudioHandleConsts::AUDIO_IO_HANDLE_NONE; AudioConfig suggestedConfig{}; - ASSERT_OK(openStream(ioHandle, config, returnIn(res, stream, suggestedConfig))); - - // TODO: only allow failure for RecommendedPlaybackAudioConfig - switch (res) { + bool retryWithSuggestedConfig = true; + if (suggestedConfigPtr == nullptr) { + suggestedConfigPtr = &suggestedConfig; + retryWithSuggestedConfig = false; + } + ASSERT_OK(openStream(ioHandle, config, returnIn(*res, mStream, *suggestedConfigPtr))); + switch (*res) { case Result::OK: - ASSERT_TRUE(stream != nullptr); - audioConfig = config; + ASSERT_TRUE(mStream != nullptr); + *suggestedConfigPtr = config; break; case Result::INVALID_ARGUMENTS: - ASSERT_TRUE(stream == nullptr); - AudioConfig suggestedConfigRetry; - // Could not open stream with config, try again with the - // suggested one - ASSERT_OK(openStream(ioHandle, suggestedConfig, - returnIn(res, stream, suggestedConfigRetry))); - // This time it must succeed - ASSERT_OK(res); - ASSERT_TRUE(stream != nullptr); - audioConfig = suggestedConfig; + ASSERT_TRUE(mStream == nullptr); + if (retryWithSuggestedConfig) { + AudioConfig suggestedConfigRetry; + ASSERT_OK(openStream(ioHandle, *suggestedConfigPtr, + returnIn(*res, mStream, suggestedConfigRetry))); + ASSERT_OK(*res); + ASSERT_TRUE(mStream != nullptr); + } break; default: - FAIL() << "Invalid return status: " << ::testing::PrintToString(res); + FAIL() << "Invalid return status: " << ::testing::PrintToString(*res); } + } + void close(bool clear, Result* res) { + auto ret = mStream->close(); + EXPECT_TRUE(ret.isOk()); + *res = ret; + if (clear) { + mStream.clear(); +#if MAJOR_VERSION <= 5 + // FIXME: there is no way to know when the remote IStream is being destroyed + // Binder does not support testing if an object is alive, thus + // wait for 100ms to let the binder destruction propagates and + // the remote device has the time to be destroyed. + // flushCommand makes sure all local command are sent, thus should reduce + // the latency between local and remote destruction. + IPCThreadState::self()->flushCommands(); + usleep(100 * 1000); +#endif + } + } + + private: + sp& mStream; +}; + +template +class OpenStreamTest : public AudioHidlTestWithDeviceConfigParameter { + protected: + OpenStreamTest() : AudioHidlTestWithDeviceConfigParameter(), helper(stream) {} + template + void testOpen(Open openStream, const AudioConfig& config) { + // TODO: only allow failure for RecommendedPlaybackAudioConfig + ASSERT_NO_FATAL_FAILURE(helper.open(openStream, config, &res, &audioConfig)); open = true; } - Return closeStream() { + Result closeStream(bool clear = true) { open = false; - auto res = stream->close(); - stream.clear(); - waitForStreamDestruction(); + helper.close(clear, &res); return res; } - static void waitForStreamDestruction() { -#if MAJOR_VERSION <= 5 - // FIXME: there is no way to know when the remote IStream is being destroyed - // Binder does not support testing if an object is alive, thus - // wait for 100ms to let the binder destruction propagates and - // the remote device has the time to be destroyed. - // flushCommand makes sure all local command are sent, thus should reduce - // the latency between local and remote destruction. - IPCThreadState::self()->flushCommands(); - usleep(100 * 1000); -#endif - } - private: void TearDown() override { if (open) { @@ -877,6 +899,7 @@ class OpenStreamTest : public AudioHidlTestWithDeviceConfigParameter { AudioConfig audioConfig; DeviceAddress address = {}; sp stream; + StreamHelper helper; bool open = false; }; @@ -1178,11 +1201,8 @@ TEST_IO_STREAM(getMmapPositionNoMmap, "Get a stream Mmap position before mapping TEST_IO_STREAM(close, "Make sure a stream can be closed", ASSERT_OK(closeStream())) // clang-format off TEST_IO_STREAM(closeTwice, "Make sure a stream can not be closed twice", - auto streamCopy = stream; - ASSERT_OK(closeStream()); - ASSERT_RESULT(Result::INVALID_STATE, streamCopy->close()); - streamCopy.clear(); - waitForStreamDestruction()) + ASSERT_OK(closeStream(false /*clear*/)); + ASSERT_EQ(Result::INVALID_STATE, closeStream())) // clang-format on static void testMmapBufferOfInvalidSize(IStream* stream) {