[vlc-devel] [PATCH] display: specify if vout_display_PlacePicture() alignment is top/bottom-left

Steve Lhomme robux4 at ycbcr.xyz
Tue Aug 25 14:22:12 CEST 2020


For OpenGL the picture alignment must be computed from the bottom-left, whereas
all other modules align from top-left.

This was handled internally by OpenGL modules, but for center vertical
alignment on odd vertical dimensions, it may round to the same value regardless
of the display coordinate system, which is incorrect.
---
 include/vlc_vout_display.h            |  5 ++-
 modules/hw/mmal/vout.c                |  2 +-
 modules/hw/vdpau/display.c            |  6 ++--
 modules/video_output/caopengllayer.m  |  8 +----
 modules/video_output/ios.m            |  2 +-
 modules/video_output/kva.c            |  4 +--
 modules/video_output/macosx.m         | 14 ++-------
 modules/video_output/opengl/display.c | 18 ++---------
 modules/video_output/vulkan/display.c |  2 +-
 modules/video_output/wayland/shm.c    |  4 +--
 modules/video_output/win32/common.c   |  6 +++-
 modules/video_output/xcb/render.c     |  2 +-
 modules/video_output/xcb/x11.c        |  4 +--
 src/video_output/display.c            | 45 ++++++++++++++++++++-------
 src/video_output/video_output.c       |  2 +-
 15 files changed, 62 insertions(+), 62 deletions(-)

