Remove AudioAttributesInternal and media.AudioFlag
There exist equivalent types from the stable package
android.media.audio.common. Switch to these types
and remove duplicates.
Update aidl<->legacy conversions to allow non-vendor
tags within AudioAttributes. Non-vendor tags only get
filtered out on the HAL boundary: cpp<->ndk.
Bug: 281850726
Test: atest audioclient_serialization_tests
Test: atest audio_aidl_ndk_conversion_tests
Test: atest audiorouting_tests
Test: atest audiosystem_tests
Merged-In: If1ff43110c54613c5c29b42a2dc3e5d832708c28
Change-Id: If1ff43110c54613c5c29b42a2dc3e5d832708c28
diff --git a/media/audioaidlconversion/AidlConversionCppNdk.cpp b/media/audioaidlconversion/AidlConversionCppNdk.cpp
index ff141eb..a0de3a6 100644
--- a/media/audioaidlconversion/AidlConversionCppNdk.cpp
+++ b/media/audioaidlconversion/AidlConversionCppNdk.cpp
@@ -19,7 +19,6 @@
#include <algorithm>
#include <map>
#include <sstream>
-#include <regex>
#include <utility>
#include <vector>
@@ -101,12 +100,6 @@
namespace {
-bool isVendorExtension(const std::string& s) {
- // Must be the same as defined in AudioAttributes.aidl and {Playback|Record}TrackMetadata.aidl
- static const std::regex vendorExtension("VX_[A-Z0-9]{3,}_[_A-Z0-9]+");
- return std::regex_match(s.begin(), s.end(), vendorExtension);
-}
-
std::vector<std::string> splitString(const std::string& s, char separator) {
std::istringstream iss(s);
std::string t;
@@ -1815,11 +1808,6 @@
return unexpected(BAD_VALUE);
}
-namespace {
-
-// TODO(b/281850726): Expose publicly once android.media.AudioFlag is removed.
-// Until that, `legacy2aidl_audio_flags_mask_t_AudioFlag` function is ambiguous.
-
ConversionResult<audio_flags_mask_t>
aidl2legacy_AudioFlag_audio_flags_mask_t(AudioFlag aidl) {
switch (aidl) {
@@ -1921,8 +1909,6 @@
enumToMask_bitmask<int32_t, AudioFlag>);
}
-} // namespace
-
ConversionResult<std::string>
aidl2legacy_AudioTags_string(const std::vector<std::string>& aidl) {
std::ostringstream tagsBuffer;
@@ -1931,17 +1917,12 @@
if (hasValue) {
tagsBuffer << AUDIO_ATTRIBUTES_TAGS_SEPARATOR;
}
- if (isVendorExtension(tag)) {
- // Note: with the current regex for vendor tags: VX_[A-Z0-9]{3,}_[_A-Z0-9]+
- // it's impossible to create a vendor tag that would contain the separator, but in case
- // the criteria changes, we double check it here.
- if (strchr(tag.c_str(), AUDIO_ATTRIBUTES_TAGS_SEPARATOR) == nullptr) {
- tagsBuffer << tag;
- hasValue = true;
- } else {
- ALOGE("Vendor extension tag is ill-formed: \"%s\"", tag.c_str());
- return unexpected(BAD_VALUE);
- }
+ if (strchr(tag.c_str(), AUDIO_ATTRIBUTES_TAGS_SEPARATOR) == nullptr) {
+ tagsBuffer << tag;
+ hasValue = true;
+ } else {
+ ALOGE("Tag is ill-formed: \"%s\"", tag.c_str());
+ return unexpected(BAD_VALUE);
}
}
return tagsBuffer.str();
@@ -1949,11 +1930,7 @@
ConversionResult<std::vector<std::string>>
legacy2aidl_string_AudioTags(const std::string& legacy) {
- auto allTags = splitString(legacy, AUDIO_ATTRIBUTES_TAGS_SEPARATOR);
- std::vector<std::string> result;
- std::copy_if(std::make_move_iterator(allTags.begin()), std::make_move_iterator(allTags.end()),
- std::back_inserter(result), isVendorExtension);
- return result;
+ return splitString(legacy, AUDIO_ATTRIBUTES_TAGS_SEPARATOR);
}
ConversionResult<audio_attributes_t>
diff --git a/media/audioaidlconversion/AidlConversionNdkCpp.cpp b/media/audioaidlconversion/AidlConversionNdkCpp.cpp
index c64a074..ecd2e5e 100644
--- a/media/audioaidlconversion/AidlConversionNdkCpp.cpp
+++ b/media/audioaidlconversion/AidlConversionNdkCpp.cpp
@@ -15,6 +15,7 @@
*/
#include <algorithm>
+#include <regex>
#include <type_traits>
#define LOG_TAG "AidlConversionNdkCpp"
@@ -34,6 +35,24 @@
namespace {
+bool isVendorExtension(const std::string& s) {
+ // Per definition in AudioAttributes.aidl and {Playback|Record}TrackMetadata.aidl
+ static const std::regex vendorExtension("VX_[A-Z0-9]{3,}_[_A-Z0-9]+");
+ return std::regex_match(s.begin(), s.end(), vendorExtension);
+}
+
+inline bool isNotVendorExtension(const std::string& s) { return !isVendorExtension(s); }
+
+void filterOutNonVendorTagsInPlace(std::vector<std::string>& tags) {
+ if (std::find_if(tags.begin(), tags.end(), isNotVendorExtension) == tags.end()) {
+ return;
+ }
+ std::vector<std::string> temp;
+ temp.reserve(tags.size());
+ std::copy_if(tags.begin(), tags.end(), std::back_inserter(temp), isVendorExtension);
+ tags = std::move(temp);
+}
+
// cpp2ndk and ndk2cpp are universal converters which work for any type,
// however they are not the most efficient way to convert due to extra
// marshaling / unmarshaling step.
@@ -99,12 +118,15 @@
} // namespace
-#define GENERATE_CONVERTERS(packageName, className) \
- ConversionResult<::aidl::packageName::className> cpp2ndk_##className( \
+#define GENERATE_CONVERTERS(packageName, className) \
+ GENERATE_CONVERTERS_IMPL(packageName, _, className)
+
+#define GENERATE_CONVERTERS_IMPL(packageName, prefix, className) \
+ ConversionResult<::aidl::packageName::className> cpp2ndk##prefix##className( \
const ::packageName::className& cpp) { \
return cpp2ndk<::aidl::packageName::className>(cpp); \
} \
- ConversionResult<::packageName::className> ndk2cpp_##className( \
+ ConversionResult<::packageName::className> ndk2cpp##prefix##className( \
const ::aidl::packageName::className& ndk) { \
return ndk2cpp<::packageName::className>(ndk); \
}
@@ -120,10 +142,46 @@
}
GENERATE_CONVERTERS(android::media::audio::common, AudioFormatDescription);
-GENERATE_CONVERTERS(android::media::audio::common, AudioHalEngineConfig);
+GENERATE_CONVERTERS_IMPL(android::media::audio::common, _Impl_, AudioHalEngineConfig);
GENERATE_CONVERTERS(android::media::audio::common, AudioMMapPolicyInfo);
GENERATE_ENUM_CONVERTERS(android::media::audio::common, AudioMMapPolicyType);
GENERATE_ENUM_CONVERTERS(android::media::audio::common, AudioMode);
GENERATE_CONVERTERS(android::media::audio::common, AudioPort);
+namespace {
+
+// Filter out all AudioAttributes tags that do not conform to the vendor extension pattern.
+template<typename T>
+void filterOutNonVendorTags(T& audioHalEngineConfig) {
+ for (auto& strategy : audioHalEngineConfig.productStrategies) {
+ for (auto& group : strategy.attributesGroups) {
+ for (auto& attr : group.attributes) {
+ filterOutNonVendorTagsInPlace(attr.tags);
+ }
+ }
+ }
+}
+
+} // namespace
+
+ConversionResult<::aidl::android::media::audio::common::AudioHalEngineConfig>
+cpp2ndk_AudioHalEngineConfig(const ::android::media::audio::common::AudioHalEngineConfig& cpp) {
+ auto conv = cpp2ndk_Impl_AudioHalEngineConfig(cpp);
+ if (conv.ok()) {
+ filterOutNonVendorTags(conv.value());
+ }
+ return conv;
+}
+
+ConversionResult<::android::media::audio::common::AudioHalEngineConfig>
+ndk2cpp_AudioHalEngineConfig(
+ const ::aidl::android::media::audio::common::AudioHalEngineConfig& ndk) {
+ auto conv = ndk2cpp_Impl_AudioHalEngineConfig(ndk);
+ if (conv.ok()) {
+ filterOutNonVendorTags(conv.value());
+ }
+ return conv;
+}
+
+
} // namespace android
diff --git a/media/audioaidlconversion/include/media/AidlConversionCppNdk-impl.h b/media/audioaidlconversion/include/media/AidlConversionCppNdk-impl.h
index fc57f72..7268464 100644
--- a/media/audioaidlconversion/include/media/AidlConversionCppNdk-impl.h
+++ b/media/audioaidlconversion/include/media/AidlConversionCppNdk-impl.h
@@ -357,6 +357,16 @@
ConversionResult<media::audio::common::AudioUsage> legacy2aidl_audio_usage_t_AudioUsage(
audio_usage_t legacy);
+ConversionResult<audio_flags_mask_t>
+aidl2legacy_AudioFlag_audio_flags_mask_t(media::audio::common::AudioFlag aidl);
+ConversionResult<media::audio::common::AudioFlag>
+legacy2aidl_audio_flags_mask_t_AudioFlag(audio_flags_mask_t legacy);
+
+ConversionResult<audio_flags_mask_t>
+aidl2legacy_int32_t_audio_flags_mask_t_mask(int32_t aidl);
+ConversionResult<int32_t>
+legacy2aidl_audio_flags_mask_t_int32_t_mask(audio_flags_mask_t legacy);
+
ConversionResult<std::string>
aidl2legacy_AudioTags_string(const std::vector<std::string>& aidl);
ConversionResult<std::vector<std::string>>
diff --git a/media/audioaidlconversion/tests/audio_aidl_ndk_conversion_tests.cpp b/media/audioaidlconversion/tests/audio_aidl_ndk_conversion_tests.cpp
index 056698f..60727b4 100644
--- a/media/audioaidlconversion/tests/audio_aidl_ndk_conversion_tests.cpp
+++ b/media/audioaidlconversion/tests/audio_aidl_ndk_conversion_tests.cpp
@@ -107,25 +107,31 @@
std::vector<std::string>{"VX_GOOGLE_41"},
std::vector<std::string>{"VX_GOOGLE_41", "VX_GOOGLE_42"}));
-TEST(AudioTags, NonVendorTags) {
+TEST(AudioTags, NonVendorTagsAllowed) {
const std::string separator(1, AUDIO_ATTRIBUTES_TAGS_SEPARATOR);
- const std::vector<std::string> initial{
- "random_string", "random" + separator + "string", "VX_GOOGLE_42"};
+ const std::vector<std::string> initial{"random_string", "VX_GOOGLE_42"};
auto conv = aidl2legacy_AudioTags_string(initial);
ASSERT_TRUE(conv.ok());
- EXPECT_EQ("VX_GOOGLE_42", conv.value());
+ EXPECT_EQ("random_string" + separator + "VX_GOOGLE_42", conv.value());
}
TEST(AudioTags, IllFormedAidlTag) {
const std::string separator(1, AUDIO_ATTRIBUTES_TAGS_SEPARATOR);
- const std::vector<std::string> initial{"VX_GOOGLE" + separator + "42", "VX_GOOGLE_42"};
- auto conv = aidl2legacy_AudioTags_string(initial);
- // Note: with the current regex for vendor tags: VX_[A-Z0-9]{3,}_[_A-Z0-9]+
- // it's impossible to create a vendor tag that would contain the separator, but in case
- // the criteria changes, we ensure that either such tags get filtered out or an error happens.
- if (conv.ok()) {
- EXPECT_EQ("VX_GOOGLE_42", conv.value());
- } else {
- EXPECT_FALSE(conv.ok()) << conv.value();
+ {
+ const std::vector<std::string> initial{"VX_GOOGLE" + separator + "42", "VX_GOOGLE_42"};
+ auto conv = aidl2legacy_AudioTags_string(initial);
+ if (conv.ok()) {
+ EXPECT_EQ("VX_GOOGLE_42", conv.value());
+ }
+ // Failing this conversion is also OK. The result depends on whether the conversion
+ // only passes through vendor tags.
+ }
+ {
+ const std::vector<std::string> initial{
+ "random_string", "random" + separator + "string", "VX_GOOGLE_42"};
+ auto conv = aidl2legacy_AudioTags_string(initial);
+ if (conv.ok()) {
+ EXPECT_EQ("VX_GOOGLE_42", conv.value());
+ }
}
}