Update AudioChannelLayout to address some issues
Changes made:
- haptic channels moved to the MSB part of the integer;
- "voice up/down link" channel moved into their own
union variant.
Bug: 188932434
Bug: 193562113
Bug: 193563345
Test: atest audio_aidl_conversion_tests
Change-Id: Idedab7142fe05ecfcbcc57c8a359a9d541395c96
diff --git a/media/libaudioclient/AidlConversion.cpp b/media/libaudioclient/AidlConversion.cpp
index 298e596..bb67b53 100644
--- a/media/libaudioclient/AidlConversion.cpp
+++ b/media/libaudioclient/AidlConversion.cpp
@@ -475,10 +475,7 @@
DEFINE_INPUT_LAYOUT(2POINT1POINT2),
DEFINE_INPUT_LAYOUT(3POINT0POINT2),
DEFINE_INPUT_LAYOUT(3POINT1POINT2),
- DEFINE_INPUT_LAYOUT(5POINT1),
- DEFINE_INPUT_LAYOUT(VOICE_UPLINK_MONO),
- DEFINE_INPUT_LAYOUT(VOICE_DNLINK_MONO),
- DEFINE_INPUT_LAYOUT(VOICE_CALL_MONO)
+ DEFINE_INPUT_LAYOUT(5POINT1)
#undef DEFINE_INPUT_LAYOUT
};
return pairs;
@@ -527,6 +524,22 @@
return pairs;
}
+const detail::AudioChannelPairs& getVoiceAudioChannelPairs() {
+ static const detail::AudioChannelPairs pairs = {
+#define DEFINE_VOICE_LAYOUT(n) \
+ { \
+ AUDIO_CHANNEL_IN_VOICE_##n, \
+ media::AudioChannelLayout::make<media::AudioChannelLayout::Tag::voiceMask>( \
+ media::AudioChannelLayout::VOICE_##n) \
+ }
+ DEFINE_VOICE_LAYOUT(UPLINK_MONO),
+ DEFINE_VOICE_LAYOUT(DNLINK_MONO),
+ DEFINE_VOICE_LAYOUT(CALL_MONO)
+#undef DEFINE_VOICE_LAYOUT
+ };
+ return pairs;
+}
+
media::AudioDeviceDescription make_AudioDeviceDescription(media::AudioDeviceType type,
const std::string& connection = "") {
media::AudioDeviceDescription result;
@@ -1080,6 +1093,17 @@
}
template<typename S, typename T>
+std::unordered_map<S, T> make_DirectMap(
+ const std::vector<std::pair<S, T>>& v1, const std::vector<std::pair<S, T>>& v2) {
+ std::unordered_map<S, T> result(v1.begin(), v1.end());
+ LOG_ALWAYS_FATAL_IF(result.size() != v1.size(), "Duplicate key elements detected in v1");
+ result.insert(v2.begin(), v2.end());
+ LOG_ALWAYS_FATAL_IF(result.size() != v1.size() + v2.size(),
+ "Duplicate key elements detected in v1+v2");
+ return result;
+}
+
+template<typename S, typename T>
std::unordered_map<T, S> make_ReverseMap(const std::vector<std::pair<S, T>>& v) {
std::unordered_map<T, S> result;
std::transform(v.begin(), v.end(), std::inserter(result, result.begin()),
@@ -1099,6 +1123,7 @@
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());
auto convert = [](const media::AudioChannelLayout& aidl, const ReverseMap& m,
const char* func, const char* type) -> ConversionResult<audio_channel_mask_t> {
@@ -1120,6 +1145,8 @@
return convert(aidl, mIdx, __func__, "index");
case Tag::layoutMask:
return convert(aidl, isOutput ? mOut : mIn, __func__, isOutput ? "output" : "input");
+ case Tag::voiceMask:
+ return convert(aidl, mVoice, __func__, "voice");
}
ALOGE("%s: unexpected tag value %d", __func__, aidl.getTag());
return unexpected(BAD_VALUE);
@@ -1130,7 +1157,8 @@
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 mIn = make_DirectMap(getInAudioChannelPairs());
+ static const DirectMap mInAndVoice = make_DirectMap(
+ getInAudioChannelPairs(), getVoiceAudioChannelPairs());
static const DirectMap mOut = make_DirectMap(getOutAudioChannelPairs());
auto convert = [](const audio_channel_mask_t legacy, const DirectMap& m,
@@ -1154,7 +1182,8 @@
if (repr == AUDIO_CHANNEL_REPRESENTATION_INDEX) {
return convert(legacy, mIdx, __func__, "index");
} else if (repr == AUDIO_CHANNEL_REPRESENTATION_POSITION) {
- return convert(legacy, isOutput ? mOut : mIn, __func__, isOutput ? "output" : "input");
+ return convert(legacy, isOutput ? mOut : mInAndVoice, __func__,
+ isOutput ? "output" : "input / voice");
}
ALOGE("%s: unknown representation %d in audio_channel_mask_t value 0x%x",
diff --git a/media/libaudioclient/aidl/android/media/AudioChannelLayout.aidl b/media/libaudioclient/aidl/android/media/AudioChannelLayout.aidl
index 9b6d59f..fdc8184 100644
--- a/media/libaudioclient/aidl/android/media/AudioChannelLayout.aidl
+++ b/media/libaudioclient/aidl/android/media/AudioChannelLayout.aidl
@@ -33,6 +33,10 @@
* number of channels in the audio frame. A channel mask per se only defines the
* presence or absence of a channel, not the order. Please see 'INTERLEAVE_*'
* constants for the platform convention of order.
+ *
+ * The structure also defines a "voice mask" which is a special case of
+ * layout mask, intended for processing voice audio from telecommunication
+ * use cases.
*/
union AudioChannelLayout {
/**
@@ -59,6 +63,12 @@
* must have at least one bit set.
*/
int layoutMask;
+ /**
+ * This variant is used for processing of voice audio input and output.
+ * It is recommended to use one of 'VOICE_*' values. The 'voiceMask' field
+ * must have at least one bit set.
+ */
+ int voiceMask;
/**
* 'INDEX_MASK_' constants define how many channels are used.
@@ -183,12 +193,6 @@
LAYOUT_STEREO | LAYOUT_HAPTIC_AB;
const int LAYOUT_FRONT_BACK =
CHANNEL_FRONT_CENTER | CHANNEL_BACK_CENTER;
- const int LAYOUT_VOICE_UPLINK_MONO =
- LAYOUT_MONO | CHANNEL_VOICE_UPLINK;
- const int LAYOUT_VOICE_DNLINK_MONO =
- LAYOUT_MONO | CHANNEL_VOICE_DNLINK;
- const int LAYOUT_VOICE_CALL_MONO =
- CHANNEL_VOICE_UPLINK | CHANNEL_VOICE_DNLINK;
/**
* Expresses the convention when stereo audio samples are stored interleaved
@@ -204,8 +208,10 @@
const int INTERLEAVE_RIGHT = 1;
/**
- * 'CHANNEL_*' constants are used to build 'LAYOUT_*' masks.
- * Each constant must have exactly one bit set.
+ * 'CHANNEL_*' constants are used to build 'LAYOUT_*' masks. Each constant
+ * must have exactly one bit set. The values do not match
+ * 'android.media.AudioFormat.CHANNEL_OUT_*' constants from the SDK
+ * for better efficiency in masks processing.
*/
const int CHANNEL_FRONT_LEFT = 1 << 0;
const int CHANNEL_FRONT_RIGHT = 1 << 1;
@@ -233,8 +239,30 @@
const int CHANNEL_LOW_FREQUENCY_2 = 1 << 23;
const int CHANNEL_FRONT_WIDE_LEFT = 1 << 24;
const int CHANNEL_FRONT_WIDE_RIGHT = 1 << 25;
- const int CHANNEL_VOICE_UPLINK = 1 << 26;
- const int CHANNEL_VOICE_DNLINK = 1 << 27;
- const int CHANNEL_HAPTIC_B = 1 << 28; // B then A to match legacy const.
- const int CHANNEL_HAPTIC_A = 1 << 29;
+ /**
+ * Haptic channels are not part of multichannel standards, however they
+ * enhance user experience when playing so they are packed together with the
+ * channels of the program. To avoid collision with positional channels the
+ * values for haptic channels start at the MSB of an integer (after the sign
+ * bit) and move down to LSB.
+ */
+ const int CHANNEL_HAPTIC_B = 1 << 29;
+ const int CHANNEL_HAPTIC_A = 1 << 30;
+
+ /**
+ * 'VOICE_*' constants define layouts for voice audio. The order of the
+ * channels in the frame is assumed to be from the LSB to MSB for all the
+ * bits set to '1'.
+ */
+ const int VOICE_UPLINK_MONO = CHANNEL_VOICE_UPLINK;
+ const int VOICE_DNLINK_MONO = CHANNEL_VOICE_DNLINK;
+ const int VOICE_CALL_MONO = CHANNEL_VOICE_UPLINK | CHANNEL_VOICE_DNLINK;
+
+ /**
+ * 'CHANNEL_VOICE_*' constants are used to build 'VOICE_*' masks. Each
+ * constant must have exactly one bit set. Use the same values as
+ * 'android.media.AudioFormat.CHANNEL_IN_VOICE_*' constants from the SDK.
+ */
+ const int CHANNEL_VOICE_UPLINK = 0x4000;
+ const int CHANNEL_VOICE_DNLINK = 0x8000;
}
diff --git a/media/libaudioclient/include/media/AudioCommonTypes.h b/media/libaudioclient/include/media/AudioCommonTypes.h
index 2a46e05..a368e74 100644
--- a/media/libaudioclient/include/media/AudioCommonTypes.h
+++ b/media/libaudioclient/include/media/AudioCommonTypes.h
@@ -56,6 +56,8 @@
return hash_combine(seed, std::hash<int32_t>{}(acl.get<Tag::indexMask>()));
case Tag::layoutMask:
return hash_combine(seed, std::hash<int32_t>{}(acl.get<Tag::layoutMask>()));
+ case Tag::voiceMask:
+ return hash_combine(seed, std::hash<int32_t>{}(acl.get<Tag::voiceMask>()));
}
return seed;
}
diff --git a/media/libaudioclient/tests/audio_aidl_legacy_conversion_tests.cpp b/media/libaudioclient/tests/audio_aidl_legacy_conversion_tests.cpp
index e549354..b6e597d 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_VoiceCall() {
+ return media::AudioChannelLayout::make<media::AudioChannelLayout::Tag::voiceMask>(
+ media::AudioChannelLayout::VOICE_CALL_MONO);
+}
+
media::AudioDeviceDescription make_AudioDeviceDescription(media::AudioDeviceType type,
const std::string& connection = "") {
media::AudioDeviceDescription result;
@@ -151,7 +156,8 @@
TEST_F(HashIdentityTest, AudioChannelLayoutHashIdentity) {
verifyHashIdentity<media::AudioChannelLayout>({
- make_ACL_None, make_ACL_Invalid, make_ACL_Stereo, make_ACL_ChannelIndex2});
+ make_ACL_None, make_ACL_Invalid, make_ACL_Stereo, make_ACL_ChannelIndex2,
+ make_ACL_VoiceCall});
}
TEST_F(HashIdentityTest, AudioDeviceDescriptionHashIdentity) {
@@ -184,6 +190,10 @@
testing::Values(media::AudioChannelLayout{}, make_ACL_Invalid(), make_ACL_Stereo(),
make_ACL_ChannelIndex2()),
testing::Values(true, false)));
+INSTANTIATE_TEST_SUITE_P(AudioChannelVoiceRoundTrip,
+ AudioChannelLayoutRoundTripTest,
+ // In legacy constants the voice call is only defined for input.
+ testing::Combine(testing::Values(make_ACL_VoiceCall()), testing::Values(false)));
class AudioDeviceDescriptionRoundTripTest :
public testing::TestWithParam<media::AudioDeviceDescription> {};
