<html><head></head><body>Hi,<br><br>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 component uses the video coordinates as window coordinates, it is buggy. Conversely, if some filters translate wrong or fail to translate, they're already buggy now and this makes no difference to this patchset.<br><br>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.<br><br>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.<br><br>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.<br><br><div class="gmail_quote">Le 21 août 2020 09:34:10 GMT+02:00, Steve Lhomme <robux4@ycbcr.xyz> a écrit :<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<pre class="k9mail">What we're talking about here is opening a can of worms. This is a whole <br>new behavior the code wasn't designed for. The whole mouse filtering <br>could do with some love, but this is not the scope of this patchset.<br><br>Example of worm we're talking about: the software deinterlace filters <br>mouse for when the picture is half height. None of the hardware filters <br>do that.<br>There is no consistency between filters. (BTW rotate doesn't handle <br>mouse events, and transform does)<br><br>I think, when it comes to UI, the responsiveness should matter the most. <br>That means do as little "heavy" processing in the UI thread. Locking <br>"sys->filter.loc"k in the UI thread would be quite heavy. That lock is <br>used to process the pixels. It can take quite a while.<br><br>In the end the mouse events are transformed according to the filter <br>dimension changes. But it's not to reflect something in the UI. It's to <br>reflect something in the video rendering. Either modifying the rendering <br>based on the mouse position or by sending "mouse-xxx" events. In the <br>former case, there is no use having that in the UI thread. In the latter <br>case, there are things like gestures/hotkeys which might benefit from <br>being in the UI thread. There is also the callback set by some <br>access/demuxer modules via ES_OUT_VOUT_SET_MOUSE_EVENT. It sounds like <br>it could benefit from being in the UI thread. In practice when the user <br>clicks it doesn't necessarily match the position the module think it's <br>at, because of the buffering in between. Doing it one frame later is <br>probably not a big deal.<br><br>So the real benefit would be for gestures/hotkeys.<br><br>The gestures only check for a movement in (all) directions, based on a <br>threshold. But if a deinterlace shows the video at double/half size, <br>that means the user actually has to moved twice more/less to reach that <br>threshold, when in fact the threshold is based on the visible area, not <br>the original picture location. So this is in fact wrong to handle <br>filtered mouse values here. (it would be worse in puzzle if you happen <br>to cross a square and the next virtual position is totally different <br>from where the mouse is actually on the screen)<br><br>The hotkeys module has 2 things. Some "intf-xxx" toggles, which can <br>popup a menu over the video. This should definitely not be filtered <br>according to the coordinates in the unfiltered picture but should match <br>the screen coordinates. It also calls vlc_player_UpdateViewpoint() with <br>the new mouse (filtered) coordinates, to change the viewpoint. Here I <br>don't think it makes sense to translate to filtered coordinates. For <br>example it the video is zoomed 2x a mouse movement of 10 pixels should <br>result in a movement of 10 pixels in the rendered video, not 5 pixels in <br>the source video. In the end the viewpoint value is only applied to <br>video and audio outputs, not the source material.<br><br>So it seems the "mouse-xxx" calls should not be filtered. They should be <br>called before the filtering is applied. That means they could be called <br>when the mouse event in received, in the UI thread. That would make the <br>gesture/hotkey more responsive. (the double click fullscreen toggle <br>should fall in that category too)<br><br>The filtered coordinates end up being used only for the few modules that <br>actually need to know exactly where in the source video the user <br>clicked/moved (using ES_OUT_VOUT_SET_MOUSE_EVENT). As explained above, I <br>think it should remain in the rendering thread for now.<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">On 20 Aug 2020, at 21:46, Rémi Denis-Courmont <remi@remlab.net> wrote:<br><br>Le jeudi 20 août 2020, 21:29:48 EEST Steve Lhomme a écrit :<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ad7fa8; padding-left: 1ex;"><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;">On 2020-08-20 18:11, Rémi Denis-Courmont wrote:<br>Hi,<br><br>Badly designed or implemented plugins are what they are. Just as big<br>loops in the UI thread are bad for interactivity, big loops in the<br>render thread are bad for timeliness. And besides those egregiously<br>broken plugins you have, e.g., rotate and transform who actually<br>translate the mouse event, and should obviously run synchronously - not<br>at the next render.<br></blockquote>Synchronously ? I have seen nothing in the UI thread, the one that will<br>display the rendered result, that says a frame is allowed to be<br>displayed ASAP. It will wait until the next loop, in that thread.<br>Putting that in the UI thread is just pretending the rendering will be<br>more responsive.<br></blockquote>Pretending? How do you need the rendering to translate mouse coordinates <br>through the transform filter? and eventually to pass mouse events to the input?<br><br>Even plotting the cursor does not need the VLC rendering. Only thing that does <br>is updating the Puzzle and such, not exactly everyday use cases.<br><br>-- <br>Rémi Denis-Courmont<hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></blockquote><hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a></pre></blockquote></div><br>-- <br>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.</body></html>