| drosca | |
| davidedmundson | |
| ngraham |
| Plasma |
The audio should not be garbled if you move fast the audio slider (it distorts because it stops and plays the feedback sound every few milliseconds)
An alternative approach is to return; if the ca_context_playing says that the sound is still playing, but that gives less "resolution"
BUG: 409525
BUG: 410716
FIXED-IN: 5.17.0
| Lint Skipped |
| Unit Tests Skipped |
I don't know why it is not showing the context of the patch 🤷♂️
I selected the plasma-pa project
Thanks @davidedmundson for the feedback,
I fixed what you suggested, but thinking about this again, the 50ms timer fixed the audio garbling but the sounds felt "too frequent", increasing the timer gave the same exact result as disabling the function altogether, so here's that.
It wasn't worth the extra complexity of the timer
Because you didn't use arc to submit the patch and Phabricator is kinda dumb about this. This should get much better with GitLab.
Removed VDG as the reviewer, seems like a technical issue all together. Also wanted to contribute, but I don't really perceive any issues when moving the slider fast :/
Sorry
Also wanted to contribute, but I don't really perceive any issues when moving the slider fast :/
This is what I am referring to
As you can see in the picture, the waveform of the feedback sound gets truncated in a "violent" way, and the abrupt truncation generates a broadband sound spike that's clearly audible
Also, having the sound start and truncate 10 times when you move the slider a bit feels less polished
Maybe a better option is to have the sound play when you release the mouse button from the slider
Should it queue up a sound so that it plays one sound at the final volume when you've stopped dragging?
Should it queue up a sound so that it plays one sound at the final volume when you've stopped dragging?
I made that it plays the sound at the final volume when you release the mouse button
With this change, if I press the volume up and volume down key on my keyboard, feedback only plays every second or third time I hit the button (depending on how fast I press it).
True
For some reason, if I measure the amount of time that it takes for ca_context_playing to switch back from 1 to 0, by busy waiting, it blocks for about two seconds, even though the actual sound lasts about 0.07 seconds
With the volume slider, for some other reason, the ca_context_playing goes back to 0 sooner, but with the volume keys it blocks the whole two seconds.
I have no idea on why it does that
updateTimer uses slider value and it takes 200 ms to react, so you can sync the timer with keyboard one by increase the interval https://phabricator.kde.org/source/plasma-pa/browse/master/applet/contents/ui/ListItemBase.qml$198
I talked with 4-5 people and they all said that giving real time feedback in the slider is useless and bad, so here's that, the feedback is given on mouse release, and every problem is automatically solved.
If you want more technical details about the alternative version, read the rest of this comment, otherwise just ignore, now everything is fine.
useless part of the comment:
I'm not sure if I am understanding this, the timer seems to only provide feedback to the slider .. ?
The main issue here is inside libcanberra (or pulseaudio): in some conditions (I found that the bug occurs mostly if no other sound is playing) ca_context_playing simply does not work correctly, i'm fairly sure that my patch is correct, I tested libcanberra independently and I can confirm that "bug"
I tested that by setting CA_PROP_CANBERRA_CACHE_CONTROL to "never", the problem seems to go away, that's probably suboptimal but it's the only decent workaround I can come up with
diff --git a/applet/contents/ui/ListItemBase.qml b/applet/contents/ui/ListItemBase.qml index d962eb5..6bdbcb7 100644 --- a/applet/contents/ui/ListItemBase.qml +++ b/applet/contents/ui/ListItemBase.qml @@ -190,6 +190,10 @@ PlasmaComponents.ListItem { // whereas PA rejected the volume change and is // still at v15 (e.g.). updateTimer.restart(); + + if (type == "sink") { + playFeedback(Index); + } } } diff --git a/src/qml/volumefeedback.cpp b/src/qml/volumefeedback.cpp index 69bc260..3e5abae 100644 --- a/src/qml/volumefeedback.cpp +++ b/src/qml/volumefeedback.cpp @@ -55,9 +55,9 @@ void VolumeFeedback::play(quint32 sinkIndex) // NB Depending on how this is desired to work, we may want to simply // skip playing, or cancel the currently playing sound and play our - // new one... for now, let's do the latter. + // new one... for now, let's do the former. if (playing) { - ca_context_cancel(context, cindex); + return; } char dev[64]; @@ -70,7 +70,7 @@ void VolumeFeedback::play(quint32 sinkIndex) cindex, CA_PROP_EVENT_DESCRIPTION, "Volume Control Feedback Sound", CA_PROP_EVENT_ID, "audio-volume-change", - CA_PROP_CANBERRA_CACHE_CONTROL, "permanent", + CA_PROP_CANBERRA_CACHE_CONTROL, "never", // XXX workaround a libcamberra bug CA_PROP_CANBERRA_ENABLE, "1", nullptr );