Skip to content
This repository has been archived by the owner on Feb 12, 2023. It is now read-only.

fix(calls): create dummy source for invalidated callback #5759

Open
wants to merge 3 commits into
base: v1.17-dev
Choose a base branch
from

Conversation

anthonybilinski
Copy link
Member

@anthonybilinski anthonybilinski commented Aug 2, 2019

Fixes mute friend and group calls when mic is switched to disabled then back to a valid source. Now ToxCall can register to the DummySource, and re-register when the source is changed, like when two normal sources are used.


This change is Reviewable

Copy link
Member

@sudden6 sudden6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 5 of 8 files at r1.
Reviewable status: 0 of 1 LGTMs obtained


src/audio/backend/openal.h, line 152 at r1 (raw file):

    std::unordered_set<AlSink*> sinks;
    std::unordered_set<AlSink*> soundSinks;
    std::unordered_set<IAudioSource*> sources;

Mixing ALSource and DummySource here is probably a bad idea, because AlSource actually represents resources in the audio backend and needs to be destroyed differently


src/audio/backend/openal.cpp, line 329 at r1 (raw file):

    std::unique_ptr<IAudioSource> source;
    if (!Settings::getInstance().getAudioInDevEnabled()) {
        source = std::unique_ptr<IAudioSource>(new DummySource{*this});

Bad dependency on the Settings singleton


src/audio/backend/openal.cpp, line 366 at r1 (raw file):

    qDebug() << "Unsubscribed from audio input device [" << sources.size() << "subscriptions left ]";

    if (sources.empty()) {

Having dummy sources in sources will keep the audio device open even though there are no real sources

@sudden6
Copy link
Member

sudden6 commented Sep 29, 2019

what do you think about IAudioControl handling dummy source/sink logic, and dispatching to a virtual private function that OpenAl implements for creating /real/ audio sources and audio sinks?

So you want to turn the IAudioControl interface into a full class, right? ATM I don't see any drawbacks from this approach.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants