Feature request: non-blocking setvcp to improve UI responsiveness

[apsun]

Hi, thanks for creating this app! As someone who's dived into ddcutil a bit myself, I'm wondering if it would be possible to make the setvcp calls non-blocking, and asynchronously update the display values using an eventual consistency model? In my testing I found that performing a setvcp call for every UI update was too slow even when not reinitializing ddcutil - coalescing updates helped improve responsiveness significantly (https://github.com/apsun/xfce4-ddc-plugin/blob/master/panel-plugin/ddcdisplay.c#L205)

The idea is that if the user adjusts a value before the previous setvcp call has returned, it would enqueue another setvcp call and return immediately. If the user changes the value again, the new value would replace the old value in the queue. Once the original setvcp call returns, it would see that a new value was set and perform another setvcp call (repeating until there are no more updates).

[digitaltrails]

Thanks for the feedback. Yes, I had wondered about further separating the backend ddcutil operations from the frontend. I was pondering a blackboard architecture. The front end would just display current values from the blackboard (model of VDUs). Setvcp would just be dispatched to background operations that would eventually update blackboard. Blackboard events would be signaled by qt-signals.

Request to move in the opposite direction

On the other hand, I have just implemented #75: when using the ddcutil-service, dragging on the controls should act immediately because the service is fast enough for that to be useful (useful because the user can see/judge the resulting brightness in the actual VDU). Having made that change, it does appear that dragging the brightness slider is fast enough to judge the resulting brightness.

When ddcutil-service isn't used, the UI only changes on mouse-release, so intermediate drag values are dropped by the UI (exec'ing the ddcutil command is too slow to allow dragging feedback). So this is kind of like dropping intermediate setvcps (they just don't happen).

Considering what other issues a queue would raise...

The background threads for preset-restoration and ambient-light-adjustments both check for alterations made by other threads/processes and take this as a signal to terminate (because someone/something else is making changes). So discarding queued values might have to allow for that, or maybe the queue entry could have a 'requester-id' or 'reason'.

When restoring resets and adjusting for ambient light, in some cases the adjustment may be stepped (to transition gradually). I'd need to make sure the queue doesn't discard transition steps (again perhaps a requester-id or reason).

Possible implementations

Looking at how to bolt this in with minimal change, I'd imagine I'd just add a queue to the ddcutil layer that fronts the ddcutil-command and ddcutil-service implementations.

A blackboard style architecture would require a rewrite. All the creeping-featuritis that infects the current codebase would make refactoring quite tricky.

Is this change really necessary

What do you find slow in the existing vdu_controls implementation, especially when used with ddcutil-service?

[apsun]

I don't think live updates while dragging the slider and asynchronous processing need to be mutually exclusive - the updates can still be live; the only difference is that duplicate calls for the same feature could be deduplicated by adding a queue.

I'm not sure if it's an issue with my monitor, but each SetVcp call on my machine takes on the order of 200ms via ddcutil-service (even worse when shelling out to ddcutil directly, which needs ~1.5s); when dragging the slider this results in the UI freezing as it tries to call SetVcp dozens of times.

$ time dbus-send --dest=com.ddcutil.DdcutilService --print-reply --type=method_call   /com/ddcutil/DdcutilObject   com.ddcutil.DdcutilInterface.SetVcp int32:1 string: byte:0x62 uint16:100 uint32:0 
method return time=1711835380.654171 sender=:1.195 -> destination=:1.287 serial=681 reply_serial=2
   int32 0
   string "OK"

real    0m0.200s
user    0m0.000s
sys     0m0.002s

$ time ddcutil setvcp 0x62 100

real    0m1.594s
user    0m0.019s
sys     0m0.166s

[digitaltrails]

I get a similar figures for ddcutil-service:

% time dbus-send --dest=com.ddcutil.DdcutilService --print-reply --type=method_call   /com/ddcutil/DdcutilObject   com.ddcutil.DdcutilInterface.SetVcp int32:1 string: byte:0x10 uint16:100 uint32:0
method return time=1711836946.161089 sender=:1.76 -> destination=:1.121 serial=447 reply_serial=2
   int32 0
   string "OK"

real    0m0.185s
user    0m0.003s
sys     0m0.000s

% time ddcutil setvcp 0x10 100

real    0m0.841s
user    0m0.005s
sys     0m0.274s

However I find dragging the brightness slider when using ddcutil-service to be OK, not completely smooth, but seemly responsive. Maybe my expectations of smooth-enough differ from yours.

I did notice that if I switch vdu_controls back to using ddcutil-command, I have to do a restart or it continues to do setvcp while dragging (so that's a bug). But if I do a restart, then drag brightness, setvcp only happens on release.

This would be on X11, I haven't tested wayland recently.

[apsun]

Interesting, this is how it looks for me:

Screencast_20240330_153559.webm

It seems as if the more ddcutil calls there are enqueued, the slower everything gets. Perhaps there is some lock contention somewhere? This is on Wayland with ddcutil 2.1.4.

[digitaltrails]

OK - I see your issue. Both from your video and if I drag often enough, there does sometimes appear to be a lag, then it goes away. Maybe libddcutil is erroring and repeating (internally) in some cases, but not all, and that might vary for different hardware and different versions of ddcutil (after ddcutil 2.0 a lot changed and keeps changing).

I take the issue doesn't occur when using ddcutil-command (disable dbus-client and restart), which doesn't call setvcp while sliding?

I don't think live updates while dragging the slider and asynchronous processing need to be mutually exclusive - the updates can still be live; the only difference is that duplicate calls for the same feature could be deduplicated by adding a queue.

Yes, I see where a dedup would basically implement a kind of throttling, so if the setvcp does not return fast enough, some intermediate values would be skipped, but some would make it through, so the drag will still be viewable.

[digitaltrails]

This is what I generally see. There is a little lag at the start of the second drag. That's what I occasionally experience.

https://github.com/digitaltrails/vdu_controls/assets/5510901/c8ddd424-1e98-4a11-b5ef-e28bc4c08aec

Just in case, I should do some new wayland testing now that KDE6 is here.

I need to think about what to do in the short and long term.

What I may do is add a flag that allows value changes while dragging and default it to off. After this release, when time allows, I'll see if I can do something such as queuing and then get rid of the flag and allow value changes while dragging for both ddcutil-command and ddcutil-service.

[digitaltrails]

In the meantime, if you'd like to disable setvcp while dragging, changing self.is_speedy = False in class VduController(QObject) is a quick way to change the behaviour.

https://github.com/digitaltrails/vdu_controls/blob/c04ddfe96b89ac40a235a2d9b91037b0f5912ac3/vdu_controls.py#L2346

[digitaltrails]

I don't know if it makes a difference, but I spotted I undercooked the minimum time between setvcp's while dragging.

I've now throttled it back to a maximum of 1 setvcp every 0.2 of a second which matches the elapsed times actually seen with ddcutil-service.

[digitaltrails]

I don't know if it makes a difference, but I spotted I undercooked the minimum time between setvcp's while dragging.

I've now throttled it back to a maximum of 1 setvcp every 0.2 of a second which matches the elapsed times actually seen with ddcutil-service.

Bumped to 250 milliseconds as a safety margin.

I wonder if it might now perform OK. I can see I had intended to throttle the load on both Qt components and ddcutil by preventing excessive updates of the UI spinner and the VDU. But, as I never encountered significant issues, I forgot to revisit the minimum time and left it way too low. I wonder if the excessive number of Qt events piling up may have been part of the problem.

Anyway, see if this helps before resorting to setting is_speady to False.

[apsun]

Thanks - that change seems to have made things much snappier! It's still a bit jittery, but at least it no longer lags behind by multiple seconds.

Screencast_20240330_200316.webm

[digitaltrails]

I tested on Wayland. Very interesting!

If I set the min time between setvcp's to less than 150 milliseconds, the Wayland vdu_controls interface became totally jerky and unresponsive.

I changed to 250 milliseconds and response was good, but dragging was a bit jerky, perhaps jumping about 2-5%. I eventually changed up to 300 millis and it seemed a bit better. I found 500 millis was about as good, so perhaps that's a safe bet.

I need to investigate whether what is causing this under Wayland:

• The pile up of setvcp requests.
• The pile up of qt events.
• Qt interaction between the spinner and the slider
• Some kind of threading/locking issue.

It seems this does not affect X11 to the same degree where it becomes very obvious. Perhaps X11 is more synchronous, so events don't pile up or cascade.

[digitaltrails]

I'm experimenting with an set_vcp_asynchronously() in the VduController object. It does restore smoothness to the UI (at least in X11). I need to fully figure out the implications when an error occurs, but I think that will pan out OK. Plus, as you suggested, I need to have a queue so the last value always get set.

Anyway, I'll continue tinkering, and if it pans out I'll check something in.

[digitaltrails]

Adjusting sliders in X11 and Wayland seems smooth now.

[apsun]

Can confirm, the sliders work perfectly now! No lag at all on my end :-) Thanks for adding this!