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

[Alexandre Janniaux]

Hi,

On Mon, Nov 02, 2020 at 04:18:11PM +0100, Steve Lhomme wrote:
> This value is not going to change the life of the module so doesn't need any
> locking.

Unfortunately like mentioned in previous threads, this is not
«true» in that mouse callback can happen even after the vout
window is closed, because the object (~=sys of the module)
implementing the behaviour might still exist and be waiting to
be destroyed/removed in the main thread. Storing the vout_window_t
change nothing to that since there's no refcounting of its lifetime.

The proper solution is Marvin's new module. My suggestion is a simple
workaround splitting the necessary lifetime checker from the lock of
the global state, avoiding the interleaving of locks, but which cannot
work for Close() because you need to actually modify the state at this
point, leaving an opportunity for a deadlock. But it helps shedding
some light on the locking pattern which your patch seems to ignore.

I agree that it usually works like this because most display are written
in a synchronous manner with the vout thread, but apple ones and
especially windowing events are asynchronous and on top of that most code
interacting with that need to be executed in the main thread.

For iOS, I will submit the new refactor (much alike Marvin's work) in
a few days as I've solved the latest issues.

Regards,
--
Alexandre Janniaux
Videolabs


> ---
>  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