From 16d654790192069ae43e357ac3bbd47c83d823d5 Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Fri, 20 Apr 2018 23:40:13 -0700 Subject: [PATCH 1/8] Audio V4: Improve VTS error messages This does not change the test logic or constraints. It only improves the error messages. Bug: 77307068 Test: atest VtsHalAudioV4_0TargetTest Change-Id: I6c79ddd014d4ab8aba4f1d0b918888f83dfb63ad Signed-off-by: Kevin Rocard --- .../test/utility/include/utility/PrettyPrintAudioTypes.h | 1 + .../core/4.0/vts/functional/AudioPrimaryHidlHalTest.cpp | 9 +++++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/audio/common/all-versions/test/utility/include/utility/PrettyPrintAudioTypes.h b/audio/common/all-versions/test/utility/include/utility/PrettyPrintAudioTypes.h index 88a67e077f..fd218200bf 100644 --- a/audio/common/all-versions/test/utility/include/utility/PrettyPrintAudioTypes.h +++ b/audio/common/all-versions/test/utility/include/utility/PrettyPrintAudioTypes.h @@ -48,6 +48,7 @@ namespace common { namespace AUDIO_HAL_VERSION { DEFINE_GTEST_PRINT_TO(AudioConfig) DEFINE_GTEST_PRINT_TO(AudioDevice) +DEFINE_GTEST_PRINT_TO(AudioFormat) DEFINE_GTEST_PRINT_TO(AudioChannelMask) } // namespace AUDIO_HAL_VERSION } // namespace common diff --git a/audio/core/4.0/vts/functional/AudioPrimaryHidlHalTest.cpp b/audio/core/4.0/vts/functional/AudioPrimaryHidlHalTest.cpp index 9484ddd4ac..5ce2cdc0be 100644 --- a/audio/core/4.0/vts/functional/AudioPrimaryHidlHalTest.cpp +++ b/audio/core/4.0/vts/functional/AudioPrimaryHidlHalTest.cpp @@ -727,11 +727,12 @@ static void testCapabilityGetter(const string& name, IStream* stream, ASSERT_OK(ret); if (currentMustBeSupported) { + ASSERT_NE(0U, capabilities.size()) << name << " must not return an empty list"; Property currentValue = extract((stream->*getter)()); - EXPECT_NE(std::find(capabilities.begin(), capabilities.end(), currentValue), - capabilities.end()) - << "current " << name << " is not in the list of the supported ones " - << toString(capabilities); + EXPECT_TRUE(std::find(capabilities.begin(), capabilities.end(), currentValue) != + capabilities.end()) + << "value returned by " << name << "() = " << testing::PrintToString(currentValue) + << " is not in the list of the supported ones " << toString(capabilities); } // Check that all declared supported values are indeed supported From 6512b6050291fbfc1ddf7d084fbc6102916c03cc Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Sat, 21 Apr 2018 00:25:08 -0700 Subject: [PATCH 2/8] Audio V4: some legacy getSupported can return NOT_SUPPORTED Legacy implementation through getParameter can not return a status_t. This is problematic for - getSupportedChannelMasks(format) - getSupportedSampleRate(format) as they should be able to return NOT_SUPPORTED if they do not support the provided format. In that case, allow the legacy implementation to return an empty string that will be converted to NOT_SUPPORTED. Test: atest VtsHalAudioV4_0TargetTest Bug: 77307068 Change-Id: I78c37caf059885e3d33e6a308876dbc0e3ef7145 Signed-off-by: Kevin Rocard --- .../include/core/all-versions/default/Stream.impl.h | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/audio/core/all-versions/default/include/core/all-versions/default/Stream.impl.h b/audio/core/all-versions/default/include/core/all-versions/default/Stream.impl.h index 7415112545..72d7a3760a 100644 --- a/audio/core/all-versions/default/include/core/all-versions/default/Stream.impl.h +++ b/audio/core/all-versions/default/include/core/all-versions/default/Stream.impl.h @@ -101,11 +101,15 @@ Return Stream::getSupportedSampleRates(AudioFormat format, halSampleRates = samplingRatesFromString(halListValue.string(), AudioParameter::valueListSeparator); sampleRates.setToExternal(halSampleRates.editArray(), halSampleRates.size()); + // Legacy get_parameter does not return a status_t, thus can not advertise of failure. + // Note that this method must succeed (non empty list) if the format is supported. + if (sampleRates.size() == 0) { + result = Result::NOT_SUPPORTED; + } } #ifdef AUDIO_HAL_VERSION_2_0 _hidl_cb(sampleRates); -#endif -#ifdef AUDIO_HAL_VERSION_4_0 +#elif AUDIO_HAL_VERSION_4_0 _hidl_cb(result, sampleRates); #endif return Void(); @@ -126,6 +130,11 @@ Return Stream::getSupportedChannelMasks(AudioFormat format, for (size_t i = 0; i < halChannelMasks.size(); ++i) { channelMasks[i] = AudioChannelBitfield(halChannelMasks[i]); } + // Legacy get_parameter does not return a status_t, thus can not advertise of failure. + // Note that this method must succeed (non empty list) if the format is supported. + if (channelMasks.size() == 0) { + result = Result::NOT_SUPPORTED; + } } #ifdef AUDIO_HAL_VERSION_2_0 _hidl_cb(channelMasks); From 92dcce09245a48fa04caaec4cfbf31859615fd4c Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Sat, 21 Apr 2018 00:28:29 -0700 Subject: [PATCH 3/8] Audio V4: XSD device category was missing hearing aid Bug: 77307068 Test: atest VtsHalAudioV4_0TargetTest Change-Id: I9de9b826d01fe90c1a7d16631d0f808b29ad4775 Signed-off-by: Kevin Rocard --- audio/4.0/config/audio_policy_configuration.xsd | 1 + 1 file changed, 1 insertion(+) diff --git a/audio/4.0/config/audio_policy_configuration.xsd b/audio/4.0/config/audio_policy_configuration.xsd index 14e4fd6192..ee17fc983b 100644 --- a/audio/4.0/config/audio_policy_configuration.xsd +++ b/audio/4.0/config/audio_policy_configuration.xsd @@ -525,6 +525,7 @@ + From dea4c3803fb07822a08c382dc26af32804c32772 Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Sat, 21 Apr 2018 00:32:06 -0700 Subject: [PATCH 4/8] Audio V4: setMode VTS incorrectly assume 0 was invalid Thus the test was always incorrectly failing. Also improve the test error messages. Bug: 77307068 Test: VtsHalAudioV4_0TargetTest Change-Id: I95db94ed99f7ca32af35422e36a95084e72279d8 Signed-off-by: Kevin Rocard --- .../utility/include/utility/PrettyPrintAudioTypes.h | 1 + .../4.0/vts/functional/AudioPrimaryHidlHalTest.cpp | 13 +++++-------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/audio/common/all-versions/test/utility/include/utility/PrettyPrintAudioTypes.h b/audio/common/all-versions/test/utility/include/utility/PrettyPrintAudioTypes.h index fd218200bf..05239accfb 100644 --- a/audio/common/all-versions/test/utility/include/utility/PrettyPrintAudioTypes.h +++ b/audio/common/all-versions/test/utility/include/utility/PrettyPrintAudioTypes.h @@ -47,6 +47,7 @@ DEFINE_GTEST_PRINT_TO(Result) namespace common { namespace AUDIO_HAL_VERSION { DEFINE_GTEST_PRINT_TO(AudioConfig) +DEFINE_GTEST_PRINT_TO(AudioMode) DEFINE_GTEST_PRINT_TO(AudioDevice) DEFINE_GTEST_PRINT_TO(AudioFormat) DEFINE_GTEST_PRINT_TO(AudioChannelMask) diff --git a/audio/core/4.0/vts/functional/AudioPrimaryHidlHalTest.cpp b/audio/core/4.0/vts/functional/AudioPrimaryHidlHalTest.cpp index 5ce2cdc0be..c8593e9efa 100644 --- a/audio/core/4.0/vts/functional/AudioPrimaryHidlHalTest.cpp +++ b/audio/core/4.0/vts/functional/AudioPrimaryHidlHalTest.cpp @@ -1272,19 +1272,16 @@ TEST_F(AudioPrimaryHidlTest, setVoiceVolume) { } TEST_F(AudioPrimaryHidlTest, setMode) { - doc::test( - "Make sure setMode always succeeds if mode is valid " - "and fails otherwise"); + doc::test("Make sure setMode always succeeds if mode is valid and fails otherwise"); // Test Invalid values - for (int mode : {-1, 0, int(AudioMode::IN_COMMUNICATION) + 1}) { - SCOPED_TRACE("mode=" + to_string(mode)); - ASSERT_RESULT(Result::INVALID_ARGUMENTS, device->setMode(AudioMode(mode))); + for (int mode : {-2, -1, int(AudioMode::IN_COMMUNICATION) + 1}) { + ASSERT_RESULT(Result::INVALID_ARGUMENTS, device->setMode(AudioMode(mode))) + << "mode=" << mode; } // Test valid values for (AudioMode mode : {AudioMode::IN_CALL, AudioMode::IN_COMMUNICATION, AudioMode::RINGTONE, AudioMode::NORMAL /* Make sure to leave the test in normal mode */}) { - SCOPED_TRACE("mode=" + toString(mode)); - ASSERT_OK(device->setMode(mode)); + ASSERT_OK(device->setMode(mode)) << "mode=" << toString(mode); } } From cff6813819524e584069d9a007aa70279892b1c8 Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Mon, 23 Apr 2018 15:28:18 -0700 Subject: [PATCH 5/8] Audio V4: fix invalid SupportedChannelMasks test Test was calling getSupportedSamplingRate instead of channel mask. Bug: 77307068 Test: atest VtsHalAudioV4_0TargetTest Change-Id: Ib6e9d017793edfc95853cab0c2955b8c801bc66f Signed-off-by: Kevin Rocard --- audio/core/4.0/vts/functional/AudioPrimaryHidlHalTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/audio/core/4.0/vts/functional/AudioPrimaryHidlHalTest.cpp b/audio/core/4.0/vts/functional/AudioPrimaryHidlHalTest.cpp index c8593e9efa..4a7318036e 100644 --- a/audio/core/4.0/vts/functional/AudioPrimaryHidlHalTest.cpp +++ b/audio/core/4.0/vts/functional/AudioPrimaryHidlHalTest.cpp @@ -758,7 +758,7 @@ Result getSupportedChannelMasks(IStream* stream, hidl_vec>& channels) { Result res; EXPECT_OK( - stream->getSupportedSampleRates(extract(stream->getFormat()), returnIn(res, channels))); + stream->getSupportedChannelMasks(extract(stream->getFormat()), returnIn(res, channels))); return res; } From 64c3932736558609f7cf657b2a63e90a937f663c Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Mon, 23 Apr 2018 17:09:44 -0700 Subject: [PATCH 6/8] Audio V4: factorize analyzeStatus This factorization had not been ported from the AOSP patch. The code is functionally identical. Test: compile Bug: 69010523 Change-Id: Ied3a657d7c219b580eb32377789096f6b2f6dc19 Signed-off-by: Kevin Rocard --- .../core/all-versions/default/Device.impl.h | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/audio/core/all-versions/default/include/core/all-versions/default/Device.impl.h b/audio/core/all-versions/default/include/core/all-versions/default/Device.impl.h index 37b7124a26..230b8de243 100644 --- a/audio/core/all-versions/default/include/core/all-versions/default/Device.impl.h +++ b/audio/core/all-versions/default/include/core/all-versions/default/Device.impl.h @@ -41,23 +41,7 @@ Device::~Device() { } Result Device::analyzeStatus(const char* funcName, int status) { - if (status != 0) { - ALOGW("Device %p %s: %s", mDevice, funcName, strerror(-status)); - } - switch (status) { - case 0: - return Result::OK; - case -EINVAL: - return Result::INVALID_ARGUMENTS; - case -ENODATA: - return Result::INVALID_STATE; - case -ENODEV: - return Result::NOT_INITIALIZED; - case -ENOSYS: - return Result::NOT_SUPPORTED; - default: - return Result::INVALID_STATE; - } + return util::analyzeStatus("Device", funcName, status); } void Device::closeInputStream(audio_stream_in_t* stream) { From 82cef8d3f9404835800c25039b3ea6009fc5da07 Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Mon, 23 Apr 2018 17:18:46 -0700 Subject: [PATCH 7/8] Audio V4: VTS Pause was testing resume Bug: 74037175 Test: VtsHalAudioV4_0TargetTest Change-Id: I9d77a59e69452e0891dc4e8e2cc2a14fef4fda41 Signed-off-by: Kevin Rocard --- audio/core/4.0/vts/functional/AudioPrimaryHidlHalTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/audio/core/4.0/vts/functional/AudioPrimaryHidlHalTest.cpp b/audio/core/4.0/vts/functional/AudioPrimaryHidlHalTest.cpp index 4a7318036e..7a0bec0154 100644 --- a/audio/core/4.0/vts/functional/AudioPrimaryHidlHalTest.cpp +++ b/audio/core/4.0/vts/functional/AudioPrimaryHidlHalTest.cpp @@ -1196,7 +1196,7 @@ TEST_P(OutputStreamTest, Pause) { doc::partialTest("The output stream does not support pause"); return; } - ASSERT_RESULT(Result::INVALID_STATE, stream->resume()); + ASSERT_RESULT(Result::INVALID_STATE, stream->pause()); } static void testDrain(IStreamOut* stream, AudioDrain type) { From d2f6be72ef843810e808b75a3971db069d928d25 Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Mon, 23 Apr 2018 21:39:43 -0700 Subject: [PATCH 8/8] Audio V4: Add missing VTS Test: atest VtsHalAudioV4_0TargetTest Bug: 74037175 Change-Id: I2f70f2ec6d4b1ee015de6b4937832903d945fdeb Signed-off-by: Kevin Rocard --- .../functional/AudioPrimaryHidlHalTest.cpp | 96 ++++++++++++++++--- 1 file changed, 84 insertions(+), 12 deletions(-) diff --git a/audio/core/4.0/vts/functional/AudioPrimaryHidlHalTest.cpp b/audio/core/4.0/vts/functional/AudioPrimaryHidlHalTest.cpp index 7a0bec0154..de0df400e5 100644 --- a/audio/core/4.0/vts/functional/AudioPrimaryHidlHalTest.cpp +++ b/audio/core/4.0/vts/functional/AudioPrimaryHidlHalTest.cpp @@ -55,6 +55,7 @@ using std::vector; using ::android::sp; using ::android::hardware::Return; using ::android::hardware::hidl_bitfield; +using ::android::hardware::hidl_enum_iterator; using ::android::hardware::hidl_handle; using ::android::hardware::hidl_string; using ::android::hardware::hidl_vec; @@ -81,6 +82,7 @@ using ::android::hardware::audio::V4_0::SourceMetadata; using ::android::hardware::audio::V4_0::SinkMetadata; using ::android::hardware::audio::common::V4_0::AudioChannelMask; using ::android::hardware::audio::common::V4_0::AudioConfig; +using ::android::hardware::audio::common::V4_0::AudioContentType; using ::android::hardware::audio::common::V4_0::AudioDevice; using ::android::hardware::audio::common::V4_0::AudioFormat; using ::android::hardware::audio::common::V4_0::AudioHandleConsts; @@ -91,6 +93,7 @@ using ::android::hardware::audio::common::V4_0::AudioMode; using ::android::hardware::audio::common::V4_0::AudioOffloadInfo; using ::android::hardware::audio::common::V4_0::AudioOutputFlag; using ::android::hardware::audio::common::V4_0::AudioSource; +using ::android::hardware::audio::common::V4_0::AudioUsage; using ::android::hardware::audio::common::V4_0::ThreadInfo; using ::android::hardware::audio::common::utils::mkBitfield; @@ -140,11 +143,11 @@ class AudioHidlTest : public HidlTest { sp AudioHidlTest::devicesFactory; TEST_F(AudioHidlTest, GetAudioDevicesFactoryService) { - doc::test("test the getService (called in SetUp)"); + doc::test("Test the getService (called in SetUp)"); } TEST_F(AudioHidlTest, OpenDeviceInvalidParameter) { - doc::test("test passing an invalid parameter to openDevice"); + doc::test("Test passing an invalid parameter to openDevice"); Result result; sp device; ASSERT_OK(devicesFactory->openDevice("Non existing device", returnIn(result, device))); @@ -152,6 +155,19 @@ TEST_F(AudioHidlTest, OpenDeviceInvalidParameter) { ASSERT_TRUE(device == nullptr); } +TEST_F(AudioHidlTest, OpenPrimaryDeviceUsingGetDevice) { + doc::test("Calling openDevice(\"primary\") should return the primary device."); + Result result; + sp baseDevice; + ASSERT_OK(devicesFactory->openDevice("primary", returnIn(result, baseDevice))); + ASSERT_OK(result); + ASSERT_TRUE(baseDevice != nullptr); + + Return> primaryDevice = IPrimaryDevice::castFrom(baseDevice); + ASSERT_TRUE(primaryDevice.isOk()); + ASSERT_TRUE(sp(primaryDevice) != nullptr); +} + ////////////////////////////////////////////////////////////////////////////// /////////////////////////////// openDevice primary /////////////////////////// ////////////////////////////////////////////////////////////////////////////// @@ -165,14 +181,11 @@ class AudioPrimaryHidlTest : public AudioHidlTest { if (device == nullptr) { Result result; - sp baseDevice; - ASSERT_OK(devicesFactory->openDevice("primary", returnIn(result, baseDevice))); + ASSERT_OK(devicesFactory->openPrimaryDevice(returnIn(result, device))); ASSERT_OK(result); - ASSERT_TRUE(baseDevice != nullptr); + ASSERT_TRUE(device != nullptr); environment->registerTearDown([] { device.clear(); }); - device = IPrimaryDevice::castFrom(baseDevice); - ASSERT_TRUE(device != nullptr); } } @@ -600,13 +613,17 @@ class OutputStreamTest : public OpenStreamTest { const AudioConfig& config = GetParam(); // TODO: test all flag combination auto flags = hidl_bitfield(AudioOutputFlag::NONE); - SourceMetadata metadata = {{{}}}; // create on track metadata testOpen( [&](AudioIoHandle handle, AudioConfig config, auto cb) { - return device->openOutputStream(handle, address, config, flags, metadata, cb); + return device->openOutputStream(handle, address, config, flags, initialMetadata, + cb); }, config); } + + protected: + const SourceMetadata initialMetadata = { + {{AudioUsage::MEDIA, AudioContentType::MUSIC, 1 /* gain */}}}; }; TEST_P(OutputStreamTest, OpenOutputStreamTest) { doc::test( @@ -637,13 +654,15 @@ class InputStreamTest : public OpenStreamTest { const AudioConfig& config = GetParam(); // TODO: test all supported flags and source auto flags = hidl_bitfield(AudioInputFlag::NONE); - SinkMetadata metadata = {{{AudioSource::DEFAULT, 1}}}; testOpen( [&](AudioIoHandle handle, AudioConfig config, auto cb) { - return device->openInputStream(handle, address, config, flags, metadata, cb); + return device->openInputStream(handle, address, config, flags, initialMetadata, cb); }, config); } + + protected: + const SinkMetadata initialMetadata = {{{AudioSource::DEFAULT, 1 /* gain */}}}; }; TEST_P(InputStreamTest, OpenInputStreamTest) { @@ -1040,8 +1059,30 @@ TEST_P(InputStreamTest, getCapturePosition) { ASSERT_RESULT(invalidStateOrNotSupported, res); } +TEST_P(InputStreamTest, updateSinkMetadata) { + doc::test("The HAL should not crash on metadata change"); + + hidl_enum_iterator range; + // Test all possible track configuration + for (AudioSource source : range) { + for (float volume : {0.0, 0.5, 1.0}) { + const SinkMetadata metadata = {{{source, volume}}}; + ASSERT_OK(stream->updateSinkMetadata(metadata)) + << "source=" << toString(source) << ", volume=" << volume; + } + } + + // Do not test concurrent capture as this is not officially supported + + // Set no metadata as if all stream track had stopped + ASSERT_OK(stream->updateSinkMetadata({})); + + // Restore initial + ASSERT_OK(stream->updateSinkMetadata(initialMetadata)); +} + ////////////////////////////////////////////////////////////////////////////// -///////////////////////////////// StreamIn /////////////////////////////////// +///////////////////////////////// StreamOut ////////////////////////////////// ////////////////////////////////////////////////////////////////////////////// TEST_P(OutputStreamTest, getLatency) { @@ -1262,6 +1303,37 @@ TEST_P(OutputStreamTest, SelectPresentation) { ASSERT_RESULT(okOrNotSupported, stream->selectPresentation(0, 0)); } +TEST_P(OutputStreamTest, updateSourceMetadata) { + doc::test("The HAL should not crash on metadata change"); + + hidl_enum_iterator usageRange; + hidl_enum_iterator contentRange; + // Test all possible track configuration + for (auto usage : usageRange) { + for (auto content : contentRange) { + for (float volume : {0.0, 0.5, 1.0}) { + const SourceMetadata metadata = {{{usage, content, volume}}}; + ASSERT_OK(stream->updateSourceMetadata(metadata)) + << "usage=" << toString(usage) << ", content=" << toString(content) + << ", volume=" << volume; + } + } + } + + // Set many track of different configuration + ASSERT_OK(stream->updateSourceMetadata( + {{{AudioUsage::MEDIA, AudioContentType::MUSIC, 0.1}, + {AudioUsage::VOICE_COMMUNICATION, AudioContentType::SPEECH, 1.0}, + {AudioUsage::ALARM, AudioContentType::SONIFICATION, 0.0}, + {AudioUsage::ASSISTANT, AudioContentType::UNKNOWN, 0.3}}})); + + // Set no metadata as if all stream track had stopped + ASSERT_OK(stream->updateSourceMetadata({})); + + // Restore initial + ASSERT_OK(stream->updateSourceMetadata(initialMetadata)); +} + ////////////////////////////////////////////////////////////////////////////// /////////////////////////////// PrimaryDevice //////////////////////////////// //////////////////////////////////////////////////////////////////////////////