diff --git a/include/vlc_vout_display.h b/include/vlc_vout_display.h
index 2a0b593a13e..9afa27d1c78 100644
--- a/include/vlc_vout_display.h
+++ b/include/vlc_vout_display.h
@@ -512,8 +512,11 @@ static inline bool vout_display_PlaceEquals(const vout_display_place_t *p1,
  * \param place Storage space for the picture placement [OUT]
  * \param source Video source format
  * \param cfg Display configuration
+ * \param bottom_left True if the alignement should be done from the bottom-left
+ *                    otherwise the top-left corner is used.
  */
-VLC_API void vout_display_PlacePicture(vout_display_place_t *place, const video_format_t *source, const vout_display_cfg_t *cfg);
+VLC_API void vout_display_PlacePicture(vout_display_place_t *place, const video_format_t *source,
+                                       const vout_display_cfg_t *cfg, bool bottom_left);
 
 /**
  * Translates mouse state.
diff --git a/modules/hw/mmal/vout.c b/modules/hw/mmal/vout.c
index 5889d3d55ec..4fd3b2eb075 100644
--- a/modules/hw/mmal/vout.c
+++ b/modules/hw/mmal/vout.c
@@ -522,7 +522,7 @@ place_dest(vout_display_sys_t * const sys,
     tcfg.display.width = sys->display_width;
     tcfg.display.height = sys->display_height;
     tcfg.is_display_filled = true;
-    vout_display_PlacePicture(&place, fmt, &tcfg);
+    vout_display_PlacePicture(&place, fmt, &tcfg, false);
 
     sys->dest_rect = place_to_mmal_rect(place);
 }
diff --git a/modules/hw/vdpau/display.c b/modules/hw/vdpau/display.c
index 5963be63718..4be6bc046bb 100644
--- a/modules/hw/vdpau/display.c
+++ b/modules/hw/vdpau/display.c
@@ -237,7 +237,7 @@ static int Control(vout_display_t *vd, int query, va_list ap)
         vout_display_place_t place;
 
         msg_Dbg(vd, "resetting pictures");
-        vout_display_PlacePicture(&place, src, cfg);
+        vout_display_PlacePicture(&place, src, cfg, false);
 
         fmt->i_width = src->i_width * place.width / src->i_visible_width;
         fmt->i_height = src->i_height * place.height / src->i_visible_height;
@@ -259,7 +259,7 @@ static int Control(vout_display_t *vd, int query, va_list ap)
         const vout_display_cfg_t *cfg = va_arg(ap, const vout_display_cfg_t *);
         vout_display_place_t place;
 
-        vout_display_PlacePicture(&place, &vd->source, cfg);
+        vout_display_PlacePicture(&place, &vd->source, cfg, false);
         if (place.width  != vd->fmt.i_visible_width
          || place.height != vd->fmt.i_visible_height)
             return VLC_EGENERIC;
@@ -427,7 +427,7 @@ static int Open(vout_display_t *vd, const vout_display_cfg_t *cfg,
         };
         vout_display_place_t place;
 
-        vout_display_PlacePicture(&place, &vd->source, cfg);
+        vout_display_PlacePicture(&place, &vd->source, cfg, false);
         sys->window = xcb_generate_id(sys->conn);
 
         xcb_void_cookie_t c =
diff --git a/modules/video_output/caopengllayer.m b/modules/video_output/caopengllayer.m
index 0dafcec000a..14018acd184 100644
--- a/modules/video_output/caopengllayer.m
+++ b/modules/video_output/caopengllayer.m
@@ -319,14 +319,8 @@ static int Control (vout_display_t *vd, int query, va_list ap)
             cfg_tmp.display.width = bounds.size.width;
             cfg_tmp.display.height = bounds.size.height;
 
-            /* 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);
+            vout_display_PlacePicture(&place, &vd->source, &cfg_tmp, true);
             if (unlikely(OpenglLock(sys->gl)))
                 // don't return an error or we need to handle VOUT_DISPLAY_RESET_PICTURES
                 return VLC_SUCCESS;
diff --git a/modules/video_output/ios.m b/modules/video_output/ios.m
index 80b75147c7e..51784b351a8 100644
--- a/modules/video_output/ios.m
+++ b/modules/video_output/ios.m
@@ -645,7 +645,7 @@ static void GLESSwap(vlc_gl_t *gl)
     cfg.display.width  = _viewSize.width * _scaleFactor;
     cfg.display.height = _viewSize.height * _scaleFactor;
 
-    vout_display_PlacePicture(place, &_voutDisplay->source, &cfg);
+    vout_display_PlacePicture(place, &_voutDisplay->source, &cfg, false);
 }
 
 - (void)reshape
diff --git a/modules/video_output/kva.c b/modules/video_output/kva.c
index 185e8269600..151ee9c1e3c 100644
--- a/modules/video_output/kva.c
+++ b/modules/video_output/kva.c
@@ -433,7 +433,7 @@ static int Control( vout_display_t *vd, int query, va_list args )
     case VOUT_DISPLAY_CHANGE_SOURCE_ASPECT:
     {
         vout_display_place_t place;
-        vout_display_PlacePicture(&place, &vd->source, vd->cfg);
+        vout_display_PlacePicture(&place, &vd->source, vd->cfg, false);
 
         sys->kvas.ulAspectWidth  = place.width;
         sys->kvas.ulAspectHeight = place.height;
@@ -938,7 +938,7 @@ static MRESULT EXPENTRY WndProc( HWND hwnd, ULONG msg, MPARAM mp1, MPARAM mp2 )
             i_movie_height = movie_rect.yTop - movie_rect.yBottom;
 
             vout_display_place_t place;
-            vout_display_PlacePicture(&place, &vd->source, vd->cfg);
+            vout_display_PlacePicture(&place, &vd->source, vd->cfg, false);
 
             int x = ( i_mouse_x - movie_rect.xLeft ) *
                     place.width / i_movie_width + place.x;
diff --git a/modules/video_output/macosx.m b/modules/video_output/macosx.m
index 15d0c305772..060113b9b14 100644
--- a/modules/video_output/macosx.m
+++ b/modules/video_output/macosx.m
@@ -321,7 +321,7 @@ static void PictureDisplay (vout_display_t *vd, picture_t *pic)
     {
         if (@available(macOS 10.14, *)) {
             vout_display_place_t place;
-            vout_display_PlacePicture(&place, &vd->source, &sys->cfg);
+            vout_display_PlacePicture(&place, &vd->source, &sys->cfg, true);
             vout_display_opengl_Viewport(vd->sys->vgl, place.x,
                                          sys->cfg.display.height - (place.y + place.height),
                                          place.width, place.height);
@@ -355,16 +355,8 @@ static int Control (vout_display_t *vd, int query, va_list ap)
 
                 /* we always use our current frame here, because we have some size constraints
                  in the ui vout provider */
-                vout_display_cfg_t cfg_tmp = *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);
+                vout_display_PlacePicture(&place, &vd->source, cfg, true);
                 @synchronized (sys->glView) {
                     sys->cfg = *cfg;
                 }
