Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit e02cc395 authored by Ján Sebechlebský's avatar Ján Sebechlebský Committed by Gerrit Code Review
Browse files

Merge changes from topic "refactor_mix" 

* changes:
  Validate consistency of audio mix criteria during registration
  Refactor mixCriteria in AudioPolicyMix.
parents 4aec31d0 193f7ca8

media/libaudioclient/AudioPolicy.cpp

+14 −9
Original line number Diff line number Diff line
@@ -71,6 +71,10 @@ status_t AudioMixMatchCriterion::writeToParcel(Parcel *parcel) const 
    return NO_ERROR;

}



bool AudioMixMatchCriterion::isExcludeCriterion() const {

    return mRule & RULE_EXCLUSION_MASK;

}



//

//  AudioMix implementation

//
@@ -91,10 +95,11 @@ status_t AudioMix::readFromParcel(Parcel *parcel) 
    if (size > MAX_CRITERIA_PER_MIX) {

        size = MAX_CRITERIA_PER_MIX;

    }

    mCriteria.reserve(size);

    for (size_t i = 0; i < size; i++) {

        AudioMixMatchCriterion criterion;

        if (criterion.readFromParcel(parcel) == NO_ERROR) {

            mCriteria.add(criterion);

            mCriteria.push_back(criterion);

        }

    }

    return NO_ERROR;
@@ -135,18 +140,18 @@ status_t AudioMix::writeToParcel(Parcel *parcel) const 
    return NO_ERROR;

}



