Update AIDL<->legacy conversion for AudioChannelLayout
Index masks can contain arbitrary bits set, thus the range
of valid values is not restricted to AUDIO_CHANNEL_INDEX_MASK_*
values from system/audio.h.
Bug: 188932434
Bug: 195346865
Test: atest audio_aidl_conversion_tests
Test: atest android.media.cts.AudioNativeTest#testInputChannelMasks
Change-Id: Ic09c89c996726d9be4563c9d55327bbe9a5b199d
diff --git a/media/libaudioclient/AidlConversion.cpp b/media/libaudioclient/AidlConversion.cpp
index 4c155fe..b0eb7bd 100644
--- a/media/libaudioclient/AidlConversion.cpp
+++ b/media/libaudioclient/AidlConversion.cpp
@@ -396,44 +396,6 @@
using AudioFormatPairs = std::vector<AudioFormatPair>;
}
-const detail::AudioChannelPairs& getIndexAudioChannelPairs() {
- static const detail::AudioChannelPairs pairs = {
-#define DEFINE_INDEX_MASK(n) \
- { \
- AUDIO_CHANNEL_INDEX_MASK_##n, \
- media::AudioChannelLayout::make<media::AudioChannelLayout::Tag::indexMask>( \
- media::AudioChannelLayout::INDEX_MASK_##n) \
- }
-
- DEFINE_INDEX_MASK(1),
- DEFINE_INDEX_MASK(2),
- DEFINE_INDEX_MASK(3),
- DEFINE_INDEX_MASK(4),
- DEFINE_INDEX_MASK(5),
- DEFINE_INDEX_MASK(6),
- DEFINE_INDEX_MASK(7),
- DEFINE_INDEX_MASK(8),
- DEFINE_INDEX_MASK(9),
- DEFINE_INDEX_MASK(10),
- DEFINE_INDEX_MASK(11),
- DEFINE_INDEX_MASK(12),
- DEFINE_INDEX_MASK(13),
- DEFINE_INDEX_MASK(14),
- DEFINE_INDEX_MASK(15),
- DEFINE_INDEX_MASK(16),
- DEFINE_INDEX_MASK(17),
- DEFINE_INDEX_MASK(18),
- DEFINE_INDEX_MASK(19),
- DEFINE_INDEX_MASK(20),
- DEFINE_INDEX_MASK(21),
- DEFINE_INDEX_MASK(22),
- DEFINE_INDEX_MASK(23),
- DEFINE_INDEX_MASK(24)
-#undef DEFINE_INDEX_MASK
- };
- return pairs;
-}
-
const detail::AudioChannelPairs& getInAudioChannelPairs() {
static const detail::AudioChannelPairs pairs = {
#define DEFINE_INPUT_LAYOUT(n) \
@@ -1096,7 +1058,6 @@
const media::AudioChannelLayout& aidl, bool isInput) {
using ReverseMap = std::unordered_map<media::AudioChannelLayout, audio_channel_mask_t>;
using Tag = media::AudioChannelLayout::Tag;
- static const ReverseMap mIdx = make_ReverseMap(getIndexAudioChannelPairs());
static const ReverseMap mIn = make_ReverseMap(getInAudioChannelPairs());
static const ReverseMap mOut = make_ReverseMap(getOutAudioChannelPairs());
static const ReverseMap mVoice = make_ReverseMap(getVoiceAudioChannelPairs());
@@ -1117,8 +1078,19 @@
return AUDIO_CHANNEL_NONE;
case Tag::invalid:
return AUDIO_CHANNEL_INVALID;
- case Tag::indexMask:
- return convert(aidl, mIdx, __func__, "index");
+ case Tag::indexMask: {
+ // Index masks do not have pre-defined values.
+ const int bits = aidl.get<Tag::indexMask>();
+ if (__builtin_popcount(bits) != 0 &&
+ __builtin_popcount(bits) <= AUDIO_CHANNEL_COUNT_MAX) {
+ return audio_channel_mask_from_representation_and_bits(
+ AUDIO_CHANNEL_REPRESENTATION_INDEX, bits);
+ } else {
+ ALOGE("%s: invalid indexMask value 0x%x in %s",
+ __func__, bits, aidl.toString().c_str());
+ return unexpected(BAD_VALUE);
+ }
+ }
case Tag::layoutMask:
return convert(aidl, isInput ? mIn : mOut, __func__, isInput ? "input" : "output");
case Tag::voiceMask:
@@ -1132,7 +1104,6 @@
audio_channel_mask_t legacy, bool isInput) {
using DirectMap = std::unordered_map<audio_channel_mask_t, media::AudioChannelLayout>;
using Tag = media::AudioChannelLayout::Tag;
- static const DirectMap mIdx = make_DirectMap(getIndexAudioChannelPairs());
static const DirectMap mInAndVoice = make_DirectMap(
getInAudioChannelPairs(), getVoiceAudioChannelPairs());
static const DirectMap mOut = make_DirectMap(getOutAudioChannelPairs());
@@ -1156,7 +1127,14 @@
const audio_channel_representation_t repr = audio_channel_mask_get_representation(legacy);
if (repr == AUDIO_CHANNEL_REPRESENTATION_INDEX) {
- return convert(legacy, mIdx, __func__, "index");
+ if (audio_channel_mask_is_valid(legacy)) {
+ const int indexMask = VALUE_OR_RETURN(
+ convertIntegral<int>(audio_channel_mask_get_bits(legacy)));
+ return media::AudioChannelLayout::make<Tag::indexMask>(indexMask);
+ } else {
+ ALOGE("%s: legacy audio_channel_mask_t value 0x%x is invalid", __func__, legacy);
+ return unexpected(BAD_VALUE);
+ }
} else if (repr == AUDIO_CHANNEL_REPRESENTATION_POSITION) {
return convert(legacy, isInput ? mInAndVoice : mOut, __func__,
isInput ? "input / voice" : "output");
diff --git a/media/libaudioclient/aidl/android/media/AudioChannelLayout.aidl b/media/libaudioclient/aidl/android/media/AudioChannelLayout.aidl
index fdc8184..3259105 100644
--- a/media/libaudioclient/aidl/android/media/AudioChannelLayout.aidl
+++ b/media/libaudioclient/aidl/android/media/AudioChannelLayout.aidl
@@ -52,9 +52,12 @@
*/
int invalid = 0;
/**
- * This variant is used for representing indexed masks. The value
- * must be one of the 'INDEX_MASK_*' constants. The 'indexMask' field
- * must have at least one bit set.
+ * This variant is used for representing indexed masks. The mask indicates
+ * what channels are used. For example, the mask that specifies to use only
+ * channels 1 and 3 when interacting with a multi-channel device is defined
+ * as a combination of the 1st and the 3rd bits and thus is equal to 5. See
+ * also the 'INDEX_MASK_*' constants. The 'indexMask' field must have at
+ * least one bit set.
*/
int indexMask;
/**
@@ -71,7 +74,10 @@
int voiceMask;
/**
- * 'INDEX_MASK_' constants define how many channels are used.
+ * 'INDEX_MASK_*' constants define how many channels are used.
+ * The mask constants below are 'canonical' masks. Each 'INDEX_MASK_N'
+ * constant declares that all N channels are used and arranges
+ * them starting from the LSB.
*/
const int INDEX_MASK_1 = (1 << 1) - 1;
const int INDEX_MASK_2 = (1 << 2) - 1;
diff --git a/media/libaudioclient/tests/audio_aidl_legacy_conversion_tests.cpp b/media/libaudioclient/tests/audio_aidl_legacy_conversion_tests.cpp
index 7f8af53..424b387 100644
--- a/media/libaudioclient/tests/audio_aidl_legacy_conversion_tests.cpp
+++ b/media/libaudioclient/tests/audio_aidl_legacy_conversion_tests.cpp
@@ -46,6 +46,11 @@
media::AudioChannelLayout::INDEX_MASK_2);
}
+media::AudioChannelLayout make_ACL_ChannelIndexArbitrary() {
+ // Use channels 1 and 3.
+ return media::AudioChannelLayout::make<media::AudioChannelLayout::Tag::indexMask>(5);
+}
+
media::AudioChannelLayout make_ACL_VoiceCall() {
return media::AudioChannelLayout::make<media::AudioChannelLayout::Tag::voiceMask>(
media::AudioChannelLayout::VOICE_CALL_MONO);
@@ -157,7 +162,7 @@
TEST_F(HashIdentityTest, AudioChannelLayoutHashIdentity) {
verifyHashIdentity<media::AudioChannelLayout>({
make_ACL_None, make_ACL_Invalid, make_ACL_Stereo, make_ACL_ChannelIndex2,
- make_ACL_VoiceCall});
+ make_ACL_ChannelIndexArbitrary, make_ACL_VoiceCall});
}
TEST_F(HashIdentityTest, AudioDeviceDescriptionHashIdentity) {
@@ -188,7 +193,7 @@
AudioChannelLayoutRoundTripTest,
testing::Combine(
testing::Values(media::AudioChannelLayout{}, make_ACL_Invalid(), make_ACL_Stereo(),
- make_ACL_ChannelIndex2()),
+ make_ACL_ChannelIndex2(), make_ACL_ChannelIndexArbitrary()),
testing::Values(false, true)));
INSTANTIATE_TEST_SUITE_P(AudioChannelVoiceRoundTrip,
AudioChannelLayoutRoundTripTest,