From 72e50e2ef1480fc3d90f0d88c7e9e3595622e75c Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Fri, 5 May 2017 14:02:55 -0700 Subject: [PATCH 01/16] Audio HAL VTS: Fix style on modified files In order to avoid style inconsistency as well as non functional modification in following patches, fix the style of all files modified for the fix of the VTS tests. Patch generated with: $ clang-format -i --style file -- Bug: 36311550 Test: compile Change-Id: I53dbcdabf959a6100e34a2ee4d0f951d525049cb --- audio/2.0/default/Device.cpp | 218 +++--- audio/2.0/default/ParametersUtil.cpp | 20 +- audio/2.0/default/PrimaryDevice.cpp | 100 +-- audio/2.0/default/StreamIn.cpp | 217 +++--- audio/2.0/default/StreamOut.cpp | 270 +++---- .../functional/AudioPrimaryHidlHalTest.cpp | 661 +++++++++++------- audio/2.0/vts/functional/utility/AssertOk.h | 25 +- 7 files changed, 847 insertions(+), 664 deletions(-) diff --git a/audio/2.0/default/Device.cpp b/audio/2.0/default/Device.cpp index da792384eb..9246f8b4e3 100644 --- a/audio/2.0/default/Device.cpp +++ b/audio/2.0/default/Device.cpp @@ -1,18 +1,18 @@ - /* - * Copyright (C) 2016 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ +/* +* Copyright (C) 2016 The Android Open Source Project +* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ #define LOG_TAG "DeviceHAL" //#define LOG_NDEBUG 0 @@ -92,7 +92,8 @@ Device::Device(audio_hw_device_t* device, const char* type) Device::~Device() { int status = audio_hw_device_close(mDevice); - ALOGW_IF(status, "Error closing audio hw device %p: %s", mDevice, strerror(-status)); + ALOGW_IF(status, "Error closing audio hw device %p: %s", mDevice, + strerror(-status)); mDevice = nullptr; } @@ -101,12 +102,18 @@ Result Device::analyzeStatus(const char* funcName, int status) { 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; + 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; } } @@ -129,59 +136,64 @@ int Device::halSetParameters(const char* keysAndValues) { } // Methods from ::android::hardware::audio::V2_0::IDevice follow. -Return Device::initCheck() { +Return Device::initCheck() { return analyzeStatus("init_check", mDevice->init_check(mDevice)); } -Return Device::setMasterVolume(float volume) { +Return Device::setMasterVolume(float volume) { Result retval(Result::NOT_SUPPORTED); if (mDevice->set_master_volume != NULL) { - retval = analyzeStatus("set_master_volume", mDevice->set_master_volume(mDevice, volume)); + retval = analyzeStatus("set_master_volume", + mDevice->set_master_volume(mDevice, volume)); } return retval; } -Return Device::getMasterVolume(getMasterVolume_cb _hidl_cb) { +Return Device::getMasterVolume(getMasterVolume_cb _hidl_cb) { Result retval(Result::NOT_SUPPORTED); float volume = 0; if (mDevice->get_master_volume != NULL) { - retval = analyzeStatus("get_master_volume", mDevice->get_master_volume(mDevice, &volume)); + retval = analyzeStatus("get_master_volume", + mDevice->get_master_volume(mDevice, &volume)); } _hidl_cb(retval, volume); return Void(); } -Return Device::setMicMute(bool mute) { +Return Device::setMicMute(bool mute) { return analyzeStatus("set_mic_mute", mDevice->set_mic_mute(mDevice, mute)); } -Return Device::getMicMute(getMicMute_cb _hidl_cb) { +Return Device::getMicMute(getMicMute_cb _hidl_cb) { bool mute = false; - Result retval = analyzeStatus("get_mic_mute", mDevice->get_mic_mute(mDevice, &mute)); + Result retval = + analyzeStatus("get_mic_mute", mDevice->get_mic_mute(mDevice, &mute)); _hidl_cb(retval, mute); return Void(); } -Return Device::setMasterMute(bool mute) { +Return Device::setMasterMute(bool mute) { Result retval(Result::NOT_SUPPORTED); if (mDevice->set_master_mute != NULL) { - retval = analyzeStatus("set_master_mute", mDevice->set_master_mute(mDevice, mute)); + retval = analyzeStatus("set_master_mute", + mDevice->set_master_mute(mDevice, mute)); } return retval; } -Return Device::getMasterMute(getMasterMute_cb _hidl_cb) { +Return Device::getMasterMute(getMasterMute_cb _hidl_cb) { Result retval(Result::NOT_SUPPORTED); bool mute = false; if (mDevice->get_master_mute != NULL) { - retval = analyzeStatus("get_master_mute", mDevice->get_master_mute(mDevice, &mute)); + retval = analyzeStatus("get_master_mute", + mDevice->get_master_mute(mDevice, &mute)); } _hidl_cb(retval, mute); return Void(); } -Return Device::getInputBufferSize( - const AudioConfig& config, getInputBufferSize_cb _hidl_cb) { +Return Device::getInputBufferSize(const AudioConfig& config, + getInputBufferSize_cb _hidl_cb) { audio_config_t halConfig; HidlUtils::audioConfigToHal(config, &halConfig); size_t halBufferSize = mDevice->get_input_buffer_size(mDevice, &halConfig); @@ -195,29 +207,25 @@ Return Device::getInputBufferSize( return Void(); } -Return Device::openOutputStream( - int32_t ioHandle, - const DeviceAddress& device, - const AudioConfig& config, - AudioOutputFlag flags, - openOutputStream_cb _hidl_cb) { +Return Device::openOutputStream(int32_t ioHandle, + const DeviceAddress& device, + const AudioConfig& config, + AudioOutputFlag flags, + openOutputStream_cb _hidl_cb) { audio_config_t halConfig; HidlUtils::audioConfigToHal(config, &halConfig); - audio_stream_out_t *halStream; - ALOGV("open_output_stream handle: %d devices: %x flags: %#x " - "srate: %d format %#x channels %x address %s", - ioHandle, - static_cast(device.device), static_cast(flags), - halConfig.sample_rate, halConfig.format, halConfig.channel_mask, - deviceAddressToHal(device).c_str()); + audio_stream_out_t* halStream; + ALOGV( + "open_output_stream handle: %d devices: %x flags: %#x " + "srate: %d format %#x channels %x address %s", + ioHandle, static_cast(device.device), + static_cast(flags), halConfig.sample_rate, + halConfig.format, halConfig.channel_mask, + deviceAddressToHal(device).c_str()); int status = mDevice->open_output_stream( - mDevice, - ioHandle, - static_cast(device.device), - static_cast(flags), - &halConfig, - &halStream, - deviceAddressToHal(device).c_str()); + mDevice, ioHandle, static_cast(device.device), + static_cast(flags), &halConfig, &halStream, + deviceAddressToHal(device).c_str()); ALOGV("open_output_stream status %d stream %p", status, halStream); sp streamOut; if (status == OK) { @@ -225,35 +233,32 @@ Return Device::openOutputStream( } AudioConfig suggestedConfig; HidlUtils::audioConfigFromHal(halConfig, &suggestedConfig); - _hidl_cb(analyzeStatus("open_output_stream", status), streamOut, suggestedConfig); + _hidl_cb(analyzeStatus("open_output_stream", status), streamOut, + suggestedConfig); return Void(); } -Return Device::openInputStream( - int32_t ioHandle, - const DeviceAddress& device, - const AudioConfig& config, - AudioInputFlag flags, - AudioSource source, - openInputStream_cb _hidl_cb) { +Return Device::openInputStream(int32_t ioHandle, + const DeviceAddress& device, + const AudioConfig& config, + AudioInputFlag flags, AudioSource source, + openInputStream_cb _hidl_cb) { audio_config_t halConfig; HidlUtils::audioConfigToHal(config, &halConfig); - audio_stream_in_t *halStream; - ALOGV("open_input_stream handle: %d devices: %x flags: %#x " - "srate: %d format %#x channels %x address %s source %d", - ioHandle, - static_cast(device.device), static_cast(flags), - halConfig.sample_rate, halConfig.format, halConfig.channel_mask, - deviceAddressToHal(device).c_str(), static_cast(source)); + audio_stream_in_t* halStream; + ALOGV( + "open_input_stream handle: %d devices: %x flags: %#x " + "srate: %d format %#x channels %x address %s source %d", + ioHandle, static_cast(device.device), + static_cast(flags), halConfig.sample_rate, + halConfig.format, halConfig.channel_mask, + deviceAddressToHal(device).c_str(), + static_cast(source)); int status = mDevice->open_input_stream( - mDevice, - ioHandle, - static_cast(device.device), - &halConfig, - &halStream, - static_cast(flags), - deviceAddressToHal(device).c_str(), - static_cast(source)); + mDevice, ioHandle, static_cast(device.device), + &halConfig, &halStream, static_cast(flags), + deviceAddressToHal(device).c_str(), + static_cast(source)); ALOGV("open_input_stream status %d stream %p", status, halStream); sp streamIn; if (status == OK) { @@ -261,7 +266,8 @@ Return Device::openInputStream( } AudioConfig suggestedConfig; HidlUtils::audioConfigFromHal(halConfig, &suggestedConfig); - _hidl_cb(analyzeStatus("open_input_stream", status), streamIn, suggestedConfig); + _hidl_cb(analyzeStatus("open_input_stream", status), streamIn, + suggestedConfig); return Void(); } @@ -269,23 +275,21 @@ Return Device::supportsAudioPatches() { return version() >= AUDIO_DEVICE_API_VERSION_3_0; } -Return Device::createAudioPatch( - const hidl_vec& sources, - const hidl_vec& sinks, - createAudioPatch_cb _hidl_cb) { +Return Device::createAudioPatch(const hidl_vec& sources, + const hidl_vec& sinks, + createAudioPatch_cb _hidl_cb) { Result retval(Result::NOT_SUPPORTED); AudioPatchHandle patch = 0; if (version() >= AUDIO_DEVICE_API_VERSION_3_0) { - std::unique_ptr halSources(HidlUtils::audioPortConfigsToHal(sources)); - std::unique_ptr halSinks(HidlUtils::audioPortConfigsToHal(sinks)); + std::unique_ptr halSources( + HidlUtils::audioPortConfigsToHal(sources)); + std::unique_ptr halSinks( + HidlUtils::audioPortConfigsToHal(sinks)); audio_patch_handle_t halPatch = AUDIO_PATCH_HANDLE_NONE; retval = analyzeStatus( - "create_audio_patch", - mDevice->create_audio_patch( - mDevice, - sources.size(), &halSources[0], - sinks.size(), &halSinks[0], - &halPatch)); + "create_audio_patch", + mDevice->create_audio_patch(mDevice, sources.size(), &halSources[0], + sinks.size(), &halSinks[0], &halPatch)); if (retval == Result::OK) { patch = static_cast(halPatch); } @@ -294,19 +298,22 @@ Return Device::createAudioPatch( return Void(); } -Return Device::releaseAudioPatch(int32_t patch) { +Return Device::releaseAudioPatch(int32_t patch) { if (version() >= AUDIO_DEVICE_API_VERSION_3_0) { return analyzeStatus( - "release_audio_patch", - mDevice->release_audio_patch(mDevice, static_cast(patch))); + "release_audio_patch", + mDevice->release_audio_patch( + mDevice, static_cast(patch))); } return Result::NOT_SUPPORTED; } -Return Device::getAudioPort(const AudioPort& port, getAudioPort_cb _hidl_cb) { +Return Device::getAudioPort(const AudioPort& port, + getAudioPort_cb _hidl_cb) { audio_port halPort; HidlUtils::audioPortToHal(port, &halPort); - Result retval = analyzeStatus("get_audio_port", mDevice->get_audio_port(mDevice, &halPort)); + Result retval = analyzeStatus("get_audio_port", + mDevice->get_audio_port(mDevice, &halPort)); AudioPort resultPort = port; if (retval == Result::OK) { HidlUtils::audioPortFromHal(halPort, &resultPort); @@ -315,36 +322,39 @@ Return Device::getAudioPort(const AudioPort& port, getAudioPort_cb _hidl_c return Void(); } -Return Device::setAudioPortConfig(const AudioPortConfig& config) { +Return Device::setAudioPortConfig(const AudioPortConfig& config) { if (version() >= AUDIO_DEVICE_API_VERSION_3_0) { struct audio_port_config halPortConfig; HidlUtils::audioPortConfigToHal(config, &halPortConfig); return analyzeStatus( - "set_audio_port_config", mDevice->set_audio_port_config(mDevice, &halPortConfig)); + "set_audio_port_config", + mDevice->set_audio_port_config(mDevice, &halPortConfig)); } return Result::NOT_SUPPORTED; } -Return Device::getHwAvSync() { +Return Device::getHwAvSync() { int halHwAvSync; Result retval = getParam(AudioParameter::keyHwAvSync, &halHwAvSync); return retval == Result::OK ? halHwAvSync : AUDIO_HW_SYNC_INVALID; } -Return Device::setScreenState(bool turnedOn) { +Return Device::setScreenState(bool turnedOn) { return setParam(AudioParameter::keyScreenState, turnedOn); } -Return Device::getParameters(const hidl_vec& keys, getParameters_cb _hidl_cb) { +Return Device::getParameters(const hidl_vec& keys, + getParameters_cb _hidl_cb) { getParametersImpl(keys, _hidl_cb); return Void(); } -Return Device::setParameters(const hidl_vec& parameters) { +Return Device::setParameters( + const hidl_vec& parameters) { return setParametersImpl(parameters); } -Return Device::debugDump(const hidl_handle& fd) { +Return Device::debugDump(const hidl_handle& fd) { if (fd.getNativeHandle() != nullptr && fd->numFds == 1) { analyzeStatus("dump", mDevice->dump(mDevice, fd->data[0])); } diff --git a/audio/2.0/default/ParametersUtil.cpp b/audio/2.0/default/ParametersUtil.cpp index 75a60b9b64..0511557da3 100644 --- a/audio/2.0/default/ParametersUtil.cpp +++ b/audio/2.0/default/ParametersUtil.cpp @@ -51,8 +51,10 @@ Result ParametersUtil::getParam(const char* name, String8* value) { } void ParametersUtil::getParametersImpl( - const hidl_vec& keys, - std::function& parameters)> cb) { + const hidl_vec& keys, + std::function& parameters)> + cb) { AudioParameter halKeys; for (size_t i = 0; i < keys.size(); ++i) { halKeys.addKey(String8(keys[i].c_str())); @@ -79,9 +81,10 @@ void ParametersUtil::getParametersImpl( cb(retval, result); } -std::unique_ptr ParametersUtil::getParams(const AudioParameter& keys) { +std::unique_ptr ParametersUtil::getParams( + const AudioParameter& keys) { String8 paramsAndValues; - char *halValues = halGetParameters(keys.keysToString().string()); + char* halValues = halGetParameters(keys.keysToString().string()); if (halValues != NULL) { paramsAndValues.setTo(halValues); free(halValues); @@ -93,7 +96,8 @@ std::unique_ptr ParametersUtil::getParams(const AudioParameter& Result ParametersUtil::setParam(const char* name, bool value) { AudioParameter param; - param.add(String8(name), String8(value ? AudioParameter::valueOn : AudioParameter::valueOff)); + param.add(String8(name), String8(value ? AudioParameter::valueOn + : AudioParameter::valueOff)); return setParams(param); } @@ -109,10 +113,12 @@ Result ParametersUtil::setParam(const char* name, const char* value) { return setParams(param); } -Result ParametersUtil::setParametersImpl(const hidl_vec& parameters) { +Result ParametersUtil::setParametersImpl( + const hidl_vec& parameters) { AudioParameter params; for (size_t i = 0; i < parameters.size(); ++i) { - params.add(String8(parameters[i].key.c_str()), String8(parameters[i].value.c_str())); + params.add(String8(parameters[i].key.c_str()), + String8(parameters[i].value.c_str())); } return setParams(params); } diff --git a/audio/2.0/default/PrimaryDevice.cpp b/audio/2.0/default/PrimaryDevice.cpp index af0b249f72..6f2268a331 100644 --- a/audio/2.0/default/PrimaryDevice.cpp +++ b/audio/2.0/default/PrimaryDevice.cpp @@ -30,56 +30,52 @@ PrimaryDevice::PrimaryDevice(audio_hw_device_t* device) PrimaryDevice::~PrimaryDevice() {} // Methods from ::android::hardware::audio::V2_0::IDevice follow. -Return PrimaryDevice::initCheck() { +Return PrimaryDevice::initCheck() { return mDevice->initCheck(); } -Return PrimaryDevice::setMasterVolume(float volume) { +Return PrimaryDevice::setMasterVolume(float volume) { return mDevice->setMasterVolume(volume); } -Return PrimaryDevice::getMasterVolume(getMasterVolume_cb _hidl_cb) { +Return PrimaryDevice::getMasterVolume(getMasterVolume_cb _hidl_cb) { return mDevice->getMasterVolume(_hidl_cb); } -Return PrimaryDevice::setMicMute(bool mute) { +Return PrimaryDevice::setMicMute(bool mute) { return mDevice->setMicMute(mute); } -Return PrimaryDevice::getMicMute(getMicMute_cb _hidl_cb) { +Return PrimaryDevice::getMicMute(getMicMute_cb _hidl_cb) { return mDevice->getMicMute(_hidl_cb); } -Return PrimaryDevice::setMasterMute(bool mute) { +Return PrimaryDevice::setMasterMute(bool mute) { return mDevice->setMasterMute(mute); } -Return PrimaryDevice::getMasterMute(getMasterMute_cb _hidl_cb) { +Return PrimaryDevice::getMasterMute(getMasterMute_cb _hidl_cb) { return mDevice->getMasterMute(_hidl_cb); } -Return PrimaryDevice::getInputBufferSize( - const AudioConfig& config, getInputBufferSize_cb _hidl_cb) { +Return PrimaryDevice::getInputBufferSize(const AudioConfig& config, + getInputBufferSize_cb _hidl_cb) { return mDevice->getInputBufferSize(config, _hidl_cb); } -Return PrimaryDevice::openOutputStream( - int32_t ioHandle, - const DeviceAddress& device, - const AudioConfig& config, - AudioOutputFlag flags, - openOutputStream_cb _hidl_cb) { +Return PrimaryDevice::openOutputStream(int32_t ioHandle, + const DeviceAddress& device, + const AudioConfig& config, + AudioOutputFlag flags, + openOutputStream_cb _hidl_cb) { return mDevice->openOutputStream(ioHandle, device, config, flags, _hidl_cb); } Return PrimaryDevice::openInputStream( - int32_t ioHandle, - const DeviceAddress& device, - const AudioConfig& config, - AudioInputFlag flags, - AudioSource source, - openInputStream_cb _hidl_cb) { - return mDevice->openInputStream(ioHandle, device, config, flags, source, _hidl_cb); + int32_t ioHandle, const DeviceAddress& device, const AudioConfig& config, + AudioInputFlag flags, AudioSource source, openInputStream_cb _hidl_cb) { + return mDevice->openInputStream(ioHandle, device, config, flags, source, + _hidl_cb); } Return PrimaryDevice::supportsAudioPatches() { @@ -87,82 +83,85 @@ Return PrimaryDevice::supportsAudioPatches() { } Return PrimaryDevice::createAudioPatch( - const hidl_vec& sources, - const hidl_vec& sinks, - createAudioPatch_cb _hidl_cb) { + const hidl_vec& sources, + const hidl_vec& sinks, createAudioPatch_cb _hidl_cb) { return mDevice->createAudioPatch(sources, sinks, _hidl_cb); } -Return PrimaryDevice::releaseAudioPatch(int32_t patch) { +Return PrimaryDevice::releaseAudioPatch(int32_t patch) { return mDevice->releaseAudioPatch(patch); } -Return PrimaryDevice::getAudioPort(const AudioPort& port, getAudioPort_cb _hidl_cb) { +Return PrimaryDevice::getAudioPort(const AudioPort& port, + getAudioPort_cb _hidl_cb) { return mDevice->getAudioPort(port, _hidl_cb); } -Return PrimaryDevice::setAudioPortConfig(const AudioPortConfig& config) { +Return PrimaryDevice::setAudioPortConfig( + const AudioPortConfig& config) { return mDevice->setAudioPortConfig(config); } -Return PrimaryDevice::getHwAvSync() { +Return PrimaryDevice::getHwAvSync() { return mDevice->getHwAvSync(); } -Return PrimaryDevice::setScreenState(bool turnedOn) { +Return PrimaryDevice::setScreenState(bool turnedOn) { return mDevice->setScreenState(turnedOn); } -Return PrimaryDevice::getParameters( - const hidl_vec& keys, getParameters_cb _hidl_cb) { +Return PrimaryDevice::getParameters(const hidl_vec& keys, + getParameters_cb _hidl_cb) { return mDevice->getParameters(keys, _hidl_cb); } -Return PrimaryDevice::setParameters(const hidl_vec& parameters) { +Return PrimaryDevice::setParameters( + const hidl_vec& parameters) { return mDevice->setParameters(parameters); } -Return PrimaryDevice::debugDump(const hidl_handle& fd) { +Return PrimaryDevice::debugDump(const hidl_handle& fd) { return mDevice->debugDump(fd); } - // Methods from ::android::hardware::audio::V2_0::IPrimaryDevice follow. -Return PrimaryDevice::setVoiceVolume(float volume) { +Return PrimaryDevice::setVoiceVolume(float volume) { return mDevice->analyzeStatus( - "set_voice_volume", - mDevice->device()->set_voice_volume(mDevice->device(), volume)); + "set_voice_volume", + mDevice->device()->set_voice_volume(mDevice->device(), volume)); } -Return PrimaryDevice::setMode(AudioMode mode) { +Return PrimaryDevice::setMode(AudioMode mode) { return mDevice->analyzeStatus( - "set_mode", - mDevice->device()->set_mode(mDevice->device(), static_cast(mode))); + "set_mode", mDevice->device()->set_mode( + mDevice->device(), static_cast(mode))); } -Return PrimaryDevice::getBtScoNrecEnabled(getBtScoNrecEnabled_cb _hidl_cb) { +Return PrimaryDevice::getBtScoNrecEnabled( + getBtScoNrecEnabled_cb _hidl_cb) { bool enabled; Result retval = mDevice->getParam(AudioParameter::keyBtNrec, &enabled); _hidl_cb(retval, enabled); return Void(); } -Return PrimaryDevice::setBtScoNrecEnabled(bool enabled) { +Return PrimaryDevice::setBtScoNrecEnabled(bool enabled) { return mDevice->setParam(AudioParameter::keyBtNrec, enabled); } -Return PrimaryDevice::getBtScoWidebandEnabled(getBtScoWidebandEnabled_cb _hidl_cb) { +Return PrimaryDevice::getBtScoWidebandEnabled( + getBtScoWidebandEnabled_cb _hidl_cb) { bool enabled; Result retval = mDevice->getParam(AUDIO_PARAMETER_KEY_BT_SCO_WB, &enabled); _hidl_cb(retval, enabled); return Void(); } -Return PrimaryDevice::setBtScoWidebandEnabled(bool enabled) { +Return PrimaryDevice::setBtScoWidebandEnabled(bool enabled) { return mDevice->setParam(AUDIO_PARAMETER_KEY_BT_SCO_WB, enabled); } -Return PrimaryDevice::getTtyMode(getTtyMode_cb _hidl_cb) { +Return PrimaryDevice::getTtyMode(getTtyMode_cb _hidl_cb) { int halMode; Result retval = mDevice->getParam(AUDIO_PARAMETER_KEY_TTY_MODE, &halMode); TtyMode mode = retval == Result::OK ? TtyMode(halMode) : TtyMode::OFF; @@ -170,18 +169,19 @@ Return PrimaryDevice::getTtyMode(getTtyMode_cb _hidl_cb) { return Void(); } -Return PrimaryDevice::setTtyMode(IPrimaryDevice::TtyMode mode) { - return mDevice->setParam(AUDIO_PARAMETER_KEY_TTY_MODE, static_cast(mode)); +Return PrimaryDevice::setTtyMode(IPrimaryDevice::TtyMode mode) { + return mDevice->setParam(AUDIO_PARAMETER_KEY_TTY_MODE, + static_cast(mode)); } -Return PrimaryDevice::getHacEnabled(getHacEnabled_cb _hidl_cb) { +Return PrimaryDevice::getHacEnabled(getHacEnabled_cb _hidl_cb) { bool enabled; Result retval = mDevice->getParam(AUDIO_PARAMETER_KEY_HAC, &enabled); _hidl_cb(retval, enabled); return Void(); } -Return PrimaryDevice::setHacEnabled(bool enabled) { +Return PrimaryDevice::setHacEnabled(bool enabled) { return mDevice->setParam(AUDIO_PARAMETER_KEY_HAC, enabled); } diff --git a/audio/2.0/default/StreamIn.cpp b/audio/2.0/default/StreamIn.cpp index e5a1a55524..2262697cde 100644 --- a/audio/2.0/default/StreamIn.cpp +++ b/audio/2.0/default/StreamIn.cpp @@ -20,8 +20,8 @@ #include #include -#include #include +#include #include "StreamIn.h" @@ -38,30 +38,26 @@ using ::android::hardware::audio::common::V2_0::ThreadInfo; namespace { class ReadThread : public Thread { - public: + public: // ReadThread's lifespan never exceeds StreamIn's lifespan. - ReadThread(std::atomic* stop, - audio_stream_in_t* stream, - StreamIn::CommandMQ* commandMQ, - StreamIn::DataMQ* dataMQ, - StreamIn::StatusMQ* statusMQ, - EventFlag* efGroup) - : Thread(false /*canCallJava*/), - mStop(stop), - mStream(stream), - mCommandMQ(commandMQ), - mDataMQ(dataMQ), - mStatusMQ(statusMQ), - mEfGroup(efGroup), - mBuffer(nullptr) { - } + ReadThread(std::atomic* stop, audio_stream_in_t* stream, + StreamIn::CommandMQ* commandMQ, StreamIn::DataMQ* dataMQ, + StreamIn::StatusMQ* statusMQ, EventFlag* efGroup) + : Thread(false /*canCallJava*/), + mStop(stop), + mStream(stream), + mCommandMQ(commandMQ), + mDataMQ(dataMQ), + mStatusMQ(statusMQ), + mEfGroup(efGroup), + mBuffer(nullptr) {} bool init() { - mBuffer.reset(new(std::nothrow) uint8_t[mDataMQ->getQuantumCount()]); + mBuffer.reset(new (std::nothrow) uint8_t[mDataMQ->getQuantumCount()]); return mBuffer != nullptr; } virtual ~ReadThread() {} - private: + private: std::atomic* mStop; audio_stream_in_t* mStream; StreamIn::CommandMQ* mCommandMQ; @@ -82,8 +78,10 @@ void ReadThread::doRead() { size_t availableToWrite = mDataMQ->availableToWrite(); size_t requestedToRead = mParameters.params.read; if (requestedToRead > availableToWrite) { - ALOGW("truncating read data from %d to %d due to insufficient data queue space", - (int32_t)requestedToRead, (int32_t)availableToWrite); + ALOGW( + "truncating read data from %d to %d due to insufficient data queue " + "space", + (int32_t)requestedToRead, (int32_t)availableToWrite); requestedToRead = availableToWrite; } ssize_t readResult = mStream->read(mStream, &mBuffer[0], requestedToRead); @@ -101,16 +99,20 @@ void ReadThread::doRead() { void ReadThread::doGetCapturePosition() { mStatus.retval = StreamIn::getCapturePositionImpl( - mStream, &mStatus.reply.capturePosition.frames, &mStatus.reply.capturePosition.time); + mStream, &mStatus.reply.capturePosition.frames, + &mStatus.reply.capturePosition.time); } bool ReadThread::threadLoop() { - // This implementation doesn't return control back to the Thread until it decides to stop, + // This implementation doesn't return control back to the Thread until it + // decides to stop, // as the Thread uses mutexes, and this can lead to priority inversion. - while(!std::atomic_load_explicit(mStop, std::memory_order_acquire)) { + while (!std::atomic_load_explicit(mStop, std::memory_order_acquire)) { uint32_t efState = 0; - mEfGroup->wait(static_cast(MessageQueueFlagBits::NOT_FULL), &efState); - if (!(efState & static_cast(MessageQueueFlagBits::NOT_FULL))) { + mEfGroup->wait(static_cast(MessageQueueFlagBits::NOT_FULL), + &efState); + if (!(efState & + static_cast(MessageQueueFlagBits::NOT_FULL))) { continue; // Nothing to do. } if (!mCommandMQ->read(&mParameters)) { @@ -125,7 +127,8 @@ bool ReadThread::threadLoop() { doGetCapturePosition(); break; default: - ALOGE("Unknown read thread command code %d", mParameters.command); + ALOGE("Unknown read thread command code %d", + mParameters.command); mStatus.retval = Result::NOT_SUPPORTED; break; } @@ -141,11 +144,13 @@ bool ReadThread::threadLoop() { } // namespace StreamIn::StreamIn(const sp& device, audio_stream_in_t* stream) - : mIsClosed(false), mDevice(device), mStream(stream), - mStreamCommon(new Stream(&stream->common)), - mStreamMmap(new StreamMmap(stream)), - mEfGroup(nullptr), mStopReadThread(false) { -} + : mIsClosed(false), + mDevice(device), + mStream(stream), + mStreamCommon(new Stream(&stream->common)), + mStreamMmap(new StreamMmap(stream)), + mEfGroup(nullptr), + mStopReadThread(false) {} StreamIn::~StreamIn() { ATRACE_CALL(); @@ -157,102 +162,108 @@ StreamIn::~StreamIn() { } if (mEfGroup) { status_t status = EventFlag::deleteEventFlag(&mEfGroup); - ALOGE_IF(status, "read MQ event flag deletion error: %s", strerror(-status)); + ALOGE_IF(status, "read MQ event flag deletion error: %s", + strerror(-status)); } mDevice->closeInputStream(mStream); mStream = nullptr; } // Methods from ::android::hardware::audio::V2_0::IStream follow. -Return StreamIn::getFrameSize() { +Return StreamIn::getFrameSize() { return audio_stream_in_frame_size(mStream); } -Return StreamIn::getFrameCount() { +Return StreamIn::getFrameCount() { return mStreamCommon->getFrameCount(); } -Return StreamIn::getBufferSize() { +Return StreamIn::getBufferSize() { return mStreamCommon->getBufferSize(); } -Return StreamIn::getSampleRate() { +Return StreamIn::getSampleRate() { return mStreamCommon->getSampleRate(); } -Return StreamIn::getSupportedSampleRates(getSupportedSampleRates_cb _hidl_cb) { +Return StreamIn::getSupportedSampleRates( + getSupportedSampleRates_cb _hidl_cb) { return mStreamCommon->getSupportedSampleRates(_hidl_cb); } -Return StreamIn::setSampleRate(uint32_t sampleRateHz) { +Return StreamIn::setSampleRate(uint32_t sampleRateHz) { return mStreamCommon->setSampleRate(sampleRateHz); } -Return StreamIn::getChannelMask() { +Return StreamIn::getChannelMask() { return mStreamCommon->getChannelMask(); } -Return StreamIn::getSupportedChannelMasks(getSupportedChannelMasks_cb _hidl_cb) { +Return StreamIn::getSupportedChannelMasks( + getSupportedChannelMasks_cb _hidl_cb) { return mStreamCommon->getSupportedChannelMasks(_hidl_cb); } -Return StreamIn::setChannelMask(AudioChannelMask mask) { +Return StreamIn::setChannelMask(AudioChannelMask mask) { return mStreamCommon->setChannelMask(mask); } -Return StreamIn::getFormat() { +Return StreamIn::getFormat() { return mStreamCommon->getFormat(); } -Return StreamIn::getSupportedFormats(getSupportedFormats_cb _hidl_cb) { +Return StreamIn::getSupportedFormats(getSupportedFormats_cb _hidl_cb) { return mStreamCommon->getSupportedFormats(_hidl_cb); } -Return StreamIn::setFormat(AudioFormat format) { +Return StreamIn::setFormat(AudioFormat format) { return mStreamCommon->setFormat(format); } -Return StreamIn::getAudioProperties(getAudioProperties_cb _hidl_cb) { +Return StreamIn::getAudioProperties(getAudioProperties_cb _hidl_cb) { return mStreamCommon->getAudioProperties(_hidl_cb); } -Return StreamIn::addEffect(uint64_t effectId) { +Return StreamIn::addEffect(uint64_t effectId) { return mStreamCommon->addEffect(effectId); } -Return StreamIn::removeEffect(uint64_t effectId) { +Return StreamIn::removeEffect(uint64_t effectId) { return mStreamCommon->removeEffect(effectId); } -Return StreamIn::standby() { +Return StreamIn::standby() { return mStreamCommon->standby(); } -Return StreamIn::getDevice() { +Return StreamIn::getDevice() { return mStreamCommon->getDevice(); } -Return StreamIn::setDevice(const DeviceAddress& address) { +Return StreamIn::setDevice(const DeviceAddress& address) { return mStreamCommon->setDevice(address); } -Return StreamIn::setConnectedState(const DeviceAddress& address, bool connected) { +Return StreamIn::setConnectedState(const DeviceAddress& address, + bool connected) { return mStreamCommon->setConnectedState(address, connected); } -Return StreamIn::setHwAvSync(uint32_t hwAvSync) { +Return StreamIn::setHwAvSync(uint32_t hwAvSync) { return mStreamCommon->setHwAvSync(hwAvSync); } -Return StreamIn::getParameters(const hidl_vec& keys, getParameters_cb _hidl_cb) { +Return StreamIn::getParameters(const hidl_vec& keys, + getParameters_cb _hidl_cb) { return mStreamCommon->getParameters(keys, _hidl_cb); } -Return StreamIn::setParameters(const hidl_vec& parameters) { +Return StreamIn::setParameters( + const hidl_vec& parameters) { return mStreamCommon->setParameters(parameters); } -Return StreamIn::debugDump(const hidl_handle& fd) { +Return StreamIn::debugDump(const hidl_handle& fd) { return mStreamCommon->debugDump(fd); } @@ -264,16 +275,17 @@ Return StreamIn::stop() { return mStreamMmap->stop(); } -Return StreamIn::createMmapBuffer(int32_t minSizeFrames, createMmapBuffer_cb _hidl_cb) { +Return StreamIn::createMmapBuffer(int32_t minSizeFrames, + createMmapBuffer_cb _hidl_cb) { return mStreamMmap->createMmapBuffer( - minSizeFrames, audio_stream_in_frame_size(mStream), _hidl_cb); + minSizeFrames, audio_stream_in_frame_size(mStream), _hidl_cb); } Return StreamIn::getMmapPosition(getMmapPosition_cb _hidl_cb) { return mStreamMmap->getMmapPosition(_hidl_cb); } -Return StreamIn::close() { +Return StreamIn::close() { if (mIsClosed) return Result::INVALID_STATE; mIsClosed = true; if (mReadThread.get()) { @@ -286,9 +298,10 @@ Return StreamIn::close() { } // Methods from ::android::hardware::audio::V2_0::IStreamIn follow. -Return StreamIn::getAudioSource(getAudioSource_cb _hidl_cb) { +Return StreamIn::getAudioSource(getAudioSource_cb _hidl_cb) { int halSource; - Result retval = mStreamCommon->getParam(AudioParameter::keyInputSource, &halSource); + Result retval = + mStreamCommon->getParam(AudioParameter::keyInputSource, &halSource); AudioSource source(AudioSource::DEFAULT); if (retval == Result::OK) { source = AudioSource(halSource); @@ -297,68 +310,69 @@ Return StreamIn::getAudioSource(getAudioSource_cb _hidl_cb) { return Void(); } -Return StreamIn::setGain(float gain) { +Return StreamIn::setGain(float gain) { return Stream::analyzeStatus("set_gain", mStream->set_gain(mStream, gain)); } -Return StreamIn::prepareForReading( - uint32_t frameSize, uint32_t framesCount, prepareForReading_cb _hidl_cb) { +Return StreamIn::prepareForReading(uint32_t frameSize, + uint32_t framesCount, + prepareForReading_cb _hidl_cb) { status_t status; - ThreadInfo threadInfo = { 0, 0 }; + ThreadInfo threadInfo = {0, 0}; // Create message queues. if (mDataMQ) { ALOGE("the client attempts to call prepareForReading twice"); - _hidl_cb(Result::INVALID_STATE, - CommandMQ::Descriptor(), DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + _hidl_cb(Result::INVALID_STATE, CommandMQ::Descriptor(), + DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); return Void(); } std::unique_ptr tempCommandMQ(new CommandMQ(1)); if (frameSize > std::numeric_limits::max() / framesCount) { - ALOGE("Requested buffer is too big, %d*%d can not fit in size_t", frameSize, framesCount); - _hidl_cb(Result::INVALID_ARGUMENTS, - CommandMQ::Descriptor(), DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + ALOGE("Requested buffer is too big, %d*%d can not fit in size_t", + frameSize, framesCount); + _hidl_cb(Result::INVALID_ARGUMENTS, CommandMQ::Descriptor(), + DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); return Void(); } - std::unique_ptr tempDataMQ(new DataMQ(frameSize * framesCount, true /* EventFlag */)); + std::unique_ptr tempDataMQ( + new DataMQ(frameSize * framesCount, true /* EventFlag */)); std::unique_ptr tempStatusMQ(new StatusMQ(1)); - if (!tempCommandMQ->isValid() || !tempDataMQ->isValid() || !tempStatusMQ->isValid()) { + if (!tempCommandMQ->isValid() || !tempDataMQ->isValid() || + !tempStatusMQ->isValid()) { ALOGE_IF(!tempCommandMQ->isValid(), "command MQ is invalid"); ALOGE_IF(!tempDataMQ->isValid(), "data MQ is invalid"); ALOGE_IF(!tempStatusMQ->isValid(), "status MQ is invalid"); - _hidl_cb(Result::INVALID_ARGUMENTS, - CommandMQ::Descriptor(), DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + _hidl_cb(Result::INVALID_ARGUMENTS, CommandMQ::Descriptor(), + DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); return Void(); } EventFlag* tempRawEfGroup{}; - status = EventFlag::createEventFlag(tempDataMQ->getEventFlagWord(), &tempRawEfGroup); - std::unique_ptr tempElfGroup(tempRawEfGroup, [](auto *ef) { - EventFlag::deleteEventFlag(&ef); }); + status = EventFlag::createEventFlag(tempDataMQ->getEventFlagWord(), + &tempRawEfGroup); + std::unique_ptr tempElfGroup( + tempRawEfGroup, [](auto* ef) { EventFlag::deleteEventFlag(&ef); }); if (status != OK || !tempElfGroup) { ALOGE("failed creating event flag for data MQ: %s", strerror(-status)); - _hidl_cb(Result::INVALID_ARGUMENTS, - CommandMQ::Descriptor(), DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + _hidl_cb(Result::INVALID_ARGUMENTS, CommandMQ::Descriptor(), + DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); return Void(); } // Create and launch the thread. auto tempReadThread = std::make_unique( - &mStopReadThread, - mStream, - tempCommandMQ.get(), - tempDataMQ.get(), - tempStatusMQ.get(), - tempElfGroup.get()); + &mStopReadThread, mStream, tempCommandMQ.get(), tempDataMQ.get(), + tempStatusMQ.get(), tempElfGroup.get()); if (!tempReadThread->init()) { - _hidl_cb(Result::INVALID_ARGUMENTS, - CommandMQ::Descriptor(), DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + _hidl_cb(Result::INVALID_ARGUMENTS, CommandMQ::Descriptor(), + DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); return Void(); } status = tempReadThread->run("reader", PRIORITY_URGENT_AUDIO); if (status != OK) { ALOGW("failed to start reader thread: %s", strerror(-status)); - _hidl_cb(Result::INVALID_ARGUMENTS, - CommandMQ::Descriptor(), DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + _hidl_cb(Result::INVALID_ARGUMENTS, CommandMQ::Descriptor(), + DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); return Void(); } @@ -369,28 +383,27 @@ Return StreamIn::prepareForReading( mEfGroup = tempElfGroup.release(); threadInfo.pid = getpid(); threadInfo.tid = mReadThread->getTid(); - _hidl_cb(Result::OK, - *mCommandMQ->getDesc(), *mDataMQ->getDesc(), *mStatusMQ->getDesc(), - threadInfo); + _hidl_cb(Result::OK, *mCommandMQ->getDesc(), *mDataMQ->getDesc(), + *mStatusMQ->getDesc(), threadInfo); return Void(); } -Return StreamIn::getInputFramesLost() { +Return StreamIn::getInputFramesLost() { return mStream->get_input_frames_lost(mStream); } // static -Result StreamIn::getCapturePositionImpl( - audio_stream_in_t *stream, uint64_t *frames, uint64_t *time) { +Result StreamIn::getCapturePositionImpl(audio_stream_in_t* stream, + uint64_t* frames, uint64_t* time) { Result retval(Result::NOT_SUPPORTED); if (stream->get_capture_position != NULL) return retval; int64_t halFrames, halTime; retval = Stream::analyzeStatus( - "get_capture_position", - stream->get_capture_position(stream, &halFrames, &halTime), - // HAL may have a stub function, always returning ENOSYS, don't - // spam the log in this case. - ENOSYS); + "get_capture_position", + stream->get_capture_position(stream, &halFrames, &halTime), + // HAL may have a stub function, always returning ENOSYS, don't + // spam the log in this case. + ENOSYS); if (retval == Result::OK) { *frames = halFrames; *time = halTime; @@ -398,14 +411,14 @@ Result StreamIn::getCapturePositionImpl( return retval; }; -Return StreamIn::getCapturePosition(getCapturePosition_cb _hidl_cb) { +Return StreamIn::getCapturePosition(getCapturePosition_cb _hidl_cb) { uint64_t frames = 0, time = 0; Result retval = getCapturePositionImpl(mStream, &frames, &time); _hidl_cb(retval, frames, time); return Void(); } -} // namespace implementation +} // namespace implementation } // namespace V2_0 } // namespace audio } // namespace hardware diff --git a/audio/2.0/default/StreamOut.cpp b/audio/2.0/default/StreamOut.cpp index 3339b63006..63b9ae352c 100644 --- a/audio/2.0/default/StreamOut.cpp +++ b/audio/2.0/default/StreamOut.cpp @@ -37,30 +37,26 @@ using ::android::hardware::audio::common::V2_0::ThreadInfo; namespace { class WriteThread : public Thread { - public: + public: // WriteThread's lifespan never exceeds StreamOut's lifespan. - WriteThread(std::atomic* stop, - audio_stream_out_t* stream, - StreamOut::CommandMQ* commandMQ, - StreamOut::DataMQ* dataMQ, - StreamOut::StatusMQ* statusMQ, - EventFlag* efGroup) - : Thread(false /*canCallJava*/), - mStop(stop), - mStream(stream), - mCommandMQ(commandMQ), - mDataMQ(dataMQ), - mStatusMQ(statusMQ), - mEfGroup(efGroup), - mBuffer(nullptr) { - } + WriteThread(std::atomic* stop, audio_stream_out_t* stream, + StreamOut::CommandMQ* commandMQ, StreamOut::DataMQ* dataMQ, + StreamOut::StatusMQ* statusMQ, EventFlag* efGroup) + : Thread(false /*canCallJava*/), + mStop(stop), + mStream(stream), + mCommandMQ(commandMQ), + mDataMQ(dataMQ), + mStatusMQ(statusMQ), + mEfGroup(efGroup), + mBuffer(nullptr) {} bool init() { - mBuffer.reset(new(std::nothrow) uint8_t[mDataMQ->getQuantumCount()]); + mBuffer.reset(new (std::nothrow) uint8_t[mDataMQ->getQuantumCount()]); return mBuffer != nullptr; } virtual ~WriteThread() {} - private: + private: std::atomic* mStop; audio_stream_out_t* mStream; StreamOut::CommandMQ* mCommandMQ; @@ -93,9 +89,8 @@ void WriteThread::doWrite() { void WriteThread::doGetPresentationPosition() { mStatus.retval = StreamOut::getPresentationPositionImpl( - mStream, - &mStatus.reply.presentationPosition.frames, - &mStatus.reply.presentationPosition.timeStamp); + mStream, &mStatus.reply.presentationPosition.frames, + &mStatus.reply.presentationPosition.timeStamp); } void WriteThread::doGetLatency() { @@ -104,12 +99,15 @@ void WriteThread::doGetLatency() { } bool WriteThread::threadLoop() { - // This implementation doesn't return control back to the Thread until it decides to stop, + // This implementation doesn't return control back to the Thread until it + // decides to stop, // as the Thread uses mutexes, and this can lead to priority inversion. - while(!std::atomic_load_explicit(mStop, std::memory_order_acquire)) { + while (!std::atomic_load_explicit(mStop, std::memory_order_acquire)) { uint32_t efState = 0; - mEfGroup->wait(static_cast(MessageQueueFlagBits::NOT_EMPTY), &efState); - if (!(efState & static_cast(MessageQueueFlagBits::NOT_EMPTY))) { + mEfGroup->wait(static_cast(MessageQueueFlagBits::NOT_EMPTY), + &efState); + if (!(efState & + static_cast(MessageQueueFlagBits::NOT_EMPTY))) { continue; // Nothing to do. } if (!mCommandMQ->read(&mStatus.replyTo)) { @@ -142,11 +140,13 @@ bool WriteThread::threadLoop() { } // namespace StreamOut::StreamOut(const sp& device, audio_stream_out_t* stream) - : mIsClosed(false), mDevice(device), mStream(stream), - mStreamCommon(new Stream(&stream->common)), - mStreamMmap(new StreamMmap(stream)), - mEfGroup(nullptr), mStopWriteThread(false) { -} + : mIsClosed(false), + mDevice(device), + mStream(stream), + mStreamCommon(new Stream(&stream->common)), + mStreamMmap(new StreamMmap(stream)), + mEfGroup(nullptr), + mStopWriteThread(false) {} StreamOut::~StreamOut() { ATRACE_CALL(); @@ -158,7 +158,8 @@ StreamOut::~StreamOut() { } if (mEfGroup) { status_t status = EventFlag::deleteEventFlag(&mEfGroup); - ALOGE_IF(status, "write MQ event flag deletion error: %s", strerror(-status)); + ALOGE_IF(status, "write MQ event flag deletion error: %s", + strerror(-status)); } mCallback.clear(); mDevice->closeOutputStream(mStream); @@ -166,100 +167,104 @@ StreamOut::~StreamOut() { } // Methods from ::android::hardware::audio::V2_0::IStream follow. -Return StreamOut::getFrameSize() { +Return StreamOut::getFrameSize() { return audio_stream_out_frame_size(mStream); } -Return StreamOut::getFrameCount() { +Return StreamOut::getFrameCount() { return mStreamCommon->getFrameCount(); } -Return StreamOut::getBufferSize() { +Return StreamOut::getBufferSize() { return mStreamCommon->getBufferSize(); } -Return StreamOut::getSampleRate() { +Return StreamOut::getSampleRate() { return mStreamCommon->getSampleRate(); } -Return StreamOut::getSupportedSampleRates(getSupportedSampleRates_cb _hidl_cb) { +Return StreamOut::getSupportedSampleRates( + getSupportedSampleRates_cb _hidl_cb) { return mStreamCommon->getSupportedSampleRates(_hidl_cb); } -Return StreamOut::setSampleRate(uint32_t sampleRateHz) { +Return StreamOut::setSampleRate(uint32_t sampleRateHz) { return mStreamCommon->setSampleRate(sampleRateHz); } -Return StreamOut::getChannelMask() { +Return StreamOut::getChannelMask() { return mStreamCommon->getChannelMask(); } -Return StreamOut::getSupportedChannelMasks(getSupportedChannelMasks_cb _hidl_cb) { +Return StreamOut::getSupportedChannelMasks( + getSupportedChannelMasks_cb _hidl_cb) { return mStreamCommon->getSupportedChannelMasks(_hidl_cb); } -Return StreamOut::setChannelMask(AudioChannelMask mask) { +Return StreamOut::setChannelMask(AudioChannelMask mask) { return mStreamCommon->setChannelMask(mask); } -Return StreamOut::getFormat() { +Return StreamOut::getFormat() { return mStreamCommon->getFormat(); } -Return StreamOut::getSupportedFormats(getSupportedFormats_cb _hidl_cb) { +Return StreamOut::getSupportedFormats(getSupportedFormats_cb _hidl_cb) { return mStreamCommon->getSupportedFormats(_hidl_cb); } -Return StreamOut::setFormat(AudioFormat format) { +Return StreamOut::setFormat(AudioFormat format) { return mStreamCommon->setFormat(format); } -Return StreamOut::getAudioProperties(getAudioProperties_cb _hidl_cb) { +Return StreamOut::getAudioProperties(getAudioProperties_cb _hidl_cb) { return mStreamCommon->getAudioProperties(_hidl_cb); } -Return StreamOut::addEffect(uint64_t effectId) { +Return StreamOut::addEffect(uint64_t effectId) { return mStreamCommon->addEffect(effectId); } -Return StreamOut::removeEffect(uint64_t effectId) { +Return StreamOut::removeEffect(uint64_t effectId) { return mStreamCommon->removeEffect(effectId); } -Return StreamOut::standby() { +Return StreamOut::standby() { return mStreamCommon->standby(); } -Return StreamOut::getDevice() { +Return StreamOut::getDevice() { return mStreamCommon->getDevice(); } -Return StreamOut::setDevice(const DeviceAddress& address) { +Return StreamOut::setDevice(const DeviceAddress& address) { return mStreamCommon->setDevice(address); } -Return StreamOut::setConnectedState(const DeviceAddress& address, bool connected) { +Return StreamOut::setConnectedState(const DeviceAddress& address, + bool connected) { return mStreamCommon->setConnectedState(address, connected); } -Return StreamOut::setHwAvSync(uint32_t hwAvSync) { +Return StreamOut::setHwAvSync(uint32_t hwAvSync) { return mStreamCommon->setHwAvSync(hwAvSync); } -Return StreamOut::getParameters( - const hidl_vec& keys, getParameters_cb _hidl_cb) { +Return StreamOut::getParameters(const hidl_vec& keys, + getParameters_cb _hidl_cb) { return mStreamCommon->getParameters(keys, _hidl_cb); } -Return StreamOut::setParameters(const hidl_vec& parameters) { +Return StreamOut::setParameters( + const hidl_vec& parameters) { return mStreamCommon->setParameters(parameters); } -Return StreamOut::debugDump(const hidl_handle& fd) { +Return StreamOut::debugDump(const hidl_handle& fd) { return mStreamCommon->debugDump(fd); } -Return StreamOut::close() { +Return StreamOut::close() { if (mIsClosed) return Result::INVALID_STATE; mIsClosed = true; if (mWriteThread.get()) { @@ -272,78 +277,79 @@ Return StreamOut::close() { } // Methods from ::android::hardware::audio::V2_0::IStreamOut follow. -Return StreamOut::getLatency() { +Return StreamOut::getLatency() { return mStream->get_latency(mStream); } -Return StreamOut::setVolume(float left, float right) { +Return StreamOut::setVolume(float left, float right) { Result retval(Result::NOT_SUPPORTED); if (mStream->set_volume != NULL) { retval = Stream::analyzeStatus( - "set_volume", mStream->set_volume(mStream, left, right)); + "set_volume", mStream->set_volume(mStream, left, right)); } return retval; } -Return StreamOut::prepareForWriting( - uint32_t frameSize, uint32_t framesCount, prepareForWriting_cb _hidl_cb) { +Return StreamOut::prepareForWriting(uint32_t frameSize, + uint32_t framesCount, + prepareForWriting_cb _hidl_cb) { status_t status; - ThreadInfo threadInfo = { 0, 0 }; + ThreadInfo threadInfo = {0, 0}; // Create message queues. if (mDataMQ) { ALOGE("the client attempts to call prepareForWriting twice"); - _hidl_cb(Result::INVALID_STATE, - CommandMQ::Descriptor(), DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + _hidl_cb(Result::INVALID_STATE, CommandMQ::Descriptor(), + DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); return Void(); } std::unique_ptr tempCommandMQ(new CommandMQ(1)); if (frameSize > std::numeric_limits::max() / framesCount) { - ALOGE("Requested buffer is too big, %d*%d can not fit in size_t", frameSize, framesCount); - _hidl_cb(Result::INVALID_ARGUMENTS, - CommandMQ::Descriptor(), DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + ALOGE("Requested buffer is too big, %d*%d can not fit in size_t", + frameSize, framesCount); + _hidl_cb(Result::INVALID_ARGUMENTS, CommandMQ::Descriptor(), + DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); return Void(); } - std::unique_ptr tempDataMQ(new DataMQ(frameSize * framesCount, true /* EventFlag */)); + std::unique_ptr tempDataMQ( + new DataMQ(frameSize * framesCount, true /* EventFlag */)); std::unique_ptr tempStatusMQ(new StatusMQ(1)); - if (!tempCommandMQ->isValid() || !tempDataMQ->isValid() || !tempStatusMQ->isValid()) { + if (!tempCommandMQ->isValid() || !tempDataMQ->isValid() || + !tempStatusMQ->isValid()) { ALOGE_IF(!tempCommandMQ->isValid(), "command MQ is invalid"); ALOGE_IF(!tempDataMQ->isValid(), "data MQ is invalid"); ALOGE_IF(!tempStatusMQ->isValid(), "status MQ is invalid"); - _hidl_cb(Result::INVALID_ARGUMENTS, - CommandMQ::Descriptor(), DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + _hidl_cb(Result::INVALID_ARGUMENTS, CommandMQ::Descriptor(), + DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); return Void(); } EventFlag* tempRawEfGroup{}; - status = EventFlag::createEventFlag(tempDataMQ->getEventFlagWord(), &tempRawEfGroup); - std::unique_ptr tempElfGroup(tempRawEfGroup,[](auto *ef) { - EventFlag::deleteEventFlag(&ef); }); + status = EventFlag::createEventFlag(tempDataMQ->getEventFlagWord(), + &tempRawEfGroup); + std::unique_ptr tempElfGroup( + tempRawEfGroup, [](auto* ef) { EventFlag::deleteEventFlag(&ef); }); if (status != OK || !tempElfGroup) { ALOGE("failed creating event flag for data MQ: %s", strerror(-status)); - _hidl_cb(Result::INVALID_ARGUMENTS, - CommandMQ::Descriptor(), DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + _hidl_cb(Result::INVALID_ARGUMENTS, CommandMQ::Descriptor(), + DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); return Void(); } // Create and launch the thread. auto tempWriteThread = std::make_unique( - &mStopWriteThread, - mStream, - tempCommandMQ.get(), - tempDataMQ.get(), - tempStatusMQ.get(), - tempElfGroup.get()); + &mStopWriteThread, mStream, tempCommandMQ.get(), tempDataMQ.get(), + tempStatusMQ.get(), tempElfGroup.get()); if (!tempWriteThread->init()) { - _hidl_cb(Result::INVALID_ARGUMENTS, - CommandMQ::Descriptor(), DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + _hidl_cb(Result::INVALID_ARGUMENTS, CommandMQ::Descriptor(), + DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); return Void(); } status = tempWriteThread->run("writer", PRIORITY_URGENT_AUDIO); if (status != OK) { ALOGW("failed to start writer thread: %s", strerror(-status)); - _hidl_cb(Result::INVALID_ARGUMENTS, - CommandMQ::Descriptor(), DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + _hidl_cb(Result::INVALID_ARGUMENTS, CommandMQ::Descriptor(), + DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); return Void(); } @@ -354,33 +360,34 @@ Return StreamOut::prepareForWriting( mEfGroup = tempElfGroup.release(); threadInfo.pid = getpid(); threadInfo.tid = mWriteThread->getTid(); - _hidl_cb(Result::OK, - *mCommandMQ->getDesc(), *mDataMQ->getDesc(), *mStatusMQ->getDesc(), - threadInfo); + _hidl_cb(Result::OK, *mCommandMQ->getDesc(), *mDataMQ->getDesc(), + *mStatusMQ->getDesc(), threadInfo); return Void(); } -Return StreamOut::getRenderPosition(getRenderPosition_cb _hidl_cb) { +Return StreamOut::getRenderPosition(getRenderPosition_cb _hidl_cb) { uint32_t halDspFrames; Result retval = Stream::analyzeStatus( - "get_render_position", mStream->get_render_position(mStream, &halDspFrames)); + "get_render_position", + mStream->get_render_position(mStream, &halDspFrames)); _hidl_cb(retval, halDspFrames); return Void(); } -Return StreamOut::getNextWriteTimestamp(getNextWriteTimestamp_cb _hidl_cb) { +Return StreamOut::getNextWriteTimestamp( + getNextWriteTimestamp_cb _hidl_cb) { Result retval(Result::NOT_SUPPORTED); int64_t timestampUs = 0; if (mStream->get_next_write_timestamp != NULL) { retval = Stream::analyzeStatus( - "get_next_write_timestamp", - mStream->get_next_write_timestamp(mStream, ×tampUs)); + "get_next_write_timestamp", + mStream->get_next_write_timestamp(mStream, ×tampUs)); } _hidl_cb(retval, timestampUs); return Void(); } -Return StreamOut::setCallback(const sp& callback) { +Return StreamOut::setCallback(const sp& callback) { if (mStream->set_callback == NULL) return Result::NOT_SUPPORTED; int result = mStream->set_callback(mStream, StreamOut::asyncCallback, this); if (result == 0) { @@ -389,14 +396,15 @@ Return StreamOut::setCallback(const sp& callback) { return Stream::analyzeStatus("set_callback", result); } -Return StreamOut::clearCallback() { +Return StreamOut::clearCallback() { if (mStream->set_callback == NULL) return Result::NOT_SUPPORTED; mCallback.clear(); return Result::OK; } // static -int StreamOut::asyncCallback(stream_callback_event_t event, void*, void *cookie) { +int StreamOut::asyncCallback(stream_callback_event_t event, void*, + void* cookie) { wp weakSelf(reinterpret_cast(cookie)); sp self = weakSelf.promote(); if (self == nullptr || self->mCallback == nullptr) return 0; @@ -418,53 +426,57 @@ int StreamOut::asyncCallback(stream_callback_event_t event, void*, void *cookie) return 0; } -Return StreamOut::supportsPauseAndResume(supportsPauseAndResume_cb _hidl_cb) { +Return StreamOut::supportsPauseAndResume( + supportsPauseAndResume_cb _hidl_cb) { _hidl_cb(mStream->pause != NULL, mStream->resume != NULL); return Void(); } -Return StreamOut::pause() { - return mStream->pause != NULL ? - Stream::analyzeStatus("pause", mStream->pause(mStream)) : - Result::NOT_SUPPORTED; +Return StreamOut::pause() { + return mStream->pause != NULL + ? Stream::analyzeStatus("pause", mStream->pause(mStream)) + : Result::NOT_SUPPORTED; } -Return StreamOut::resume() { - return mStream->resume != NULL ? - Stream::analyzeStatus("resume", mStream->resume(mStream)) : - Result::NOT_SUPPORTED; +Return StreamOut::resume() { + return mStream->resume != NULL + ? Stream::analyzeStatus("resume", mStream->resume(mStream)) + : Result::NOT_SUPPORTED; } -Return StreamOut::supportsDrain() { +Return StreamOut::supportsDrain() { return mStream->drain != NULL; } -Return StreamOut::drain(AudioDrain type) { - return mStream->drain != NULL ? - Stream::analyzeStatus( - "drain", mStream->drain(mStream, static_cast(type))) : - Result::NOT_SUPPORTED; +Return StreamOut::drain(AudioDrain type) { + return mStream->drain != NULL + ? Stream::analyzeStatus( + "drain", + mStream->drain(mStream, + static_cast(type))) + : Result::NOT_SUPPORTED; } -Return StreamOut::flush() { - return mStream->flush != NULL ? - Stream::analyzeStatus("flush", mStream->flush(mStream)) : - Result::NOT_SUPPORTED; +Return StreamOut::flush() { + return mStream->flush != NULL + ? Stream::analyzeStatus("flush", mStream->flush(mStream)) + : Result::NOT_SUPPORTED; } // static -Result StreamOut::getPresentationPositionImpl( - audio_stream_out_t *stream, uint64_t *frames, TimeSpec *timeStamp) { +Result StreamOut::getPresentationPositionImpl(audio_stream_out_t* stream, + uint64_t* frames, + TimeSpec* timeStamp) { Result retval(Result::NOT_SUPPORTED); if (stream->get_presentation_position == NULL) return retval; struct timespec halTimeStamp; retval = Stream::analyzeStatus( - "get_presentation_position", - stream->get_presentation_position(stream, frames, &halTimeStamp), - // Don't logspam on EINVAL--it's normal for get_presentation_position - // to return it sometimes. EAGAIN may be returned by A2DP audio HAL - // implementation. - EINVAL, EAGAIN); + "get_presentation_position", + stream->get_presentation_position(stream, frames, &halTimeStamp), + // Don't logspam on EINVAL--it's normal for get_presentation_position + // to return it sometimes. EAGAIN may be returned by A2DP audio HAL + // implementation. + EINVAL, EAGAIN); if (retval == Result::OK) { timeStamp->tvSec = halTimeStamp.tv_sec; timeStamp->tvNSec = halTimeStamp.tv_nsec; @@ -472,9 +484,10 @@ Result StreamOut::getPresentationPositionImpl( return retval; } -Return StreamOut::getPresentationPosition(getPresentationPosition_cb _hidl_cb) { +Return StreamOut::getPresentationPosition( + getPresentationPosition_cb _hidl_cb) { uint64_t frames = 0; - TimeSpec timeStamp = { 0, 0 }; + TimeSpec timeStamp = {0, 0}; Result retval = getPresentationPositionImpl(mStream, &frames, &timeStamp); _hidl_cb(retval, frames, timeStamp); return Void(); @@ -488,9 +501,10 @@ Return StreamOut::stop() { return mStreamMmap->stop(); } -Return StreamOut::createMmapBuffer(int32_t minSizeFrames, createMmapBuffer_cb _hidl_cb) { +Return StreamOut::createMmapBuffer(int32_t minSizeFrames, + createMmapBuffer_cb _hidl_cb) { return mStreamMmap->createMmapBuffer( - minSizeFrames, audio_stream_out_frame_size(mStream), _hidl_cb); + minSizeFrames, audio_stream_out_frame_size(mStream), _hidl_cb); } Return StreamOut::getMmapPosition(getMmapPosition_cb _hidl_cb) { diff --git a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp index 074903f665..e1c9549541 100644 --- a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp +++ b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp @@ -36,9 +36,9 @@ #include #include -#include "utility/ReturnIn.h" #include "utility/AssertOk.h" #include "utility/PrettyPrintAudioTypes.h" +#include "utility/ReturnIn.h" using std::string; using std::to_string; @@ -59,7 +59,8 @@ using ::android::hardware::audio::V2_0::IDevicesFactory; using ::android::hardware::audio::V2_0::IStream; using ::android::hardware::audio::V2_0::IStreamIn; using ::android::hardware::audio::V2_0::TimeSpec; -using ReadParameters = ::android::hardware::audio::V2_0::IStreamIn::ReadParameters; +using ReadParameters = + ::android::hardware::audio::V2_0::IStreamIn::ReadParameters; using ReadStatus = ::android::hardware::audio::V2_0::IStreamIn::ReadStatus; using ::android::hardware::audio::V2_0::IStreamOut; using ::android::hardware::audio::V2_0::IStreamOutCallback; @@ -84,16 +85,20 @@ using utility::returnIn; namespace doc { /** Document the current test case. - * Eg: calling `doc::test("Dump the state of the hal")` in the "debugDump" test will output: - * - * see https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuide.md#logging-additional-information + * Eg: calling `doc::test("Dump the state of the hal")` in the "debugDump" test + * will output: + * + * see + https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuide.md#logging-additional-information */ void test(const std::string& testCaseDocumentation) { ::testing::Test::RecordProperty("description", testCaseDocumentation); } -/** Document why a test was not fully run. Usually due to an optional feature not implemented. */ +/** Document why a test was not fully run. Usually due to an optional feature + * not implemented. */ void partialTest(const std::string& reason) { ::testing::Test::RecordProperty("partialyRunTest", reason); } @@ -101,16 +106,17 @@ void partialTest(const std::string& reason) { // Register callback for static object destruction // Avoid destroying static objects after main return. -// Post main return destruction leads to incorrect gtest timing measurements as well as harder +// Post main return destruction leads to incorrect gtest timing measurements as +// well as harder // debuging if anything goes wrong during destruction. class Environment : public ::testing::Environment { -public: - using TearDownFunc = std::function; - void registerTearDown(TearDownFunc&& tearDown) { - tearDowns.push_back(std::move(tearDown)); + public: + using TearDownFunc = std::function; + void registerTearDown(TearDownFunc&& tearDown) { + tearDowns.push_back(std::move(tearDown)); } -private: + private: void TearDown() override { // Call the tear downs in reverse order of insertion for (auto& tearDown : tearDowns) { @@ -123,7 +129,7 @@ private: static Environment* environment; class HidlTest : public ::testing::VtsHalHidlTargetTestBase { -protected: + protected: // Convenient member to store results Result res; }; @@ -134,18 +140,19 @@ protected: // Test all audio devices class AudioHidlTest : public HidlTest { -public: + public: void SetUp() override { - ASSERT_NO_FATAL_FAILURE(HidlTest::SetUp()); // setup base + ASSERT_NO_FATAL_FAILURE(HidlTest::SetUp()); // setup base if (devicesFactory == nullptr) { - environment->registerTearDown([]{ devicesFactory.clear(); }); - devicesFactory = ::testing::VtsHalHidlTargetTestBase::getService(); + environment->registerTearDown([] { devicesFactory.clear(); }); + devicesFactory = ::testing::VtsHalHidlTargetTestBase::getService< + IDevicesFactory>(); } ASSERT_TRUE(devicesFactory != nullptr); } -protected: + protected: // Cache the devicesFactory retrieval to speed up each test by ~0.5s static sp devicesFactory; }; @@ -171,26 +178,27 @@ TEST_F(AudioHidlTest, OpenDeviceInvalidParameter) { // Test the primary device class AudioPrimaryHidlTest : public AudioHidlTest { -public: + public: /** Primary HAL test are NOT thread safe. */ void SetUp() override { - ASSERT_NO_FATAL_FAILURE(AudioHidlTest::SetUp()); // setup base + ASSERT_NO_FATAL_FAILURE(AudioHidlTest::SetUp()); // setup base if (device == nullptr) { IDevicesFactory::Result result; sp baseDevice; - ASSERT_OK(devicesFactory->openDevice(IDevicesFactory::Device::PRIMARY, - returnIn(result, baseDevice))); + ASSERT_OK( + devicesFactory->openDevice(IDevicesFactory::Device::PRIMARY, + returnIn(result, baseDevice))); ASSERT_OK(result); ASSERT_TRUE(baseDevice != nullptr); - environment->registerTearDown([]{ device.clear(); }); + environment->registerTearDown([] { device.clear(); }); device = IPrimaryDevice::castFrom(baseDevice); ASSERT_TRUE(device != nullptr); } } -protected: + protected: // Cache the device opening to speed up each test by ~0.5s static sp device; }; @@ -211,15 +219,15 @@ TEST_F(AudioPrimaryHidlTest, Init) { template class AccessorPrimaryHidlTest : public AudioPrimaryHidlTest { -protected: - + protected: /** Test a property getter and setter. */ template - void testAccessors(const string& propertyName, const vector& valuesToTest, - Setter setter, Getter getter, + void testAccessors(const string& propertyName, + const vector& valuesToTest, Setter setter, + Getter getter, const vector& invalidValues = {}) { - - Property initialValue; // Save initial value to restore it at the end of the test + Property initialValue; // Save initial value to restore it at the end + // of the test ASSERT_OK((device.get()->*getter)(returnIn(res, initialValue))); ASSERT_OK(res); @@ -235,17 +243,21 @@ protected: } for (Property invalidValue : invalidValues) { - SCOPED_TRACE("Try to set " + propertyName + " with the invalid value " + + SCOPED_TRACE("Try to set " + propertyName + + " with the invalid value " + testing::PrintToString(invalidValue)); - EXPECT_RESULT(Result::INVALID_ARGUMENTS, (device.get()->*setter)(invalidValue)); + EXPECT_RESULT(Result::INVALID_ARGUMENTS, + (device.get()->*setter)(invalidValue)); } - ASSERT_OK((device.get()->*setter)(initialValue)); // restore initial value + ASSERT_OK( + (device.get()->*setter)(initialValue)); // restore initial value } /** Test the getter and setter of an optional feature. */ template - void testOptionalAccessors(const string& propertyName, const vector& valuesToTest, + void testOptionalAccessors(const string& propertyName, + const vector& valuesToTest, Setter setter, Getter getter, const vector& invalidValues = {}) { doc::test("Test the optional " + propertyName + " getters and setter"); @@ -257,10 +269,11 @@ protected: doc::partialTest(propertyName + " getter is not supported"); return; } - ASSERT_OK(res); // If it is supported it must succeed + ASSERT_OK(res); // If it is supported it must succeed } // The feature is supported, test it - testAccessors(propertyName, valuesToTest, setter, getter, invalidValues); + testAccessors(propertyName, valuesToTest, setter, getter, + invalidValues); } }; @@ -268,12 +281,15 @@ using BoolAccessorPrimaryHidlTest = AccessorPrimaryHidlTest; TEST_F(BoolAccessorPrimaryHidlTest, MicMuteTest) { doc::test("Check that the mic can be muted and unmuted"); - testAccessors("mic mute", {true, false, true}, &IDevice::setMicMute, &IDevice::getMicMute); + testAccessors("mic mute", {true, false, true}, &IDevice::setMicMute, + &IDevice::getMicMute); // TODO: check that the mic is really muted (all sample are 0) } TEST_F(BoolAccessorPrimaryHidlTest, MasterMuteTest) { - doc::test("If master mute is supported, try to mute and unmute the master output"); + doc::test( + "If master mute is supported, try to mute and unmute the master " + "output"); testOptionalAccessors("master mute", {true, false, true}, &IDevice::setMasterMute, &IDevice::getMasterMute); // TODO: check that the master volume is really muted @@ -282,7 +298,7 @@ TEST_F(BoolAccessorPrimaryHidlTest, MasterMuteTest) { using FloatAccessorPrimaryHidlTest = AccessorPrimaryHidlTest; TEST_F(FloatAccessorPrimaryHidlTest, MasterVolumeTest) { doc::test("Test the master volume if supported"); - testOptionalAccessors("master volume", {0, 0.5, 1}, + testOptionalAccessors("master volume", {0, 0.5, 1}, &IDevice::setMasterVolume, &IDevice::getMasterVolume, {-0.1, 1.1, NAN, INFINITY, -INFINITY, 1 + std::numeric_limits::epsilon()}); @@ -294,7 +310,7 @@ TEST_F(FloatAccessorPrimaryHidlTest, MasterVolumeTest) { ////////////////////////////////////////////////////////////////////////////// class AudioPatchPrimaryHidlTest : public AudioPrimaryHidlTest { -protected: + protected: bool areAudioPatchesSupported() { auto result = device->supportsAudioPatches(); EXPECT_TRUE(result.isOk()); @@ -311,27 +327,30 @@ TEST_F(AudioPatchPrimaryHidlTest, AudioPatches) { // TODO: test audio patches } - ////////////////////////////////////////////////////////////////////////////// //////////////// Required and recommended audio format support /////////////// -// From: https://source.android.com/compatibility/android-cdd.html#5_4_audio_recording -// From: https://source.android.com/compatibility/android-cdd.html#5_5_audio_playback +// From: +// https://source.android.com/compatibility/android-cdd.html#5_4_audio_recording +// From: +// https://source.android.com/compatibility/android-cdd.html#5_5_audio_playback /////////// TODO: move to the beginning of the file for easier update //////// ////////////////////////////////////////////////////////////////////////////// class AudioConfigPrimaryTest : public AudioPatchPrimaryHidlTest { -public: + public: // Cache result ? static const vector getRequiredSupportPlaybackAudioConfig() { - return combineAudioConfig({AudioChannelMask::OUT_STEREO, AudioChannelMask::OUT_MONO}, - {8000, 11025, 16000, 22050, 32000, 44100}, - {AudioFormat::PCM_16_BIT}); + return combineAudioConfig( + {AudioChannelMask::OUT_STEREO, AudioChannelMask::OUT_MONO}, + {8000, 11025, 16000, 22050, 32000, 44100}, + {AudioFormat::PCM_16_BIT}); } - static const vector getRecommendedSupportPlaybackAudioConfig() { - return combineAudioConfig({AudioChannelMask::OUT_STEREO, AudioChannelMask::OUT_MONO}, - {24000, 48000}, - {AudioFormat::PCM_16_BIT}); + static const vector + getRecommendedSupportPlaybackAudioConfig() { + return combineAudioConfig( + {AudioChannelMask::OUT_STEREO, AudioChannelMask::OUT_MONO}, + {24000, 48000}, {AudioFormat::PCM_16_BIT}); } static const vector getSupportedPlaybackAudioConfig() { @@ -346,8 +365,7 @@ public: {AudioFormat::PCM_16_BIT}); } static const vector getRecommendedSupportCaptureAudioConfig() { - return combineAudioConfig({AudioChannelMask::IN_STEREO}, - {22050, 48000}, + return combineAudioConfig({AudioChannelMask::IN_STEREO}, {22050, 48000}, {AudioFormat::PCM_16_BIT}); } static const vector getSupportedCaptureAudioConfig() { @@ -355,13 +373,13 @@ public: // as declared in the policy configuration return {}; } -private: + + private: static const vector combineAudioConfig( - vector channelMasks, - vector sampleRates, - vector formats) { + vector channelMasks, vector sampleRates, + vector formats) { vector configs; - for (auto channelMask: channelMasks) { + for (auto channelMask : channelMasks) { for (auto sampleRate : sampleRates) { for (auto format : formats) { AudioConfig config{}; @@ -383,28 +401,34 @@ private: * As the only parameter changing are channel mask and sample rate, * only print those ones in the test name. */ -static string generateTestName(const testing::TestParamInfo& info) { +static string generateTestName( + const testing::TestParamInfo& info) { const AudioConfig& config = info.param; - return to_string(info.index) + "__" + to_string(config.sampleRateHz)+ "_" + - // "MONO" is more clear than "FRONT_LEFT" - ((config.channelMask == AudioChannelMask::OUT_MONO || - config.channelMask == AudioChannelMask::IN_MONO) ? - "MONO" : toString(config.channelMask)); + return to_string(info.index) + "__" + to_string(config.sampleRateHz) + "_" + + // "MONO" is more clear than "FRONT_LEFT" + ((config.channelMask == AudioChannelMask::OUT_MONO || + config.channelMask == AudioChannelMask::IN_MONO) + ? "MONO" + : toString(config.channelMask)); } ////////////////////////////////////////////////////////////////////////////// ///////////////////////////// getInputBufferSize ///////////////////////////// ////////////////////////////////////////////////////////////////////////////// -// FIXME: execute input test only if platform declares android.hardware.microphone +// FIXME: execute input test only if platform declares +// android.hardware.microphone // how to get this value ? is it a property ??? -class AudioCaptureConfigPrimaryTest : public AudioConfigPrimaryTest, - public ::testing::WithParamInterface { -protected: - void inputBufferSizeTest(const AudioConfig& audioConfig, bool supportRequired) { +class AudioCaptureConfigPrimaryTest + : public AudioConfigPrimaryTest, + public ::testing::WithParamInterface { + protected: + void inputBufferSizeTest(const AudioConfig& audioConfig, + bool supportRequired) { uint64_t bufferSize; - ASSERT_OK(device->getInputBufferSize(audioConfig, returnIn(res, bufferSize))); + ASSERT_OK( + device->getInputBufferSize(audioConfig, returnIn(res, bufferSize))); switch (res) { case Result::INVALID_ARGUMENTS: @@ -416,36 +440,46 @@ protected: EXPECT_GT(bufferSize, uint64_t(0)); break; default: - FAIL() << "Invalid return status: " << ::testing::PrintToString(res); + FAIL() << "Invalid return status: " + << ::testing::PrintToString(res); } } }; -// Test that the required capture config and those declared in the policy are indeed supported +// Test that the required capture config and those declared in the policy are +// indeed supported class RequiredInputBufferSizeTest : public AudioCaptureConfigPrimaryTest {}; TEST_P(RequiredInputBufferSizeTest, RequiredInputBufferSizeTest) { - doc::test("Input buffer size must be retrievable for a format with required support."); + doc::test( + "Input buffer size must be retrievable for a format with required " + "support."); inputBufferSizeTest(GetParam(), true); } INSTANTIATE_TEST_CASE_P( - RequiredInputBufferSize, RequiredInputBufferSizeTest, - ::testing::ValuesIn(AudioConfigPrimaryTest::getRequiredSupportCaptureAudioConfig()), - &generateTestName); + RequiredInputBufferSize, RequiredInputBufferSizeTest, + ::testing::ValuesIn( + AudioConfigPrimaryTest::getRequiredSupportCaptureAudioConfig()), + &generateTestName); INSTANTIATE_TEST_CASE_P( - SupportedInputBufferSize, RequiredInputBufferSizeTest, - ::testing::ValuesIn(AudioConfigPrimaryTest::getSupportedCaptureAudioConfig()), - &generateTestName); + SupportedInputBufferSize, RequiredInputBufferSizeTest, + ::testing::ValuesIn( + AudioConfigPrimaryTest::getSupportedCaptureAudioConfig()), + &generateTestName); -// Test that the recommended capture config are supported or lead to a INVALID_ARGUMENTS return +// Test that the recommended capture config are supported or lead to a +// INVALID_ARGUMENTS return class OptionalInputBufferSizeTest : public AudioCaptureConfigPrimaryTest {}; TEST_P(OptionalInputBufferSizeTest, OptionalInputBufferSizeTest) { - doc::test("Input buffer size should be retrievable for a format with recommended support."); + doc::test( + "Input buffer size should be retrievable for a format with recommended " + "support."); inputBufferSizeTest(GetParam(), false); } INSTANTIATE_TEST_CASE_P( - RecommendedCaptureAudioConfigSupport, OptionalInputBufferSizeTest, - ::testing::ValuesIn(AudioConfigPrimaryTest::getRecommendedSupportCaptureAudioConfig()), - &generateTestName); + RecommendedCaptureAudioConfigSupport, OptionalInputBufferSizeTest, + ::testing::ValuesIn( + AudioConfigPrimaryTest::getRecommendedSupportCaptureAudioConfig()), + &generateTestName); ////////////////////////////////////////////////////////////////////////////// /////////////////////////////// setScreenState /////////////////////////////// @@ -457,7 +491,8 @@ TEST_F(AudioPrimaryHidlTest, setScreenState) { auto ret = device->setScreenState(turnedOn); ASSERT_TRUE(ret.isOk()); Result result = ret; - ASSERT_TRUE(result == Result::OK || result == Result::NOT_SUPPORTED) << toString(result); + ASSERT_TRUE(result == Result::OK || result == Result::NOT_SUPPORTED) + << toString(result); } } @@ -492,10 +527,11 @@ static void testDebugDump(DebugDump debugDump) { handle.setTo(nativeHandle, true /*take ownership*/); // TODO: debugDump does not return a Result. - // This mean that the hal can not report that it not implementing the function. + // This mean that the hal can not report that it not implementing the + // function. ASSERT_OK(debugDump(handle)); - rewind(file); // can not fail + rewind(file); // can not fail // Check that at least one bit was written by the hal char buff; @@ -505,7 +541,8 @@ static void testDebugDump(DebugDump debugDump) { TEST_F(AudioPrimaryHidlTest, debugDump) { doc::test("Check that the hal can dump its state without error"); - testDebugDump([this](const auto& handle){ return device->debugDump(handle); }); + testDebugDump( + [this](const auto& handle) { return device->debugDump(handle); }); } TEST_F(AudioPrimaryHidlTest, debugDumpInvalidArguments) { @@ -520,14 +557,16 @@ TEST_F(AudioPrimaryHidlTest, debugDumpInvalidArguments) { template class OpenStreamTest : public AudioConfigPrimaryTest, public ::testing::WithParamInterface { -protected: + protected: template void testOpen(Open openStream, const AudioConfig& config) { // 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; + AudioIoHandle ioHandle = + (AudioIoHandle)AudioHandleConsts::AUDIO_IO_HANDLE_NONE; AudioConfig suggestedConfig{}; - ASSERT_OK(openStream(ioHandle, config, returnIn(res, stream, suggestedConfig))); + ASSERT_OK(openStream(ioHandle, config, + returnIn(res, stream, suggestedConfig))); // TODO: only allow failure for RecommendedPlaybackAudioConfig switch (res) { @@ -538,16 +577,19 @@ protected: 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))); + // 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; break; default: - FAIL() << "Invalid return status: " << ::testing::PrintToString(res); + FAIL() << "Invalid return status: " + << ::testing::PrintToString(res); } open = true; } @@ -556,15 +598,15 @@ protected: open = false; return stream->close(); } -private: + + private: void TearDown() override { if (open) { ASSERT_OK(stream->close()); } } -protected: - + protected: AudioConfig audioConfig; DeviceAddress address = {}; sp stream; @@ -575,66 +617,84 @@ protected: class OutputStreamTest : public OpenStreamTest { virtual void SetUp() override { - ASSERT_NO_FATAL_FAILURE(OpenStreamTest::SetUp()); // setup base + ASSERT_NO_FATAL_FAILURE(OpenStreamTest::SetUp()); // setup base address.device = AudioDevice::OUT_DEFAULT; const AudioConfig& config = GetParam(); - AudioOutputFlag flags = AudioOutputFlag::NONE; // TODO: test all flag combination - testOpen([&](AudioIoHandle handle, AudioConfig config, auto cb) - { return device->openOutputStream(handle, address, config, flags, cb); }, - config); + AudioOutputFlag flags = + AudioOutputFlag::NONE; // TODO: test all flag combination + testOpen( + [&](AudioIoHandle handle, AudioConfig config, auto cb) { + return device->openOutputStream(handle, address, config, flags, + cb); + }, + config); } }; TEST_P(OutputStreamTest, OpenOutputStreamTest) { - doc::test("Check that output streams can be open with the required and recommended config"); + doc::test( + "Check that output streams can be open with the required and " + "recommended config"); // Open done in SetUp } INSTANTIATE_TEST_CASE_P( - RequiredOutputStreamConfigSupport, OutputStreamTest, - ::testing::ValuesIn(AudioConfigPrimaryTest::getRequiredSupportPlaybackAudioConfig()), - &generateTestName); + RequiredOutputStreamConfigSupport, OutputStreamTest, + ::testing::ValuesIn( + AudioConfigPrimaryTest::getRequiredSupportPlaybackAudioConfig()), + &generateTestName); INSTANTIATE_TEST_CASE_P( - SupportedOutputStreamConfig, OutputStreamTest, - ::testing::ValuesIn(AudioConfigPrimaryTest::getSupportedPlaybackAudioConfig()), - &generateTestName); + SupportedOutputStreamConfig, OutputStreamTest, + ::testing::ValuesIn( + AudioConfigPrimaryTest::getSupportedPlaybackAudioConfig()), + &generateTestName); INSTANTIATE_TEST_CASE_P( - RecommendedOutputStreamConfigSupport, OutputStreamTest, - ::testing::ValuesIn(AudioConfigPrimaryTest::getRecommendedSupportPlaybackAudioConfig()), - &generateTestName); + RecommendedOutputStreamConfigSupport, OutputStreamTest, + ::testing::ValuesIn( + AudioConfigPrimaryTest::getRecommendedSupportPlaybackAudioConfig()), + &generateTestName); ////////////////////////////// openInputStream ////////////////////////////// class InputStreamTest : public OpenStreamTest { - virtual void SetUp() override { - ASSERT_NO_FATAL_FAILURE(OpenStreamTest::SetUp()); // setup base + ASSERT_NO_FATAL_FAILURE(OpenStreamTest::SetUp()); // setup base address.device = AudioDevice::IN_DEFAULT; const AudioConfig& config = GetParam(); - AudioInputFlag flags = AudioInputFlag::NONE; // TODO: test all flag combination - AudioSource source = AudioSource::DEFAULT; // TODO: test all flag combination - testOpen([&](AudioIoHandle handle, AudioConfig config, auto cb) - { return device->openInputStream(handle, address, config, flags, source, cb); }, - config); + AudioInputFlag flags = + AudioInputFlag::NONE; // TODO: test all flag combination + AudioSource source = + AudioSource::DEFAULT; // TODO: test all flag combination + testOpen( + [&](AudioIoHandle handle, AudioConfig config, auto cb) { + return device->openInputStream(handle, address, config, flags, + source, cb); + }, + config); } }; TEST_P(InputStreamTest, OpenInputStreamTest) { - doc::test("Check that input streams can be open with the required and recommended config"); + doc::test( + "Check that input streams can be open with the required and " + "recommended config"); // Open done in setup } INSTANTIATE_TEST_CASE_P( - RequiredInputStreamConfigSupport, InputStreamTest, - ::testing::ValuesIn(AudioConfigPrimaryTest::getRequiredSupportCaptureAudioConfig()), - &generateTestName); + RequiredInputStreamConfigSupport, InputStreamTest, + ::testing::ValuesIn( + AudioConfigPrimaryTest::getRequiredSupportCaptureAudioConfig()), + &generateTestName); INSTANTIATE_TEST_CASE_P( - SupportedInputStreamConfig, InputStreamTest, - ::testing::ValuesIn(AudioConfigPrimaryTest::getSupportedCaptureAudioConfig()), - &generateTestName); + SupportedInputStreamConfig, InputStreamTest, + ::testing::ValuesIn( + AudioConfigPrimaryTest::getSupportedCaptureAudioConfig()), + &generateTestName); INSTANTIATE_TEST_CASE_P( - RecommendedInputStreamConfigSupport, InputStreamTest, - ::testing::ValuesIn(AudioConfigPrimaryTest::getRecommendedSupportCaptureAudioConfig()), - &generateTestName); + RecommendedInputStreamConfigSupport, InputStreamTest, + ::testing::ValuesIn( + AudioConfigPrimaryTest::getRecommendedSupportCaptureAudioConfig()), + &generateTestName); ////////////////////////////////////////////////////////////////////////////// ////////////////////////////// IStream getters /////////////////////////////// @@ -654,49 +714,63 @@ static R extract(Return ret) { /* Could not find a way to write a test for two parametrized class fixure * thus use this macro do duplicate tests for Input and Output stream */ #define TEST_IO_STREAM(test_name, documentation, code) \ - TEST_P(InputStreamTest, test_name) { \ - doc::test(documentation); \ - code; \ - } \ - TEST_P(OutputStreamTest, test_name) { \ - doc::test(documentation); \ - code; \ + TEST_P(InputStreamTest, test_name) { \ + doc::test(documentation); \ + code; \ + } \ + TEST_P(OutputStreamTest, test_name) { \ + doc::test(documentation); \ + code; \ } -TEST_IO_STREAM(GetFrameCount, "Check that the stream frame count == the one it was opened with", - ASSERT_EQ(audioConfig.frameCount, extract(stream->getFrameCount()))) +TEST_IO_STREAM( + GetFrameCount, + "Check that the stream frame count == the one it was opened with", + ASSERT_EQ(audioConfig.frameCount, extract(stream->getFrameCount()))) -TEST_IO_STREAM(GetSampleRate, "Check that the stream sample rate == the one it was opened with", - ASSERT_EQ(audioConfig.sampleRateHz, extract(stream->getSampleRate()))) +TEST_IO_STREAM( + GetSampleRate, + "Check that the stream sample rate == the one it was opened with", + ASSERT_EQ(audioConfig.sampleRateHz, extract(stream->getSampleRate()))) -TEST_IO_STREAM(GetChannelMask, "Check that the stream channel mask == the one it was opened with", - ASSERT_EQ(audioConfig.channelMask, extract(stream->getChannelMask()))) +TEST_IO_STREAM( + GetChannelMask, + "Check that the stream channel mask == the one it was opened with", + ASSERT_EQ(audioConfig.channelMask, extract(stream->getChannelMask()))) -TEST_IO_STREAM(GetFormat, "Check that the stream format == the one it was opened with", +TEST_IO_STREAM(GetFormat, + "Check that the stream format == the one it was opened with", ASSERT_EQ(audioConfig.format, extract(stream->getFormat()))) // TODO: for now only check that the framesize is not incoherent -TEST_IO_STREAM(GetFrameSize, "Check that the stream frame size == the one it was opened with", +TEST_IO_STREAM(GetFrameSize, + "Check that the stream frame size == the one it was opened with", ASSERT_GT(extract(stream->getFrameSize()), 0U)) -TEST_IO_STREAM(GetBufferSize, "Check that the stream buffer size== the one it was opened with", - ASSERT_GE(extract(stream->getBufferSize()), \ - extract(stream->getFrameSize()))); +TEST_IO_STREAM(GetBufferSize, + "Check that the stream buffer size== the one it was opened with", + ASSERT_GE(extract(stream->getBufferSize()), + extract(stream->getFrameSize()))); template -static void testCapabilityGetter(const string& name, IStream* stream, Property currentValue, - CapabilityGetter capablityGetter, Getter getter, Setter setter) { +static void testCapabilityGetter(const string& name, IStream* stream, + Property currentValue, + CapabilityGetter capablityGetter, + Getter getter, Setter setter) { hidl_vec capabilities; ASSERT_OK((stream->*capablityGetter)(returnIn(capabilities))); if (capabilities.size() == 0) { - // The default hal should probably return a NOT_SUPPORTED if the hal does not expose - // capability retrieval. For now it returns an empty list if not implemented + // The default hal should probably return a NOT_SUPPORTED if the hal + // does not expose + // capability retrieval. For now it returns an empty list if not + // implemented doc::partialTest(name + " is not supported"); return; }; - // TODO: This code has never been tested on a hal that supports getSupportedSampleRates + // TODO: This code has never been tested on a hal that supports + // getSupportedSampleRates EXPECT_NE(std::find(capabilities.begin(), capabilities.end(), currentValue), - capabilities.end()) + capabilities.end()) << "current " << name << " is not in the list of the supported ones " << toString(capabilities); @@ -707,22 +781,27 @@ static void testCapabilityGetter(const string& name, IStream* stream, Property c } } -TEST_IO_STREAM(SupportedSampleRate, "Check that the stream sample rate is declared as supported", - testCapabilityGetter("getSupportedSampleRate", stream.get(), \ - extract(stream->getSampleRate()), \ - &IStream::getSupportedSampleRates, \ - &IStream::getSampleRate, &IStream::setSampleRate)) +TEST_IO_STREAM(SupportedSampleRate, + "Check that the stream sample rate is declared as supported", + testCapabilityGetter("getSupportedSampleRate", stream.get(), + extract(stream->getSampleRate()), + &IStream::getSupportedSampleRates, + &IStream::getSampleRate, + &IStream::setSampleRate)) -TEST_IO_STREAM(SupportedChannelMask, "Check that the stream channel mask is declared as supported", - testCapabilityGetter("getSupportedChannelMask", stream.get(), \ - extract(stream->getChannelMask()), \ - &IStream::getSupportedChannelMasks, \ - &IStream::getChannelMask, &IStream::setChannelMask)) +TEST_IO_STREAM(SupportedChannelMask, + "Check that the stream channel mask is declared as supported", + testCapabilityGetter("getSupportedChannelMask", stream.get(), + extract(stream->getChannelMask()), + &IStream::getSupportedChannelMasks, + &IStream::getChannelMask, + &IStream::setChannelMask)) -TEST_IO_STREAM(SupportedFormat, "Check that the stream format is declared as supported", - testCapabilityGetter("getSupportedFormat", stream.get(), \ - extract(stream->getFormat()), \ - &IStream::getSupportedFormats, \ +TEST_IO_STREAM(SupportedFormat, + "Check that the stream format is declared as supported", + testCapabilityGetter("getSupportedFormat", stream.get(), + extract(stream->getFormat()), + &IStream::getSupportedFormats, &IStream::getFormat, &IStream::setFormat)) static void testGetDevice(IStream* stream, AudioDevice expectedDevice) { @@ -732,44 +811,53 @@ static void testGetDevice(IStream* stream, AudioDevice expectedDevice) { ASSERT_EQ(expectedDevice, device); } -TEST_IO_STREAM(GetDevice, "Check that the stream device == the one it was opened with", - areAudioPatchesSupported() ? doc::partialTest("Audio patches are supported") : \ - testGetDevice(stream.get(), address.device)) +TEST_IO_STREAM(GetDevice, + "Check that the stream device == the one it was opened with", + areAudioPatchesSupported() + ? doc::partialTest("Audio patches are supported") + : testGetDevice(stream.get(), address.device)) static void testSetDevice(IStream* stream, const DeviceAddress& address) { DeviceAddress otherAddress = address; - otherAddress.device = (address.device & AudioDevice::BIT_IN) == 0 ? - AudioDevice::OUT_SPEAKER : AudioDevice::IN_BUILTIN_MIC; + otherAddress.device = (address.device & AudioDevice::BIT_IN) == 0 + ? AudioDevice::OUT_SPEAKER + : AudioDevice::IN_BUILTIN_MIC; EXPECT_OK(stream->setDevice(otherAddress)); - ASSERT_OK(stream->setDevice(address)); // Go back to the original value + ASSERT_OK(stream->setDevice(address)); // Go back to the original value } -TEST_IO_STREAM(SetDevice, "Check that the stream can be rerouted to SPEAKER or BUILTIN_MIC", - areAudioPatchesSupported() ? doc::partialTest("Audio patches are supported") : \ - testSetDevice(stream.get(), address)) +TEST_IO_STREAM( + SetDevice, + "Check that the stream can be rerouted to SPEAKER or BUILTIN_MIC", + areAudioPatchesSupported() ? doc::partialTest("Audio patches are supported") + : testSetDevice(stream.get(), address)) -static void testGetAudioProperties(IStream* stream, AudioConfig expectedConfig) { +static void testGetAudioProperties(IStream* stream, + AudioConfig expectedConfig) { uint32_t sampleRateHz; AudioChannelMask mask; AudioFormat format; stream->getAudioProperties(returnIn(sampleRateHz, mask, format)); - // FIXME: the qcom hal it does not currently negotiate the sampleRate & channel mask + // FIXME: the qcom hal it does not currently negotiate the sampleRate & + // channel mask EXPECT_EQ(expectedConfig.sampleRateHz, sampleRateHz); EXPECT_EQ(expectedConfig.channelMask, mask); EXPECT_EQ(expectedConfig.format, format); } -TEST_IO_STREAM(GetAudioProperties, - "Check that the stream audio properties == the ones it was opened with", - testGetAudioProperties(stream.get(), audioConfig)) +TEST_IO_STREAM( + GetAudioProperties, + "Check that the stream audio properties == the ones it was opened with", + testGetAudioProperties(stream.get(), audioConfig)) static void testConnectedState(IStream* stream) { DeviceAddress address = {}; using AD = AudioDevice; - for (auto device : {AD::OUT_HDMI, AD::OUT_WIRED_HEADPHONE, AD::IN_USB_HEADSET}) { + for (auto device : + {AD::OUT_HDMI, AD::OUT_WIRED_HEADPHONE, AD::IN_USB_HEADSET}) { address.device = device; ASSERT_OK(stream->setConnectedState(address, true)); @@ -777,13 +865,15 @@ static void testConnectedState(IStream* stream) { } } TEST_IO_STREAM(SetConnectedState, - "Check that the stream can be notified of device connection and deconnection", + "Check that the stream can be notified of device connection and " + "deconnection", testConnectedState(stream.get())) - -static auto invalidArgsOrNotSupported = {Result::INVALID_ARGUMENTS, Result::NOT_SUPPORTED}; +static auto invalidArgsOrNotSupported = {Result::INVALID_ARGUMENTS, + Result::NOT_SUPPORTED}; TEST_IO_STREAM(SetHwAvSync, "Try to set hardware sync to an invalid value", - ASSERT_RESULT(invalidArgsOrNotSupported, stream->setHwAvSync(666))) + ASSERT_RESULT(invalidArgsOrNotSupported, + stream->setHwAvSync(666))) TEST_IO_STREAM(GetHwAvSync, "Get hardware sync can not fail", ASSERT_TRUE(device->getHwAvSync().isOk())) @@ -799,7 +889,8 @@ static void checkGetParameter(IStream* stream, hidl_vec keys, } } -/* Get/Set parameter is intended to be an opaque channel between vendors app and their HALs. +/* Get/Set parameter is intended to be an opaque channel between vendors app and + * their HALs. * Thus can not be meaningfully tested. * TODO: Doc missing. Should asking for an empty set of params raise an error ? */ @@ -807,22 +898,27 @@ TEST_IO_STREAM(getEmptySetParameter, "Retrieve the values of an empty set", checkGetParameter(stream.get(), {} /* keys */, {Result::OK, Result::INVALID_ARGUMENTS})) - -TEST_IO_STREAM(getNonExistingParameter, "Retrieve the values of an non existing parameter", +TEST_IO_STREAM(getNonExistingParameter, + "Retrieve the values of an non existing parameter", checkGetParameter(stream.get(), {"Non existing key"} /* keys */, {Result::INVALID_ARGUMENTS})) -static vector okOrInvalidArguments = {Result::OK, Result::INVALID_ARGUMENTS}; -TEST_IO_STREAM(setEmptySetParameter, "Set the values of an empty set of parameters", +static vector okOrInvalidArguments = {Result::OK, + Result::INVALID_ARGUMENTS}; +TEST_IO_STREAM(setEmptySetParameter, + "Set the values of an empty set of parameters", ASSERT_RESULT(okOrInvalidArguments, stream->setParameters({}))) -TEST_IO_STREAM(setNonExistingParameter, "Set the values of an non existing parameter", - ASSERT_RESULT(Result::INVALID_ARGUMENTS, - stream->setParameters({{"non existing key", "0"}}))) +TEST_IO_STREAM( + setNonExistingParameter, "Set the values of an non existing parameter", + ASSERT_RESULT(Result::INVALID_ARGUMENTS, + stream->setParameters({{"non existing key", "0"}}))) TEST_IO_STREAM(DebugDump, "Check that a stream can dump its state without error", - testDebugDump([this](const auto& handle){ return stream->debugDump(handle); })) + testDebugDump([this](const auto& handle) { + return stream->debugDump(handle); + })) TEST_IO_STREAM(DebugDumpInvalidArguments, "Check that the stream dump doesn't crash on invalid arguments", @@ -834,39 +930,47 @@ TEST_IO_STREAM(DebugDumpInvalidArguments, TEST_IO_STREAM(AddNonExistingEffect, "Adding a non existing effect should fail", ASSERT_RESULT(Result::INVALID_ARGUMENTS, stream->addEffect(666))) -TEST_IO_STREAM(RemoveNonExistingEffect, "Removing a non existing effect should fail", - ASSERT_RESULT(Result::INVALID_ARGUMENTS, stream->removeEffect(666))) +TEST_IO_STREAM(RemoveNonExistingEffect, + "Removing a non existing effect should fail", + ASSERT_RESULT(Result::INVALID_ARGUMENTS, + stream->removeEffect(666))) -//TODO: positive tests +// TODO: positive tests ////////////////////////////////////////////////////////////////////////////// /////////////////////////////// Control //////////////////////////////// ////////////////////////////////////////////////////////////////////////////// TEST_IO_STREAM(standby, "Make sure the stream can be put in stanby", - ASSERT_OK(stream->standby())) // can not fail + ASSERT_OK(stream->standby())) // can not fail -static vector invalidStateOrNotSupported = {Result::INVALID_STATE, Result::NOT_SUPPORTED}; +static vector invalidStateOrNotSupported = {Result::INVALID_STATE, + Result::NOT_SUPPORTED}; -TEST_IO_STREAM(startNoMmap, "Starting a mmaped stream before mapping it should fail", +TEST_IO_STREAM(startNoMmap, + "Starting a mmaped stream before mapping it should fail", ASSERT_RESULT(invalidStateOrNotSupported, stream->start())) -TEST_IO_STREAM(stopNoMmap, "Stopping a mmaped stream before mapping it should fail", +TEST_IO_STREAM(stopNoMmap, + "Stopping a mmaped stream before mapping it should fail", ASSERT_RESULT(invalidStateOrNotSupported, stream->stop())) -TEST_IO_STREAM(getMmapPositionNoMmap, "Get a stream Mmap position before mapping it should fail", +TEST_IO_STREAM(getMmapPositionNoMmap, + "Get a stream Mmap position before mapping it should fail", ASSERT_RESULT(invalidStateOrNotSupported, stream->stop())) -TEST_IO_STREAM(close, "Make sure a stream can be closed", ASSERT_OK(closeStream())) +TEST_IO_STREAM(close, "Make sure a stream can be closed", + ASSERT_OK(closeStream())) TEST_IO_STREAM(closeTwice, "Make sure a stream can not be closed twice", - ASSERT_OK(closeStream()); \ + ASSERT_OK(closeStream()); ASSERT_RESULT(Result::INVALID_STATE, closeStream())) static void testCreateTooBigMmapBuffer(IStream* stream) { MmapBufferInfo info; Result res; // Assume that int max is a value too big to be allocated - // This is true currently with a 32bit media server, but might not when it will run in 64 bit + // This is true currently with a 32bit media server, but might not when it + // will run in 64 bit auto minSizeFrames = std::numeric_limits::max(); ASSERT_OK(stream->createMmapBuffer(minSizeFrames, returnIn(res, info))); ASSERT_RESULT(invalidArgsOrNotSupported, res); @@ -875,7 +979,6 @@ static void testCreateTooBigMmapBuffer(IStream* stream) { TEST_IO_STREAM(CreateTooBigMmapBuffer, "Create mmap buffer too big should fail", testCreateTooBigMmapBuffer(stream.get())) - static void testGetMmapPositionOfNonMmapedStream(IStream* stream) { Result res; MmapPosition position; @@ -883,33 +986,36 @@ static void testGetMmapPositionOfNonMmapedStream(IStream* stream) { ASSERT_RESULT(invalidArgsOrNotSupported, res); } -TEST_IO_STREAM(GetMmapPositionOfNonMmapedStream, - "Retrieving the mmap position of a non mmaped stream should fail", - testGetMmapPositionOfNonMmapedStream(stream.get())) +TEST_IO_STREAM( + GetMmapPositionOfNonMmapedStream, + "Retrieving the mmap position of a non mmaped stream should fail", + testGetMmapPositionOfNonMmapedStream(stream.get())) ////////////////////////////////////////////////////////////////////////////// ///////////////////////////////// StreamIn /////////////////////////////////// ////////////////////////////////////////////////////////////////////////////// TEST_P(InputStreamTest, GetAudioSource) { - doc::test("Retrieving the audio source of an input stream should always succeed"); + doc::test( + "Retrieving the audio source of an input stream should always succeed"); AudioSource source; ASSERT_OK(stream->getAudioSource(returnIn(res, source))); ASSERT_OK(res); ASSERT_EQ(AudioSource::DEFAULT, source); } -static void testUnitaryGain(std::function (float)> setGain) { +static void testUnitaryGain(std::function(float)> setGain) { for (float value : {0.0, 0.01, 0.5, 0.09, 1.0}) { SCOPED_TRACE("value=" + to_string(value)); ASSERT_OK(setGain(value)); } - for (float value : (float[]){-INFINITY,-1.0, -0.0, - 1.0 + std::numeric_limits::epsilon(), 2.0, INFINITY, - NAN}) { + for (float value : (float[]){-INFINITY, -1.0, -0.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] ? + // FIXME: Missing api doc. What should the impl do if the volume is + // outside [0,1] ? ASSERT_RESULT(Result::INVALID_ARGUMENTS, setGain(value)); } } @@ -919,27 +1025,34 @@ TEST_P(InputStreamTest, SetGain) { testUnitaryGain([this](float volume) { return stream->setGain(volume); }); } -static void testPrepareForReading(IStreamIn* stream, uint32_t frameSize, uint32_t framesCount) { +static void testPrepareForReading(IStreamIn* stream, uint32_t frameSize, + uint32_t framesCount) { Result res; // Ignore output parameters as the call should fail - ASSERT_OK(stream->prepareForReading(frameSize, framesCount, - [&res](auto r, auto&, auto&, auto&, auto&) { res = r; })); + ASSERT_OK(stream->prepareForReading( + frameSize, framesCount, + [&res](auto r, auto&, auto&, auto&, auto&) { res = r; })); EXPECT_RESULT(invalidArgsOrNotSupported, res); } TEST_P(InputStreamTest, PrepareForReadingWithHugeBuffer) { - doc::test("Preparing a stream for reading with a 2^32 sized buffer should fail"); - testPrepareForReading(stream.get(), 1, std::numeric_limits::max()); + doc::test( + "Preparing a stream for reading with a 2^32 sized buffer should fail"); + testPrepareForReading(stream.get(), 1, + std::numeric_limits::max()); } TEST_P(InputStreamTest, PrepareForReadingCheckOverflow) { - doc::test("Preparing a stream for reading with a overflowing sized buffer should fail"); + doc::test( + "Preparing a stream for reading with a overflowing sized buffer should " + "fail"); auto uintMax = std::numeric_limits::max(); testPrepareForReading(stream.get(), uintMax, uintMax); } TEST_P(InputStreamTest, GetInputFramesLost) { - doc::test("The number of frames lost on a never started stream should be 0"); + doc::test( + "The number of frames lost on a never started stream should be 0"); auto ret = stream->getInputFramesLost(); ASSERT_TRUE(ret.isOk()); uint32_t framesLost{ret}; @@ -947,7 +1060,9 @@ TEST_P(InputStreamTest, GetInputFramesLost) { } TEST_P(InputStreamTest, getCapturePosition) { - doc::test("The capture position of a non prepared stream should not be retrievable"); + doc::test( + "The capture position of a non prepared stream should not be " + "retrievable"); uint64_t frames; uint64_t time; ASSERT_OK(stream->getCapturePosition(returnIn(res, frames, time))); @@ -973,24 +1088,31 @@ TEST_P(OutputStreamTest, setVolume) { doc::partialTest("setVolume is not supported"); return; } - testUnitaryGain([this](float volume) { return stream->setVolume(volume, volume); }); + testUnitaryGain( + [this](float volume) { return stream->setVolume(volume, volume); }); } -static void testPrepareForWriting(IStreamOut* stream, uint32_t frameSize, uint32_t framesCount) { +static void testPrepareForWriting(IStreamOut* stream, uint32_t frameSize, + uint32_t framesCount) { Result res; // Ignore output parameters as the call should fail - ASSERT_OK(stream->prepareForWriting(frameSize, framesCount, - [&res](auto r, auto&, auto&, auto&, auto&) { res = r; })); + ASSERT_OK(stream->prepareForWriting( + frameSize, framesCount, + [&res](auto r, auto&, auto&, auto&, auto&) { res = r; })); EXPECT_RESULT(invalidArgsOrNotSupported, res); } TEST_P(OutputStreamTest, PrepareForWriteWithHugeBuffer) { - doc::test("Preparing a stream for writing with a 2^32 sized buffer should fail"); - testPrepareForWriting(stream.get(), 1, std::numeric_limits::max()); + doc::test( + "Preparing a stream for writing with a 2^32 sized buffer should fail"); + testPrepareForWriting(stream.get(), 1, + std::numeric_limits::max()); } TEST_P(OutputStreamTest, PrepareForWritingCheckOverflow) { - doc::test("Preparing a stream for writing with a overflowing sized buffer should fail"); + doc::test( + "Preparing a stream for writing with a overflowing sized buffer should " + "fail"); auto uintMax = std::numeric_limits::max(); testPrepareForWriting(stream.get(), uintMax, uintMax); } @@ -1010,7 +1132,8 @@ struct Capability { }; TEST_P(OutputStreamTest, SupportsPauseAndResumeAndDrain) { - doc::test("Implementation must expose pause, resume and drain capabilities"); + doc::test( + "Implementation must expose pause, resume and drain capabilities"); Capability(stream.get()); } @@ -1044,16 +1167,19 @@ class MockOutCallbacks : public IStreamOutCallback { Return onError() override { return {}; } }; -static bool isAsyncModeSupported(IStreamOut *stream) { +static bool isAsyncModeSupported(IStreamOut* stream) { auto res = stream->setCallback(new MockOutCallbacks); - stream->clearCallback(); // try to restore the no callback state, ignore any error - auto okOrNotSupported = { Result::OK, Result::NOT_SUPPORTED }; + stream->clearCallback(); // try to restore the no callback state, ignore + // any error + auto okOrNotSupported = {Result::OK, Result::NOT_SUPPORTED}; EXPECT_RESULT(okOrNotSupported, res); return res.isOk() ? res == Result::OK : false; } TEST_P(OutputStreamTest, SetCallback) { - doc::test("If supported, registering callback for async operation should never fail"); + doc::test( + "If supported, registering callback for async operation should never " + "fail"); if (!isAsyncModeSupported(stream.get())) { doc::partialTest("The stream does not support async operations"); return; @@ -1063,7 +1189,9 @@ TEST_P(OutputStreamTest, SetCallback) { } TEST_P(OutputStreamTest, clearCallback) { - doc::test("If supported, clearing a callback to go back to sync operation should not fail"); + doc::test( + "If supported, clearing a callback to go back to sync operation should " + "not fail"); if (!isAsyncModeSupported(stream.get())) { doc::partialTest("The stream does not support async operations"); return; @@ -1074,7 +1202,9 @@ TEST_P(OutputStreamTest, clearCallback) { } TEST_P(OutputStreamTest, Resume) { - doc::test("If supported, a stream should fail to resume if not previously paused"); + doc::test( + "If supported, a stream should fail to resume if not previously " + "paused"); if (!Capability(stream.get()).resume) { doc::partialTest("The output stream does not support resume"); return; @@ -1083,7 +1213,9 @@ TEST_P(OutputStreamTest, Resume) { } TEST_P(OutputStreamTest, Pause) { - doc::test("If supported, a stream should fail to pause if not previously started"); + doc::test( + "If supported, a stream should fail to pause if not previously " + "started"); if (!Capability(stream.get()).pause) { doc::partialTest("The output stream does not support pause"); return; @@ -1091,7 +1223,7 @@ TEST_P(OutputStreamTest, Pause) { ASSERT_RESULT(Result::INVALID_STATE, stream->resume()); } -static void testDrain(IStreamOut *stream, AudioDrain type) { +static void testDrain(IStreamOut* stream, AudioDrain type) { if (!Capability(stream).drain) { doc::partialTest("The output stream does not support drain"); return; @@ -1121,7 +1253,9 @@ TEST_P(OutputStreamTest, FlushStop) { } TEST_P(OutputStreamTest, GetPresentationPositionStop) { - doc::test("If supported, a stream should always succeed to retrieve the presentation position"); + doc::test( + "If supported, a stream should always succeed to retrieve the " + "presentation position"); uint64_t frames; TimeSpec mesureTS; ASSERT_OK(stream->getPresentationPosition(returnIn(res, frames, mesureTS))); @@ -1134,63 +1268,66 @@ TEST_P(OutputStreamTest, GetPresentationPositionStop) { struct timespec currentTS; ASSERT_EQ(0, clock_gettime(CLOCK_MONOTONIC, ¤tTS)) << errno; - auto toMicroSec = [](uint64_t sec, auto nsec) { return sec * 1e+6 + nsec / 1e+3; }; + auto toMicroSec = [](uint64_t sec, auto nsec) { + return sec * 1e+6 + nsec / 1e+3; + }; auto currentTime = toMicroSec(currentTS.tv_sec, currentTS.tv_nsec); auto mesureTime = toMicroSec(mesureTS.tvSec, mesureTS.tvNSec); - ASSERT_PRED2([](auto c, auto m){ return c - m < 1e+6; }, currentTime, mesureTime); + ASSERT_PRED2([](auto c, auto m) { return c - m < 1e+6; }, currentTime, + mesureTime); } - ////////////////////////////////////////////////////////////////////////////// /////////////////////////////// PrimaryDevice //////////////////////////////// ////////////////////////////////////////////////////////////////////////////// - TEST_F(AudioPrimaryHidlTest, setVoiceVolume) { doc::test("Make sure setVoiceVolume only succeed if volume is in [0,1]"); - testUnitaryGain([this](float volume) { return device->setVoiceVolume(volume); }); + testUnitaryGain( + [this](float volume) { return device->setVoiceVolume(volume); }); } TEST_F(AudioPrimaryHidlTest, setMode) { doc::test("Make sure setMode always succeeds if mode is valid"); - for (AudioMode mode : {AudioMode::IN_CALL, AudioMode::IN_COMMUNICATION, - AudioMode::RINGTONE, AudioMode::CURRENT, - AudioMode::NORMAL /* Make sure to leave the test in normal mode */ }) { + for (AudioMode mode : + {AudioMode::IN_CALL, AudioMode::IN_COMMUNICATION, AudioMode::RINGTONE, + AudioMode::CURRENT, + AudioMode::NORMAL /* Make sure to leave the test in normal mode */}) { SCOPED_TRACE("mode=" + toString(mode)); ASSERT_OK(device->setMode(mode)); } // FIXME: Missing api doc. What should the impl do if the mode is invalid ? - ASSERT_RESULT(Result::INVALID_ARGUMENTS, device->setMode(AudioMode::INVALID)); + ASSERT_RESULT(Result::INVALID_ARGUMENTS, + device->setMode(AudioMode::INVALID)); } - TEST_F(BoolAccessorPrimaryHidlTest, BtScoNrecEnabled) { doc::test("Query and set the BT SCO NR&EC state"); testOptionalAccessors("BtScoNrecEnabled", {true, false, true}, - &IPrimaryDevice::setBtScoNrecEnabled, - &IPrimaryDevice::getBtScoNrecEnabled); + &IPrimaryDevice::setBtScoNrecEnabled, + &IPrimaryDevice::getBtScoNrecEnabled); } TEST_F(BoolAccessorPrimaryHidlTest, setGetBtScoWidebandEnabled) { doc::test("Query and set the SCO whideband state"); testOptionalAccessors("BtScoWideband", {true, false, true}, - &IPrimaryDevice::setBtScoWidebandEnabled, - &IPrimaryDevice::getBtScoWidebandEnabled); + &IPrimaryDevice::setBtScoWidebandEnabled, + &IPrimaryDevice::getBtScoWidebandEnabled); } using TtyModeAccessorPrimaryHidlTest = AccessorPrimaryHidlTest; TEST_F(TtyModeAccessorPrimaryHidlTest, setGetTtyMode) { doc::test("Query and set the TTY mode state"); - testOptionalAccessors("TTY mode", {TtyMode::OFF, TtyMode::HCO, TtyMode::VCO, TtyMode::FULL}, - &IPrimaryDevice::setTtyMode, &IPrimaryDevice::getTtyMode); + testOptionalAccessors( + "TTY mode", {TtyMode::OFF, TtyMode::HCO, TtyMode::VCO, TtyMode::FULL}, + &IPrimaryDevice::setTtyMode, &IPrimaryDevice::getTtyMode); } TEST_F(BoolAccessorPrimaryHidlTest, setGetHac) { doc::test("Query and set the HAC state"); - testAccessors("HAC", {true, false, true}, - &IPrimaryDevice::setHacEnabled, - &IPrimaryDevice::getHacEnabled); + testAccessors("HAC", {true, false, true}, &IPrimaryDevice::setHacEnabled, + &IPrimaryDevice::getHacEnabled); } ////////////////////////////////////////////////////////////////////////////// diff --git a/audio/2.0/vts/functional/utility/AssertOk.h b/audio/2.0/vts/functional/utility/AssertOk.h index 10b088c374..6891dc4af8 100644 --- a/audio/2.0/vts/functional/utility/AssertOk.h +++ b/audio/2.0/vts/functional/utility/AssertOk.h @@ -14,14 +14,15 @@ * limitations under the License. */ -#include #include +#include #include namespace detail { -// This is a detail namespace, thus it is OK to import a class as nobody else is allowed to use it +// This is a detail namespace, thus it is OK to import a class as nobody else is +// allowed to use it using ::android::hardware::Return; using ::android::hardware::audio::V2_0::Result; @@ -29,27 +30,28 @@ inline void assertResult(Result expected, Result result) { ASSERT_EQ(expected, result); } -inline void assertResult(Result expected, const Return &ret) { +inline void assertResult(Result expected, const Return& ret) { ASSERT_TRUE(ret.isOk()); Result result = ret; assertResult(expected, result); } -inline void assertResult(const std::vector &expected, Result result) { +inline void assertResult(const std::vector& expected, Result result) { if (std::find(expected.begin(), expected.end(), result) != expected.end()) { - return; // result is in expected + return; // result is in expected } FAIL() << "Expected result " << ::testing::PrintToString(result) << " to be one of " << ::testing::PrintToString(expected); } -inline void assertResult(const std::vector &expected, const Return &ret) { +inline void assertResult(const std::vector& expected, + const Return& ret) { ASSERT_TRUE(ret.isOk()); Result result = ret; assertResult(expected, result); } -inline void assertOk(const Return &ret) { +inline void assertOk(const Return& ret) { ASSERT_TRUE(ret.isOk()); } @@ -57,15 +59,16 @@ inline void assertOk(Result result) { assertResult(Result::OK, result); } -inline void assertOk(const Return &ret) { +inline void assertOk(const Return& ret) { assertResult(Result::OK, ret); } - } // Test anything provided is and contains only OK #define ASSERT_OK(ret) ASSERT_NO_FATAL_FAILURE(detail::assertOk(ret)) #define EXPECT_OK(ret) EXPECT_NO_FATAL_FAILURE(detail::assertOk(ret)) -#define ASSERT_RESULT(expected, ret) ASSERT_NO_FATAL_FAILURE(detail::assertResult(expected, ret)) -#define EXPECT_RESULT(expected, ret) EXPECT_NO_FATAL_FAILURE(detail::assertResult(expected, ret)) +#define ASSERT_RESULT(expected, ret) \ + ASSERT_NO_FATAL_FAILURE(detail::assertResult(expected, ret)) +#define EXPECT_RESULT(expected, ret) \ + EXPECT_NO_FATAL_FAILURE(detail::assertResult(expected, ret)) From fa3b4a9334ac3c747c121652441f402a1bd3f45e Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Tue, 2 May 2017 17:38:34 -0700 Subject: [PATCH 02/16] Audio HAL VTS: differentiate getParam success/failure/not_implemented When sending parameters to the HAL (and some getters are implemented with getParameters), the client expect a status consistent with the other HIDL methods. Ie: not implemented or success and failure. Unfortunately, the legacy get_parameter interface, which currently most Audio HIDL implementation are a wrapper around, do not return such error code. Get parameters return a list of key values. - If a requested key does not return a key value pair, consider it not implemented - If a requested key returns a key not followed by a correct value, consider it a failure - otherwise it is a success Test: vts-tradefed run vts --module VtsHalAudioV2_0Target Test: call/play music/record/video... Bug: 36311550 Change-Id: Id6711e9c1974fe5a336b6de83a9b6d14f74437c9 Signed-off-by: Kevin Rocard --- audio/2.0/default/ParametersUtil.cpp | 53 ++++++++++++------- .../functional/AudioPrimaryHidlHalTest.cpp | 13 +++-- 2 files changed, 40 insertions(+), 26 deletions(-) diff --git a/audio/2.0/default/ParametersUtil.cpp b/audio/2.0/default/ParametersUtil.cpp index 0511557da3..5cc60db981 100644 --- a/audio/2.0/default/ParametersUtil.cpp +++ b/audio/2.0/default/ParametersUtil.cpp @@ -22,11 +22,31 @@ namespace audio { namespace V2_0 { namespace implementation { +// Static method and not private method to avoid leaking status_t dependency +static Result getHalStatusToResult(status_t status) { + switch (status) { + case OK: + return Result::OK; + case BAD_VALUE: // Nothing was returned, probably because the HAL does + // not handle it + return Result::NOT_SUPPORTED; + case INVALID_OPERATION: // Conversion from string to the requested type + // failed + return Result::INVALID_ARGUMENTS; + default: // Should not happen + ALOGW("Unexpected status returned by getParam: %u", status); + return Result::INVALID_ARGUMENTS; + } +} + Result ParametersUtil::getParam(const char* name, bool* value) { String8 halValue; Result retval = getParam(name, &halValue); *value = false; if (retval == Result::OK) { + if (halValue.empty()) { + return Result::NOT_SUPPORTED; + } *value = !(halValue == AudioParameter::valueOff); } return retval; @@ -37,8 +57,7 @@ Result ParametersUtil::getParam(const char* name, int* value) { AudioParameter keys; keys.addKey(halName); std::unique_ptr params = getParams(keys); - status_t halStatus = params->getInt(halName, *value); - return halStatus == OK ? Result::OK : Result::INVALID_ARGUMENTS; + return getHalStatusToResult(params->getInt(halName, *value)); } Result ParametersUtil::getParam(const char* name, String8* value) { @@ -46,8 +65,7 @@ Result ParametersUtil::getParam(const char* name, String8* value) { AudioParameter keys; keys.addKey(halName); std::unique_ptr params = getParams(keys); - status_t halStatus = params->get(halName, *value); - return halStatus == OK ? Result::OK : Result::INVALID_ARGUMENTS; + return getHalStatusToResult(params->get(halName, *value)); } void ParametersUtil::getParametersImpl( @@ -60,23 +78,20 @@ void ParametersUtil::getParametersImpl( halKeys.addKey(String8(keys[i].c_str())); } std::unique_ptr halValues = getParams(halKeys); - Result retval(Result::INVALID_ARGUMENTS); + Result retval = + halValues->size() == keys.size() ? Result::OK : Result::NOT_SUPPORTED; hidl_vec result; - if (halValues->size() > 0) { - result.resize(halValues->size()); - String8 halKey, halValue; - for (size_t i = 0; i < halValues->size(); ++i) { - status_t status = halValues->getAt(i, halKey, halValue); - if (status != OK) { - result.resize(0); - break; - } - result[i].key = halKey.string(); - result[i].value = halValue.string(); - } - if (result.size() != 0) { - retval = Result::OK; + result.resize(halValues->size()); + String8 halKey, halValue; + for (size_t i = 0; i < halValues->size(); ++i) { + status_t status = halValues->getAt(i, halKey, halValue); + if (status != OK) { + result.resize(0); + retval = getHalStatusToResult(status); + break; } + result[i].key = halKey.string(); + result[i].value = halValue.string(); } cb(retval, result); } diff --git a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp index e1c9549541..b8e34549ff 100644 --- a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp +++ b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp @@ -878,8 +878,8 @@ TEST_IO_STREAM(SetHwAvSync, "Try to set hardware sync to an invalid value", TEST_IO_STREAM(GetHwAvSync, "Get hardware sync can not fail", ASSERT_TRUE(device->getHwAvSync().isOk())) -static void checkGetParameter(IStream* stream, hidl_vec keys, - vector expectedResults) { +static void checkGetNoParameter(IStream* stream, hidl_vec keys, + vector expectedResults) { hidl_vec parameters; Result res; ASSERT_OK(stream->getParameters(keys, returnIn(res, parameters))); @@ -892,16 +892,15 @@ static void checkGetParameter(IStream* stream, hidl_vec keys, /* Get/Set parameter is intended to be an opaque channel between vendors app and * their HALs. * Thus can not be meaningfully tested. - * TODO: Doc missing. Should asking for an empty set of params raise an error ? */ TEST_IO_STREAM(getEmptySetParameter, "Retrieve the values of an empty set", - checkGetParameter(stream.get(), {} /* keys */, - {Result::OK, Result::INVALID_ARGUMENTS})) + checkGetNoParameter(stream.get(), {} /* keys */, {Result::OK})) TEST_IO_STREAM(getNonExistingParameter, "Retrieve the values of an non existing parameter", - checkGetParameter(stream.get(), {"Non existing key"} /* keys */, - {Result::INVALID_ARGUMENTS})) + checkGetNoParameter(stream.get(), + {"Non existing key"} /* keys */, + {Result::NOT_SUPPORTED})) static vector okOrInvalidArguments = {Result::OK, Result::INVALID_ARGUMENTS}; From f8500dcb5a22b3f21dbcdadf04cf3d667d499b51 Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Wed, 3 May 2017 10:43:21 -0700 Subject: [PATCH 03/16] Audio HAL VTS: Allow OK when setting a non existing parameter setHwAvSync and setParameters were implemented in the pre-hidl interface as set_parameters. Unfortunately set_parameters did not return an error if a key was not implemented. As most HIDL implementation will be a wrapper around the pre-hidl interface, allow those functions to return OK on not implemented key. Test: vts-tradefed run vts --module VtsHalAudioV2_0Target Test: call/play music/record/video... Bug: 36311550 Change-Id: Icfcaa02b7d63e03375fddc90dc5a803754c1874f Signed-off-by: Kevin Rocard --- .../vts/functional/AudioPrimaryHidlHalTest.cpp | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp index b8e34549ff..20a093fbf7 100644 --- a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp +++ b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp @@ -869,10 +869,10 @@ TEST_IO_STREAM(SetConnectedState, "deconnection", testConnectedState(stream.get())) -static auto invalidArgsOrNotSupported = {Result::INVALID_ARGUMENTS, - Result::NOT_SUPPORTED}; +static auto invalidArgsOrNotSupportedOrOK = {Result::INVALID_ARGUMENTS, + Result::NOT_SUPPORTED, Result::OK}; TEST_IO_STREAM(SetHwAvSync, "Try to set hardware sync to an invalid value", - ASSERT_RESULT(invalidArgsOrNotSupported, + ASSERT_RESULT(invalidArgsOrNotSupportedOrOK, stream->setHwAvSync(666))) TEST_IO_STREAM(GetHwAvSync, "Get hardware sync can not fail", @@ -902,15 +902,17 @@ TEST_IO_STREAM(getNonExistingParameter, {"Non existing key"} /* keys */, {Result::NOT_SUPPORTED})) -static vector okOrInvalidArguments = {Result::OK, - Result::INVALID_ARGUMENTS}; TEST_IO_STREAM(setEmptySetParameter, "Set the values of an empty set of parameters", - ASSERT_RESULT(okOrInvalidArguments, stream->setParameters({}))) + ASSERT_RESULT(Result::OK, stream->setParameters({}))) TEST_IO_STREAM( setNonExistingParameter, "Set the values of an non existing parameter", - ASSERT_RESULT(Result::INVALID_ARGUMENTS, + // Unfortunately, the set_parameter legacy interface did not return any + // error code when a key is not supported. + // To allow implementation to just wrapped the legacy one, consider OK as a + // valid result for setting a non existing parameter. + ASSERT_RESULT(invalidArgsOrNotSupportedOrOK, stream->setParameters({{"non existing key", "0"}}))) TEST_IO_STREAM(DebugDump, @@ -964,6 +966,8 @@ TEST_IO_STREAM(closeTwice, "Make sure a stream can not be closed twice", ASSERT_OK(closeStream()); ASSERT_RESULT(Result::INVALID_STATE, closeStream())) +static auto invalidArgsOrNotSupported = {Result::INVALID_ARGUMENTS, + Result::NOT_SUPPORTED}; static void testCreateTooBigMmapBuffer(IStream* stream) { MmapBufferInfo info; Result res; From 04364edefcc39a5f620156a35209b760eb66a916 Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Tue, 2 May 2017 18:16:00 -0700 Subject: [PATCH 04/16] Audio HAL VTS: Sanitize setMode input Some values of AudioMode are exposed although implementation detail. Make sure the client can not use them. Test: vts-tradefed run vts --module VtsHalAudioV2_0Target Test: call/play music/record/video... Bug: 36311550 Change-Id: If513c2a06efa8a92459f0af80c63232fc63302b0 Signed-off-by: Kevin Rocard --- audio/2.0/default/PrimaryDevice.cpp | 12 ++++++++++++ .../vts/functional/AudioPrimaryHidlHalTest.cpp | 16 ++++++++++------ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/audio/2.0/default/PrimaryDevice.cpp b/audio/2.0/default/PrimaryDevice.cpp index 6f2268a331..4e8f30f924 100644 --- a/audio/2.0/default/PrimaryDevice.cpp +++ b/audio/2.0/default/PrimaryDevice.cpp @@ -132,6 +132,18 @@ Return PrimaryDevice::setVoiceVolume(float volume) { } Return PrimaryDevice::setMode(AudioMode mode) { + // INVALID, CURRENT, CNT, MAX are reserved for internal use. + // TODO: remove the values from the HIDL interface + switch (mode) { + case AudioMode::NORMAL: + case AudioMode::RINGTONE: + case AudioMode::IN_CALL: + case AudioMode::IN_COMMUNICATION: + break; // Valid values + default: + return Result::INVALID_ARGUMENTS; + }; + return mDevice->analyzeStatus( "set_mode", mDevice->device()->set_mode( mDevice->device(), static_cast(mode))); diff --git a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp index 20a093fbf7..2700ef1b4a 100644 --- a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp +++ b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp @@ -1291,18 +1291,22 @@ TEST_F(AudioPrimaryHidlTest, setVoiceVolume) { } TEST_F(AudioPrimaryHidlTest, setMode) { - doc::test("Make sure setMode always succeeds if mode is valid"); + doc::test( + "Make sure setMode always succeeds if mode is valid " + "and fails otherwise"); + // Test Invalid values + for (AudioMode mode : + {AudioMode::INVALID, AudioMode::CURRENT, AudioMode::CNT}) { + SCOPED_TRACE("mode=" + toString(mode)); + ASSERT_RESULT(Result::INVALID_ARGUMENTS, device->setMode(mode)); + } + // Test valid values for (AudioMode mode : {AudioMode::IN_CALL, AudioMode::IN_COMMUNICATION, AudioMode::RINGTONE, - AudioMode::CURRENT, AudioMode::NORMAL /* Make sure to leave the test in normal mode */}) { SCOPED_TRACE("mode=" + toString(mode)); ASSERT_OK(device->setMode(mode)); } - - // FIXME: Missing api doc. What should the impl do if the mode is invalid ? - ASSERT_RESULT(Result::INVALID_ARGUMENTS, - device->setMode(AudioMode::INVALID)); } TEST_F(BoolAccessorPrimaryHidlTest, BtScoNrecEnabled) { From c07df49e455ef67f880a3ca29ce585a213bccdde Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Tue, 2 May 2017 18:31:24 -0700 Subject: [PATCH 05/16] Audio HAL VTS: refactor prepareFor{Reading,Writing} Those functions had lots of copy paste on errors and the following patch will even add more error detections. Refactor the hidl_cb call to avoid all duplication. Note that both function should be refactored as 99% identical. Test: vts-tradefed run vts --module VtsHalAudioV2_0Target Test: call/play music/record/video... Bug: 36311550 Change-Id: I40d6926b4f9f5e3aba51e878f55fb013f4ca09c1 Signed-off-by: Kevin Rocard --- audio/2.0/default/StreamIn.cpp | 27 +++++++++++++++------------ audio/2.0/default/StreamOut.cpp | 24 ++++++++++++++---------- 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/audio/2.0/default/StreamIn.cpp b/audio/2.0/default/StreamIn.cpp index 2262697cde..59029be522 100644 --- a/audio/2.0/default/StreamIn.cpp +++ b/audio/2.0/default/StreamIn.cpp @@ -319,19 +319,25 @@ Return StreamIn::prepareForReading(uint32_t frameSize, prepareForReading_cb _hidl_cb) { status_t status; ThreadInfo threadInfo = {0, 0}; + + // Wrap the _hidl_cb to return an error + auto sendError = [this, &threadInfo, &_hidl_cb](Result result) { + _hidl_cb(result, CommandMQ::Descriptor(), DataMQ::Descriptor(), + StatusMQ::Descriptor(), threadInfo); + + }; + // Create message queues. if (mDataMQ) { ALOGE("the client attempts to call prepareForReading twice"); - _hidl_cb(Result::INVALID_STATE, CommandMQ::Descriptor(), - DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + sendError(Result::INVALID_STATE); return Void(); } std::unique_ptr tempCommandMQ(new CommandMQ(1)); if (frameSize > std::numeric_limits::max() / framesCount) { ALOGE("Requested buffer is too big, %d*%d can not fit in size_t", frameSize, framesCount); - _hidl_cb(Result::INVALID_ARGUMENTS, CommandMQ::Descriptor(), - DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + sendError(Result::INVALID_ARGUMENTS); return Void(); } std::unique_ptr tempDataMQ( @@ -343,8 +349,7 @@ Return StreamIn::prepareForReading(uint32_t frameSize, ALOGE_IF(!tempCommandMQ->isValid(), "command MQ is invalid"); ALOGE_IF(!tempDataMQ->isValid(), "data MQ is invalid"); ALOGE_IF(!tempStatusMQ->isValid(), "status MQ is invalid"); - _hidl_cb(Result::INVALID_ARGUMENTS, CommandMQ::Descriptor(), - DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + sendError(Result::INVALID_ARGUMENTS); return Void(); } EventFlag* tempRawEfGroup{}; @@ -354,8 +359,7 @@ Return StreamIn::prepareForReading(uint32_t frameSize, tempRawEfGroup, [](auto* ef) { EventFlag::deleteEventFlag(&ef); }); if (status != OK || !tempElfGroup) { ALOGE("failed creating event flag for data MQ: %s", strerror(-status)); - _hidl_cb(Result::INVALID_ARGUMENTS, CommandMQ::Descriptor(), - DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + sendError(Result::INVALID_ARGUMENTS); return Void(); } @@ -364,15 +368,14 @@ Return StreamIn::prepareForReading(uint32_t frameSize, &mStopReadThread, mStream, tempCommandMQ.get(), tempDataMQ.get(), tempStatusMQ.get(), tempElfGroup.get()); if (!tempReadThread->init()) { - _hidl_cb(Result::INVALID_ARGUMENTS, CommandMQ::Descriptor(), - DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + ALOGW("failed to start reader thread: %s", strerror(-status)); + sendError(Result::INVALID_ARGUMENTS); return Void(); } status = tempReadThread->run("reader", PRIORITY_URGENT_AUDIO); if (status != OK) { ALOGW("failed to start reader thread: %s", strerror(-status)); - _hidl_cb(Result::INVALID_ARGUMENTS, CommandMQ::Descriptor(), - DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + sendError(Result::INVALID_ARGUMENTS); return Void(); } diff --git a/audio/2.0/default/StreamOut.cpp b/audio/2.0/default/StreamOut.cpp index 63b9ae352c..fc055be546 100644 --- a/audio/2.0/default/StreamOut.cpp +++ b/audio/2.0/default/StreamOut.cpp @@ -295,11 +295,18 @@ Return StreamOut::prepareForWriting(uint32_t frameSize, prepareForWriting_cb _hidl_cb) { status_t status; ThreadInfo threadInfo = {0, 0}; + + // Wrap the _hidl_cb to return an error + auto sendError = [this, &threadInfo, &_hidl_cb](Result result) { + _hidl_cb(result, CommandMQ::Descriptor(), DataMQ::Descriptor(), + StatusMQ::Descriptor(), threadInfo); + + }; + // Create message queues. if (mDataMQ) { ALOGE("the client attempts to call prepareForWriting twice"); - _hidl_cb(Result::INVALID_STATE, CommandMQ::Descriptor(), - DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + sendError(Result::INVALID_STATE); return Void(); } std::unique_ptr tempCommandMQ(new CommandMQ(1)); @@ -320,8 +327,7 @@ Return StreamOut::prepareForWriting(uint32_t frameSize, ALOGE_IF(!tempCommandMQ->isValid(), "command MQ is invalid"); ALOGE_IF(!tempDataMQ->isValid(), "data MQ is invalid"); ALOGE_IF(!tempStatusMQ->isValid(), "status MQ is invalid"); - _hidl_cb(Result::INVALID_ARGUMENTS, CommandMQ::Descriptor(), - DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + sendError(Result::INVALID_ARGUMENTS); return Void(); } EventFlag* tempRawEfGroup{}; @@ -331,8 +337,7 @@ Return StreamOut::prepareForWriting(uint32_t frameSize, tempRawEfGroup, [](auto* ef) { EventFlag::deleteEventFlag(&ef); }); if (status != OK || !tempElfGroup) { ALOGE("failed creating event flag for data MQ: %s", strerror(-status)); - _hidl_cb(Result::INVALID_ARGUMENTS, CommandMQ::Descriptor(), - DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + sendError(Result::INVALID_ARGUMENTS); return Void(); } @@ -341,15 +346,14 @@ Return StreamOut::prepareForWriting(uint32_t frameSize, &mStopWriteThread, mStream, tempCommandMQ.get(), tempDataMQ.get(), tempStatusMQ.get(), tempElfGroup.get()); if (!tempWriteThread->init()) { - _hidl_cb(Result::INVALID_ARGUMENTS, CommandMQ::Descriptor(), - DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + ALOGW("failed to start writer thread: %s", strerror(-status)); + sendError(Result::INVALID_ARGUMENTS); return Void(); } status = tempWriteThread->run("writer", PRIORITY_URGENT_AUDIO); if (status != OK) { ALOGW("failed to start writer thread: %s", strerror(-status)); - _hidl_cb(Result::INVALID_ARGUMENTS, CommandMQ::Descriptor(), - DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + sendError(Result::INVALID_ARGUMENTS); return Void(); } From 195205b323b7d93cf4d477445469759b27371f45 Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Tue, 2 May 2017 18:34:59 -0700 Subject: [PATCH 06/16] Audio HAL VTS: Sanitize prepareFor{Writing,Reading} input size Return an error if framesCount or frameSize are null to avoid a division by zero when calculating the buffer size. The message queues are allocated with a buffer size but if two big they will assert not return an error. Thus take some margin on the buffer size check. Note that both function should be refactored as 99% identical. Test: vts-tradefed run vts --module VtsHalAudioV2_0Target Test: call/play music/record/video... Bug: 36311550 Change-Id: I0576e9016ef2e567c8d4e171c6237883d9865db9 Signed-off-by: Kevin Rocard --- audio/2.0/default/StreamIn.cpp | 17 +++++++++++++++-- audio/2.0/default/StreamOut.cpp | 19 +++++++++++++++---- .../functional/AudioPrimaryHidlHalTest.cpp | 12 ++++++++++++ 3 files changed, 42 insertions(+), 6 deletions(-) diff --git a/audio/2.0/default/StreamIn.cpp b/audio/2.0/default/StreamIn.cpp index 59029be522..9feef15cdf 100644 --- a/audio/2.0/default/StreamIn.cpp +++ b/audio/2.0/default/StreamIn.cpp @@ -334,8 +334,21 @@ Return StreamIn::prepareForReading(uint32_t frameSize, return Void(); } std::unique_ptr tempCommandMQ(new CommandMQ(1)); - if (frameSize > std::numeric_limits::max() / framesCount) { - ALOGE("Requested buffer is too big, %d*%d can not fit in size_t", + + // Check frameSize and framesCount + if (frameSize == 0 || framesCount == 0) { + ALOGE("Null frameSize (%u) or framesCount (%u)", frameSize, + framesCount); + sendError(Result::INVALID_ARGUMENTS); + return Void(); + } + // A message queue asserts if it can not handle the requested buffer, + // thus the client has to guess the maximum size it can handle + // Choose an arbitrary margin for the overhead of a message queue + size_t metadataOverhead = 100000; + if (frameSize > + (std::numeric_limits::max() - metadataOverhead) / framesCount) { + ALOGE("Buffer too big: %u*%u bytes can not fit in a message queue", frameSize, framesCount); sendError(Result::INVALID_ARGUMENTS); return Void(); diff --git a/audio/2.0/default/StreamOut.cpp b/audio/2.0/default/StreamOut.cpp index fc055be546..eb53259f9c 100644 --- a/audio/2.0/default/StreamOut.cpp +++ b/audio/2.0/default/StreamOut.cpp @@ -311,11 +311,22 @@ Return StreamOut::prepareForWriting(uint32_t frameSize, } std::unique_ptr tempCommandMQ(new CommandMQ(1)); - if (frameSize > std::numeric_limits::max() / framesCount) { - ALOGE("Requested buffer is too big, %d*%d can not fit in size_t", + // Check frameSize and framesCount + if (frameSize == 0 || framesCount == 0) { + ALOGE("Null frameSize (%u) or framesCount (%u)", frameSize, + framesCount); + sendError(Result::INVALID_ARGUMENTS); + return Void(); + } + // A message queue asserts if it can not handle the requested buffer, + // thus the client has to guess the maximum size it can handle + size_t metadataOverhead = + 100000; // Arbitrary margin for the overhead of a message queue + if (frameSize > + (std::numeric_limits::max() - metadataOverhead) / framesCount) { + ALOGE("Buffer too big: %u*%u bytes can not fit in a message queue", frameSize, framesCount); - _hidl_cb(Result::INVALID_ARGUMENTS, CommandMQ::Descriptor(), - DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + sendError(Result::INVALID_ARGUMENTS); return Void(); } std::unique_ptr tempDataMQ( diff --git a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp index 2700ef1b4a..f49c9261f3 100644 --- a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp +++ b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp @@ -1038,6 +1038,12 @@ static void testPrepareForReading(IStreamIn* stream, uint32_t frameSize, EXPECT_RESULT(invalidArgsOrNotSupported, res); } +TEST_P(InputStreamTest, PrepareForReadingWithZeroBuffer) { + doc::test( + "Preparing a stream for reading with a 0 sized buffer should fail"); + testPrepareForReading(stream.get(), 0, 0); +} + TEST_P(InputStreamTest, PrepareForReadingWithHugeBuffer) { doc::test( "Preparing a stream for reading with a 2^32 sized buffer should fail"); @@ -1105,6 +1111,12 @@ static void testPrepareForWriting(IStreamOut* stream, uint32_t frameSize, EXPECT_RESULT(invalidArgsOrNotSupported, res); } +TEST_P(OutputStreamTest, PrepareForWriteWithZeroBuffer) { + doc::test( + "Preparing a stream for writing with a 0 sized buffer should fail"); + testPrepareForWriting(stream.get(), 0, 0); +} + TEST_P(OutputStreamTest, PrepareForWriteWithHugeBuffer) { doc::test( "Preparing a stream for writing with a 2^32 sized buffer should fail"); From 5e5783daef84deecd53bfb1e94a0177e667b71db Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Tue, 2 May 2017 18:41:46 -0700 Subject: [PATCH 07/16] Audio HAL VTS: debugDump can only test for crash DebugDump does not return an error code, thus the implementation can not return not implemented. As a result, the test can not expect any output from the function. Only test that the call does not crash and add a log if the function is probably not implemented in the test. Test: vts-tradefed run vts --module VtsHalAudioV2_0Target Test: call/play music/record/video... Bug: 36311550 Change-Id: I2c18958bceb1eb638491f9afce9d8e8025ccd3ec Signed-off-by: Kevin Rocard --- .../functional/AudioPrimaryHidlHalTest.cpp | 24 +++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp index f49c9261f3..0bb0769dd0 100644 --- a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp +++ b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp @@ -102,6 +102,11 @@ void test(const std::string& testCaseDocumentation) { void partialTest(const std::string& reason) { ::testing::Test::RecordProperty("partialyRunTest", reason); } + +/** Add a note to the test. */ +void note(const std::string& note) { + ::testing::Test::RecordProperty("note", note); +} } // Register callback for static object destruction @@ -516,36 +521,41 @@ TEST_F(AudioPrimaryHidlTest, getParameters) { template static void testDebugDump(DebugDump debugDump) { + // Dump in a temporary file + // Note that SELinux must be deactivate for this test to work FILE* file = tmpfile(); ASSERT_NE(nullptr, file) << errno; + // Wrap the temporary file file descriptor in a native handle auto* nativeHandle = native_handle_create(1, 0); ASSERT_NE(nullptr, nativeHandle); nativeHandle->data[0] = fileno(file); + // Wrap this native handle in a hidl handle hidl_handle handle; handle.setTo(nativeHandle, true /*take ownership*/); + ASSERT_OK(debugDump(handle)); + + // Check that at least one bit was written by the hal // TODO: debugDump does not return a Result. // This mean that the hal can not report that it not implementing the // function. - ASSERT_OK(debugDump(handle)); - rewind(file); // can not fail - - // Check that at least one bit was written by the hal char buff; - ASSERT_EQ(size_t{1}, fread(&buff, sizeof(buff), 1, file)); + if (fread(&buff, sizeof(buff), 1, file) != 1) { + doc::note("debugDump does not seem implemented"); + } EXPECT_EQ(0, fclose(file)) << errno; } -TEST_F(AudioPrimaryHidlTest, debugDump) { +TEST_F(AudioPrimaryHidlTest, DebugDump) { doc::test("Check that the hal can dump its state without error"); testDebugDump( [this](const auto& handle) { return device->debugDump(handle); }); } -TEST_F(AudioPrimaryHidlTest, debugDumpInvalidArguments) { +TEST_F(AudioPrimaryHidlTest, DebugDumpInvalidArguments) { doc::test("Check that the hal dump doesn't crash on invalid arguments"); ASSERT_OK(device->debugDump(hidl_handle())); } From 4aefd1c1ff3f92fd02fdfb76d7feda93cdea2909 Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Tue, 2 May 2017 18:58:58 -0700 Subject: [PATCH 08/16] Audio HAL VTS: Getter test assert logic was incorrect Test: vts-tradefed run vts --module VtsHalAudioV2_0Target Test: call/play music/record/video... Bug: 36311550 Change-Id: Iaf2d71829a15b12dcf56e825773c8a697896a264 Signed-off-by: Kevin Rocard --- audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp index 0bb0769dd0..6315af81de 100644 --- a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp +++ b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp @@ -594,7 +594,7 @@ class OpenStreamTest : public AudioConfigPrimaryTest, returnIn(res, stream, suggestedConfigRetry))); // This time it must succeed ASSERT_OK(res); - ASSERT_TRUE(stream == nullptr); + ASSERT_TRUE(stream != nullptr); audioConfig = suggestedConfig; break; default: @@ -895,7 +895,9 @@ static void checkGetNoParameter(IStream* stream, hidl_vec keys, ASSERT_OK(stream->getParameters(keys, returnIn(res, parameters))); ASSERT_RESULT(expectedResults, res); if (res == Result::OK) { - ASSERT_EQ(0U, parameters.size()); + for (auto& parameter : parameters) { + ASSERT_EQ(0U, parameter.value.size()) << toString(parameter); + } } } From 8f8730c7629fe87d8a2123359da2e1a20f926d0e Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Tue, 2 May 2017 19:34:29 -0700 Subject: [PATCH 09/16] Audio HAL VTS: getDevice() == NONE => not supported getDevice does not return a Result, thus it can not return NOT_SUPPORTED. Consider NONE as not supported. Test: vts-tradefed run vts --module VtsHalAudioV2_0Target Test: call/play music/record/video... Bug: 36311550 Change-Id: I3b6f7a1fbc1d1535faf549f5b031461cb39d1722 Signed-off-by: Kevin Rocard --- audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp index 6315af81de..c31026f18c 100644 --- a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp +++ b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp @@ -815,10 +815,15 @@ TEST_IO_STREAM(SupportedFormat, &IStream::getFormat, &IStream::setFormat)) static void testGetDevice(IStream* stream, AudioDevice expectedDevice) { + // Unfortunately the interface does not allow the implementation to return + // NOT_SUPPORTED + // Thus allow NONE as signaling that the call is not supported. auto ret = stream->getDevice(); ASSERT_TRUE(ret.isOk()); AudioDevice device = ret; - ASSERT_EQ(expectedDevice, device); + ASSERT_TRUE(device == expectedDevice || device == AudioDevice::NONE) + << "Expected: " << ::testing::PrintToString(expectedDevice) + << "\n Actual: " << ::testing::PrintToString(device); } TEST_IO_STREAM(GetDevice, From 476e38fd3146989efbac1e02e0e918d1deed9fff Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Wed, 3 May 2017 10:52:43 -0700 Subject: [PATCH 10/16] Audio HAL VTS: GetPresentationPosition may return 0 on stop stream GetPresentationPosition returns the last time a sample was written to the hardware. Calling it on a stop stream should return 0 sample written, but the timestamps of that measure is of little importance. Thus allow this timestamp to be 0 indicating that the measure was never actually made. Test: vts-tradefed run vts --module VtsHalAudioV2_0Target Test: call/play music/record/video... Bug: 36311550 Change-Id: I3c33b60f98e8cbea269a7739cc1889af932dcff5 Signed-off-by: Kevin Rocard --- audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp index c31026f18c..965388673f 100644 --- a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp +++ b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp @@ -1297,6 +1297,13 @@ TEST_P(OutputStreamTest, GetPresentationPositionStop) { } ASSERT_EQ(0U, frames); + if (mesureTS.tvNSec == 0 && mesureTS.tvSec == 0) { + // As the stream has never written a frame yet, + // the timestamp does not really have a meaning, allow to return 0 + return; + } + + // Make sure the return measure is not more than 1s old. struct timespec currentTS; ASSERT_EQ(0, clock_gettime(CLOCK_MONOTONIC, ¤tTS)) << errno; From 98390a6c2cff5b5c75b7236b40a6b72b5b116f17 Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Wed, 3 May 2017 11:16:05 -0700 Subject: [PATCH 11/16] Audio HAL VTS: Some methods are optional Although the method documentation does not say it, some HIDL interface methods are optional. Update the tests to allow NOT_SUPPORTED to be returned. Test: vts-tradefed run vts --module VtsHalAudioV2_0Target Test: call/play music/record/video... Bug: 36311550 Change-Id: If31acc2dbdb6d1d563910e85c99401c48f4f3f86 Signed-off-by: Kevin Rocard --- .../functional/AudioPrimaryHidlHalTest.cpp | 35 +++++++++++++------ 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp index 965388673f..f90ff05e3c 100644 --- a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp +++ b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp @@ -1020,6 +1020,10 @@ TEST_P(InputStreamTest, GetAudioSource) { "Retrieving the audio source of an input stream should always succeed"); AudioSource source; ASSERT_OK(stream->getAudioSource(returnIn(res, source))); + if (res == Result::NOT_SUPPORTED) { + doc::partialTest("getAudioSource is not supported"); + return; + } ASSERT_OK(res); ASSERT_EQ(AudioSource::DEFAULT, source); } @@ -1040,9 +1044,22 @@ static void testUnitaryGain(std::function(float)> setGain) { } } +static void testOptionalUnitaryGain( + std::function(float)> setGain, string debugName) { + auto result = setGain(1); + ASSERT_TRUE(result.isOk()); + if (result == Result::NOT_SUPPORTED) { + doc::partialTest(debugName + " is not supported"); + return; + } + testUnitaryGain(setGain); +} + TEST_P(InputStreamTest, SetGain) { doc::test("The gain of an input stream should only be set between [0,1]"); - testUnitaryGain([this](float volume) { return stream->setGain(volume); }); + testOptionalUnitaryGain( + [this](float volume) { return stream->setGain(volume); }, + "InputStream::setGain"); } static void testPrepareForReading(IStreamIn* stream, uint32_t frameSize, @@ -1108,14 +1125,9 @@ TEST_P(OutputStreamTest, getLatency) { TEST_P(OutputStreamTest, setVolume) { doc::test("Try to set the output volume"); - auto result = stream->setVolume(1, 1); - ASSERT_TRUE(result.isOk()); - if (result == Result::NOT_SUPPORTED) { - doc::partialTest("setVolume is not supported"); - return; - } - testUnitaryGain( - [this](float volume) { return stream->setVolume(volume, volume); }); + testOptionalUnitaryGain( + [this](float volume) { return stream->setVolume(volume, volume); }, + "setVolume"); } static void testPrepareForWriting(IStreamOut* stream, uint32_t frameSize, @@ -1369,8 +1381,9 @@ TEST_F(TtyModeAccessorPrimaryHidlTest, setGetTtyMode) { TEST_F(BoolAccessorPrimaryHidlTest, setGetHac) { doc::test("Query and set the HAC state"); - testAccessors("HAC", {true, false, true}, &IPrimaryDevice::setHacEnabled, - &IPrimaryDevice::getHacEnabled); + testOptionalAccessors("HAC", {true, false, true}, + &IPrimaryDevice::setHacEnabled, + &IPrimaryDevice::getHacEnabled); } ////////////////////////////////////////////////////////////////////////////// From c4f1b2f86a0dd9337577192372219e868a6c59f3 Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Wed, 3 May 2017 11:19:25 -0700 Subject: [PATCH 12/16] Audio HAL VTS: setGain allow -0.0 Previously -0.0 was consider an invalid value as not in the range [0,1]. But it is quite difficult in C++ to differentiate -0.0 and 0.0 as -0.0 == 0.0 and such difference has no impact in practice. Thus leave the implementation support or not -0.0. Test: vts-tradefed run vts --module VtsHalAudioV2_0Target Test: call/play music/record/video... Bug: 36311550 Change-Id: Ia0ebcb325f77adcf8471620f418da1cbe8995e36 Signed-off-by: Kevin Rocard --- .../vts/functional/AudioPrimaryHidlHalTest.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp index f90ff05e3c..0af5c940fb 100644 --- a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp +++ b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp @@ -1029,19 +1029,20 @@ TEST_P(InputStreamTest, GetAudioSource) { } static void testUnitaryGain(std::function(float)> setGain) { - for (float value : {0.0, 0.01, 0.5, 0.09, 1.0}) { - SCOPED_TRACE("value=" + to_string(value)); - ASSERT_OK(setGain(value)); - } - for (float value : (float[]){-INFINITY, -1.0, -0.0, - 1.0 + std::numeric_limits::epsilon(), - 2.0, INFINITY, NAN}) { + 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)); } + // 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)); + } } static void testOptionalUnitaryGain( From 304b6c810eb0209d5c548d63252670ce2396b020 Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Wed, 3 May 2017 10:57:06 -0700 Subject: [PATCH 13/16] Audio HAL VTS: stopped stream state getters may return INVALID_STATE A never started stream should have its render position and next write timestamp at 0 or indicate that the state is invalid. Test: vts-tradefed run vts --module VtsHalAudioV2_0Target Test: call/play music/record/video... Bug: 36311550 Change-Id: I62e16066bb22101ee8f75154fc6c85a66be2f402 Signed-off-by: Kevin Rocard --- .../functional/AudioPrimaryHidlHalTest.cpp | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp index 0af5c940fb..46a2f78c5a 100644 --- a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp +++ b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp @@ -1182,27 +1182,39 @@ TEST_P(OutputStreamTest, SupportsPauseAndResumeAndDrain) { Capability(stream.get()); } +template +static void checkInvalidStateOr0(Result res, Value value) { + switch (res) { + case Result::INVALID_STATE: + break; + case Result::OK: + ASSERT_EQ(0U, value); + break; + default: + FAIL() << "Unexpected result " << toString(res); + } +} + TEST_P(OutputStreamTest, GetRenderPosition) { - doc::test("The render position should be 0 on a not started"); + doc::test("A new stream render position should be 0 or INVALID_STATE"); uint32_t dspFrames; ASSERT_OK(stream->getRenderPosition(returnIn(res, dspFrames))); if (res == Result::NOT_SUPPORTED) { doc::partialTest("getRenderPosition is not supported"); return; } - ASSERT_OK(res); - ASSERT_EQ(0U, dspFrames); + checkInvalidStateOr0(res, dspFrames); } TEST_P(OutputStreamTest, GetNextWriteTimestamp) { - doc::test("The render position of a stream just created should be 0"); + doc::test("A new stream next write timestamp should be 0 or INVALID_STATE"); uint64_t timestampUs; ASSERT_OK(stream->getNextWriteTimestamp(returnIn(res, timestampUs))); if (res == Result::NOT_SUPPORTED) { doc::partialTest("getNextWriteTimestamp is not supported"); return; } - ASSERT_EQ(Result::INVALID_STATE, res); + checkInvalidStateOr0(res, timestampUs); } /** Stub implementation of out stream callback. */ From f26f67a16b1c29993309380e3b33178bf35c4507 Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Tue, 2 May 2017 19:21:58 -0700 Subject: [PATCH 14/16] Audio HAL VTS: Improve ASSERT of Result and Return Previously tests on Result and Return were using ASSERT_NO_FATAL_FAILURE and helper methods. This leaded to complex error messages were the error did not pointed to the helper methods instead of the ASSERT_RESULT call. Additionally SCOPE_TRACE messages are repeated for each ASSERT_NO_FATAL_FAILURE level. Use ::testing::AssertionResult to improve drastically the error messages. Test: vts-tradefed run vts --module VtsHalAudioV2_0Target Test: call/play music/record/video... Bug: 36311550 Change-Id: If705502546606d678df3f1966b0cd9f3ef8c2529 Signed-off-by: Kevin Rocard --- .../functional/AudioPrimaryHidlHalTest.cpp | 24 ++--- audio/2.0/vts/functional/utility/AssertOk.h | 94 ++++++++++++++----- 2 files changed, 81 insertions(+), 37 deletions(-) diff --git a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp index 46a2f78c5a..447ceaad66 100644 --- a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp +++ b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp @@ -318,7 +318,7 @@ class AudioPatchPrimaryHidlTest : public AudioPrimaryHidlTest { protected: bool areAudioPatchesSupported() { auto result = device->supportsAudioPatches(); - EXPECT_TRUE(result.isOk()); + EXPECT_IS_OK(result); return result; } }; @@ -494,10 +494,10 @@ TEST_F(AudioPrimaryHidlTest, setScreenState) { doc::test("Check that the hal can receive the screen state"); for (bool turnedOn : {false, true, true, false, false}) { auto ret = device->setScreenState(turnedOn); - ASSERT_TRUE(ret.isOk()); + ASSERT_IS_OK(ret); Result result = ret; - ASSERT_TRUE(result == Result::OK || result == Result::NOT_SUPPORTED) - << toString(result); + auto okOrNotSupported = {Result::OK, Result::NOT_SUPPORTED}; + ASSERT_RESULT(okOrNotSupported, result); } } @@ -715,7 +715,7 @@ INSTANTIATE_TEST_CASE_P( template static R extract(Return ret) { if (!ret.isOk()) { - ADD_FAILURE(); + EXPECT_IS_OK(ret); return R{}; } return ret; @@ -819,7 +819,7 @@ static void testGetDevice(IStream* stream, AudioDevice expectedDevice) { // NOT_SUPPORTED // Thus allow NONE as signaling that the call is not supported. auto ret = stream->getDevice(); - ASSERT_TRUE(ret.isOk()); + ASSERT_IS_OK(ret); AudioDevice device = ret; ASSERT_TRUE(device == expectedDevice || device == AudioDevice::NONE) << "Expected: " << ::testing::PrintToString(expectedDevice) @@ -891,7 +891,7 @@ TEST_IO_STREAM(SetHwAvSync, "Try to set hardware sync to an invalid value", stream->setHwAvSync(666))) TEST_IO_STREAM(GetHwAvSync, "Get hardware sync can not fail", - ASSERT_TRUE(device->getHwAvSync().isOk())) + ASSERT_IS_OK(device->getHwAvSync())); static void checkGetNoParameter(IStream* stream, hidl_vec keys, vector expectedResults) { @@ -1048,7 +1048,7 @@ static void testUnitaryGain(std::function(float)> setGain) { static void testOptionalUnitaryGain( std::function(float)> setGain, string debugName) { auto result = setGain(1); - ASSERT_TRUE(result.isOk()); + ASSERT_IS_OK(result); if (result == Result::NOT_SUPPORTED) { doc::partialTest(debugName + " is not supported"); return; @@ -1098,7 +1098,7 @@ TEST_P(InputStreamTest, GetInputFramesLost) { doc::test( "The number of frames lost on a never started stream should be 0"); auto ret = stream->getInputFramesLost(); - ASSERT_TRUE(ret.isOk()); + ASSERT_IS_OK(ret); uint32_t framesLost{ret}; ASSERT_EQ(0U, framesLost); } @@ -1120,7 +1120,7 @@ TEST_P(InputStreamTest, getCapturePosition) { TEST_P(OutputStreamTest, getLatency) { doc::test("Make sure latency is over 0"); auto result = stream->getLatency(); - ASSERT_TRUE(result.isOk()); + ASSERT_IS_OK(result); ASSERT_GT(result, 0U); } @@ -1166,7 +1166,7 @@ struct Capability { Capability(IStreamOut* stream) { EXPECT_OK(stream->supportsPauseAndResume(returnIn(pause, resume))); auto ret = stream->supportsDrain(); - EXPECT_TRUE(ret.isOk()); + EXPECT_IS_OK(ret); if (ret.isOk()) { drain = ret; } @@ -1301,7 +1301,7 @@ TEST_P(OutputStreamTest, DrainEarlyNotify) { TEST_P(OutputStreamTest, FlushStop) { doc::test("If supported, a stream should always succeed to flush"); auto ret = stream->flush(); - ASSERT_TRUE(ret.isOk()); + ASSERT_IS_OK(ret); if (ret == Result::NOT_SUPPORTED) { doc::partialTest("Flush is not supported"); return; diff --git a/audio/2.0/vts/functional/utility/AssertOk.h b/audio/2.0/vts/functional/utility/AssertOk.h index 6891dc4af8..4c8440e18c 100644 --- a/audio/2.0/vts/functional/utility/AssertOk.h +++ b/audio/2.0/vts/functional/utility/AssertOk.h @@ -26,49 +26,93 @@ namespace detail { using ::android::hardware::Return; using ::android::hardware::audio::V2_0::Result; -inline void assertResult(Result expected, Result result) { - ASSERT_EQ(expected, result); +template +inline ::testing::AssertionResult assertIsOk(const char* expr, + const Return& ret) { + return ::testing::AssertionResult(ret.isOk()) + << "Expected: " << expr + << "\n to be an OK Return but it is not: " << ret.description(); } -inline void assertResult(Result expected, const Return& ret) { - ASSERT_TRUE(ret.isOk()); - Result result = ret; - assertResult(expected, result); +// Call continuation if the provided result isOk +template +inline ::testing::AssertionResult continueIfIsOk(const char* expr, + const Return& ret, + Continuation continuation) { + auto isOkStatus = assertIsOk(expr, ret); + return !isOkStatus ? isOkStatus : continuation(); } -inline void assertResult(const std::vector& expected, Result result) { +// Expect two equal Results +inline ::testing::AssertionResult assertResult(const char* e_expr, + const char* r_expr, + Result expected, Result result) { + return ::testing::AssertionResult(expected == result) + << "Value of: " << r_expr + << "\n Actual: " << ::testing::PrintToString(result) + << "\nExpected: " << e_expr + << "\nWhich is: " << ::testing::PrintToString(expected); +} + +// Expect two equal Results one being wrapped in an OK Return +inline ::testing::AssertionResult assertResult(const char* e_expr, + const char* r_expr, + Result expected, + const Return& ret) { + return continueIfIsOk(r_expr, ret, [&] { + return assertResult(e_expr, r_expr, expected, Result{ret}); + }); +} + +// Expect a Result to be part of a list of Results +inline ::testing::AssertionResult assertResult( + const char* e_expr, const char* r_expr, const std::vector& expected, + Result result) { if (std::find(expected.begin(), expected.end(), result) != expected.end()) { - return; // result is in expected + return ::testing::AssertionSuccess(); // result is in expected } - FAIL() << "Expected result " << ::testing::PrintToString(result) - << " to be one of " << ::testing::PrintToString(expected); + return ::testing::AssertionFailure() + << "Value of: " << r_expr + << "\n Actual: " << ::testing::PrintToString(result) + << "\nExpected one of: " << e_expr + << "\n Which is: " << ::testing::PrintToString(expected); } -inline void assertResult(const std::vector& expected, - const Return& ret) { - ASSERT_TRUE(ret.isOk()); - Result result = ret; - assertResult(expected, result); +// Expect a Result wrapped in an OK Return to be part of a list of Results +inline ::testing::AssertionResult assertResult( + const char* e_expr, const char* r_expr, const std::vector& expected, + const Return& ret) { + return continueIfIsOk(r_expr, ret, [&] { + return assertResult(e_expr, r_expr, expected, Result{ret}); + }); } -inline void assertOk(const Return& ret) { - ASSERT_TRUE(ret.isOk()); +inline ::testing::AssertionResult assertOk(const char* expr, + const Return& ret) { + return assertIsOk(expr, ret); } -inline void assertOk(Result result) { - assertResult(Result::OK, result); +inline ::testing::AssertionResult assertOk(const char* expr, Result result) { + return ::testing::AssertionResult(result == Result::OK) + << "Expected success: " << expr + << "\nActual: " << ::testing::PrintToString(result); } -inline void assertOk(const Return& ret) { - assertResult(Result::OK, ret); +inline ::testing::AssertionResult assertOk(const char* expr, + const Return& ret) { + return continueIfIsOk(expr, ret, + [&] { return assertOk(expr, Result{ret}); }); } } +#define ASSERT_IS_OK(ret) ASSERT_PRED_FORMAT1(detail::assertIsOk, ret) +#define EXPECT_IS_OK(ret) EXPECT_PRED_FORMAT1(detail::assertIsOk, ret) + // Test anything provided is and contains only OK -#define ASSERT_OK(ret) ASSERT_NO_FATAL_FAILURE(detail::assertOk(ret)) -#define EXPECT_OK(ret) EXPECT_NO_FATAL_FAILURE(detail::assertOk(ret)) +#define ASSERT_OK(ret) ASSERT_PRED_FORMAT1(detail::assertOk, ret) +#define EXPECT_OK(ret) EXPECT_PRED_FORMAT1(detail::assertOk, ret) #define ASSERT_RESULT(expected, ret) \ - ASSERT_NO_FATAL_FAILURE(detail::assertResult(expected, ret)) + ASSERT_PRED_FORMAT2(detail::assertResult, expected, ret) #define EXPECT_RESULT(expected, ret) \ - EXPECT_NO_FATAL_FAILURE(detail::assertResult(expected, ret)) + EXPECT_PRED_FORMAT2(detail::assertResult, expected, ret) From 96f46c4a23e8acce582bba8d6fc6b1d5a61af319 Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Mon, 8 May 2017 11:53:07 -0700 Subject: [PATCH 15/16] Audio HAL VTS: Log test unexpected behaviour Some test output infos that are useful to understand how they run. Unfortunately the xml report does not seem to be saved by VTS tradefed. Thus output them in logcat. Test: vts-tradefed run vts --module VtsHalAudioV2_0Target Test: call/play music/record/video... Bug: 36311550 Change-Id: I9a2cc10160c3b1c8f81db0464efbc6b26600cadc Signed-off-by: Kevin Rocard --- audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp index 447ceaad66..83a1db04c8 100644 --- a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp +++ b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp @@ -83,6 +83,10 @@ using ::android::hardware::audio::common::V2_0::ThreadInfo; using utility::returnIn; +const char* getTestName() { + return ::testing::UnitTest::GetInstance()->current_test_info()->name(); +} + namespace doc { /** Document the current test case. * Eg: calling `doc::test("Dump the state of the hal")` in the "debugDump" test @@ -100,11 +104,13 @@ void test(const std::string& testCaseDocumentation) { /** Document why a test was not fully run. Usually due to an optional feature * not implemented. */ void partialTest(const std::string& reason) { + LOG(INFO) << "Test " << getTestName() << " partially run: " << reason; ::testing::Test::RecordProperty("partialyRunTest", reason); } /** Add a note to the test. */ void note(const std::string& note) { + LOG(INFO) << "Test " << getTestName() << " noted: " << note; ::testing::Test::RecordProperty("note", note); } } From 4c030024dfa9cdf924cb771580e13d237f1ec813 Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Mon, 8 May 2017 17:08:11 -0700 Subject: [PATCH 16/16] Audio HAL: A volume/gain 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: Ia4880bdff6111cbcdae6a4ebee921eddae141ee4 Signed-off-by: Kevin Rocard --- audio/2.0/default/Device.cpp | 14 ++++++++----- audio/2.0/default/StreamIn.cpp | 5 +++++ audio/2.0/default/StreamOut.cpp | 15 ++++++++----- audio/2.0/default/Util.h | 37 +++++++++++++++++++++++++++++++++ 4 files changed, 61 insertions(+), 10 deletions(-) create mode 100644 audio/2.0/default/Util.h diff --git a/audio/2.0/default/Device.cpp b/audio/2.0/default/Device.cpp index 9246f8b4e3..280d32058e 100644 --- a/audio/2.0/default/Device.cpp +++ b/audio/2.0/default/Device.cpp @@ -30,6 +30,7 @@ #include "HidlUtils.h" #include "StreamIn.h" #include "StreamOut.h" +#include "Util.h" namespace android { namespace hardware { @@ -141,12 +142,15 @@ Return Device::initCheck() { } Return Device::setMasterVolume(float volume) { - Result retval(Result::NOT_SUPPORTED); - if (mDevice->set_master_volume != NULL) { - retval = analyzeStatus("set_master_volume", - mDevice->set_master_volume(mDevice, volume)); + if (mDevice->set_master_volume == NULL) { + return Result::NOT_SUPPORTED; } - return retval; + if (!isGainNormalized(volume)) { + ALOGW("Can not set a master volume (%f) outside [0,1]", volume); + return Result::INVALID_ARGUMENTS; + } + return analyzeStatus("set_master_volume", + mDevice->set_master_volume(mDevice, volume)); } Return Device::getMasterVolume(getMasterVolume_cb _hidl_cb) { diff --git a/audio/2.0/default/StreamIn.cpp b/audio/2.0/default/StreamIn.cpp index 9feef15cdf..abd0497fe1 100644 --- a/audio/2.0/default/StreamIn.cpp +++ b/audio/2.0/default/StreamIn.cpp @@ -24,6 +24,7 @@ #include #include "StreamIn.h" +#include "Util.h" using ::android::hardware::audio::V2_0::MessageQueueFlagBits; @@ -311,6 +312,10 @@ Return StreamIn::getAudioSource(getAudioSource_cb _hidl_cb) { } Return StreamIn::setGain(float gain) { + if (!isGainNormalized(gain)) { + ALOGW("Can not set a stream input gain (%f) outside [0,1]", gain); + return Result::INVALID_ARGUMENTS; + } return Stream::analyzeStatus("set_gain", mStream->set_gain(mStream, gain)); } diff --git a/audio/2.0/default/StreamOut.cpp b/audio/2.0/default/StreamOut.cpp index eb53259f9c..e48497f6c6 100644 --- a/audio/2.0/default/StreamOut.cpp +++ b/audio/2.0/default/StreamOut.cpp @@ -25,6 +25,7 @@ #include #include "StreamOut.h" +#include "Util.h" namespace android { namespace hardware { @@ -282,12 +283,16 @@ Return StreamOut::getLatency() { } Return StreamOut::setVolume(float left, float right) { - Result retval(Result::NOT_SUPPORTED); - if (mStream->set_volume != NULL) { - retval = Stream::analyzeStatus( - "set_volume", mStream->set_volume(mStream, left, right)); + if (mStream->set_volume == NULL) { + return Result::NOT_SUPPORTED; } - return retval; + if (!isGainNormalized(left)) { + ALOGW("Can not set a stream output volume {%f, %f} outside [0,1]", left, + right); + return Result::INVALID_ARGUMENTS; + } + return Stream::analyzeStatus("set_volume", + mStream->set_volume(mStream, left, right)); } Return StreamOut::prepareForWriting(uint32_t frameSize, diff --git a/audio/2.0/default/Util.h b/audio/2.0/default/Util.h new file mode 100644 index 0000000000..72eea5037b --- /dev/null +++ b/audio/2.0/default/Util.h @@ -0,0 +1,37 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef ANDROID_HARDWARE_AUDIO_V2_0_UTIL_H +#define ANDROID_HARDWARE_AUDIO_V2_0_UTIL_H + +namespace android { +namespace hardware { +namespace audio { +namespace V2_0 { +namespace implementation { + +/** @return true if gain is between 0 and 1 included. */ +constexpr bool isGainNormalized(float gain) { + return gain >= 0.0 && gain <= 1.0; +} + +} // namespace implementation +} // namespace V2_0 +} // namespace audio +} // namespace hardware +} // namespace android + +#endif // ANDROID_HARDWARE_AUDIO_V2_0_UTIL_H