[vlc-devel] [PATCH 4/5] caopengllayer: call vout_window_ReportMouseXXX directly

[Alexandre Janniaux]

Hi,

On Tue, Nov 03, 2020 at 09:50:29AM +0100, Steve Lhomme wrote:
> Simply removing the @synchronized in this patch should be sufficient. The
> _voutDisplay value never changes and is set before the object is used by the
> rest of the code.

The vd is expected to be set to nil to tackle the usual race
condition with main thread. You cannot just remove the lock
to make a deadlock disappear. As I've mentioned initially,
you can reduce the scope of the lock to be able to ensure
that it's not taken by display callbacks, but you'll still
have the issue in Close that you cannot workaround, or you
can move the state lock out of the display lock, meaning into
a window module (Marvin's and mine typically), to achieve the
locking in the correct order.

> If calling the core calls back the module then the thread protection should
> happen there. There are a few places left protecting the OpenGL object. From
> the core it's only in prepare/display callbacks.
>
> On 2020-11-02 16:18, Steve Lhomme wrote:
> > No need to use the old vout_display_SendMouseXXX calls or lock the
> > VLCCAOpenGLLayer.
> >
> > It's probably possible to set the value in the initializer so it's always set.
> > ---
> >   modules/video_output/caopengllayer.m | 47 ++++++++++++----------------
> >   1 file changed, 20 insertions(+), 27 deletions(-)
> >
> > diff --git a/modules/video_output/caopengllayer.m b/modules/video_output/caopengllayer.m
> > index 587817d1530..ef424047410 100644
> > --- a/modules/video_output/caopengllayer.m
> > +++ b/modules/video_output/caopengllayer.m
> > @@ -82,6 +82,7 @@ static void OpenglSwap         (vlc_gl_t *gl);
> >   @interface VLCCAOpenGLLayer : CAOpenGLLayer
> >   @property (nonatomic, readwrite) vout_display_t* voutDisplay;
> > + at property (nonatomic, readwrite) vout_window_t*  window;
> >   @property (nonatomic, readwrite) CGLContextObj glContext;
> >   @end
> > @@ -155,6 +156,7 @@ static int Open (vout_display_t *vd, const vout_display_cfg_t *cfg,
> >           [CATransaction begin];
> >           sys->cgLayer = [[VLCCAOpenGLLayer alloc] init];
> > +        [sys->cgLayer setVoutWindow:cfg->window];
> >           [sys->cgLayer setVoutDisplay:vd];
> >           [sys->cgLayer performSelectorOnMainThread:@selector(display)
> > @@ -424,6 +426,11 @@ static void *OurGetProcAddress (vlc_gl_t *gl, const char *name)
> >       return self;
> >   }
> > +- (void)setVoutWindow:(vout_window_t *)aWindow
> > +{
> > +    _window = aWindow;
> > +}
> > +
> >   - (void)setVoutDisplay:(vout_display_t *)aVd
> >   {
> >       _voutDisplay = aVd;
> > @@ -510,41 +517,27 @@ static void *OurGetProcAddress (vlc_gl_t *gl, const char *name)
> >   - (void)mouseButtonDown:(int)buttonNumber
> >   {
> > -    @synchronized (self) {
> > -        if (_voutDisplay) {
> > -            if (buttonNumber == 0)
> > -                vout_display_SendEventMousePressed (_voutDisplay, MOUSE_BUTTON_LEFT);
> > -            else if (buttonNumber == 1)
> > -                vout_display_SendEventMousePressed (_voutDisplay, MOUSE_BUTTON_RIGHT);
> > -            else
> > -                vout_display_SendEventMousePressed (_voutDisplay, MOUSE_BUTTON_CENTER);
> > -        }
> > -    }
> > +    if (buttonNumber == 0)
> > +        vout_window_ReportMousePressed (_window, MOUSE_BUTTON_LEFT);
> > +    else if (buttonNumber == 1)
> > +        vout_window_ReportMousePressed (_window, MOUSE_BUTTON_RIGHT);
> > +    else
> > +        vout_window_ReportMousePressed (_window, MOUSE_BUTTON_CENTER);
> >   }
> >   - (void)mouseButtonUp:(int)buttonNumber
> >   {
> > -    @synchronized (self) {
> > -        if (_voutDisplay) {
> > -            if (buttonNumber == 0)
> > -                vout_display_SendEventMouseReleased (_voutDisplay, MOUSE_BUTTON_LEFT);
> > -            else if (buttonNumber == 1)
> > -                vout_display_SendEventMouseReleased (_voutDisplay, MOUSE_BUTTON_RIGHT);
> > -            else
> > -                vout_display_SendEventMouseReleased (_voutDisplay, MOUSE_BUTTON_CENTER);
> > -        }
> > -    }
> > +    if (buttonNumber == 0)
> > +        vout_window_ReportMouseReleased (_window, MOUSE_BUTTON_LEFT);
> > +    else if (buttonNumber == 1)
> > +        vout_window_ReportMouseReleased (_window, MOUSE_BUTTON_RIGHT);
> > +    else
> > +        vout_window_ReportMouseReleased (_window, MOUSE_BUTTON_CENTER);
> >   }
> >   - (void)mouseMovedToX:(double)xValue Y:(double)yValue
> >   {
> > -    @synchronized (self) {
> > -        if (_voutDisplay) {
> > -            vout_display_SendMouseMovedDisplayCoordinates (_voutDisplay,
> > -                                                           xValue,
> > -                                                           yValue);
> > -        }
> > -    }
> > +    vout_window_ReportMouseMoved (_window, xValue, yValue);
> >   }
> >   @end
> > --
> > 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