@@ -627,7 +619,7 @@ static void OpenglSwap (vlc_gl_t *gl)
             sys->cfg.display.width  = bounds.size.width;
             sys->cfg.display.height = bounds.size.height;
 
-            vout_display_PlacePicture(&place, &vd->source, &sys->cfg);
+            vout_display_PlacePicture(&place, &vd->source, &sys->cfg, true);
             // FIXME: this call leads to a fatal mutex locking error in vout_ChangeDisplaySize()
             // vout_window_ReportSize(sys->embed, bounds.size.width, bounds.size.height);
         }
diff --git a/modules/video_output/opengl/display.c b/modules/video_output/opengl/display.c
index 2db08ccc03b..258a7205c0f 100644
--- a/modules/video_output/opengl/display.c
+++ b/modules/video_output/opengl/display.c
@@ -206,16 +206,6 @@ static void PictureDisplay (vout_display_t *vd, picture_t *pic)
     }
 }
 
-static void
-FlipVerticalAlign(vout_display_cfg_t *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;
-}
-
 static int Control (vout_display_t *vd, int query, va_list ap)
 {
     vout_display_sys_t *sys = vd->sys;
@@ -234,9 +224,7 @@ static int Control (vout_display_t *vd, int query, va_list ap)
         vout_display_cfg_t cfg = *va_arg(ap, const vout_display_cfg_t *);
         const video_format_t *src = &vd->source;
 
-        FlipVerticalAlign(&cfg);
-
-        vout_display_PlacePicture(&sys->place, src, &cfg);
+        vout_display_PlacePicture(&sys->place, src, &cfg, true);
         sys->place_changed = true;
         vlc_gl_Resize (sys->gl, cfg.display.width, cfg.display.height);
         return VLC_SUCCESS;
@@ -247,9 +235,7 @@ static int Control (vout_display_t *vd, int query, va_list ap)
       {
         vout_display_cfg_t cfg = *va_arg(ap, const vout_display_cfg_t *);
 
-        FlipVerticalAlign(&cfg);
-
-        vout_display_PlacePicture(&sys->place, &vd->source, &cfg);
+        vout_display_PlacePicture(&sys->place, &vd->source, &cfg, true);
         sys->place_changed = true;
         return VLC_SUCCESS;
       }
diff --git a/modules/video_output/vulkan/display.c b/modules/video_output/vulkan/display.c
index 226165f2838..5fc90e7570a 100644
--- a/modules/video_output/vulkan/display.c
+++ b/modules/video_output/vulkan/display.c
@@ -348,7 +348,7 @@ static int Control(vout_display_t *vd, int query, va_list ap)
     case VOUT_DISPLAY_CHANGE_SOURCE_CROP:
     case VOUT_DISPLAY_CHANGE_ZOOM: {
         vout_display_cfg_t cfg = *va_arg (ap, const vout_display_cfg_t *);
-        vout_display_PlacePicture(&sys->place, &vd->source, &cfg);
+        vout_display_PlacePicture(&sys->place, &vd->source, &cfg, false);
         return VLC_SUCCESS;
     }
 
diff --git a/modules/video_output/wayland/shm.c b/modules/video_output/wayland/shm.c
index 77acabdd0c2..5752ab6a351 100644
--- a/modules/video_output/wayland/shm.c
+++ b/modules/video_output/wayland/shm.c
@@ -166,7 +166,7 @@ static int Control(vout_display_t *vd, int query, va_list ap)
             video_format_t src;
             assert(sys->viewport == NULL);
 
-            vout_display_PlacePicture(&place, &vd->source, cfg);
+            vout_display_PlacePicture(&place, &vd->source, cfg, false);
             video_format_ApplyRotation(&src, &vd->source);
 
             fmt->i_width  = src.i_width * place.width
@@ -198,7 +198,7 @@ static int Control(vout_display_t *vd, int query, va_list ap)
                 vout_display_place_t place;
 
                 video_format_ApplyRotation(&fmt, &vd->source);
-                vout_display_PlacePicture(&place, &vd->source, cfg);
+                vout_display_PlacePicture(&place, &vd->source, cfg, false);
 
                 wp_viewport_set_source(sys->viewport,
                                 wl_fixed_from_int(fmt.i_x_offset),
diff --git a/modules/video_output/win32/common.c b/modules/video_output/win32/common.c
index 60fb4e0f25a..f1fc7904c81 100644
--- a/modules/video_output/win32/common.c
+++ b/modules/video_output/win32/common.c
@@ -98,17 +98,21 @@ void CommonPlacePicture(vout_display_t *vd, display_win32_area_t *area, vout_dis
 {
     /* Update the window position and size */
     vout_display_cfg_t place_cfg = area->vdcfg;
+    bool bottom_left;
 
 #if (defined(MODULE_NAME_IS_glwin32))
+    bottom_left = true;
     /* Reverse vertical alignment as the GL tex are Y inverted */
     if (place_cfg.align.vertical == VLC_VIDEO_ALIGN_TOP)
         place_cfg.align.vertical = VLC_VIDEO_ALIGN_BOTTOM;
     else if (place_cfg.align.vertical == VLC_VIDEO_ALIGN_BOTTOM)
         place_cfg.align.vertical = VLC_VIDEO_ALIGN_TOP;
+#else
+    bottom_left = false;
 #endif
 
     vout_display_place_t before_place = area->place;
-    vout_display_PlacePicture(&area->place, &vd->source, &place_cfg);
+    vout_display_PlacePicture(&area->place, &vd->source, &place_cfg, bottom_left);
 
     /* Signal the change in size/position */
     if (!vout_display_PlaceEquals(&before_place, &area->place))
diff --git a/modules/video_output/xcb/render.c b/modules/video_output/xcb/render.c
index c3e71933f35..bb2dc04ced0 100644
--- a/modules/video_output/xcb/render.c
+++ b/modules/video_output/xcb/render.c
@@ -281,7 +281,7 @@ static void CreateBuffers(vout_display_t *vd, const vout_display_cfg_t *cfg)
                               sys->format.argb, 0, NULL);
 
     vout_display_place_t *place = &sys->place;
-    vout_display_PlacePicture(place, fmt, cfg);
+    vout_display_PlacePicture(place, fmt, cfg, false);
 
     /* Homogeneous coordinates transform from destination(place)
      * to source(fmt) */
diff --git a/modules/video_output/xcb/x11.c b/modules/video_output/xcb/x11.c
index 05e012f3897..65b7f31d897 100644
--- a/modules/video_output/xcb/x11.c
+++ b/modules/video_output/xcb/x11.c
@@ -150,7 +150,7 @@ static int Control(vout_display_t *vd, int query, va_list ap)
         vout_display_place_t place;
         int ret = VLC_SUCCESS;
 
-        vout_display_PlacePicture(&place, &vd->source, cfg);
+        vout_display_PlacePicture(&place, &vd->source, cfg, false);
 
         uint32_t mask = XCB_CONFIG_WINDOW_X | XCB_CONFIG_WINDOW_Y;
         const uint32_t values[] = {
@@ -313,7 +313,7 @@ static int Open (vout_display_t *vd, const vout_display_cfg_t *cfg,
     };
     vout_display_place_t place;
 
-    vout_display_PlacePicture(&place, &vd->source, cfg);
+    vout_display_PlacePicture(&place, &vd->source, cfg, false);
     sys->window = xcb_generate_id (conn);
     sys->gc = xcb_generate_id (conn);
 
diff --git a/src/video_output/display.c b/src/video_output/display.c
index 0112425ac14..688ec3f79b5 100644
--- a/src/video_output/display.c
+++ b/src/video_output/display.c
@@ -143,7 +143,8 @@ void vout_display_GetDefaultDisplaySize(unsigned *width, unsigned *height,
 /* */
 void vout_display_PlacePicture(vout_display_place_t *place,
                                const video_format_t *source,
-                               const vout_display_cfg_t *cfg)
+                               const vout_display_cfg_t *cfg,
+                               bool bottom_left)
 {
     /* vout_display_PlacePicture() is called from vd plugins. They should not
      * care about the initial window properties. */
@@ -210,16 +211,36 @@ void vout_display_PlacePicture(vout_display_place_t *place,
         break;
     }
 
-    switch (cfg->align.vertical) {
-    case VLC_VIDEO_ALIGN_TOP:
-        place->y = 0;
-        break;
-    case VLC_VIDEO_ALIGN_BOTTOM:
-        place->y = cfg->display.height - place->height;
-        break;
-    default:
-        place->y = ((int)cfg->display.height - (int)place->height) / 2;
-        break;
+
+    if (bottom_left)
+    {
+        switch (cfg->align.vertical) {
+        case VLC_VIDEO_ALIGN_TOP:
+            place->y = cfg->display.height - place->height;
+            break;
+        case VLC_VIDEO_ALIGN_BOTTOM:
+            place->y = 0;
+            break;
+        default:
+            // round to upper value
+            place->y = ((int)cfg->display.height - (int)place->height + 1) / 2;
+            break;
+        }
+    }
+    else
+    {
+        switch (cfg->align.vertical) {
+        case VLC_VIDEO_ALIGN_TOP:
+            place->y = 0;
+            break;
+        case VLC_VIDEO_ALIGN_BOTTOM:
+            place->y = cfg->display.height - place->height;
+            break;
+        default:
+            // round to lower value
+            place->y = ((int)cfg->display.height - (int)place->height) / 2;
+            break;
+        }
     }
 }
 
@@ -229,7 +250,7 @@ void vout_display_TranslateMouseState(vout_display_t *vd, vlc_mouse_t *video,
     vout_display_place_t place;
 
     /* Translate window coordinates to video coordinates */
-    vout_display_PlacePicture(&place, &vd->source, vd->cfg);
+    vout_display_PlacePicture(&place, &vd->source, vd->cfg, false);
 
     if (place.width <= 0 || place.height <= 0) {
         memset(video, 0, sizeof (*video));
diff --git a/src/video_output/video_output.c b/src/video_output/video_output.c
index 4f48754cf2e..16da94d7064 100644
--- a/src/video_output/video_output.c
+++ b/src/video_output/video_output.c
@@ -1257,7 +1257,7 @@ static int ThreadDisplayRenderPicture(vout_thread_sys_t *vout, bool is_forced)
     video_format_t fmt_spu;
     if (do_dr_spu) {
         vout_display_place_t place;
-        vout_display_PlacePicture(&place, &vd->source, vd->cfg);
+        vout_display_PlacePicture(&place, &vd->source, vd->cfg, false);
 
         fmt_spu = vd->source;
         if (fmt_spu.i_width * fmt_spu.i_height < place.width * place.height) {
-- 
2.26.2



More information about the vlc-devel mailing list