D4140: Rework playing of volume feedback

[Chris Holland]

Zren added a comment.


  Is the `onVolumeChange` call delayed? I'm not sure if it's fired when we set it, or PulseAudio gets back to us.
  
  Does this fire `onVolumeChange` immediately?
  
    sinkModel.preferredSink.volume = volume;
  
  If so, you're currently calling
  
    playFeedback(); // feedback.sinkIndex = sinkModel.preferredSink.index;
  
  after you change the volume. Which means `feedback.sinkIndex` isn't set the first volume change, but it is still set until the next time you change the volume. Since it's not `-1` after the first "volume change", it will play feedback, even if it's from something else.
  
  If this is so, then we probably want to move the call before we assign the volume. Maybe even rename the function to `playFeedbackNextVolumeChange()` as well.
  
    diff --git a/applet/contents/ui/main.qml b/applet/contents/ui/main.qml
    index 1c9b759..cf58149 100644
    --- a/applet/contents/ui/main.qml
    +++ b/applet/contents/ui/main.qml
    @@ -78,10 +78,10 @@ Item {
             }
             var volume = boundVolume(sinkModel.preferredSink.volume + volumeStep);
             var percent = volumePercent(volume, maxVolumeValue);
    +        playFeedbackNextVolumeChange();
             sinkModel.preferredSink.muted = percent == 0;
             sinkModel.preferredSink.volume = volume;
             osd.show(percent);
    -        playFeedback();
         }
     
         function decreaseVolume() {
  
  Also, does `sinkModel.preferredSink.volume` = `sinkModel.preferredSink.volume` fire `onVolumeChange` too?
  
  Eg: `increaseVolume()` setting it to 100% when it's already at 100%.

REPOSITORY
  R115 Plasma Audio Volume Applet

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

To: drosca, #plasma
Cc: Zren, broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20180110/e188ca30/attachment.html>