[vlc-devel] [PATCH 02/17] video_output: translate the mouse coordinates to display coordinates early

Alexandre Janniaux ajanni at videolabs.io
Mon Nov 2 11:15:10 CET 2020


Hi,

On Mon, Nov 02, 2020 at 10:35:09AM +0100, Steve Lhomme wrote:
> It seems the issue is that code in macosx.m
>
>     @synchronized(self) {
>         _hasPendingReshape = NO;
>     }
>
> It might use an atomic bool for this variable. Using less locks is always a
> good idea when possible. I don't know if/how Objective-C can handle atomics.
>
> On the other hand the original patch is not mandatory and doesn't improve
> anything much. So if necessary I'm OK with reverting it.

I read the issue as «you cannot get the display lock for mouse update, since
display can currently report the mouse state, thus already under display lock».

Is that different from reality?

Using the work from Marvin, we'll be able to remove the mouse API from display
on MacOSX, so finally solve this issue.

Regards,
--
Alexandre Janniaux
Videolabs

> On 2020-10-31 17:49, Zhao Zhili wrote:
> > Hi Steve,
> >
> > > On Sep 14, 2020, at 10:03 PM, Steve Lhomme <robux4 at ycbcr.xyz> wrote:
> > >
> > > We can do this in the UI thread, the processing is light.
> > > Locking the display_lock doesn't have a big impact on lag as it's only taking
> > > time when reloading a display module, which is very rare.
> > > ---
> > > src/video_output/video_output.c | 21 ++++++++++++---------
> > > 1 file changed, 12 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/src/video_output/video_output.c b/src/video_output/video_output.c
> > > index cbc66b9dbc0..ed1e3bfcb07 100644
> > > --- a/src/video_output/video_output.c
> > > +++ b/src/video_output/video_output.c
> > > @@ -389,7 +389,14 @@ void vout_MouseState(vout_thread_t *vout, const vlc_mouse_t *mouse)
> > >      assert(mouse);
> > >      vout_control_cmd_t cmd;
> > >      vout_control_cmd_Init(&cmd, VOUT_CONTROL_MOUSE_STATE);
> > > -    cmd.mouse = *mouse;
> > > +
> > > +    /* Translate window coordinates to video coordinates */
> > > +    vlc_mutex_lock(&sys->display_lock);
> >
> > This change leads to a deadlock on macOS.
> >
> > * thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGSTOP
> >    * frame #0: 0x00007fff6dcdd882 libsystem_kernel.dylib`__psynch_cvwait + 10
> >      frame #1: 0x00007fff6dd9e425 libsystem_pthread.dylib`_pthread_cond_wait + 698
> >      frame #2: 0x000000010023d27c libvlccore.dylib`vlc_atomic_wait(addr=0x000000010ef31ff0, value=2) at wait.c:84:9
> >      frame #3: 0x0000000100226d4e libvlccore.dylib`vlc_mutex_lock(mtx=0x000000010ef31ff0) at threads.c:175:9
> >      frame #4: 0x00000001001eb968 libvlccore.dylib`vout_MouseState(vout=0x000000010ef31d50, mouse=0x000000010ed8e4a8) at video_output.c:389:5
> >      frame #5: 0x00000001002005f4 libvlccore.dylib`vout_display_window_MouseEvent(window=0x000000010ed8e370, ev=0x00007ffeefbfdd80) at window.c:321:5
> >      frame #6: 0x0000000107fc11b9 libvout_macosx_plugin.dylib`vout_window_SendMouseEvent(window=0x000000010ed8e370, mouse=0x00007ffeefbfdd80) at vlc_vout_window.h:595:9
> >      frame #7: 0x0000000107fc1239 libvout_macosx_plugin.dylib`vout_window_ReportMouseMoved(window=0x000000010ed8e370, x=2038, y=943) at vlc_vout_window.h:614:5
> >      frame #8: 0x0000000107fc05b8 libvout_macosx_plugin.dylib`vout_display_SendMouseMovedDisplayCoordinates(vd=0x0000000110911340, m_x=2038, m_y=943) at vlc_vout_display.h:467:5
> >      frame #9: 0x0000000107fc0507 libvout_macosx_plugin.dylib`-[VLCOpenGLVideoView mouseMoved:](self=0x0000000110911da0, _cmd="mouseMoved:", o_event=0x0000000100676520) at macosx.m:778:17
> >
> >
> >    thread #42
> >      frame #0: 0x00007fff6dcdc55e libsystem_kernel.dylib`__ulock_wait + 10
> >      frame #1: 0x00007fff6dd909c3 libsystem_platform.dylib`_os_unfair_lock_lock_slow + 160
> >      frame #2: 0x00007fff6c9e8dc7 libobjc.A.dylib`objc_sync_enter + 27
> >      frame #3: 0x0000000107fbf4e4 libvout_macosx_plugin.dylib`-[VLCOpenGLVideoView setVoutFlushing:](self=0x0000000110911da0, _cmd="setVoutFlushing:", flushing=YES) at macosx.m:550:5
> >      frame #4: 0x0000000107fc0cf0 libvout_macosx_plugin.dylib`PictureDisplay(vd=0x0000000110911340, pic=0x000000010ae1d640) at macosx.m:331:5
> >      frame #5: 0x00000001001f2b09 libvlccore.dylib`vout_display_Display(vd=0x0000000110911340, picture=0x000000010ae1d640) at vlc_vout_display.h:431:9
> >      frame #6: 0x00000001001f2127 libvlccore.dylib`ThreadDisplayRenderPicture(vout=0x000000010ef31d50, render_now=false) at video_output.c:1446:5
> >      frame #7: 0x00000001001edf69 libvlccore.dylib`ThreadDisplayPicture(vout=0x000000010ef31d50, deadline=0x00007000061d6fa0) at video_output.c:1572:15
> >      frame #8: 0x00000001001f0409 libvlccore.dylib`Thread(object=0x000000010ef31d50) at video_output.c:1914:16
> >      frame #9: 0x00007fff6dd9e109 libsystem_pthread.dylib`_pthread_start + 148
> >      frame #10: 0x00007fff6dd99b8b libsystem_pthread.dylib`thread_start + 15
> >
> >
> >
> > > +    if (sys->display)
> > > +        vout_display_TranslateMouseState(sys->display, &cmd.mouse, mouse);
> > > +    else
> > > +        cmd.mouse = *mouse;
> > > +    vlc_mutex_unlock(&sys->display_lock);
> > >
> > >      vout_control_Push(&sys->control, &cmd);
> > > }
> > > @@ -1678,17 +1685,13 @@ void vout_ChangeSpuRate(vout_thread_t *vout, size_t channel_id, float rate)
> > > static void ThreadProcessMouseState(vout_thread_sys_t *p_vout,
> > >                                      const vlc_mouse_t *win_mouse)
> > > {
> > > -    vlc_mouse_t vid_mouse, tmp1, tmp2, *m;
> > > +    vlc_mouse_t tmp1, tmp2;
> > > +    const vlc_mouse_t *m;
> > >      vout_thread_t *vout = &p_vout->obj;
> > >      vout_thread_sys_t *sys = p_vout;
> > >
> > > -    /* Translate window coordinates to video coordinates */
> > > -    vlc_mutex_lock(&sys->display_lock);
> > > -    vout_display_TranslateMouseState(sys->display, &vid_mouse, win_mouse);
> > > -    vlc_mutex_unlock(&sys->display_lock);
> > > -
> > > -    /* Then pass up the filter chains. */
> > > -    m = &vid_mouse;
> > > +    /* pass mouse coordinates in the filter chains. */
> > > +    m = win_mouse;
> > >      vlc_mutex_lock(&sys->filter.lock);
> > >      if (sys->filter.chain_static && sys->filter.chain_interactive) {
> > >          if (!filter_chain_MouseFilter(sys->filter.chain_interactive,
> > > --
> > > 2.26.2
> > >
> > > _______________________________________________
> > > 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
> >
> _______________________________________________
> 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