[vlc-devel] [PATCH v3 00/18] Get rid of the control command API

[Steve Lhomme]

What we're talking about here is opening a can of worms. This is a whole 
new behavior the code wasn't designed for. The whole mouse filtering 
could do with some love, but this is not the scope of this patchset.

Example of worm we're talking about: the software deinterlace filters 
mouse for when the picture is half height. None of the hardware filters 
do that.
There is no consistency between filters. (BTW rotate doesn't handle 
mouse events, and transform does)

I think, when it comes to UI, the responsiveness should matter the most. 
That means do as little "heavy" processing in the UI thread. Locking 
"sys->filter.loc"k in the UI thread would be quite heavy. That lock is 
used to process the pixels. It can take quite a while.

In the end the mouse events are transformed according to the filter 
dimension changes. But it's not to reflect something in the UI. It's to 
reflect something in the video rendering. Either modifying the rendering 
based on the mouse position or by sending "mouse-xxx" events. In the 
former case, there is no use having that in the UI thread. In the latter 
case, there are things like gestures/hotkeys which might benefit from 
being in the UI thread. There is also the callback set by some 
access/demuxer modules via ES_OUT_VOUT_SET_MOUSE_EVENT. It sounds like 
it could benefit from being in the UI thread. In practice when the user 
clicks it doesn't necessarily match the position the module think it's 
at, because of the buffering in between. Doing it one frame later is 
probably not a big deal.

So the real benefit would be for gestures/hotkeys.

The gestures only check for a movement in (all) directions, based on a 
threshold. But if a deinterlace shows the video at double/half size, 
that means the user actually has to moved twice more/less to reach that 
threshold, when in fact the threshold is based on the visible area, not 
the original picture location. So this is in fact wrong to handle 
filtered mouse values here. (it would be worse in puzzle if you happen 
to cross a square and the next virtual position is totally different 
from where the mouse is actually on the screen)

The hotkeys module has 2 things. Some "intf-xxx" toggles, which can 
popup a menu over the video. This should definitely not be filtered 
according to the coordinates in the unfiltered picture but should match 
the screen coordinates. It also calls vlc_player_UpdateViewpoint() with 
the new mouse (filtered) coordinates, to change the viewpoint. Here I 
don't think it makes sense to translate to filtered coordinates. For 
example it the video is zoomed 2x a mouse movement of 10 pixels should 
result in a movement of 10 pixels in the rendered video, not 5 pixels in 
the source video. In the end the viewpoint value is only applied to 
video and audio outputs, not the source material.

So it seems the "mouse-xxx" calls should not be filtered. They should be 
called before the filtering is applied. That means they could be called 
when the mouse event in received, in the UI thread. That would make the 
gesture/hotkey more responsive. (the double click fullscreen toggle 
should fall in that category too)

The filtered coordinates end up being used only for the few modules that 
actually need to know exactly where in the source video the user 
clicked/moved (using ES_OUT_VOUT_SET_MOUSE_EVENT). As explained above, I 
think it should remain in the rendering thread for now.

> On 20 Aug 2020, at 21:46, Rémi Denis-Courmont <remi at remlab.net> wrote:
> 
> Le jeudi 20 août 2020, 21:29:48 EEST Steve Lhomme a écrit :
>>> On 2020-08-20 18:11, Rémi Denis-Courmont wrote:
>>> Hi,
>>> 
>>> Badly designed or implemented plugins are what they are. Just as big
>>> loops in the UI thread are bad for interactivity, big loops in the
>>> render thread are bad for timeliness. And besides those egregiously
>>> broken plugins you have, e.g., rotate and transform who actually
>>> translate the mouse event, and should obviously run synchronously - not
>>> at the next render.
>> 
>> Synchronously ? I have seen nothing in the UI thread, the one that will
>> display the rendered result, that says a frame is allowed to be
>> displayed ASAP. It will wait until the next loop, in that thread.
>> Putting that in the UI thread is just pretending the rendering will be
>> more responsive.
> 
> Pretending? How do you need the rendering to translate mouse coordinates 
> through the transform filter? and eventually to pass mouse events to the input?
> 
> Even plotting the cursor does not need the VLC rendering. Only thing that does 
> is updating the Puzzle and such, not exactly everyday use cases.
> 
> -- 
> Rémi Denis-Courmont
> 
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel