[vlc-devel] [PATCH v2 1/4] macosx: store the vout_window_t in the VLCOpenGLVideoView

[Alexandre Janniaux]

On Tue, Nov 03, 2020 at 10:38:27AM +0100, Steve Lhomme wrote:
> On 2020-11-03 10:12, Alexandre Janniaux wrote:
> > Hi,
> >
> > Again storing the window changes nothing to the problem,
> > you can get an instance of the vout_window from the vout
> > display and it can disappear just like vout_display can
> > disappear. Additionally, you cannot wait for an event
> > coming from the main thread in Close() because the main
> > thread can be stuck waiting for the vout to close,
> > without running the main loop.
>
> I kept this patch that was in the previous branch. But it's not needed to
> solve the issue mentioned by Zhao Zhili. The juicy part in the v2 patchset
> is
> [PATCH v2 2/4] macosx: wait until the View is removed from its parent in
> Close
>
> If the UI thread is blocking indefinitely then we have another serious
> problem.  If it's momentary it's fine. We should wait until the core can do
> whatever it wants with the window.

I mean, this has been discussed extensively through Thomas's
issue with VLCKit, this is not new and this is how it works
on Apple system, I don't see any issue with it and Apple code
is especially designed for this with automatic refcounting.
Maybe you have a better solution for this but I honestly
doubt you can provide what UI expects in a simple way while
requesting those constraints to disappear.

The fact that display or window needs to be destroyed
asynchronously is far from being an issue and, here you
basically have two solutions: refcounting and enforced order
of deletion. The last one is the way you're suggesting it,
the first one is what is supposed to be implemented.

Basically, in a simpler way:

 - apple video modules live in main thread, so you need
   to do things in main thread.

 - since refcounting is automatically handled, there is
   no cost in killing the display object (ie. stopping the
   interaction between the module implementation and VLC)
   before killing the internal implementation, which again
   needs to happen in a different thread anyway.

 - the only case we need to wait for the main thread is in
   Open() where you actually need to return a value.

I hope Apple threading model is clearer.

Regards,
--
Alexandre Janniaux
Videolabs

> > Regards,
> > --
> > Alexandre Janniaux
> > Videolabs
> >
> > On Tue, Nov 03, 2020 at 08:46:44AM +0100, Steve Lhomme wrote:
> > > This value is not going to change the life of the module so doesn't need any
> > > locking.
> > > ---
> > >   modules/video_output/macosx.m | 8 ++++++++
> > >   1 file changed, 8 insertions(+)
> > >
> > > diff --git a/modules/video_output/macosx.m b/modules/video_output/macosx.m
> > > index 9ce48deaafc..bcb71579d68 100644
> > > --- a/modules/video_output/macosx.m
> > > +++ b/modules/video_output/macosx.m
> > > @@ -96,8 +96,10 @@ vlc_module_end ()
> > >   @interface VLCOpenGLVideoView : NSOpenGLView
> > >   {
> > >       vout_display_t *vd;
> > > +    vout_window_t *window;
> > >       BOOL _hasPendingReshape;
> > >   }
> > > +- (void)setVoutWindow:(vout_window_t *)aWindow;
> > >   - (void)setVoutDisplay:(vout_display_t *)vd;
> > >   - (void)setVoutFlushing:(BOOL)flushing;
> > >   @end
> > > @@ -189,6 +191,7 @@ static int Open (vout_display_t *vd, const vout_display_cfg_t *cfg,
> > >               goto error;
> > >           }
> > >
> > > +        [sys->glView setVoutWindow:cfg->window];
> > >           [sys->glView setVoutDisplay:vd];
> > >
> > >           /* We don't wait, that means that we'll have to be careful about releasing
> > > @@ -528,6 +531,11 @@ static void OpenglSwap (vlc_gl_t *gl)
> > >       [self setFrame:[parentView bounds]];
> > >   }
> > >
> > > +- (void)setVoutWindow:(vout_window_t *)aWindow
> > > +{
> > > +    window = aWindow;
> > > +}
> > > +
> > >   /**
> > >    * Gets called by the Close and Open methods.
> > >    * (Non main thread).
> > > --
> > > 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