[vlc-devel] [PATCH 2/3] ios: uiview: add new vout_window module

Alexandre Janniaux ajanni at videolabs.io
Thu Nov 5 09:43:17 CET 2020


Hi,

Thank you for the thoroughful review.

Answers inline. ;)

On Thu, Nov 05, 2020 at 07:59:58AM +0100, Steve Lhomme wrote:
> Comments below.
>
> On 2020-11-04 17:54, Alexandre Janniaux wrote:
> > Refactor code from the ios vout display to create a dedicated UIView
> > which handle the touch events and window resizing.
> >
> > The resizing events are emitted after any children UIView layer is added
> > to the UIView vout window so that it doesn't block the main thread under
> > display lock while the display module is being created. Indeed, it would
> > result in a lock inversion and thus potential deadlocks.
> > ---
> >   modules/video_output/Makefile.am    |  15 +-
> >   modules/video_output/apple/uiview.m | 390 ++++++++++++++++++++++++++++
> >   modules/video_output/ios.m          |  66 ++---
> >   3 files changed, 421 insertions(+), 50 deletions(-)
> >   create mode 100644 modules/video_output/apple/uiview.m
> >
> > diff --git a/modules/video_output/Makefile.am b/modules/video_output/Makefile.am
> > index c42fdc474a..e6edf9327d 100644
> > --- a/modules/video_output/Makefile.am
> > +++ b/modules/video_output/Makefile.am
> > @@ -52,11 +52,22 @@ libvout_ios_plugin_la_CFLAGS = $(AM_CFLAGS) $(OPENGL_COMMONCFLAGS) -DUSE_OPENGL_
> >   libvout_ios_plugin_la_LIBADD = libvlc_opengles.la
> >   libvout_ios_plugin_la_LDFLAGS = $(AM_LDFLAGS) -rpath '$(voutdir)' \
> >   	-Wl,-framework,Foundation,-framework,OpenGLES,-framework,QuartzCore,-framework,UIKit
> > +
> > +libuiview_window_plugin_la_SOURCES = video_output/apple/uiview.m
> > +libuiview_window_plugin_la_LDFLAGS = $(AM_LDFLAGS) \
> > +	-Wl,-framework,Foundation,-framework,OpenGLES,-framework,QuartzCore,-framework,UIKit
> > +libuiview_window_plugin_la_OBJCFLAGS = $(AM_OBJCFLAGS) -fobjc-arc
> > +
> >   if HAVE_IOS
> > -vout_LTLIBRARIES += libvout_ios_plugin.la libglinterop_cvpx_plugin.la
> > +vout_LTLIBRARIES += libvout_ios_plugin.la \
> > +	libglinterop_cvpx_plugin.la \
> > +	libuiview_window_plugin.la
> >   endif
> >   if HAVE_TVOS
> > -vout_LTLIBRARIES += libvout_ios_plugin.la libglinterop_cvpx_plugin.la
> > +vout_LTLIBRARIES += \
> > +	libvout_ios_plugin.la \
> > +	libglinterop_cvpx_plugin.la \
> > +	libuiview_window_plugin.la
> >   endif
> >   libglinterop_vaapi_plugin_la_SOURCES = video_output/opengl/interop_vaapi.c \
> > diff --git a/modules/video_output/apple/uiview.m b/modules/video_output/apple/uiview.m
> > new file mode 100644
> > index 0000000000..f99ad8bbea
> > --- /dev/null
> > +++ b/modules/video_output/apple/uiview.m
> > @@ -0,0 +1,390 @@
> > +/*****************************************************************************
> > + * uiview.m: iOS UIView vout window provider
> > + *****************************************************************************
> > + * Copyright (C) 2001-2017 VLC authors and VideoLAN
> > + * Copyright (C) 2020 Videolabs
> > + *
> > + * Authors: Pierre d'Herbemont <pdherbemont at videolan dot org>
> > + *          Felix Paul Kühne <fkuehne at videolan dot org>
> > + *          David Fuhrmann <david dot fuhrmann at googlemail dot com>
> > + *          Rémi Denis-Courmont
> > + *          Laurent Aimar <fenrir _AT_ videolan _DOT_ org>
> > + *          Eric Petit <titer at m0k.org>
> > + *          Alexandre Janniaux <ajanni at videolabs.io>
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU Lesser General Public License as published by
> > + * the Free Software Foundation; either version 2.1 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public License
> > + * along with this program; if not, write to the Free Software Foundation,
> > + * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
> > + *****************************************************************************/
> > +
> > +/**
> > + * @file uiview.m
> > + * @brief UIView implementation as a vout_window provider
> > + *
> > + * This UIView window provider mostly handle resizing constraints from upper
> > + * views and provides event forwarding to VLC. It is usable for any kind of
> > + * subview and in particular can be used to implement a CAEAGLLayer in a
> > + * vlc_gl_t provider as well as a CAMetalLayer, or other CALayer based video
> > + * output in general.
> > + *
> > + * In particular, UI event will be forwarded to the core without the display
> > + * lock thanks to this implementation, but vout display implementation will
> > + * need to let the event pass through to this UIView.
> > + *
> > + * Note that this module is asynchronous with the usual VLC execution flow:
> > + * except during Open(), where a status code is needed and synchronization
> > + * must be done with the main thread, everything is forwarded to and
> > + * asynchronously executed by the main thread. In particular, the closing
> > + * of this module must be done asynchronously to not require the main thread
> > + * to run, and the hosting application will need to drain the main thread
> > + * dispatch queue. For iOS, it basically means nothing more than running the
> > + * usual UIApplicationMain.
> > + */
> > +
> > +#import <UIKit/UIKit.h>
> > +#import <OpenGLES/EAGL.h>
> > +#import <OpenGLES/ES2/gl.h>
> > +#import <OpenGLES/ES2/glext.h>
> > +#import <QuartzCore/QuartzCore.h>
> > +#import <dlfcn.h>
> > +
> > +#ifdef HAVE_CONFIG_H
> > +# import "config.h"
> > +#endif
> > +
> > +#import <vlc_common.h>
> > +#import <vlc_plugin.h>
> > +#import <vlc_dialog.h>
> > +#import <vlc_mouse.h>
> > +#import <vlc_vout_window.h>
> > +
> > + at interface VLCVideoUIView : UIView {
> > +    /* VLC window object, set to NULL under _mutex lock when closing. */
> > +    vout_window_t *_wnd;
> > +    vlc_mutex_t _mutex;
> > +
> > +    /* Parent view defined by libvlc_media_player_set_nsobject. */
> > +    UIView *_viewContainer;
> > +
> > +    /* Window observer for mouse-like events. */
> > +    UITapGestureRecognizer *_tapRecognizer;
> > +
> > +    /* Window state */
> > +    BOOL _enabled;
> > +    int _subviews;
> > +}
> > +
> > ++ (void)getNewView:(struct vout_window_t*)wnd;
> > +- (id)initWithFrame:(CGRect)frame;
> > +- (BOOL)fetchViewContainer;
> > +- (void)detachFromParent;
> > +- (void)tapRecognized:(UITapGestureRecognizer *)tapRecognizer;
> > +- (void)enable;
> > +- (void)disable;
> > + at end
> > +
> > +/*****************************************************************************
> > + * Our UIView object
> > + *****************************************************************************/
> > + at implementation VLCVideoUIView
> > +
> > ++ (void)getNewView:(struct vout_window_t*)wnd
> > +{
> > +    VLCVideoUIView *uiview = [self alloc];
> > +    if (uiview == nil)
> > +        return;
> > +
> > +    uiview->_wnd = wnd;
> > +    uiview->_wnd->sys = (__bridge_retained void*)[uiview initWithFrame:CGRectMake(0.,0.,320.,240.)];
> > +}
> > +
> > +- (id)initWithFrame:(CGRect)frame
> > +{
> > +    _enabled = NO;
> > +    _subviews = 0;
> > +
> > +    self = [super initWithFrame:frame];
> > +    if (!self)
> > +        return nil;
> > +
> > +    vlc_mutex_init(&_mutex);
> > +
> > +    /* The window is controlled by the host application through the UIView
> > +     * sizing mechanisms. */
> > +    self.autoresizingMask = UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleHeight;
> > +
> > +    if (![self fetchViewContainer])
> > +        return nil;
> > +
> > +    /* add tap gesture recognizer for DVD menus and stuff */
> > +    _tapRecognizer = [[UITapGestureRecognizer alloc]
> > +        initWithTarget:self action:@selector(tapRecognized:)];
> > +
> > +    return self;
> > +}
> > +
> > +- (BOOL)fetchViewContainer
> > +{
> > +    @try {
> > +        /* get the object we will draw into */
> > +        UIView *viewContainer = (__bridge UIView*)var_InheritAddress (_wnd, "drawable-nsobject");
> > +        if (unlikely(viewContainer == nil)) {
> > +            msg_Err(_wnd, "provided view container is nil");
> > +            return NO;
> > +        }
> > +
> > +        if (unlikely(![viewContainer respondsToSelector:@selector(isKindOfClass:)])) {
> > +            msg_Err(_wnd, "void pointer not an ObjC object");
> > +            return NO;
> > +        }
> > +
> > +        if (![viewContainer isKindOfClass:[UIView class]]) {
> > +            msg_Err(_wnd, "passed ObjC object not of class UIView");
> > +            return NO;
> > +        }
> > +
> > +        /* We need to store the view container because we'll add our view
> > +         * only when a subview (hopefully able to handle the rendering) will
> > +         * get created. The main reason for this is that we cannot report
> > +         * events from the window until the display is opened, otherwise a
> > +         * race condition involving locking both the main thread and the lock
> > +         * in the core for the display are happening. */
> > +        _viewContainer = viewContainer;
> > +
> > +        self.frame = viewContainer.bounds;
> > +        [self reshape];
> > +
> > +        return YES;
> > +    } @catch (NSException *exception) {
> > +        msg_Err(_wnd, "Handling the view container failed due to an Obj-C exception (%s, %s", [exception.name UTF8String], [exception.reason UTF8String]);
> > +        return NO;
> > +    }
> > +}
> > +
> > +- (void)detachFromParent
> > +{
> > +    vlc_mutex_lock(&_mutex);
> > +    _wnd = NULL;
>
> Using a atomic_uintptr_t would save you a lock between the vout thread and
> the UI/main thread during the Close.

No, it won't work, same TOCTOU as explained by Rémi:

    vout_window_t *wnd = atomic_load(&atomic_wnd);
    // the real window object might be destroyed here
    do_something_with_wnd(wnd);
    // UB because wnd might have been freed.


> > +    vlc_mutex_unlock(&_mutex);
> > +}
> > +
> > +/**
> > + * We track whether we currently have a children view, which will be able
>
> Nitpicking: *child

Thanks! fixed locally!

> > + * to do the actual rendering. Depending on how many children view we had
> > + * and whether we are in enabled state or not, we add or remove the view
> > + * from/to the parent UIView.
> > + * This is needed because we cannot emit resize event from the main
> > + * thread as long as the display is not created, since the display will
> > + * need the main thread too. It would non-deterministically deadlock
> > + * otherwise.
> > + */
> >
> > +
> > +- (void)didAddSubview:(UIView*)subview
> > +{
> > +    _subviews++;
>
> Are we expected to have more than one child view at a time ? I don't think
> the core will ever add more than one display per window. So is the UI
> allowed to add other views to this view ? Or can we prevent other views from
> being added ?

If we write a CALayer display using CALayers for subtitles, maybe.

> > +    if (_enabled && _subviews == 1)
>
> I don't think checking the _enabled here is needed. I don't think the flag
> is useful in general. I think the core is responsible for doing balanced
> enable/disable calls of the window. (ie this is defensive programming)

Indeed, I don't remember exactly why I needed this at the first
place, but I'll check without it again. It might be an artifact
of the time the UIView was added to the parent view directly at
Enable() or when the core was disabling the window with the
display still opened.

> > +        [_viewContainer addSubview:self];
> > +
> > +    VLC_UNUSED(subview);
> > +}
> > +
> > +- (void)willRemoveSubview:(UIView*)subview
> > +{
> > +    _subviews--;
> > +    if (_enabled && _subviews == 0)
> > +        [self removeFromSuperview];
> > +
> > +    VLC_UNUSED(subview);
> > +}
> > +
> > +/**
> > + * Vout window operations implemention, which are expected to be run on
> > + * the main thread only. Core C wrappers below must typically use
> > + * dispatch_async with dispatch_get_main_queue() to call them.
> > + *
> > + * The addition of the UIView to the parent UIView might happen later
> > + * if there's no subview attached yet.
> > + */
> > +
> > +- (void)enable
> > +{
> > +    if (_enabled)
> > +        return;
> > +
> > +    if (_subviews > 0)
> > +        [_viewContainer addSubview:self];
>
> This looks exactly like the call in didAddSubview. Do we add the same view
> twice to the _viewContainer ? It sounds like the didAddSubview (and
> willRemoveSubview) is not needed at all.

As it is done, the real addSubview can be done later. Like you mentioned,
it's quite some defensive programming that has been done to handle the
disable case and previous behaviours. I'll check with a cleaner version.

> > +    _enabled = YES;
> > +
> > +    /* Bind tapRecognizer. */
> > +    [self addGestureRecognizer:_tapRecognizer];
> > +    _tapRecognizer.cancelsTouchesInView = NO;
> > +}
> > +
> > +- (void)disable
> > +{
> > +    if (!_enabled)
> > +        return;
> > +
> > +    _enabled = NO;
> > +    if (_subviews > 0)
> > +        [self removeFromSuperview];
> > +
> > +    [[NSNotificationCenter defaultCenter] removeObserver:self];
> > +    [_tapRecognizer.view removeGestureRecognizer:_tapRecognizer];
> > +}
> > +
> > +/**
> > + * Window state tracking and reporting
> > + */
> > +
> > +- (void)didMoveToWindow
> > +{
> > +    self.contentScaleFactor = self.window.screen.scale;
> > +}
> > +
> > +- (void)layoutSubviews
> > +{
> > +    [self reshape];
> > +}
> > +
> > +- (void)reshape
> > +{
> > +    assert([NSThread isMainThread]);
> > +
> > +    vlc_mutex_lock(&_mutex);
> > +    if (_wnd == NULL)
> > +        goto end;
> > +
> > +    CGSize viewSize = [self bounds].size;
>
> This can probably be done outside of the mutex.

Right.

> > +    CGFloat scaleFactor = self.contentScaleFactor;
>
> contentScaleFactor seems only used in the UI/main thread so doesn't need the
> mutex either.

Right. The lock is for the call below (because of the TOCTOU mentioned
above).

> > +
> > +    vout_window_ReportSize(_wnd,
> > +            viewSize.width * scaleFactor,
> > +            viewSize.height * scaleFactor);
> > +end:
> > +    vlc_mutex_unlock(&_mutex);
> > +}
> > +
> > +- (void)tapRecognized:(UITapGestureRecognizer *)tapRecognizer
> > +{
> > +    vlc_mutex_lock(&_mutex);
> > +
> > +    if (_wnd == NULL)
> > +        goto end;
> > +
> > +    UIGestureRecognizerState state = [tapRecognizer state];
> > +    CGPoint touchPoint = [tapRecognizer locationInView:self];
> > +    CGFloat scaleFactor = self.contentScaleFactor;
>
> These lines can also be outside of the mutex. They are always used in the
> UI/main thread.

Right. The lock is for the calls below too.

> > +
> > +    vout_window_ReportMouseMoved(_wnd,
> > +            (int)touchPoint.x * scaleFactor, (int)touchPoint.y * scaleFactor);
> > +    vout_window_ReportMousePressed(_wnd, MOUSE_BUTTON_LEFT);
> > +    vout_window_ReportMouseReleased(_wnd, MOUSE_BUTTON_LEFT);
> > +end:
> > +    vlc_mutex_unlock(&_mutex);
> > +}
> > +
> > +- (void)updateConstraints
> > +{
> > +    [super updateConstraints];
> > +    [self reshape];
> > +}
> > +
> > +/* Subview are expected to fill the whole frame so tell the compositor
> > + * that it doesn't have to bother with what's behind the window. */
> > +- (BOOL)isOpaque { return YES; }
> > +
> > +/* Prevent the subviews (which are renderers only) to get events so that
> > + * they can be dispatched from this vout_window module. */
> > +- (BOOL)acceptsFirstResponder { return YES; }
> > + at end
> > +
> > +/**
> > + * C core wrapper of the vout window operations for the ObjC module.
> > + */
> > +
> > +static int Enable(vout_window_t *wnd, const vout_window_cfg_t *cfg)
> > +{
> > +    VLCVideoUIView *sys = (__bridge VLCVideoUIView *)wnd->sys;
> > +    dispatch_async(dispatch_get_main_queue(), ^{
> > +        [sys enable];
> > +    });
> > +    return VLC_SUCCESS;
> > +}
> > +
> > +static void Disable(vout_window_t *wnd)
> > +{
> > +    VLCVideoUIView *sys = (__bridge VLCVideoUIView *)wnd->sys;
> > +    dispatch_async(dispatch_get_main_queue(), ^{
> > +        [sys disable];
> > +    });
> > +}
> > +
> > +static void Close(vout_window_t *wnd)
> > +{
> > +    VLCVideoUIView *sys = (__bridge_transfer VLCVideoUIView*)wnd->sys;
> > +
> > +    /* The UIView must not be attached before releasing */
> > +    Disable(wnd);
>
> This should be the job of the core. It must not close a window that it
> didn't disable first.

It didn't seem to happen, and since the UIView would still be
referenced by the superview it was leading to memory leaks.
I'll try again on master.

> > +
> > +    /* We need to signal the asynchronous implementation that we have been
> > +     * closed and cannot used _wnd anymore. */
> > +    [sys detachFromParent];
> > +
> > +    sys = nil;
>
> What is the effect of this line ? This is a local variable in C-looking
> code.
>

After ad-hoc testing, it seems arc doesn't need it to release
correctly, I'll remove it. But mostly originally because that's
not C code. ;)

