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

Steve Lhomme robux4 at ycbcr.xyz
Tue Aug 25 16:20:14 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            | 10 +++++++++-
 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            | 16 +++++++++++-----
 src/video_output/video_output.c       |  2 +-
 15 files changed, 45 insertions(+), 55 deletions(-)

diff --git a/include/vlc_vout_display.h b/include/vlc_vout_display.h
index 2a0b593a13e..0610c4134ba 100644
--- a/include/vlc_vout_display.h
+++ b/include/vlc_vout_display.h
@@ -500,6 +500,11 @@ static inline bool vout_display_PlaceEquals(const vout_display_place_t *p1,
             p1->y == p2->y && p1->height == p2->height;
 }
 
+enum vout_place_origin {
+   VOUT_ORIGIN_TOP_LEFT,
+   VOUT_ORIGIN_BOTTOM_LEFT,
+};
+
 /**
  * Computes the intended picture placement inside the display.
  *
@@ -512,8 +517,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 origin whether the alignement should be done from the bottom-left or
+ *               the top-left corner.
  */
-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, enum vout_place_origin origin);
 
 /**
  * Translates mouse state.
diff --git a/modules/hw/mmal/vout.c b/modules/hw/mmal/vout.c
index e0eeeb0513b..6d4c8a2c7c5 100644
--- a/modules/hw/mmal/vout.c
+++ b/modules/hw/mmal/vout.c
@@ -524,7 +524,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, VOUT_ORIGIN_TOP_LEFT);
 
     sys->dest_rect = place_to_mmal_rect(place);
 }
diff --git a/modules/hw/vdpau/display.c b/modules/hw/vdpau/display.c
index 5963be63718..61d4c1a1430 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, VOUT_ORIGIN_TOP_LEFT);
 
         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, VOUT_ORIGIN_TOP_LEFT);
         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, VOUT_ORIGIN_TOP_LEFT);
         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..3eeaba0ce10 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, VOUT_ORIGIN_BOTTOM_LEFT);
             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..771cbbb649e 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, VOUT_ORIGIN_TOP_LEFT);
 }
 
 - (void)reshape
diff --git a/modules/video_output/kva.c b/modules/video_output/kva.c
index 185e8269600..09d5fcf8add 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, VOUT_ORIGIN_TOP_LEFT);
 
         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, VOUT_ORIGIN_TOP_LEFT);
 
             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..12d18ab842f 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, VOUT_ORIGIN_BOTTOM_LEFT);
             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, VOUT_ORIGIN_BOTTOM_LEFT);
                 @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, VOUT_ORIGIN_BOTTOM_LEFT);
             // 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..fae920dd6ec 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, VOUT_ORIGIN_BOTTOM_LEFT);
         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, VOUT_ORIGIN_BOTTOM_LEFT);
         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..119583d1379 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, VOUT_ORIGIN_TOP_LEFT);
         return VLC_SUCCESS;
     }
 
diff --git a/modules/video_output/wayland/shm.c b/modules/video_output/wayland/shm.c
index 77acabdd0c2..811682b8f96 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, VOUT_ORIGIN_TOP_LEFT);
             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, VOUT_ORIGIN_TOP_LEFT);
 
                 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..5e65da9071a 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;
+    enum vout_place_origin origin;
 
 #if (defined(MODULE_NAME_IS_glwin32))
+    origin = VOUT_ORIGIN_BOTTOM_LEFT;
     /* 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
+    origin = VOUT_ORIGIN_TOP_LEFT;
 #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, origin);
 
     /* 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..cede80fd246 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, VOUT_ORIGIN_TOP_LEFT);
 
     /* 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..6ea5b82d908 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, VOUT_ORIGIN_TOP_LEFT);
 
         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, VOUT_ORIGIN_TOP_LEFT);
     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..5d59c7af2a1 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,
+                               enum vout_place_origin origin)
 {
     /* vout_display_PlacePicture() is called from vd plugins. They should not
      * care about the initial window properties. */
@@ -212,13 +213,18 @@ void vout_display_PlacePicture(vout_display_place_t *place,
 
     switch (cfg->align.vertical) {
     case VLC_VIDEO_ALIGN_TOP:
-        place->y = 0;
+        place->y = origin == VOUT_ORIGIN_BOTTOM_LEFT ? (cfg->display.height - place->height) : 0;
         break;
     case VLC_VIDEO_ALIGN_BOTTOM:
-        place->y = cfg->display.height - place->height;
+        place->y = origin == VOUT_ORIGIN_TOP_LEFT    ? (cfg->display.height - place->height) : 0;
         break;
     default:
-        place->y = ((int)cfg->display.height - (int)place->height) / 2;
+        if (origin == VOUT_ORIGIN_BOTTOM_LEFT)
+            // round to upper value
+            place->y = ((int)cfg->display.height - (int)place->height + 1) / 2;
+        else
+            // round to lower value
+            place->y = ((int)cfg->display.height - (int)place->height) / 2;
         break;
     }
 }
@@ -229,7 +235,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, VOUT_ORIGIN_TOP_LEFT);
 
     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..edb029893c4 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, VOUT_ORIGIN_TOP_LEFT);
 
         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