D19994: Add microphone indicator

David Rosca noreply at phabricator.kde.org
Wed Mar 27 18:52:37 GMT 2019


drosca added a comment.


  Looks good.
  
  Personally I would work directly with the SourceOutputMap (and maybe duplicate it as list here in MicrophoneIndicator) instead of working with the models. I'm not saying that it's wrong / you should change it though, just a note.

INLINE COMMENTS

> microphoneindicator.cpp:42
> +    : QObject(parent)
> +    // what about "virtual streams"?
> +    , m_sourceModel(new QPulseAudio::SourceModel(this))

Just ignore virtual streams? We only care about real devices here.

> microphoneindicator.cpp:198
> +        return;
> +
> +    }

newline

> microphoneindicator.cpp:202
> +    // Otherwise unmute the devices we muted
> +    for (auto &idx : m_mutedIndices) {
> +        if (!idx.isValid()) {

qAsConst(m_mutedIndices)

> microphoneindicator.cpp:253
> +    m_osd->showMicrophone(volumePercent(preferredSource));
> +
> +}

newline

> microphoneindicator.cpp:274
> +        } else {
> +            // this can never be empty, right? It's always at least the process name
> +            names.append(name);

Yes, this should never be empty.

> plugin.cpp:67
>      qmlRegisterSingletonType(uri, 0, 1, "PulseAudio", pulseaudio_singleton);
> +    qmlRegisterSingletonType<MicrophoneIndicator>(uri, 0, 1, "MicrophoneIndicator",
> +        [](QQmlEngine *engine, QJSEngine *jsEngine) -> QObject* {

Any reason why you used singleton?

REPOSITORY
  R115 Plasma Audio Volume Applet

REVISION DETAIL
  https://phabricator.kde.org/D19994

To: broulik, #plasma, #vdg, drosca
Cc: apol, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20190327/b3e5cee5/attachment.html>


More information about the Plasma-devel mailing list