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

[Alexandre Janniaux]

Hi,

On Mon, Nov 02, 2020 at 02:11:39PM +0200, Rémi Denis-Courmont wrote:
> Le maanantaina 2. marraskuuta 2020, 13.46.58 EET zhilizhao(赵志立) a écrit :
> > > On Nov 2, 2020, at 6:15 PM, Alexandre Janniaux <ajanni at videolabs.io>
> > > wrote:
> > >
> > > 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.
> >
> > So it’s OK to just revert? The master branch is totally unusable now.
>
> I don't think so. It seems that there's a lock inversion in the MacOS video
> display plugin that just happens to become (more) visible with the patch.

Somehow,

    display lock -> display callback -> lock(self)
        vs
    main thread callback -> lock(self) -> vout_MouseState -> new display lock()

The lock inversion didn't exist because the last display lock didn't exist.

Regards,
--
Alexandre Janniaux
Videolabs