From 2089f2c351a0e1b6f749f824dc1b3e4d8add613e Mon Sep 17 00:00:00 2001 From: Jan Sebechlebsky Date: Tue, 20 Sep 2022 16:59:18 +0200 Subject: [PATCH] Fix mix role deduction and validation in AudioMixingRule.Builder Bug: 247783063 Test: atest AudioMixingRuleUnitTests Change-Id: Ib4ee69ec3465af835eb10643b3568f01ef2034cb --- .../media/audiopolicy/AudioMixingRule.java | 24 ++++--- .../AudioMixingRuleUnitTests.java | 72 +++++++++++++++++++ 2 files changed, 87 insertions(+), 9 deletions(-) diff --git a/media/java/android/media/audiopolicy/AudioMixingRule.java b/media/java/android/media/audiopolicy/AudioMixingRule.java index 11d51f91027dd..6aead4337245c 100644 --- a/media/java/android/media/audiopolicy/AudioMixingRule.java +++ b/media/java/android/media/audiopolicy/AudioMixingRule.java @@ -554,7 +554,12 @@ public class AudioMixingRule { throw new IllegalArgumentException("Illegal argument for mix role"); } - Log.i("AudioMixingRule", "Builder setTargetMixRole " + mixRole); + if (mCriteria.stream().map(AudioMixMatchCriterion::getRule) + .anyMatch(mixRole == MIX_ROLE_PLAYERS + ? AudioMixingRule::isRecorderRule : AudioMixingRule::isPlayerRule)) { + throw new IllegalArgumentException( + "Target mix role is not compatible with mix rules."); + } mTargetMixType = mixRole == MIX_ROLE_INJECTOR ? AudioMix.MIX_TYPE_RECORDERS : AudioMix.MIX_TYPE_PLAYERS; return this; @@ -611,17 +616,15 @@ public class AudioMixingRule { */ private Builder addRuleInternal(AudioAttributes attrToMatch, Integer intProp, int rule) throws IllegalArgumentException { - // as rules are added to the Builder, we verify they are consistent with the type - // of mix being built. When adding the first rule, the mix type is MIX_TYPE_INVALID. + // If mix type is invalid and added rule is valid only for the players / recorders, + // adjust the mix type accordingly. + // Otherwise, if the mix type was already deduced or set explicitly, verify the rule + // is valid for the mix type. if (mTargetMixType == AudioMix.MIX_TYPE_INVALID) { if (isPlayerRule(rule)) { mTargetMixType = AudioMix.MIX_TYPE_PLAYERS; } else if (isRecorderRule(rule)) { mTargetMixType = AudioMix.MIX_TYPE_RECORDERS; - } else { - // For rules which are not player or recorder specific (e.g. RULE_MATCH_UID), - // the default mix type is MIX_TYPE_PLAYERS. - mTargetMixType = AudioMix.MIX_TYPE_PLAYERS; } } else if ((isPlayerRule(rule) && (mTargetMixType != AudioMix.MIX_TYPE_PLAYERS)) || (isRecorderRule(rule)) && (mTargetMixType != AudioMix.MIX_TYPE_RECORDERS)) @@ -679,8 +682,11 @@ public class AudioMixingRule { * @return a new {@link AudioMixingRule} object */ public AudioMixingRule build() { - return new AudioMixingRule(mTargetMixType, mCriteria, - mAllowPrivilegedMediaPlaybackCapture, mVoiceCommunicationCaptureAllowed); + return new AudioMixingRule( + mTargetMixType == AudioMix.MIX_TYPE_INVALID + ? AudioMix.MIX_TYPE_PLAYERS : mTargetMixType, + mCriteria, mAllowPrivilegedMediaPlaybackCapture, + mVoiceCommunicationCaptureAllowed); } } } diff --git a/media/tests/AudioPolicyTest/src/com/android/audiopolicytest/AudioMixingRuleUnitTests.java b/media/tests/AudioPolicyTest/src/com/android/audiopolicytest/AudioMixingRuleUnitTests.java index 3fb618207d512..ad7ab971d5f2e 100644 --- a/media/tests/AudioPolicyTest/src/com/android/audiopolicytest/AudioMixingRuleUnitTests.java +++ b/media/tests/AudioPolicyTest/src/com/android/audiopolicytest/AudioMixingRuleUnitTests.java @@ -17,9 +17,13 @@ package com.android.audiopolicytest; import static android.media.AudioAttributes.USAGE_MEDIA; +import static android.media.MediaRecorder.AudioSource.VOICE_RECOGNITION; +import static android.media.audiopolicy.AudioMixingRule.MIX_ROLE_INJECTOR; import static android.media.audiopolicy.AudioMixingRule.MIX_ROLE_PLAYERS; +import static android.media.audiopolicy.AudioMixingRule.RULE_EXCLUDE_ATTRIBUTE_CAPTURE_PRESET; import static android.media.audiopolicy.AudioMixingRule.RULE_EXCLUDE_ATTRIBUTE_USAGE; import static android.media.audiopolicy.AudioMixingRule.RULE_EXCLUDE_UID; +import static android.media.audiopolicy.AudioMixingRule.RULE_MATCH_ATTRIBUTE_CAPTURE_PRESET; import static android.media.audiopolicy.AudioMixingRule.RULE_MATCH_ATTRIBUTE_USAGE; import static android.media.audiopolicy.AudioMixingRule.RULE_MATCH_UID; @@ -53,6 +57,8 @@ import org.junit.runner.RunWith; public class AudioMixingRuleUnitTests { private static final AudioAttributes USAGE_MEDIA_AUDIO_ATTRIBUTES = new AudioAttributes.Builder().setUsage(USAGE_MEDIA).build(); + private static final AudioAttributes CAPTURE_PRESET_VOICE_RECOGNITION_AUDIO_ATTRIBUTES = + new AudioAttributes.Builder().setCapturePreset(VOICE_RECOGNITION).build(); private static final int TEST_UID = 42; private static final int OTHER_UID = 77; @@ -137,6 +143,46 @@ public class AudioMixingRuleUnitTests { .build()); } + @Test + public void injectorMixTypeDeductionWithGenericRuleSucceeds() { + AudioMixingRule rule = new AudioMixingRule.Builder() + // UID rule can be used both with MIX_ROLE_PLAYERS and MIX_ROLE_INJECTOR. + .addMixRule(RULE_MATCH_UID, TEST_UID) + // Capture preset rule is only valid for injector, MIX_ROLE_INJECTOR should + // be deduced. + .addMixRule(RULE_MATCH_ATTRIBUTE_CAPTURE_PRESET, + CAPTURE_PRESET_VOICE_RECOGNITION_AUDIO_ATTRIBUTES) + .build(); + + assertEquals(rule.getTargetMixRole(), MIX_ROLE_INJECTOR); + assertThat(rule.getCriteria(), containsInAnyOrder( + isAudioMixMatchUidCriterion(TEST_UID), + isAudioMixMatchCapturePresetCriterion(VOICE_RECOGNITION))); + } + + @Test + public void settingTheMixTypeToIncompatibleInjectorMixFails() { + assertThrows(IllegalArgumentException.class, + () -> new AudioMixingRule.Builder() + .addMixRule(RULE_MATCH_ATTRIBUTE_CAPTURE_PRESET, + CAPTURE_PRESET_VOICE_RECOGNITION_AUDIO_ATTRIBUTES) + // Capture preset cannot be defined for MIX_ROLE_PLAYERS. + .setTargetMixRole(MIX_ROLE_PLAYERS) + .build()); + } + + @Test + public void addingPlayersOnlyRuleWithInjectorsOnlyRuleFails() { + assertThrows(IllegalArgumentException.class, + () -> new AudioMixingRule.Builder() + // MIX_ROLE_PLAYERS only rule. + .addMixRule(RULE_MATCH_ATTRIBUTE_USAGE, USAGE_MEDIA_AUDIO_ATTRIBUTES) + // MIX ROLE_INJECTOR only rule. + .addMixRule(RULE_MATCH_ATTRIBUTE_CAPTURE_PRESET, + CAPTURE_PRESET_VOICE_RECOGNITION_AUDIO_ATTRIBUTES) + .build()); + } + private static Matcher isAudioMixUidCriterion(int uid, boolean exclude) { return new CustomTypeSafeMatcher("uid mix criterion") { @@ -160,6 +206,32 @@ public class AudioMixingRuleUnitTests { return isAudioMixUidCriterion(uid, /*exclude=*/ false); } + private static Matcher isAudioMixCapturePresetCriterion(int audioSource, boolean exclude) { + return new CustomTypeSafeMatcher("uid mix criterion") { + @Override + public boolean matchesSafely(AudioMixMatchCriterion item) { + int expectedRule = exclude + ? RULE_EXCLUDE_ATTRIBUTE_CAPTURE_PRESET + : RULE_MATCH_ATTRIBUTE_CAPTURE_PRESET; + AudioAttributes attributes = item.getAudioAttributes(); + return item.getRule() == expectedRule + && attributes != null && attributes.getCapturePreset() == audioSource; + } + + @Override + public void describeMismatchSafely( + AudioMixMatchCriterion item, Description mismatchDescription) { + mismatchDescription.appendText( + String.format("is not %s criterion with capture preset %d", + exclude ? "exclude" : "match", audioSource)); + } + }; + } + + private static Matcher isAudioMixMatchCapturePresetCriterion(int audioSource) { + return isAudioMixCapturePresetCriterion(audioSource, /*exclude=*/ false); + } + private static Matcher isAudioMixUsageCriterion(int usage, boolean exclude) { return new CustomTypeSafeMatcher("usage mix criterion") { @Override