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

Commit e0dc36d0 authored by Francois Gaffie's avatar Francois Gaffie
Browse files

[BUG] AudioFlinger: Patch Panel: Fix SwBridge Patch leak



Audio HAL implementors have to manage unique patch ID.
It was observed that some patches were leaking, aka created but never
released. These patches are the playback patch of SW Bridges.

This CL fixes this leak by preventing to remove Plaback patch of a SW Bridge

When a SW Bridge is created, 2 patches are created. The patch involving
the SwOutput and the sink device is attached to the SwOutput mix handle.
When this SwOutput is routed/rerouted, the setOutputDevices will create
also a new Patch on AudioFlinger.
The remove stale audio patch section may destroy this patch.
Thus, when releasing the SwBridge patch, the Playback patch is not found
and releaseAudioPatch is never called on AudioHAL.

This CL prevents to erase the AF Playback Patch of a Sw Bridge.
It also prevents to erase the AF Patch used to route the SwOutput that may
be also involved in a SwBridge.

Bug: 187173301

Test: (manuel) make a first playback on an SwOutput, stop it, create a SwAudio
Patch, and release it twice. Playback Patch shall always be released,
hense HAL API called.
(functional on emulator):
adb shell ./data/AudioPolicyEmulatorTests --gtest_filter=*AudioPatchTest*
adb shell ./data/AudioPolicyEmulatorTests --gtest_filter=*AudioSourceTest*

Signed-off-by: default avatarFrancois Gaffie <francois.gaffie@renault.com>
Change-Id: I24e371cd69e04aba2bf74b88336f6323cd88ad8b
parent 5dd9da8b
Loading
Loading
Loading
Loading
+16 −8
Original line number Diff line number Diff line
@@ -111,7 +111,8 @@ status_t AudioFlinger::PatchPanel::getAudioPort(struct audio_port *port __unused

/* Connect a patch between several source and sink ports */
status_t AudioFlinger::PatchPanel::createAudioPatch(const struct audio_patch *patch,
                                   audio_patch_handle_t *handle)
                                   audio_patch_handle_t *handle,
                                   bool endpointPatch)
{
    if (handle == NULL || patch == NULL) {
        return BAD_VALUE;
@@ -174,7 +175,7 @@ status_t AudioFlinger::PatchPanel::createAudioPatch(const struct audio_patch *pa
        }
    }

    Patch newPatch{*patch};
    Patch newPatch{*patch, endpointPatch};
    audio_module_handle_t insertedModule = AUDIO_MODULE_HANDLE_NONE;

    switch (patch->sources[0].type) {
@@ -396,12 +397,17 @@ status_t AudioFlinger::PatchPanel::createAudioPatch(const struct audio_patch *pa
            }

            // remove stale audio patch with same output as source if any
            // Prevent to remove endpoint patches (involved in a SwBridge)
            // Prevent to remove AudioPatch used to route an output involved in an endpoint.
            if (!endpointPatch) {
                for (auto& iter : mPatches) {
                if (iter.second.mAudioPatch.sources[0].ext.mix.handle == thread->id()) {
                    if (iter.second.mAudioPatch.sources[0].ext.mix.handle == thread->id() &&
                            !iter.second.mIsEndpointPatch) {
                        erasePatch(iter.first);
                        break;
                    }
                }
            }
        } break;
        default:
            status = BAD_VALUE;
@@ -435,7 +441,8 @@ status_t AudioFlinger::PatchPanel::Patch::createConnections(PatchPanel *panel)
    status_t status = panel->createAudioPatch(
            PatchBuilder().addSource(mAudioPatch.sources[0]).
                addSink(mRecord.thread(), { .source = AUDIO_SOURCE_MIC }).patch(),
            mRecord.handlePtr());
            mRecord.handlePtr(),
            true /*endpointPatch*/);
    if (status != NO_ERROR) {
        *mRecord.handlePtr() = AUDIO_PATCH_HANDLE_NONE;
        return status;
@@ -445,7 +452,8 @@ status_t AudioFlinger::PatchPanel::Patch::createConnections(PatchPanel *panel)
    if (mAudioPatch.num_sinks != 0) {
        status = panel->createAudioPatch(
                PatchBuilder().addSource(mPlayback.thread()).addSink(mAudioPatch.sinks[0]).patch(),
                mPlayback.handlePtr());
                mPlayback.handlePtr(),
                true /*endpointPatch*/);
        if (status != NO_ERROR) {
            *mPlayback.handlePtr() = AUDIO_PATCH_HANDLE_NONE;
            return status;
+7 −2
Original line number Diff line number Diff line
@@ -56,7 +56,8 @@ public:

    /* Create a patch between several source and sink ports */
    status_t createAudioPatch(const struct audio_patch *patch,
                                       audio_patch_handle_t *handle);
                              audio_patch_handle_t *handle,
                              bool endpointPatch = false);

    /* Release a patch */
    status_t releaseAudioPatch(audio_patch_handle_t handle);
@@ -161,7 +162,8 @@ public:

    class Patch final {
    public:
        explicit Patch(const struct audio_patch &patch) : mAudioPatch(patch) {}
        Patch(const struct audio_patch &patch, bool endpointPatch) :
            mAudioPatch(patch), mIsEndpointPatch(endpointPatch) {}
        Patch() = default;
        ~Patch();
        Patch(const Patch& other) noexcept {
@@ -170,6 +172,7 @@ public:
            mPlayback = other.mPlayback;
            mRecord = other.mRecord;
            mThread = other.mThread;
            mIsEndpointPatch = other.mIsEndpointPatch;
        }
        Patch(Patch&& other) noexcept { swap(other); }
        Patch& operator=(Patch&& other) noexcept {
@@ -184,6 +187,7 @@ public:
            swap(mPlayback, other.mPlayback);
            swap(mRecord, other.mRecord);
            swap(mThread, other.mThread);
            swap(mIsEndpointPatch, other.mIsEndpointPatch);
        }

        friend void swap(Patch &a, Patch &b) noexcept {
@@ -218,6 +222,7 @@ public:
        Endpoint<RecordThread, RecordThread::PatchRecord> mRecord;

        wp<ThreadBase> mThread;
        bool mIsEndpointPatch;
    };

    // Call with AudioFlinger mLock held