From fcf186bd5949f3bbb4274f8f7bf4ab9e4497d541 Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Wed, 3 May 2017 11:16:05 -0700 Subject: [PATCH 1/2] Audio HAL VTS: Some methods are not optional Some mandatory methods were allowed to return NOT_SUPPORTED although their implementations is mandatory. Test: vts-tradefed run vts --module VtsHalAudioV2_0Target Test: call/play music/record/video... Bug: 36311550 Change-Id: Ibe4b3cf73257309975ed11269a38315051fa9064 Signed-off-by: Kevin Rocard --- audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp index 83a1db04c8..f2e7aacfcd 100644 --- a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp +++ b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp @@ -1076,7 +1076,7 @@ static void testPrepareForReading(IStreamIn* stream, uint32_t frameSize, ASSERT_OK(stream->prepareForReading( frameSize, framesCount, [&res](auto r, auto&, auto&, auto&, auto&) { res = r; })); - EXPECT_RESULT(invalidArgsOrNotSupported, res); + EXPECT_RESULT(Result::INVALID_ARGUMENTS, res); } TEST_P(InputStreamTest, PrepareForReadingWithZeroBuffer) { @@ -1144,7 +1144,7 @@ static void testPrepareForWriting(IStreamOut* stream, uint32_t frameSize, ASSERT_OK(stream->prepareForWriting( frameSize, framesCount, [&res](auto r, auto&, auto&, auto&, auto&) { res = r; })); - EXPECT_RESULT(invalidArgsOrNotSupported, res); + EXPECT_RESULT(Result::INVALID_ARGUMENTS, res); } TEST_P(OutputStreamTest, PrepareForWriteWithZeroBuffer) { From a1d6ea4ba76c96cd613ee81eb204bc3041a219f7 Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Mon, 8 May 2017 17:08:11 -0700 Subject: [PATCH 2/2] Audio HAL: A speech volume outside of [0,1] is an error Hals are supposed to received normalized volumes, between 0 and 1. Previously volumes outside [0,1] were clamp to this range. This clamping has the capability to hide bugs thus return an error if such volume is received. Test: vts-tradefed run vts --module VtsHalAudioV2_0Target Test: call/play music/record/video... Bug: 36311550 Change-Id: Iab70f9c651540ea2434d10939d28c1c842db19e0 Signed-off-by: Kevin Rocard --- audio/2.0/default/PrimaryDevice.cpp | 5 +++++ audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp | 10 +++------- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/audio/2.0/default/PrimaryDevice.cpp b/audio/2.0/default/PrimaryDevice.cpp index 4e8f30f924..746d873806 100644 --- a/audio/2.0/default/PrimaryDevice.cpp +++ b/audio/2.0/default/PrimaryDevice.cpp @@ -17,6 +17,7 @@ #define LOG_TAG "PrimaryDeviceHAL" #include "PrimaryDevice.h" +#include "Util.h" namespace android { namespace hardware { @@ -126,6 +127,10 @@ Return PrimaryDevice::debugDump(const hidl_handle& fd) { // Methods from ::android::hardware::audio::V2_0::IPrimaryDevice follow. Return PrimaryDevice::setVoiceVolume(float volume) { + if (!isGainNormalized(volume)) { + ALOGW("Can not set a voice volume (%f) outside [0,1]", volume); + return Result::INVALID_ARGUMENTS; + } return mDevice->analyzeStatus( "set_voice_volume", mDevice->device()->set_voice_volume(mDevice->device(), volume)); diff --git a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp index f2e7aacfcd..27f7aa8096 100644 --- a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp +++ b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp @@ -1038,16 +1038,12 @@ static void testUnitaryGain(std::function(float)> setGain) { for (float value : (float[]){-INFINITY, -1.0, 1.0 + std::numeric_limits::epsilon(), 2.0, INFINITY, NAN}) { - SCOPED_TRACE("value=" + to_string(value)); - // FIXME: NAN should never be accepted - // FIXME: Missing api doc. What should the impl do if the volume is - // outside [0,1] ? - ASSERT_RESULT(Result::INVALID_ARGUMENTS, setGain(value)); + EXPECT_RESULT(Result::INVALID_ARGUMENTS, setGain(value)) << "value=" + << value; } // Do not consider -0.0 as an invalid value as it is == with 0.0 for (float value : {-0.0, 0.0, 0.01, 0.5, 0.09, 1.0 /* Restore volume*/}) { - SCOPED_TRACE("value=" + to_string(value)); - ASSERT_OK(setGain(value)); + EXPECT_OK(setGain(value)) << "value=" << value; } }