[vlc-devel] [PATCH] vout: ios: fix gpu_ReturnNotPermittedKillClient crashes
Thomas Guillem
thomas at gllm.fr
Mon Mar 5 19:34:27 CET 2018
Oh, this is wrong.
I need to fix all ios vout callbacks to not wait the main thread in order to avoid deadlocks.
On Mon, Mar 5, 2018, at 17:31, Thomas Guillem wrote:
> cf. https://developer.apple.com/library/content/qa/qa1766/_index.html
>
> No OpenGLES call should be made (even [eaglContext release]) once the
> application entered background.
>
> To fix this issue, this commit disable any rendering (_appActive = NO;) and
> release the EAGLContext from the applicationDidEnterBackground notification.
>
> This commits adds a lock and a condition in order to let the main thread know
> when the VLC vout thread unlocked the EAGLContext (because we don't want to
> release this context while it's being used by the vout thread).
>
> This commit also fixes the previousEaglContext variable being overridden when
> the glESView is locked from the main thread and from the vout thread. This
> variable should only by read/write from a same thread.
> ---
> modules/video_output/ios.m | 138 +++++++++++++++++++++++++++++++++------------
> 1 file changed, 103 insertions(+), 35 deletions(-)
>
> diff --git a/modules/video_output/ios.m b/modules/video_output/ios.m
> index f405e36c3c..c128d6d475 100644
> --- a/modules/video_output/ios.m
> +++ b/modules/video_output/ios.m
> @@ -88,10 +88,13 @@ vlc_module_end ()
> @interface VLCOpenGLES2VideoView : UIView {
> vout_display_t *_voutDisplay;
> EAGLContext *_eaglContext;
> - EAGLContext *_previousEaglContext;
> GLuint _renderBuffer;
> GLuint _frameBuffer;
>
> + vlc_mutex_t lock_mutex;
> + vlc_cond_t lock_wait;
> + unsigned lock_count;
> +
> BOOL _bufferNeedReset;
> BOOL _appActive;
> }
> @@ -99,14 +102,13 @@ vlc_module_end ()
> @property (readonly) GLuint frameBuffer;
> @property (readwrite) vout_display_t* voutDisplay;
> @property (readonly) EAGLContext* eaglContext;
> - at property (readonly) BOOL isAppActive;
> @property GLuint shaderProgram;
>
> - (void)createBuffers;
> - (void)destroyBuffers;
> - (void)resetBuffers;
> -- (void)lock;
> -- (void)unlock;
> +- (bool)lock:(EAGLContext **)previousEaglContext;
> +- (void)unlock:(EAGLContext *)previousEaglContext;
>
> - (void)reshape;
> - (void)propagateDimensionsToVoutCore;
> @@ -130,6 +132,7 @@ struct vout_display_sys_t
> struct gl_sys
> {
> VLCOpenGLES2VideoView *glESView;
> + EAGLContext *previousEaglContext;
> };
>
> static void *OurGetProcAddress(vlc_gl_t *gl, const char *name)
> @@ -229,6 +232,14 @@ static int Open(vlc_object_t *this)
>
> selector:@selector(applicationStateChanged:)
>
> name:UIApplicationDidBecomeActiveNotification
> object:nil];
> + [[NSNotificationCenter defaultCenter] addObserver:sys->glESView
> +
> selector:@selector(applicationStateChanged:)
> +
> name:UIApplicationDidEnterBackgroundNotification
> + object:nil];
> + [[NSNotificationCenter defaultCenter] addObserver:sys->glESView
> +
> selector:@selector(applicationStateChanged:)
> +
> name:UIApplicationWillEnterForegroundNotification
> + object:nil];
> [sys->glESView reshape];
> return VLC_SUCCESS;
>
> @@ -398,10 +409,8 @@ static int OpenglESLock(vlc_gl_t *gl)
> {
> struct gl_sys *sys = gl->sys;
>
> - if (unlikely(![sys->glESView isAppActive]))
> + if (![sys->glESView lock:&sys->previousEaglContext])
> return VLC_EGENERIC;
> -
> - [sys->glESView lock];
> [sys->glESView resetBuffers];
> return VLC_SUCCESS;
> }
> @@ -410,15 +419,14 @@ static void OpenglESUnlock(vlc_gl_t *gl)
> {
> struct gl_sys *sys = gl->sys;
>
> - [sys->glESView unlock];
> + [sys->glESView unlock:sys->previousEaglContext];
> }
>
> static void OpenglESSwap(vlc_gl_t *gl)
> {
> struct gl_sys *sys = gl->sys;
>
> - if (likely([sys->glESView isAppActive]))
> - [[sys->glESView eaglContext] presentRenderbuffer:GL_RENDERBUFFER];
> + [[sys->glESView eaglContext] presentRenderbuffer:GL_RENDERBUFFER];
> }
>
>
> @@ -426,7 +434,7 @@ static void OpenglESSwap(vlc_gl_t *gl)
> * Our UIView object
>
> *****************************************************************************/
> @implementation VLCOpenGLES2VideoView
> - at synthesize voutDisplay = _voutDisplay, eaglContext = _eaglContext,
> isAppActive = _appActive;
> + at synthesize voutDisplay = _voutDisplay;
>
> + (Class)layerClass
> {
> @@ -450,18 +458,25 @@ static void OpenglESSwap(vlc_gl_t *gl)
> if (unlikely(!_appActive))
> return nil;
>
> + vlc_mutex_init(&lock_mutex);
> + vlc_cond_init(&lock_wait);
> + lock_count = 1;
> +
> /* the following creates a new OpenGL ES context with the API version we
> * need if there is already an active context created by another OpenGL
> * provider we cache it and restore analog to the lock/unlock pattern used
> * through-out the class */
> - _previousEaglContext = [EAGLContext currentContext];
> + EAGLContext *previousEaglContext = [EAGLContext currentContext];
>
> _eaglContext = [[EAGLContext alloc]
> initWithAPI:kEAGLRenderingAPIOpenGLES2];
>
> - if (unlikely(!_eaglContext))
> - return nil;
> - if (unlikely(![EAGLContext setCurrentContext:_eaglContext]))
> + if (unlikely(!_eaglContext)
> + || unlikely(![EAGLContext setCurrentContext:_eaglContext]))
> + {
> + vlc_mutex_destroy(&lock_mutex);
> + vlc_cond_destroy(&lock_wait);
> return nil;
> + }
>
> CAEAGLLayer *layer = (CAEAGLLayer *)self.layer;
> layer.drawableProperties = [NSDictionary
> dictionaryWithObject:kEAGLColorFormatRGBA8 forKey:
> kEAGLDrawablePropertyColorFormat];
> @@ -469,7 +484,7 @@ static void OpenglESSwap(vlc_gl_t *gl)
>
> self.autoresizingMask = UIViewAutoresizingFlexibleWidth |
> UIViewAutoresizingFlexibleHeight;
>
> - [self unlock];
> + [self unlock:previousEaglContext];
>
> return self;
> }
> @@ -545,7 +560,13 @@ static void OpenglESSwap(vlc_gl_t *gl)
> - (void)dealloc
> {
> [[NSNotificationCenter defaultCenter] removeObserver:self];
> - [_eaglContext release];
> + if (_eaglContext)
> + {
> + assert(lock_count == 0);
> + [_eaglContext release];
> + }
> + vlc_mutex_destroy(&lock_mutex);
> + vlc_cond_destroy(&lock_wait);
> [super dealloc];
> }
>
> @@ -565,11 +586,9 @@ static void OpenglESSwap(vlc_gl_t *gl)
> return;
> }
>
> - if (unlikely(!_appActive)) {
> + EAGLContext *previousEaglContext;
> + if (![self lock:&previousEaglContext])
> return;
> - }
> -
> - [self lock];
>
> glDisable(GL_DEPTH_TEST);
>
> @@ -587,7 +606,7 @@ static void OpenglESSwap(vlc_gl_t *gl)
> msg_Err(_voutDisplay, "Failed to make complete framebuffer
> object %x", glCheckFramebufferStatus(GL_FRAMEBUFFER));
> }
>
> - [self unlock];
> + [self unlock:previousEaglContext];
> }
>
> - (void)destroyBuffers
> @@ -600,7 +619,9 @@ static void OpenglESSwap(vlc_gl_t *gl)
> return;
> }
>
> - [self lock];
> + EAGLContext *previousEaglContext;
> + if (![self lock:&previousEaglContext])
> + return;
>
> /* clear frame buffer */
> glDeleteFramebuffers(1, &_frameBuffer);
> @@ -610,7 +631,7 @@ static void OpenglESSwap(vlc_gl_t *gl)
> glDeleteRenderbuffers(1, &_renderBuffer);
> _renderBuffer = 0;
>
> - [self unlock];
> + [self unlock:previousEaglContext];
> }
>
> - (void)resetBuffers
> @@ -622,15 +643,36 @@ static void OpenglESSwap(vlc_gl_t *gl)
> }
> }
>
> -- (void)lock
> +- (bool)lock:(EAGLContext **)previousEaglContext
> {
> - _previousEaglContext = [EAGLContext currentContext];
> - [EAGLContext setCurrentContext:_eaglContext];
> + vlc_mutex_lock(&lock_mutex);
> +
> + if (unlikely(!_appActive))
> + {
> + vlc_mutex_unlock(&lock_mutex);
> + return false;
> + }
> +
> + *previousEaglContext = [EAGLContext currentContext];
> +
> + bool locked = [EAGLContext setCurrentContext:_eaglContext];
> +
> + if (locked)
> + lock_count++;
> +
> + vlc_mutex_unlock(&lock_mutex);
> + return locked;
> }
>
> -- (void)unlock
> +- (void)unlock:(EAGLContext *)previousEaglContext
> {
> - [EAGLContext setCurrentContext:_previousEaglContext];
> + vlc_mutex_lock(&lock_mutex);
> +
> + [EAGLContext setCurrentContext:previousEaglContext];
> +
> + lock_count--;
> + vlc_mutex_unlock(&lock_mutex);
> + vlc_cond_signal(&lock_wait);
> }
>
> - (void)layoutSubviews
> @@ -650,7 +692,9 @@ static void OpenglESSwap(vlc_gl_t *gl)
> return;
> }
>
> - [self lock];
> + EAGLContext *previousEaglContext;
> + if (![self lock:&previousEaglContext])
> + return;
>
> CGSize viewSize = [self bounds].size;
> CGFloat scaleFactor = self.contentScaleFactor;
> @@ -670,7 +714,7 @@ static void OpenglESSwap(vlc_gl_t *gl)
>
> // x / y are top left corner, but we need the lower left one
> glViewport(place.x, place.y, place.width, place.height);
> - [self unlock];
> + [self unlock:previousEaglContext];
> }
>
> - (void)tapRecognized:(UITapGestureRecognizer *)tapRecognizer
> @@ -688,14 +732,38 @@ static void OpenglESSwap(vlc_gl_t *gl)
>
> - (void)applicationStateChanged:(NSNotification *)notification
> {
> - @synchronized (self) {
> - if ([[notification name]
> isEqualToString:UIApplicationWillResignActiveNotification]
> - || [[notification name]
> isEqualToString:UIApplicationDidEnterBackgroundNotification]
> - || [[notification name]
> isEqualToString:UIApplicationWillTerminateNotification])
> + vlc_mutex_lock(&lock_mutex);
> +
> + if ([[notification name]
> isEqualToString:UIApplicationWillResignActiveNotification])
> _appActive = NO;
> + else if ([[notification name]
> isEqualToString:UIApplicationDidEnterBackgroundNotification])
> + {
> + _appActive = NO;
> + if (_eaglContext != nil)
> + {
> + /* Wait for the vout to unlock the eagl context before
> releasing it. */
> + int ret = 0;
> + mtime_t deadline = INT64_C(2000000); // 2seconds
> + while (ret == 0 && lock_count != 0)
> + ret = vlc_cond_timedwait(&lock_wait, &lock_mutex,
> deadline);
> +
> + if (ret == 0)
> + [_eaglContext release];
> + else
> + msg_Err(_voutDisplay, "VLC vout timed out ! Leaking
> eagl context...");
> + _eaglContext = nil;
> + }
> + }
> else
> + {
> + assert([[notification name]
> isEqualToString:UIApplicationWillEnterForegroundNotification]
> + || [[notification name]
> isEqualToString:UIApplicationDidBecomeActiveNotification]);
> + if (_eaglContext == nil)
> + _eaglContext = [[EAGLContext alloc]
> initWithAPI:kEAGLRenderingAPIOpenGLES2];
> _appActive = YES;
> }
> +
> + vlc_mutex_unlock(&lock_mutex);
> }
>
> - (void)updateConstraints
> --
> 2.11.0
>
> _______________________________________________
> 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