[vlc-commits] [Git][videolan/vlc][master] 6 commits: macosx.m: use vlc_obj_calloc for sys

Jean-Baptiste Kempf (@jbk) gitlab at videolan.org
Fri Apr 29 11:19:51 UTC 2022



Jean-Baptiste Kempf pushed to branch master at VideoLAN / VLC


Commits:
b688611d by Alexandre Janniaux at 2022-04-29T13:07:27+02:00
macosx.m: use vlc_obj_calloc for sys

This also prevent the leak of sys happening when the wrong type of
window is used when opening the display, and move the test after the
trivial check for window type.

- - - - -
e67a8c4f by Alexandre Janniaux at 2022-04-29T13:07:27+02:00
macosx.m: remove drawable code

The drawable NSObject is now provided through the window and the code is
the same both for a vout_window-provided NSObject or a libvlc NSObject
drawable so we can simplify here now.

- - - - -
34511d42 by Alexandre Janniaux at 2022-04-29T13:07:27+02:00
macosx.m: remove sys->embed

sys->embed was confusing when the NSView could come from either the
drawable variable or the window provider, and the view is now given
through the display configuration in any case, so it doesn't need to be
stored again.

- - - - -
2f21c868 by Alexandre Janniaux at 2022-04-29T13:07:27+02:00
macosx: ignore DISPLAY_SIZE and handle resize internally

The current vout display module is resizing from its own view and tries
to notify the video view changes to the parent. But it can already
resize, so it doesn't need the core to resize the input picture, and
would already know the size it can render to.

It makes reporting the vout display size from the vout useless, and even
harmful since the display will re-enter itself on resize.

- - - - -
73c57341 by Alexandre Janniaux at 2022-04-29T13:07:27+02:00
macosx: disable pending reshape mechanism

The _pendingReshape mechanism was used to avoid redrawing the view if it
has been updated externally, and avoid re-render the view when it's
being drawn already but:
 - `drawRect:` won't be called if we didn't change the window, except
   the first time which will likely trigger a clear.
 - When the window state changes, we were already setting
   _pendingReshape to YES.

Overall, we can let the UI system decide when it requires a re-draw and
react to that instead of trying to workaround when to re-draw.

- - - - -
2cb6eea1 by Alexandre Janniaux at 2022-04-29T13:07:27+02:00
macosx: rework resizing for vout_display

Rework resizing as a display-only module without windowing.

Since there's no window to resize this component (and it handles resize
internally), we can accept the display size events without changing
the rendering state, and will only react to the internal resize events
from the embedded NSView.

Fix #26846
Refs #25264

- - - - -


1 changed file:

- modules/video_output/macosx.m


Changes:

=====================================
modules/video_output/macosx.m
=====================================
@@ -93,10 +93,10 @@ vlc_module_end ()
 @interface VLCOpenGLVideoView : NSOpenGLView
 {
     vout_display_t *vd;
-    BOOL _hasPendingReshape;
 }
 - (void)setVoutDisplay:(vout_display_t *)vd;
 - (void)setVoutFlushing:(BOOL)flushing;
+- (void)render;
 @end
 
 
@@ -105,7 +105,6 @@ typedef struct vout_display_sys_t
     VLCOpenGLVideoView *glView;
     id<VLCVideoViewEmbedding> container;
 
-    vout_window_t *embed;
     vlc_gl_t *gl;
     vout_display_opengl_t *vgl;
 
@@ -113,6 +112,7 @@ typedef struct vout_display_sys_t
     bool has_first_frame;
 
     vout_display_cfg_t cfg;
+    vout_display_place_t place;
 } vout_display_sys_t;
 
 struct gl_sys
@@ -141,11 +141,12 @@ static const struct vlc_display_operations ops = {
 static int Open (vout_display_t *vd,
                  video_format_t *fmt, vlc_video_context *context)
 {
-    vout_display_sys_t *sys = calloc (1, sizeof(*sys));
 
     if (vd->cfg->window->type != VOUT_WINDOW_TYPE_NSOBJECT)
         return VLC_EGENERIC;
 
+    vout_display_sys_t *sys = vlc_obj_calloc (vd, 1, sizeof(*sys));
+
     if (!sys)
         return VLC_ENOMEM;
     sys->cfg = *vd->cfg;
@@ -155,21 +156,12 @@ static int Open (vout_display_t *vd,
             msg_Err (vd, "no OpenGL hardware acceleration found. this can lead to slow output and unexpected results");
 
         vd->sys = sys;
-        sys->embed = NULL;
         sys->vgl = NULL;
         sys->gl = NULL;
 
         /* Get the drawable object */
-        id container = var_CreateGetAddress (vd, "drawable-nsobject");
-        if (!container) {
-            sys->embed = vd->cfg->window;
-            container = sys->embed->handle.nsobject;
-
-            if (!container) {
-                msg_Err(vd, "No drawable-nsobject nor vout_window_t found, passing over.");
-                goto error;
-            }
-        }
+        id container = vd->cfg->window->handle.nsobject;
+        assert(container != nil);
 
         /* This will be released in Close(), on
          * main thread, after we are done using it. */
@@ -246,10 +238,6 @@ static int Open (vout_display_t *vd,
 
         vd->ops = &ops;
 
-        /* */
-        // FIXME: this call leads to a fatal mutex locking error in vout_ChangeDisplaySize()
-        // vout_window_ReportSize(sys->embed, fmt->i_visible_width, fmt->i_visible_height);
-
         return VLC_SUCCESS;
 
     error:
@@ -294,8 +282,6 @@ static void Close(vout_display_t *vd)
             [glView removeFromSuperview];
             [glView release];
         });
-
-        free (sys);
     }
 }
 