> > +}
> > +
> > +static const struct vout_window_operations window_ops =
> > +{
> > +    .enable = Enable,
> > +    .disable = Disable,
> > +    .destroy = Close,
> > +};
> > +
> > +static int Open(vout_window_t *wnd)
> > +{
> > +    wnd->sys = NULL;
> > +
> > +    var_Create(vlc_object_parent(wnd), "ios-eaglcontext", VLC_VAR_ADDRESS);
>
> Do we really need to create a variable in the parent ?
> If some pointer needs to be shared by the window with the rest of the
> decoder/render/filter it sounds like what should be put in a decoder device.

Actually, that's an artifact from the display, it should not
be used like and it's mostly sanitized to the future opengl
implementation in latter patches, but will be removed as soon
as ci_filters don't need it anymore.

It doesn't make sense in decoder device today, because it's
currently used to choose the decoder used and the rest of the
chain but you could have --dec-dev=none (avcodec) and ci_filters
without issues. Like I mentioned in previous different threads
we need to clearly redefine the decoder device (is it a decoder
choice/hint module or more a video device?) before going this
way.

In addition, this context is created by the display, not by
the window, and you could have some Metal context instead,
so it cannot happen. We have other ways of interaction that
don't need this hack, but I'm not porting the whole iOS video
output in this patchset (otherwise, I would have sent the rest).

Thank you for catching this!

> > +
> > +    dispatch_sync(dispatch_get_main_queue(), ^{
> > +        [VLCVideoUIView getNewView:wnd];
> > +    });
> > +
> > +    if (wnd->sys == NULL)
> > +    {
> > +        msg_Err(wnd, "Creating OpenGL ES 2 view failed");
> > +        var_Destroy(vlc_object_parent(wnd), "ios-eaglcontext");
>
> This is not destroyed in Close.

I'll remove it.

> > +        goto error;
> > +    }
> > +
> > +    wnd->type = VOUT_WINDOW_TYPE_NSOBJECT;
> > +    wnd->handle.nsobject = wnd->sys;
> > +    wnd->ops = &window_ops;
> > +
> > +    return VLC_SUCCESS;
> > +
> > +error:
> > +    return VLC_EGENERIC;
> > +}
> > +
> > +vlc_module_begin ()
> > +    set_shortname("UIView")
> > +    set_description("iOS UIView vout window provider")
> > +    set_category(CAT_VIDEO)
> > +    set_subcategory(SUBCAT_VIDEO_VOUT)
> > +    set_capability("vout window", 300)
> > +    set_callback(Open)
> > +
> > +    add_shortcut("uiview", "ios")
> > +vlc_module_end ()
> > diff --git a/modules/video_output/ios.m b/modules/video_output/ios.m
> > index 07c8355d45..d00bbcd3a5 100644
> > --- a/modules/video_output/ios.m
> > +++ b/modules/video_output/ios.m
> > @@ -98,7 +98,6 @@ vlc_module_end ()
> >       BOOL _placeInvalidated;
> >       UIView *_viewContainer;
> > -    UITapGestureRecognizer *_tapRecognizer;
> >       /* Written from MT, read locked from vout */
> >       vout_display_place_t _place;
> > @@ -109,7 +108,7 @@ vlc_module_end ()
> >       vout_display_cfg_t _cfg;
> >   }
> > -- (id)initWithFrame:(CGRect)frame andVD:(vout_display_t*)vd;
> > +- (id)initWithFrame:(CGRect)frame andVD:(vout_display_t*)vd window:(vout_window_t*)wnd;
>
> Usually the convention is to use readable text, it should be "andWindow:"
> rather than "window:".

