From a1d6ea4ba76c96cd613ee81eb204bc3041a219f7 Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Mon, 8 May 2017 17:08:11 -0700 Subject: [PATCH] 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; } }