@@ -319,25 +305,30 @@ static void PictureRender (vout_display_t *vd, picture_t *pic, subpicture_t *sub
 static void PictureDisplay (vout_display_t *vd, picture_t *pic)
 {
     vout_display_sys_t *sys = vd->sys;
-    VLC_UNUSED(pic);
-    [sys->glView setVoutFlushing:YES];
-    if (vlc_gl_MakeCurrent(sys->gl) == VLC_SUCCESS)
+    (void)pic;
+
+    @synchronized(sys->glView)
     {
-        if (@available(macOS 10.14, *)) {
-            vout_display_place_t place;
-            vout_display_PlacePicture(&place, vd->source, &sys->cfg);
-            vout_display_opengl_Viewport(sys->vgl, place.x,
-                                         sys->cfg.display.height - (place.y + place.height),
-                                         place.width, place.height);
+        [sys->glView setVoutFlushing:YES];
+        if (vlc_gl_MakeCurrent(sys->gl) == VLC_SUCCESS)
+        {
+            [sys->glView render];
+            vlc_gl_ReleaseCurrent(sys->gl);
         }
-
-        vout_display_opengl_Display(sys->vgl);
-        vlc_gl_ReleaseCurrent(sys->gl);
+        [sys->glView setVoutFlushing:NO];
     }
-    [sys->glView setVoutFlushing:NO];
     sys->has_first_frame = true;
 }
 
+static void UpdatePlace (vout_display_t *vd, const vout_display_cfg_t *cfg)
+{
+    vout_display_sys_t *sys = vd->sys;
+    vout_display_place_t place;
+    /* We never receive resize from the core, so provide the size ourselves */
+    vout_display_PlacePicture(&place, vd->source, cfg);
+    sys->place = place;
+}
+
 static int Control (vout_display_t *vd, int query)
 {
     vout_display_sys_t *sys = vd->sys;
@@ -348,41 +339,28 @@ static int Control (vout_display_t *vd, int query)
     @autoreleasepool {
         switch (query)
         {
+            /* We handle the resizing ourselves, nothing to report either. */
+            case VOUT_DISPLAY_CHANGE_DISPLAY_SIZE:
+                return VLC_SUCCESS;
+
             case VOUT_DISPLAY_CHANGE_DISPLAY_FILLED:
             case VOUT_DISPLAY_CHANGE_ZOOM:
             case VOUT_DISPLAY_CHANGE_SOURCE_ASPECT:
             case VOUT_DISPLAY_CHANGE_SOURCE_CROP:
-            case VOUT_DISPLAY_CHANGE_DISPLAY_SIZE:
             {
-                /* we always use our current frame here, because we have some size constraints
-                 in the ui vout provider */
-                vout_display_cfg_t cfg_tmp = *vd->cfg;
-
-                /* Reverse vertical alignment as the GL tex are Y inverted */
-                if (cfg_tmp.align.vertical == VLC_VIDEO_ALIGN_TOP)
-                    cfg_tmp.align.vertical = VLC_VIDEO_ALIGN_BOTTOM;
-                else if (cfg_tmp.align.vertical == VLC_VIDEO_ALIGN_BOTTOM)
-                    cfg_tmp.align.vertical = VLC_VIDEO_ALIGN_TOP;
-
-                vout_display_place_t place;
-                vout_display_PlacePicture(&place, vd->source, &cfg_tmp);
-                @synchronized (sys->glView) {
-                    sys->cfg = *vd->cfg;
+                @synchronized(sys->glView) {
+                    vout_display_cfg_t cfg;
+                    cfg = *vd->cfg;
+                    /* Reverse vertical alignment as the GL tex are Y inverted */
+                    if (cfg.align.vertical == VLC_VIDEO_ALIGN_TOP)
+                        cfg.align.vertical = VLC_VIDEO_ALIGN_BOTTOM;
+                    else if (cfg.align.vertical == VLC_VIDEO_ALIGN_BOTTOM)
+                        cfg.align.vertical = VLC_VIDEO_ALIGN_TOP;
+                    cfg.display.width = sys->cfg.display.width;
+                    cfg.display.height = sys->cfg.display.height;
+                    sys->cfg = cfg;
+                    UpdatePlace(vd, &cfg);
                 }
-
-                if (vlc_gl_MakeCurrent (sys->gl) != VLC_SUCCESS)
-                    return VLC_SUCCESS;
-                vout_display_opengl_SetOutputSize(sys->vgl, place.width, place.height);
-
-                /* For resize, we call glViewport in reshape and not here.
-                 This has the positive side effect that we avoid erratic sizing as we animate every resize. */
-                if (query != VOUT_DISPLAY_CHANGE_DISPLAY_SIZE)
-                    // x / y are top left corner, but we need the lower left one
-                    vout_display_opengl_Viewport(sys->vgl, place.x,
-                                                 cfg_tmp.display.height - (place.y + place.height),
-                                                 place.width, place.height);
-                vlc_gl_ReleaseCurrent (sys->gl);
-
                 return VLC_SUCCESS;
             }
 
@@ -539,27 +517,8 @@ static void OpenglSwap (vlc_gl_t *gl)
 {
     if (!flushing)
         return;
-    @synchronized(self) {
-        _hasPendingReshape = NO;
-    }
-}
-
-/**
- * Can -drawRect skip rendering?.
- */
-- (BOOL)canSkipRendering
-{
-    VLCAssertMainThread();
-
-    @synchronized(self) {
-        if (!vd)
-            return false;
-        vout_display_sys_t *sys = vd->sys;
-        return !_hasPendingReshape && sys->has_first_frame;
-    }
 }
 
-
 /**
  * Local method that locks the gl context.
  */
@@ -585,27 +544,24 @@ static void OpenglSwap (vlc_gl_t *gl)
 /**
  * Local method that force a rendering of a frame.
  * This will get called if Cocoa forces us to redraw (via -drawRect).
+ *
+ * NOTE: must be called in a @synchronized() block with sys->glView or self.
  */
 - (void)render
 {
-    VLCAssertMainThread();
-
-    // We may have taken some times to take the opengl Lock.
-    // Check here to see if we can just skip the frame as well.
-    if ([self canSkipRendering])
+    if (!vd) {
+        glClear (GL_COLOR_BUFFER_BIT);
         return;
+    }
 
-    vout_display_sys_t *sys;
+    vout_display_sys_t *sys = vd->sys;
 
-    BOOL hasFirstFrame;
-    @synchronized(self) { // vd can be accessed from multiple threads
-        sys = vd ? vd->sys : NULL;
-        hasFirstFrame = sys && sys->has_first_frame;
-    }
+    vout_display_opengl_Viewport(sys->vgl, sys->place.x, sys->place.y,
+                                 sys->place.width, sys->place.height);
+    vout_display_opengl_SetOutputSize(sys->vgl, sys->place.width, sys->place.height);
 
-    if (hasFirstFrame)
-        // This will lock gl.
-        vout_display_opengl_Display(sys->vgl);
+    if (sys->has_first_frame)
+        vout_display_opengl_Display (sys->vgl);
     else
         glClear (GL_COLOR_BUFFER_BIT);
 }
@@ -622,34 +578,13 @@ static void OpenglSwap (vlc_gl_t *gl)
     vout_display_place_t place;
 
     @synchronized(self) {
-        if (vd) {
-            vout_display_sys_t *sys = vd->sys;
-            sys->cfg.display.width  = bounds.size.width;
-            sys->cfg.display.height = bounds.size.height;
-
-            vout_display_PlacePicture(&place, vd->source, &sys->cfg);
-            // FIXME: this call leads to a fatal mutex locking error in vout_ChangeDisplaySize()
-            // vout_window_ReportSize(sys->embed, bounds.size.width, bounds.size.height);
-        }
-    }
-
-    if ([self lockgl]) {
-        // x / y are top left corner, but we need the lower left one
-        glViewport (place.x, bounds.size.height - (place.y + place.height),
-                    place.width, place.height);
-
-        @synchronized(self) {
-            // This may be cleared before -drawRect is being called,
-            // in this case we'll skip the rendering.
-            // This will save us for rendering two frames (or more) for nothing
-            // (one by the vout, one (or more) by drawRect)
-            _hasPendingReshape = YES;
-        }
-
-        [self unlockgl];
-
-        [super reshape];
+        if (vd == NULL) return;
+        vout_display_sys_t *sys = vd->sys;
+        sys->cfg.display.width  = bounds.size.width;
+        sys->cfg.display.height = bounds.size.height;
+        UpdatePlace(vd, &sys->cfg);
     }
+    [super reshape];
 }
 
 /**
@@ -675,16 +610,14 @@ static void OpenglSwap (vlc_gl_t *gl)
 {
     VLCAssertMainThread();
 
-    if ([self canSkipRendering])
-        return;
-
-    BOOL success = [self lockgl];
-    if (!success)
-        return;
-
-    [self render];
+    @synchronized(self) {
+        BOOL success = [self lockgl];
+        if (!success)
+            return;
 
-    [self unlockgl];
+        [self render];
+        [self unlockgl];
+    }
 }
 
 - (void)renewGState



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/b4cd6a8bd807c8b7e0e43c22770cc43dd18e6315...2cb6eea124b86f6be7ad74cc99e5510594596343

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/b4cd6a8bd807c8b7e0e43c22770cc43dd18e6315...2cb6eea124b86f6be7ad74cc99e5510594596343
You're receiving this email because of your account on code.videolan.org.


VideoLAN code repository instance


More information about the vlc-commits mailing list