D11021: [Media controller] Add simple volume control

Kai Uwe Broulik noreply at phabricator.kde.org
Mon Mar 5 08:45:31 UTC 2018


broulik added a comment.


  Thanks for your patch! I thought about doing that for a long time, actually.
  You can add `BUG: 386588` to your commit message (on its own line) to indiate that it fixes this bug.
  
  Can you please also implement a volume OSD? We have a mediaPlayerVolumeChanged in plasmashell OSD service but never used it. I think this could be done in the `SetVolume` operation by adding a new argument `showOsd` or so that you then set. To implement an OSD you just need to place a DBus call like so:
  
    QDBusMessage msg = QDBusMessage::createMethodCall(
        QStringLiteral("org.kde.plasmashell"),
        QStringLiteral("/org/kde/osdService"),
        QStringLiteral("org.kde.osdService"),
        QStringLiteral("mediaPlayerVolumeChanged")
    );
    
    msg.setArguments({
        50, // volume in percent (0-100)
        user visible player name (identity), // "VLC media player"
        icon name of player // "vlc"
    });
    
    QDBusConnection::sessionBus().asyncCall(msg);

INLINE COMMENTS

> main.qml:153
> +                var volume = mpris2Source.currentData.Volume || 0.0
> +                volume += (wheel.angleDelta.y / 120) * 0.03
> +                if (volume < 0) volume = 0.0

Did you check this works fine with touchpads? Here Qt sends a gazillion of tiny wheel events.

Please enforce a limit of e.g. 100%, I almost deafened myself last time I messed with volume on Mpris

> main.qml:154
> +                volume += (wheel.angleDelta.y / 120) * 0.03
> +                if (volume < 0) volume = 0.0
> +                var service = mpris2Source.serviceForSource(mpris2Source.current)

volume = Math.max(0, volume);

> multiplexedservice.cpp:136
> +            if (m_control) {
> +                double vol = m_control->playerInterface()->volume() + 0.05;
> +                m_control->playerInterface()->setVolume(vol);

No need to abbreviate

  const qreal newVolume = ...

> multiplexedservice.cpp:137
> +                double vol = m_control->playerInterface()->volume() + 0.05;
> +                m_control->playerInterface()->setVolume(vol);
> +            }

Also enforce a limit here,

  qBound(0.0, vol, 1.0);

REPOSITORY
  R120 Plasma Workspace

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

To: Pitel, #plasma
Cc: broulik, nicolasfella, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20180305/e1fa6a26/attachment-0001.html>


More information about the Plasma-devel mailing list