[vlc-devel] [PATCH v2 01/19] display: specify if vout_display_PlacePicture() alignment is top/bottom-left
Steve Lhomme
robux4 at ycbcr.xyz
Wed Aug 26 08:50:26 CEST 2020
On 2020-08-25 17:17, Romain Vimont wrote:
> On Tue, Aug 25, 2020 at 04:20:14PM +0200, Steve Lhomme wrote:
>> For OpenGL the picture alignment must be computed from the bottom-left, whereas
>> all other modules align from top-left.
>
> IMO, the core should not be aware of any "internal" vflip by the vout.
I'm not sure what you mean by "be aware". The
vout_display_PlacePicture() function is in the core. But the core itself
doesn't know anything about the orientation/origin of the display. This
state is private to the display module(s).
> It should just request the alignment it wants, and the vout handles it.
This is what is done and is not changed.
>> 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.
>
> What is rounded to the same value in that case?
You have a 1000x4 video. You want it aligned to the bottom in a 1000x7
window.
In the core/top-left-origin case, the placement is (0,3).
In the opengl/bottom-left-origin, the placement is (0,4).
>> ---
>> 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
>>
>> _______________________________________________
>> 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