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

Commit 78f7f9a5 authored by Mikhail Naganov's avatar Mikhail Naganov
Browse files

libaudiohal@aidl: Align ownership of patches with the framework 

DeviceHalAidl creates patches when opening streams.
Later these patches can be "re-created" by the framework,
that means, the framework creates a patch for the same
pair of device and mix port configurations via
'createAudioPatch' call. In this case, ownership of
the patch must be transferred to the framework, it will
release it via 'releaseAudioPatch' later.

Properly separate ownership of port configs between streams
and patches to avoid attempting to release port configs
belonging to patches.

Bug: 302132812
Bug: 303742495
Test: run `atest CtsMediaAudioTestCases \
           --test-filter=".*AudioPlaybackCaptureTest#testPlaybackCaptureDoS`
      and check for errors from the AIDL HAL
Change-Id: I695af339e7293a12d07cf0d35d10a4c29008a3d2
parent ac9d4e75

media/libaudiohal/impl/DeviceHalAidl.cpp

+2 −2
Original line number Diff line number Diff line
@@ -477,7 +477,7 @@ status_t DeviceHalAidl::openOutputStream( 
    {

        std::lock_guard l(mLock);

        mCallbacks.emplace(cbCookie, Callbacks{});

        mMapper.addStream(*outStream, aidlPatch.id);

        mMapper.addStream(*outStream, mixPortConfig.id, aidlPatch.id);

    }

    if (streamCb) streamCb->setCookie(cbCookie);

    eventCb->setCookie(cbCookie);
@@ -543,7 +543,7 @@ status_t DeviceHalAidl::openInputStream( 
            std::move(ret.stream), mVendorExt, this /*micInfoProvider*/);

    {

        std::lock_guard l(mLock);

        mMapper.addStream(*inStream, aidlPatch.id);

        mMapper.addStream(*inStream, mixPortConfig.id, aidlPatch.id);

    }

    cleanups.disarmAll();

    return OK;

media/libaudiohal/impl/Hal2AidlMapper.cpp

+51 −41
Original line number Diff line number Diff line
@@ -88,8 +88,9 @@ Hal2AidlMapper::Hal2AidlMapper(const std::string& instance, const std::shared_pt 
        : mInstance(instance), mModule(module) {

}



void Hal2AidlMapper::addStream(const sp<StreamHalInterface>& stream, int32_t patchId) {

    mStreams.insert(std::pair(stream, patchId));

void Hal2AidlMapper::addStream(

        const sp<StreamHalInterface>& stream, int32_t portConfigId, int32_t patchId) {

    mStreams.insert(std::pair(stream, std::pair(portConfigId, patchId)));

}



bool Hal2AidlMapper::audioDeviceMatches(const AudioDevice& device, const AudioPort& p) {
@@ -164,8 +165,15 @@ status_t Hal2AidlMapper::createOrUpdatePatch( 
    } else {

        bool created = false;

        RETURN_STATUS_IF_ERROR(findOrCreatePatch(patch, &patch, &created));

        // Since no cleanup of the patch is needed, 'created' is ignored.

        // No cleanup of the patch is needed, it is managed by the framework.

        *patchId = patch.id;

        if (!created) {

            // The framework might have "created" a patch which already existed due to

            // stream creation. Need to release the ownership from the stream.

            for (auto& s : mStreams) {

                if (s.second.second == patch.id) s.second.second = -1;

            }

        }

    }

    return OK;

}
@@ -622,23 +630,17 @@ status_t Hal2AidlMapper::initialize() { 
    return OK;

}



bool Hal2AidlMapper::isPortHeldByAStream(int32_t portId) {

bool Hal2AidlMapper::isPortBeingHeld(int32_t portId) {

    // It is assumed that mStreams has already been cleaned up.

    for (const auto& streamPair : mStreams) {

        int32_t patchId = streamPair.second;

        auto patchIt = mPatches.find(patchId);

        if (patchIt == mPatches.end()) continue;

        for (int32_t id : patchIt->second.sourcePortConfigIds) {

            auto portConfigIt = mPortConfigs.find(id);

            if (portConfigIt != mPortConfigs.end() && portConfigIt->second.portId == portId) {

                return true;

            }

    for (const auto& s : mStreams) {

        if (portConfigBelongsToPort(s.second.first, portId)) return true;

    }

        for (int32_t id : patchIt->second.sinkPortConfigIds) {

            auto portConfigIt = mPortConfigs.find(id);

            if (portConfigIt != mPortConfigs.end() && portConfigIt->second.portId == portId) {

                return true;

    for (const auto& [_, patch] : mPatches) {

        for (int32_t id : patch.sourcePortConfigIds) {

            if (portConfigBelongsToPort(id, portId)) return true;

        }

        for (int32_t id : patch.sinkPortConfigIds) {

            if (portConfigBelongsToPort(id, portId)) return true;

        }

    }

    return false;
@@ -686,21 +688,26 @@ status_t Hal2AidlMapper::prepareToOpenStream( 
}



status_t Hal2AidlMapper::releaseAudioPatch(int32_t patchId) {

    return releaseAudioPatches({patchId});

}



status_t Hal2AidlMapper::releaseAudioPatches(const std::set<int32_t>& patchIds) {

    status_t result = OK;

    for (const auto patchId : patchIds) {

        if (auto it = mPatches.find(patchId); it != mPatches.end()) {

            mPatches.erase(it);

            if (ndk::ScopedAStatus status = mModule->resetAudioPatch(patchId); !status.isOk()) {

                ALOGE("%s: error while resetting patch %d: %s",

                        __func__, patchId, status.getDescription().c_str());

            return statusTFromBinderStatus(status);

        }

        return OK;

                result = statusTFromBinderStatus(status);

            }

        } else {

            ALOGE("%s: patch id %d not found", __func__, patchId);

    return BAD_VALUE;

            result = BAD_VALUE;

        }



void Hal2AidlMapper::resetPatch(int32_t patchId) {

    (void)releaseAudioPatch(patchId);

    }

    resetUnusedPortConfigs();

    return result;

}



void Hal2AidlMapper::resetPortConfig(int32_t portConfigId) {
@@ -716,22 +723,22 @@ void Hal2AidlMapper::resetPortConfig(int32_t portConfigId) { 
    ALOGE("%s: port config id %d not found", __func__, portConfigId);

}



void Hal2AidlMapper::resetUnusedPatches() {

    // Since patches can be created independently of streams via 'createAudioPatch',

void Hal2AidlMapper::resetUnusedPatchesAndPortConfigs() {

    // Since patches can be created independently of streams via 'createOrUpdatePatch',

    // here we only clean up patches for released streams.

    std::set<int32_t> patchesToRelease;

    for (auto it = mStreams.begin(); it != mStreams.end(); ) {

        if (auto streamSp = it->first.promote(); streamSp) {

            ++it;

        } else {

            resetPatch(it->second);

            it = mStreams.erase(it);

            if (const int32_t patchId = it->second.second; patchId != -1) {

                patchesToRelease.insert(patchId);

            }

            it = mStreams.erase(it);

        }

    }



void Hal2AidlMapper::resetUnusedPatchesAndPortConfigs() {

    resetUnusedPatches();

    resetUnusedPortConfigs();

    // 'releaseAudioPatches' also resets unused port configs.

    releaseAudioPatches(patchesToRelease);

}



void Hal2AidlMapper::resetUnusedPortConfigs() {
@@ -749,6 +756,9 @@ void Hal2AidlMapper::resetUnusedPortConfigs() { 
    for (int32_t id : mInitialPortConfigIds) {

        portConfigIds.erase(id);

    }

    for (const auto& s : mStreams) {

        portConfigIds.erase(s.second.first);

    }

    std::set<int32_t> retryDeviceDisconnection;

    for (const auto& portConfigAndIdPair : portConfigIds) {

        resetPortConfig(portConfigAndIdPair.first);
@@ -758,7 +768,7 @@ void Hal2AidlMapper::resetUnusedPortConfigs() { 
        }

    }

    for (int32_t portId : retryDeviceDisconnection) {

        if (!isPortHeldByAStream(portId)) {

        if (!isPortBeingHeld(portId)) {

            if (auto status = mModule->disconnectExternalDevice(portId); status.isOk()) {

                eraseConnectedPort(portId);

                ALOGD("%s: executed postponed external device disconnection for port ID %d",
@@ -848,7 +858,7 @@ status_t Hal2AidlMapper::setDevicePortConnectedState(const AudioPort& devicePort 
        }

        // Streams are closed by AudioFlinger independently from device disconnections.

        // It is possible that the stream has not been closed yet.

        if (!isPortHeldByAStream(portId)) {

        if (!isPortBeingHeld(portId)) {

            RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(

                            mModule->disconnectExternalDevice(portId)));

            eraseConnectedPort(portId);

media/libaudiohal/impl/Hal2AidlMapper.h

+14 −5
Original line number Diff line number Diff line
@@ -44,7 +44,7 @@ class Hal2AidlMapper { 
            const std::string& instance,

            const std::shared_ptr<::aidl::android::hardware::audio::core::IModule>& module);



    void addStream(const sp<StreamHalInterface>& stream, int32_t patchId);

    void addStream(const sp<StreamHalInterface>& stream, int32_t portConfigId, int32_t patchId);

    status_t createOrUpdatePatch(

            const std::vector<::aidl::android::media::audio::common::AudioPortConfig>& sources,

            const std::vector<::aidl::android::media::audio::common::AudioPortConfig>& sinks,
@@ -114,7 +114,12 @@ class Hal2AidlMapper { 
    // Answers the question "whether portID 'first' is reachable from portID 'second'?"

    // It's not a map because both portIDs are known. The matrix is symmetric.

    using RoutingMatrix = std::set<std::pair<int32_t, int32_t>>;

    using Streams = std::map<wp<StreamHalInterface>, int32_t /*patch ID*/>;

    // There is always a port config ID set. The patch ID is set after stream

    // creation, and can be set to '-1' later if the framework happens to create

    // a patch between the same endpoints. In that case, the ownership of the patch

    // is on the framework.

    using Streams = std::map<wp<StreamHalInterface>,

            std::pair<int32_t /*port config ID*/, int32_t /*patch ID*/>>;



    const std::string mInstance;

    const std::shared_ptr<::aidl::android::hardware::audio::core::IModule> mModule;
@@ -147,10 +152,14 @@ class Hal2AidlMapper { 
            const std::optional<::aidl::android::media::audio::common::AudioConfig>& config,

            const std::optional<::aidl::android::media::audio::common::AudioIoFlags>& flags,

            int32_t ioHandle);

    bool isPortHeldByAStream(int32_t portId);

    void resetPatch(int32_t patchId);

    bool isPortBeingHeld(int32_t portId);

    bool portConfigBelongsToPort(int32_t portConfigId, int32_t portId) {

        auto it = mPortConfigs.find(portConfigId);

        return it != mPortConfigs.end() && it->second.portId == portId;

    }

    status_t releaseAudioPatches(const std::set<int32_t>& patchIds);

    void resetPatch(int32_t patchId) { (void)releaseAudioPatch(patchId); }

    void resetPortConfig(int32_t portConfigId);

    void resetUnusedPatches();

    void resetUnusedPortConfigs();

    status_t updateAudioPort(

            int32_t portId, ::aidl::android::media::audio::common::AudioPort* port);