I can rename into andWindow if you want.

> >   - (void)cleanAndRelease:(BOOL)flushed;
> >   - (BOOL)makeCurrent:(EAGLContext **)previousEaglContext withGL:(vlc_gl_t *)gl;
> >   - (void)releaseCurrent:(EAGLContext *)previousEaglContext;
> > @@ -158,7 +157,8 @@ static const struct vlc_display_operations ops = {
> >   static int Open(vout_display_t *vd, const vout_display_cfg_t *cfg,
> >                   video_format_t *fmt, vlc_video_context *context)
> >   {
> > -    if (vout_display_cfg_IsWindowed(cfg))
> > +    vout_window_t *wnd = cfg->window;
> > +    if (wnd->type != VOUT_WINDOW_TYPE_NSOBJECT)
> >           return VLC_EGENERIC;
> >       vout_display_sys_t *sys = vlc_obj_calloc(VLC_OBJECT(vd), 1, sizeof(*sys));
> > @@ -176,8 +176,8 @@ static int Open(vout_display_t *vd, const vout_display_cfg_t *cfg,
> >           [VLCOpenGLES2VideoView performSelectorOnMainThread:@selector(getNewView:)
> >                                                   withObject:[NSArray arrayWithObjects:
> > -                                                           [NSValue valueWithPointer:&sys->glESView],
> > -                                                           [NSValue valueWithPointer:vd], nil]
> > +                                                           [NSValue valueWithPointer:vd],
> > +                                                           [NSValue valueWithPointer:wnd], nil]
> >                                                waitUntilDone:YES];
> >           if (!sys->glESView) {
> >               msg_Err(vd, "Creating OpenGL ES 2 view failed");
> > @@ -353,12 +353,14 @@ static void GLESSwap(vlc_gl_t *gl)
> >   + (void)getNewView:(NSArray *)value
> >   {
> > -    id *ret = [[value objectAtIndex:0] pointerValue];
> > -    vout_display_t *vd = [[value objectAtIndex:1] pointerValue];
> > -    *ret = [[self alloc] initWithFrame:CGRectMake(0.,0.,320.,240.) andVD:vd];
> > +    vout_display_t *vd = [[value objectAtIndex:0] pointerValue];
> > +    vout_window_t *wnd = [[value objectAtIndex:1] pointerValue];
> > +
> > +    struct vout_display_sys_t *sys = vd->sys;
> > +    sys->glESView = [[self alloc] initWithFrame:CGRectMake(0.,0.,320.,240.) andVD:vd window:wnd];
> >   }
> > -- (id)initWithFrame:(CGRect)frame andVD:(vout_display_t*)vd
> > +- (id)initWithFrame:(CGRect)frame andVD:(vout_display_t*)vd window:(vout_window_t*)wnd
> >   {
> >       _appActive = ([UIApplication sharedApplication].applicationState == UIApplicationStateActive);
> >       if (unlikely(!_appActive))
> > @@ -403,7 +405,7 @@ static void GLESSwap(vlc_gl_t *gl)
> >       self.autoresizingMask = UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleHeight;
> > -    if (![self fetchViewContainer])
> > +    if (![self bindToWindow: wnd])
> >       {
> >           [_eaglContext release];
> >           [self release];
> > @@ -431,11 +433,11 @@ static void GLESSwap(vlc_gl_t *gl)
> >       return self;
> >   }
> > -- (BOOL)fetchViewContainer
> > +- (BOOL)bindToWindow:(vout_window_t*)wnd
> >   {
> >       @try {
> > +        UIView *viewContainer = wnd->handle.nsobject;
> >           /* get the object we will draw into */
> > -        UIView *viewContainer = var_InheritAddress (_voutDisplay, "drawable-nsobject");
>
> Nitpicking, the modified line should remain in the same place.

I should probably change the comment instead.

> >           if (unlikely(viewContainer == nil)) {
> >               msg_Err(_voutDisplay, "provided view container is nil");
> >               return NO;
> > @@ -462,20 +464,10 @@ static void GLESSwap(vlc_gl_t *gl)
> >           [_viewContainer addSubview:self];
> > -        /* add tap gesture recognizer for DVD menus and stuff */
> > -        _tapRecognizer = [[UITapGestureRecognizer alloc] initWithTarget:self
> > -                                                                 action:@selector(tapRecognized:)];
> > -        if (_viewContainer.window
> > -         && _viewContainer.window.rootViewController
> > -         && _viewContainer.window.rootViewController.view)
> > -            [_viewContainer.superview addGestureRecognizer:_tapRecognizer];
> > -        _tapRecognizer.cancelsTouchesInView = NO;
> >           return YES;
> >       } @catch (NSException *exception) {
> >           msg_Err(_voutDisplay, "Handling the view container failed due to an Obj-C exception (%s, %s", [exception.name UTF8String], [exception.reason UTF8String]);
> >           vout_display_sys_t *sys = _voutDisplay->sys;
> > -        if (_tapRecognizer)
> > -            [_tapRecognizer release];
> >           return NO;
> >       }
> >   }
> > @@ -484,9 +476,6 @@ static void GLESSwap(vlc_gl_t *gl)
> >   {
> >       [[NSNotificationCenter defaultCenter] removeObserver:self];
> > -    [_tapRecognizer.view removeGestureRecognizer:_tapRecognizer];
> > -    [_tapRecognizer release];
> > -
> >       [self removeFromSuperview];
> >       [_viewContainer release];
> > @@ -677,27 +666,6 @@ static void GLESSwap(vlc_gl_t *gl)
> >       vlc_mutex_unlock(&_mutex);
> >   }
> > -- (void)tapRecognized:(UITapGestureRecognizer *)tapRecognizer
> > -{
> > -    vlc_mutex_lock(&_mutex);
> > -    if (!_voutDisplay)
> > -    {
> > -        vlc_mutex_unlock(&_mutex);
> > -        return;
> > -    }
> > -
> > -    UIGestureRecognizerState state = [tapRecognizer state];
> > -    CGPoint touchPoint = [tapRecognizer locationInView:self];
> > -    CGFloat scaleFactor = self.contentScaleFactor;
> > -    vout_display_SendMouseMovedDisplayCoordinates(_voutDisplay,
> > -                                                  (int)touchPoint.x * scaleFactor, (int)touchPoint.y * scaleFactor);
> > -
> > -    vout_display_SendEventMousePressed(_voutDisplay, MOUSE_BUTTON_LEFT);
> > -    vout_display_SendEventMouseReleased(_voutDisplay, MOUSE_BUTTON_LEFT);
> > -
> > -    vlc_mutex_unlock(&_mutex);
> > -}
> > -
> >   - (void)updateVoutCfg:(const vout_display_cfg_t *)cfg withVGL:(vout_display_opengl_t *)vgl
> >   {
> >       if (memcmp(&_cfg, cfg, sizeof(vout_display_cfg_t)) == 0)
> > @@ -775,9 +743,11 @@ static void GLESSwap(vlc_gl_t *gl)
> >       return YES;
> >   }
> > -- (BOOL)acceptsFirstResponder
> > +- (UIView *)hitTest:(CGPoint)point withEvent:(UIEvent *)event
> >   {
> > -    return YES;
> > +    /* Disable events for this view, as the vout_window view will be the one
> > +     * handling them. */
> > +    return nil;
> >   }
> >   @end
> > --
> > 2.29.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


More information about the vlc-devel mailing list