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

Steve Lhomme robux4 at ycbcr.xyz
Fri Aug 21 16:19:44 CEST 2020


On 2020-08-21 9:53, Rémi Denis-Courmont wrote:
> Hi,
> 
> Mouse coordinates have to be translated to original video coordinates 
> for menus. We've done it, without filter callbacks, for years. No way 
> we're removing that, regardless of which thread does it. If some 

Where did I suggest that ? In fact I said the opposite:

"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)."

> component uses the video coordinates as window coordinates, it is buggy. 

No. As I explained the "mouse-XXX" (more precisely "mouse-moved", 
"mouse-button-down", "mouse-clicked" and "fullscreen") are used 
exclusively by UI components to do UI stuff. They do not care about what 
is inside the video. The fact they are currently translated before 
calling these is a *bug*.

There is however libvlc_video_get_cursor() which says the coordinates 
are according to the original video dimensions, not the displayed 
dimensions. So we need to get the translated values in there. It should 
not rely on "mouse-moved" but a different variable.

(also I just realized "mouse-clicked" is write only).

> Conversely, if some filters translate wrong or fail to translate, 
> they're already buggy now and this makes no difference to this patchset.
> 
> And maybe it's not currently practical to filter the mouse events on the 
> video window thread. That's not an excuse for regressing the mouse event 
> processing. Currently it is mostly immediate, not deferred to the next 
> picture, even if it's not literally synchronous.

I am not changing that. In v4 the UI interaction is actually more 
responsive as it all happens in the UI thread.

> And I'm not convinced that doing heavy mouse event processing on the 
> render thread is any better than on the UI thread. Doing heavy 
> processing on mouse event at all sounds like a bad idea. I don't think 
> we should base the architecture on badly designed and rarely used filters.
> 
> With that said, sure, we need to define what the locking rule is for 
> mouse events. Taking the whole filter chain lock on the UI thread is not 
> ideal.
> 
> Le 21 août 2020 09:34:10 GMT+02:00, Steve Lhomme <robux4 at ycbcr.xyz> a 
> écrit :
> 
>     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
> 
>     ------------------------------------------------------------------------
>     vlc-devel mailing list
>     To unsubscribe or modify your subscription options:
>     https://mailman.videolan.org/listinfo/vlc-devel
> 
> 
> -- 
> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser 
> ma brièveté.
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
> 


More information about the vlc-devel mailing list