void AudioMix::setExcludeUid(uid_t uid) const {

void AudioMix::setExcludeUid(uid_t uid) {

    AudioMixMatchCriterion crit;

    crit.mRule = RULE_EXCLUDE_UID;

    crit.mValue.mUid = uid;

    mCriteria.add(crit);

    mCriteria.push_back(crit);

}



void AudioMix::setMatchUid(uid_t uid) const {

void AudioMix::setMatchUid(uid_t uid) {

    AudioMixMatchCriterion crit;

    crit.mRule = RULE_MATCH_UID;

    crit.mValue.mUid = uid;

    mCriteria.add(crit);

    mCriteria.push_back(crit);

}



bool AudioMix::hasUidRule(bool match, uid_t uid) const {
@@ -169,18 +174,18 @@ bool AudioMix::hasMatchUidRule() const { 
    return false;

}



void AudioMix::setExcludeUserId(int userId) const {

void AudioMix::setExcludeUserId(int userId) {

    AudioMixMatchCriterion crit;

    crit.mRule = RULE_EXCLUDE_USERID;

    crit.mValue.mUserId = userId;

    mCriteria.add(crit);

    mCriteria.push_back(crit);

}



void AudioMix::setMatchUserId(int userId) const {

void AudioMix::setMatchUserId(int userId) {

    AudioMixMatchCriterion crit;

    crit.mRule = RULE_MATCH_USERID;

    crit.mValue.mUserId = userId;

    mCriteria.add(crit);

    mCriteria.push_back(crit);

}



bool AudioMix::hasUserIdRule(bool match, int userId) const {

media/libaudioclient/include/media/AudioPolicy.h

+9 −7
Original line number Diff line number Diff line
@@ -72,6 +72,7 @@ public: 
    status_t readFromParcel(Parcel *parcel);

    status_t writeToParcel(Parcel *parcel) const;



    bool isExcludeCriterion() const;

    union {

        audio_usage_t   mUsage;

        audio_source_t  mSource;
@@ -88,23 +89,24 @@ public: 
    static const uint32_t kCbFlagNotifyActivity = 0x1;



    AudioMix() {}

    AudioMix(Vector<AudioMixMatchCriterion> criteria, uint32_t mixType, audio_config_t format,

             uint32_t routeFlags, String8 registrationId, uint32_t flags) :

    AudioMix(const std::vector<AudioMixMatchCriterion> &criteria, uint32_t mixType,

             audio_config_t format, uint32_t routeFlags,const String8 &registrationId,

             uint32_t flags) :

        mCriteria(criteria), mMixType(mixType), mFormat(format),

        mRouteFlags(routeFlags), mDeviceAddress(registrationId), mCbFlags(flags){}



    status_t readFromParcel(Parcel *parcel);

    status_t writeToParcel(Parcel *parcel) const;



    void setExcludeUid(uid_t uid) const;

    void setMatchUid(uid_t uid) const;

    void setExcludeUid(uid_t uid);

    void setMatchUid(uid_t uid);

    /** returns true if this mix has a rule to match or exclude the given uid */

    bool hasUidRule(bool match, uid_t uid) const;

    /** returns true if this mix has a rule for uid match (any uid) */

    bool hasMatchUidRule() const;



    void setExcludeUserId(int userId) const;

    void setMatchUserId(int userId) const;

    void setExcludeUserId(int userId);

    void setMatchUserId(int userId);

    /** returns true if this mix has a rule to match or exclude the given userId */

    bool hasUserIdRule(bool match, int userId) const;

    /** returns true if this mix has a rule for userId match (any userId) */
@@ -112,7 +114,7 @@ public: 
    /** returns true if this mix can be used for uid-device affinity routing */

    bool isDeviceAffinityCompatible() const;



    mutable Vector<AudioMixMatchCriterion> mCriteria;

    std::vector<AudioMixMatchCriterion> mCriteria;

    uint32_t        mMixType;

    audio_config_t  mFormat;

    uint32_t        mRouteFlags;

media/libaudioclient/tests/audioclient_serialization_tests.cpp

+3 −2
Original line number Diff line number Diff line
@@ -189,12 +189,13 @@ TEST_F(SerializationTest, AudioVolumeGroupBinderization) { 
TEST_F(SerializationTest, AudioMixBinderization) {

    for (int j = 0; j < 512; j++) {

        const std::string msg{"Test AMBinderization for seed::" + std::to_string(mSeed)};

        Vector<AudioMixMatchCriterion> criteria;

        std::vector<AudioMixMatchCriterion> criteria;

        criteria.reserve(16);

        for (int i = 0; i < 16; i++) {

            AudioMixMatchCriterion ammc{kUsages[rand() % kUsages.size()],

                                        kInputSources[rand() % kInputSources.size()],

                                        kMixMatchRules[rand() % kMixMatchRules.size()]};

            criteria.add(ammc);

            criteria.push_back(ammc);

        }

        audio_config_t config{};

        config.sample_rate = 48000;

services/audiopolicy/common/managerdefinitions/include/AudioPolicyMix.h

+2 −2
Original line number Diff line number Diff line
@@ -55,7 +55,7 @@ public: 
    status_t getAudioPolicyMix(audio_devices_t deviceType,

            const String8& address, sp<AudioPolicyMix> &policyMix) const;



    status_t registerMix(AudioMix mix, sp<SwAudioOutputDescriptor> desc);

    status_t registerMix(const AudioMix& mix, const sp<SwAudioOutputDescriptor>& desc);



    status_t unregisterMix(const AudioMix& mix);
@@ -124,7 +124,7 @@ public: 
    void dump(String8 *dst) const;



private:

    enum class MixMatchStatus { MATCH, NO_MATCH, INVALID_MIX };

    enum class MixMatchStatus { MATCH, NO_MATCH };

    MixMatchStatus mixMatch(const AudioMix* mix, size_t mixIndex,

                            const audio_attributes_t& attributes,

                            const audio_config_base_t& config,

services/audiopolicy/common/managerdefinitions/src/AudioPolicyMix.cpp

+58 −68
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ 
#define LOG_TAG "APM_AudioPolicyMix"

//#define LOG_NDEBUG 0



#include <algorithm>

#include "AudioPolicyMix.h"

#include "TypeConverter.h"

#include "HwModule.h"
@@ -25,6 +26,34 @@ 
#include <AudioOutputDescriptor.h>



namespace android {

namespace {



// Consistency checks: for each "dimension" of rules (usage, uid...), we can

// only have MATCH rules, or EXCLUDE rules in each dimension, not a combination.

bool areMixCriteriaConsistent(const std::vector<AudioMixMatchCriterion>& criteria) {

    std::set<uint32_t> positiveCriteria;

    for (const AudioMixMatchCriterion& c : criteria) {

        if (c.isExcludeCriterion()) {

            continue;

        }

        positiveCriteria.insert(c.mRule);

    }



    auto isConflictingCriterion = [&positiveCriteria](const AudioMixMatchCriterion& c) {

        uint32_t ruleWithoutExclusion = c.mRule & ~RULE_EXCLUSION_MASK;

        return c.isExcludeCriterion() &&

               (positiveCriteria.find(ruleWithoutExclusion) != positiveCriteria.end());

    };

    return std::none_of(criteria.begin(), criteria.end(), isConflictingCriterion);

}



template <typename Predicate>

void EraseCriteriaIf(std::vector<AudioMixMatchCriterion>& v,

                     const Predicate& predicate) {

    v.erase(std::remove_if(v.begin(), v.end(), predicate), v.end());

}



} // namespace



void AudioPolicyMix::dump(String8 *dst, int spaces, int index) const

{
@@ -78,7 +107,8 @@ void AudioPolicyMix::dump(String8 *dst, int spaces, int index) const 
    }

}



status_t AudioPolicyMixCollection::registerMix(AudioMix mix, sp<SwAudioOutputDescriptor> desc)

status_t AudioPolicyMixCollection::registerMix(const AudioMix& mix,

                                               const sp<SwAudioOutputDescriptor>& desc)

{

    for (size_t i = 0; i < size(); i++) {

        const sp<AudioPolicyMix>& registeredMix = itemAt(i);
@@ -89,12 +119,17 @@ status_t AudioPolicyMixCollection::registerMix(AudioMix mix, sp<SwAudioOutputDes 
            return BAD_VALUE;

        }

    }

    sp<AudioPolicyMix> policyMix = new AudioPolicyMix(mix);

    if (!areMixCriteriaConsistent(mix.mCriteria)) {

        ALOGE("registerMix(): Mix contains inconsistent criteria "

              "(MATCH & EXCLUDE criteria of the same type)");

        return BAD_VALUE;

    }

    sp<AudioPolicyMix> policyMix = sp<AudioPolicyMix>::make(mix);

    add(policyMix);

    ALOGD("registerMix(): adding mix for dev=0x%x addr=%s",

            policyMix->mDeviceType, policyMix->mDeviceAddress.string());



    if (desc != 0) {

    if (desc != nullptr) {

        desc->mPolicyMix = policyMix;

        policyMix->setOutput(desc);

    }
@@ -177,15 +212,9 @@ status_t AudioPolicyMixCollection::getOutputForAttr( 
            continue; // Primary output already found

        }



        switch (mixMatch(policyMix.get(), i, attributes, config, uid)) {

            case MixMatchStatus::INVALID_MIX:

                // The mix has contradictory rules, ignore it

                // TODO: reject invalid mix at registration

                continue;

            case MixMatchStatus::NO_MATCH:

        if(mixMatch(policyMix.get(), i, attributes, config, uid) == MixMatchStatus::NO_MATCH) {

            ALOGV("%s: Mix %zu: does not match", __func__, i);

            continue; // skip the mix

            case MixMatchStatus::MATCH:;

        }



        if (primaryOutputMix) {
@@ -331,23 +360,6 @@ AudioPolicyMixCollection::MixMatchStatus AudioPolicyMixCollection::mixMatch( 
                break;

            }



            // consistency checks: for each "dimension" of rules (usage, uid...), we can

            // only have MATCH rules, or EXCLUDE rules in each dimension, not a combination

            if (hasUsageMatchRules && hasUsageExcludeRules) {

                ALOGE("getOutputForAttr: invalid combination of RULE_MATCH_ATTRIBUTE_USAGE"

                        " and RULE_EXCLUDE_ATTRIBUTE_USAGE in mix %zu", mixIndex);

                return MixMatchStatus::INVALID_MIX;

            }

            if (hasUidMatchRules && hasUidExcludeRules) {

                ALOGE("getOutputForAttr: invalid combination of RULE_MATCH_UID"

                        " and RULE_EXCLUDE_UID in mix %zu", mixIndex);

                return MixMatchStatus::INVALID_MIX;

            }

            if (hasUserIdMatchRules && hasUserIdExcludeRules) {

                ALOGE("getOutputForAttr: invalid combination of RULE_MATCH_USERID"

                        " and RULE_EXCLUDE_USERID in mix %zu", mixIndex);

                    return MixMatchStatus::INVALID_MIX;

            }



            if ((hasUsageExcludeRules && usageExclusionFound)

                    || (hasUidExcludeRules && uidExclusionFound)
@@ -500,7 +512,7 @@ status_t AudioPolicyMixCollection::setUidDeviceAffinities(uid_t uid, 
    //     AND it doesn't have a "match uid" rule

    //   THEN add a rule to exclude the uid

    for (size_t i = 0; i < size(); i++) {

        const AudioPolicyMix *mix = itemAt(i).get();

        AudioPolicyMix *mix = itemAt(i).get();

        if (!mix->isDeviceAffinityCompatible()) {

            continue;

        }
@@ -530,27 +542,16 @@ status_t AudioPolicyMixCollection::setUidDeviceAffinities(uid_t uid, 
status_t AudioPolicyMixCollection::removeUidDeviceAffinities(uid_t uid) {

    // for each player mix: remove existing rules that match or exclude this uid

    for (size_t i = 0; i < size(); i++) {

        bool foundUidRule = false;

        const AudioPolicyMix *mix = itemAt(i).get();

        AudioPolicyMix *mix = itemAt(i).get();

        if (!mix->isDeviceAffinityCompatible()) {

            continue;

        }

        std::vector<size_t> criteriaToRemove;

        for (size_t j = 0; j < mix->mCriteria.size(); j++) {

            const uint32_t rule = mix->mCriteria[j].mRule;



        // is this rule excluding the uid? (not considering uid match rules

        // as those are not used for uid-device affinity)

            if (rule == RULE_EXCLUDE_UID

                    && uid == mix->mCriteria[j].mValue.mUid) {

                foundUidRule = true;

                criteriaToRemove.insert(criteriaToRemove.begin(), j);

            }

        }

        if (foundUidRule) {

            for (size_t j = 0; j < criteriaToRemove.size(); j++) {

                mix->mCriteria.removeAt(criteriaToRemove[j]);

            }

        }

        EraseCriteriaIf(mix->mCriteria, [uid](const AudioMixMatchCriterion& c) {

            return c.mRule == RULE_EXCLUDE_UID && c.mValue.mUid == uid;

        });

    }

    return NO_ERROR;

}
@@ -585,7 +586,7 @@ status_t AudioPolicyMixCollection::setUserIdDeviceAffinities(int userId, 
    //    "match userId" rule for this userId, return an error

    //    (adding a userId-device affinity would result in contradictory rules)

    for (size_t i = 0; i < size(); i++) {

        const AudioPolicyMix* mix = itemAt(i).get();

        AudioPolicyMix* mix = itemAt(i).get();

        if (!mix->isDeviceAffinityCompatible()) {

            continue;

        }
@@ -602,7 +603,7 @@ status_t AudioPolicyMixCollection::setUserIdDeviceAffinities(int userId, 
    //     AND it doesn't have a "match userId" rule

    //   THEN add a rule to exclude the userId

    for (size_t i = 0; i < size(); i++) {

        const AudioPolicyMix *mix = itemAt(i).get();

        AudioPolicyMix *mix = itemAt(i).get();

        if (!mix->isDeviceAffinityCompatible()) {

            continue;

        }
@@ -632,27 +633,16 @@ status_t AudioPolicyMixCollection::setUserIdDeviceAffinities(int userId, 
status_t AudioPolicyMixCollection::removeUserIdDeviceAffinities(int userId) {

    // for each player mix: remove existing rules that match or exclude this userId

    for (size_t i = 0; i < size(); i++) {

        bool foundUserIdRule = false;

        const AudioPolicyMix *mix = itemAt(i).get();

        AudioPolicyMix *mix = itemAt(i).get();

        if (!mix->isDeviceAffinityCompatible()) {

            continue;

        }

        std::vector<size_t> criteriaToRemove;

        for (size_t j = 0; j < mix->mCriteria.size(); j++) {

            const uint32_t rule = mix->mCriteria[j].mRule;



        // is this rule excluding the userId? (not considering userId match rules

        // as those are not used for userId-device affinity)

            if (rule == RULE_EXCLUDE_USERID

                    && userId == mix->mCriteria[j].mValue.mUserId) {

                foundUserIdRule = true;

                criteriaToRemove.insert(criteriaToRemove.begin(), j);

            }

        }

        if (foundUserIdRule) {

            for (size_t j = 0; j < criteriaToRemove.size(); j++) {

                mix->mCriteria.removeAt(criteriaToRemove[j]);

            }

        }

        EraseCriteriaIf(mix->mCriteria, [userId](const AudioMixMatchCriterion& c) {

            return c.mRule == RULE_EXCLUDE_USERID && c.mValue.mUserId == userId;

        });

    }

    return NO_ERROR;

}