Audio HAL: fixes for issues discovered after client conversion
Several issues addressed:
-- added IDevice.supportsAudioPatches to query whether
create/removeAudioPatch is actually supported by HAL;
-- IStreamOutCallback proxy needs to be owned by IStreamOut
implementation. In order for the client to reset the reference,
added method IStreamOut.clearCallback;
-- IDevice.open{Input|Output}Stream need to return a "suggested" audio
config from HAL;
-- code for converting between system/audio.h and HIDL
data structures has been moved to
android.hardware.audio.common@2.0-util library for reuse;
-- added a workaround for the issue with QC effects HAL trying to write
into the input parameters buffer, which is r/o by Binder design.
Bug: 30222631
Change-Id: I64af24d79c12d6ac3b0f87d085a821913e29237b
Test: tried using with WIP HIDL client on N5X
diff --git a/audio/effect/2.0/default/Conversions.cpp b/audio/effect/2.0/default/Conversions.cpp
index ef2374c..e7d4c46 100644
--- a/audio/effect/2.0/default/Conversions.cpp
+++ b/audio/effect/2.0/default/Conversions.cpp
@@ -18,6 +18,7 @@
#include <stdio.h>
#include "Conversions.h"
+#include "HidlUtils.h"
namespace android {
namespace hardware {
@@ -28,8 +29,8 @@
void effectDescriptorFromHal(
const effect_descriptor_t& halDescriptor, EffectDescriptor* descriptor) {
- uuidFromHal(halDescriptor.type, &descriptor->type);
- uuidFromHal(halDescriptor.uuid, &descriptor->uuid);
+ HidlUtils::uuidFromHal(halDescriptor.type, &descriptor->type);
+ HidlUtils::uuidFromHal(halDescriptor.uuid, &descriptor->uuid);
descriptor->flags = EffectFlags(halDescriptor.flags);
descriptor->cpuLoad = halDescriptor.cpuLoad;
descriptor->memoryUsage = halDescriptor.memoryUsage;
@@ -38,22 +39,6 @@
halDescriptor.implementor, descriptor->implementor.size());
}
-void uuidFromHal(const effect_uuid_t& halUuid, Uuid* uuid) {
- uuid->timeLow = halUuid.timeLow;
- uuid->timeMid = halUuid.timeMid;
- uuid->versionAndTimeHigh = halUuid.timeHiAndVersion;
- uuid->variantAndClockSeqHigh = halUuid.clockSeq;
- memcpy(uuid->node.data(), halUuid.node, uuid->node.size());
-}
-
-void uuidToHal(const Uuid& uuid, effect_uuid_t* halUuid) {
- halUuid->timeLow = uuid.timeLow;
- halUuid->timeMid = uuid.timeMid;
- halUuid->timeHiAndVersion = uuid.versionAndTimeHigh;
- halUuid->clockSeq = uuid.variantAndClockSeqHigh;
- memcpy(halUuid->node, uuid.node.data(), uuid.node.size());
-}
-
std::string uuidToString(const effect_uuid_t& halUuid) {
char str[64];
snprintf(str, sizeof(str), "%08x-%04x-%04x-%04x-%02x%02x%02x%02x%02x%02x",
diff --git a/audio/effect/2.0/default/Conversions.h b/audio/effect/2.0/default/Conversions.h
index 5348ae6..7cef362 100644
--- a/audio/effect/2.0/default/Conversions.h
+++ b/audio/effect/2.0/default/Conversions.h
@@ -29,13 +29,10 @@
namespace V2_0 {
namespace implementation {
-using ::android::hardware::audio::common::V2_0::Uuid;
using ::android::hardware::audio::effect::V2_0::EffectDescriptor;
void effectDescriptorFromHal(
const effect_descriptor_t& halDescriptor, EffectDescriptor* descriptor);
-void uuidFromHal(const effect_uuid_t& halUuid, Uuid* uuid);
-void uuidToHal(const Uuid& uuid, effect_uuid_t* halUuid);
std::string uuidToString(const effect_uuid_t& halUuid);
} // namespace implementation
diff --git a/audio/effect/2.0/default/Effect.cpp b/audio/effect/2.0/default/Effect.cpp
index 82d0292..6553da4 100644
--- a/audio/effect/2.0/default/Effect.cpp
+++ b/audio/effect/2.0/default/Effect.cpp
@@ -14,7 +14,6 @@
* limitations under the License.
*/
-#include <memory>
#include <memory.h>
#define LOG_TAG "EffectHAL"
@@ -56,10 +55,15 @@
}
// static
-template<typename T> void Effect::hidlVecToHal(
- const hidl_vec<T>& vec, uint32_t* halDataSize, void** halData) {
- *halDataSize = static_cast<T>(vec.size() * sizeof(T));
- *halData = static_cast<void*>(const_cast<T*>(&vec[0]));
+template<typename T> std::unique_ptr<uint8_t[]> Effect::hidlVecToHal(
+ const hidl_vec<T>& vec, uint32_t* halDataSize) {
+ // Due to bugs in HAL, they may attempt to write into the provided
+ // input buffer. The original binder buffer is r/o, thus it is needed
+ // to create a r/w version.
+ *halDataSize = vec.size() * sizeof(T);
+ std::unique_ptr<uint8_t[]> halData(new uint8_t[*halDataSize]);
+ memcpy(&halData[0], &vec[0], *halDataSize);
+ return halData;
}
// static
@@ -393,12 +397,13 @@
Return<void> Effect::setAndGetVolume(
const hidl_vec<uint32_t>& volumes, setAndGetVolume_cb _hidl_cb) {
uint32_t halDataSize;
- void *halData;
- hidlVecToHal(volumes, &halDataSize, &halData);
+ std::unique_ptr<uint8_t[]> halData = hidlVecToHal(volumes, &halDataSize);
uint32_t halResultSize = halDataSize;
uint32_t halResult[volumes.size()];
Result retval = sendCommandReturningData(
- EFFECT_CMD_SET_VOLUME, "SET_VOLUME", halDataSize, halData, &halResultSize, halResult);
+ EFFECT_CMD_SET_VOLUME, "SET_VOLUME",
+ halDataSize, &halData[0],
+ &halResultSize, halResult);
hidl_vec<uint32_t> result;
if (retval == Result::OK) {
result.setToExternal(&halResult[0], halResultSize);
@@ -528,13 +533,12 @@
uint32_t resultMaxSize,
command_cb _hidl_cb) {
uint32_t halDataSize;
- void *halData;
- hidlVecToHal(data, &halDataSize, &halData);
+ std::unique_ptr<uint8_t[]> halData = hidlVecToHal(data, &halDataSize);
uint32_t halResultSize = resultMaxSize;
std::unique_ptr<uint8_t[]> halResult(new uint8_t[halResultSize]);
memset(&halResult[0], 0, halResultSize);
status_t status = (*mHandle)->command(
- mHandle, commandId, halDataSize, halData, &halResultSize, &halResult[0]);
+ mHandle, commandId, halDataSize, &halData[0], &halResultSize, &halResult[0]);
hidl_vec<uint8_t> result;
if (status == OK) {
result.setToExternal(&halResult[0], halResultSize);
diff --git a/audio/effect/2.0/default/Effect.h b/audio/effect/2.0/default/Effect.h
index e27a7e4..ff57051 100644
--- a/audio/effect/2.0/default/Effect.h
+++ b/audio/effect/2.0/default/Effect.h
@@ -17,6 +17,7 @@
#ifndef HIDL_GENERATED_android_hardware_audio_effect_V2_0_Effect_H_
#define HIDL_GENERATED_android_hardware_audio_effect_V2_0_Effect_H_
+#include <memory>
#include <vector>
#include <android/hardware/audio/effect/2.0/IEffect.h>
@@ -180,8 +181,8 @@
virtual ~Effect();
template<typename T> static size_t alignedSizeIn(size_t s);
- template<typename T> static void hidlVecToHal(
- const hidl_vec<T>& vec, uint32_t* halDataSize, void** halData);
+ template<typename T> std::unique_ptr<uint8_t[]> hidlVecToHal(
+ const hidl_vec<T>& vec, uint32_t* halDataSize);
static void effectAuxChannelsConfigFromHal(
const channel_config_t& halConfig, EffectAuxChannelsConfig* config);
static void effectAuxChannelsConfigToHal(
diff --git a/audio/effect/2.0/default/EffectsFactory.cpp b/audio/effect/2.0/default/EffectsFactory.cpp
index 2b5d70b..2bfe6af 100644
--- a/audio/effect/2.0/default/EffectsFactory.cpp
+++ b/audio/effect/2.0/default/EffectsFactory.cpp
@@ -33,8 +33,9 @@
#include "AutomaticGainControlEffect.h"
#include "BassBoostEffect.h"
#include "Conversions.h"
-#include "EffectsFactory.h"
#include "DownmixEffect.h"
+#include "EffectsFactory.h"
+#include "HidlUtils.h"
#include "Effect.h"
#include "EffectMap.h"
#include "EnvironmentalReverbEffect.h"
@@ -131,7 +132,7 @@
Return<void> EffectsFactory::getDescriptor(const Uuid& uid, getDescriptor_cb _hidl_cb) {
effect_uuid_t halUuid;
- uuidToHal(uid, &halUuid);
+ HidlUtils::uuidToHal(uid, &halUuid);
effect_descriptor_t halDescriptor;
status_t status = EffectGetDescriptor(&halUuid, &halDescriptor);
EffectDescriptor descriptor;
@@ -153,7 +154,7 @@
Return<void> EffectsFactory::createEffect(
const Uuid& uid, int32_t session, int32_t ioHandle, createEffect_cb _hidl_cb) {
effect_uuid_t halUuid;
- uuidToHal(uid, &halUuid);
+ HidlUtils::uuidToHal(uid, &halUuid);
effect_handle_t handle;
Result retval(Result::OK);
status_t status = EffectCreate(&halUuid, session, ioHandle, &handle);