Downmix: Fix setting an invalid channel mask has side effects
Attempting to set an invalid channel mask has the side effect
of actually setting it. A legacy bug that should be fixed
because it can cause an invalid state of the downmixer.
Test: atest downmix_tests
Bug: 202162797
Change-Id: I6cc03c5bf68492e8442b5d1ae62b89c4a77c5916
diff --git a/media/libeffects/downmix/EffectDownmix.cpp b/media/libeffects/downmix/EffectDownmix.cpp
index 90bb410..4efab84 100644
--- a/media/libeffects/downmix/EffectDownmix.cpp
+++ b/media/libeffects/downmix/EffectDownmix.cpp
@@ -647,6 +647,12 @@
ALOGE("Downmix_Configure error: invalid config");
return -EINVAL;
}
+ // when configuring the effect, do not allow a blank or unsupported channel mask
+ if (!Downmix_validChannelMask(pConfig->inputCfg.channels)) {
+ ALOGE("Downmix_Configure error: input channel mask(0x%x) not supported",
+ pConfig->inputCfg.channels);
+ return -EINVAL;
+ }
if (&pDwmModule->config != pConfig) {
memcpy(&pDwmModule->config, pConfig, sizeof(effect_config_t));
@@ -657,12 +663,6 @@
pDownmixer->apply_volume_correction = false;
pDownmixer->input_channel_count = 8; // matches default input of AUDIO_CHANNEL_OUT_7POINT1
} else {
- // when configuring the effect, do not allow a blank or unsupported channel mask
- if (!Downmix_validChannelMask(pConfig->inputCfg.channels)) {
- ALOGE("Downmix_Configure error: input channel mask(0x%x) not supported",
- pConfig->inputCfg.channels);
- return -EINVAL;
- }
pDownmixer->input_channel_count =
audio_channel_count_from_out_mask(pConfig->inputCfg.channels);
}
diff --git a/media/libeffects/downmix/tests/downmix_tests.cpp b/media/libeffects/downmix/tests/downmix_tests.cpp
index 26c7c31..a6da654 100644
--- a/media/libeffects/downmix/tests/downmix_tests.cpp
+++ b/media/libeffects/downmix/tests/downmix_tests.cpp
@@ -253,6 +253,27 @@
ASSERT_EQ(0, err);
}
+ // This test assumes the channel mask is invalid.
+ void testInvalidChannelMask(audio_channel_mask_t invalidChannelMask) {
+ reconfig(48000 /* sampleRate */, invalidChannelMask);
+ const int32_t sessionId = 0;
+ const int32_t ioId = 0;
+ int32_t err = AUDIO_EFFECT_LIBRARY_INFO_SYM.create_effect(
+ &downmix_uuid_, sessionId, ioId, &handle_);
+ ASSERT_EQ(0, err);
+
+ const struct effect_interface_s * const downmixApi = *handle_;
+ int32_t reply = 0;
+ uint32_t replySize = (uint32_t)sizeof(reply);
+ err = (downmixApi->command)(
+ handle_, EFFECT_CMD_SET_CONFIG,
+ sizeof(effect_config_t), &config_, &replySize, &reply);
+ ASSERT_EQ(0, err);
+ ASSERT_NE(0, reply); // error has occurred.
+ err = AUDIO_EFFECT_LIBRARY_INFO_SYM.release_effect(handle_);
+ ASSERT_EQ(0, err);
+ }
+
private:
void reconfig(int sampleRate, audio_channel_mask_t channelMask) {
config_.inputCfg.accessMode = EFFECT_BUFFER_ACCESS_READ;
@@ -299,6 +320,16 @@
int inputChannelCount_{};
};
+TEST(DownmixTestSimple, invalidChannelMask) {
+ // Fill in a dummy test method to use DownmixTest outside of a parameterized test.
+ class DownmixTestComplete : public DownmixTest {
+ void TestBody() override {}
+ } downmixtest;
+
+ constexpr auto INVALID_CHANNEL_MASK = audio_channel_mask_t(1 << 31);
+ downmixtest.testInvalidChannelMask(INVALID_CHANNEL_MASK);
+}
+
TEST_P(DownmixTest, basic) {
testBalance(kSampleRates[std::get<0>(GetParam())],
kChannelPositionMasks[std::get<1>(GetParam())]);