[vlc-devel] [PATCH 14/20] opengl: flip the alignment once on Open

Alexandre Janniaux ajanni at videolabs.io
Tue Aug 25 10:39:53 CEST 2020


Hi,

This seems wrong. Alignment is reversed in OpenGL because
coordinate system is reversed too, so ppicture_place data
need to be reversed too.

We don't want to report anything to the core and we want
the correct behaviour when doing control. This breaks both
points.

I'm not sure why you remove the FlipVerticalAlign too.

Regards,
--
Alexandre Janniaux
Videolabs

On Tue, Aug 25, 2020 at 09:30:02AM +0200, Steve Lhomme wrote:
> ---
>  modules/video_output/caopengllayer.m  | 12 +++++------
>  modules/video_output/macosx.m         | 18 +++++++----------
>  modules/video_output/opengl/display.c | 29 ++++++++-------------------
>  3 files changed, 21 insertions(+), 38 deletions(-)
>
> diff --git a/modules/video_output/caopengllayer.m b/modules/video_output/caopengllayer.m
> index e81f8210117..fbdbc8b59dc 100644
> --- a/modules/video_output/caopengllayer.m
> +++ b/modules/video_output/caopengllayer.m
> @@ -213,6 +213,12 @@ static int Open (vout_display_t *vd, vout_display_cfg_t *cfg,
>              outputSize = [sys->container visibleRect].size;
>          vout_window_ReportSize(sys->embed, (int)outputSize.width, (int)outputSize.height);
>
> +        /* 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;
> +
>          return VLC_SUCCESS;
>
>      bailout:
> @@ -317,12 +323,6 @@ 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);
>              if (unlikely(OpenglLock(sys->gl)))
> diff --git a/modules/video_output/macosx.m b/modules/video_output/macosx.m
> index 80247f1f29a..ffa6b83ee0f 100644
> --- a/modules/video_output/macosx.m
> +++ b/modules/video_output/macosx.m
> @@ -241,6 +241,12 @@ static int Open (vout_display_t *vd, vout_display_cfg_t *cfg,
>          vd->control = Control;
>          vd->close   = Close;
>
> +        /* 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;
> +
>          /* */
>          // 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);
> @@ -350,18 +356,8 @@ static int Control (vout_display_t *vd, int query, va_list ap)
>              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);
> +                vout_display_PlacePicture(&place, &vd->source, vd->cfg);
>                  @synchronized (sys->glView) {
>                      sys->cfg = *vd->cfg;
>                  }
> diff --git a/modules/video_output/opengl/display.c b/modules/video_output/opengl/display.c
> index 2ef8df9910b..a5738310531 100644
> --- a/modules/video_output/opengl/display.c
> +++ b/modules/video_output/opengl/display.c
> @@ -141,6 +141,12 @@ static int Open(vout_display_t *vd, vout_display_cfg_t *cfg,
>      if (sys->vgl == NULL)
>          goto error;
>
> +    /* 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;
> +
>      vd->sys = sys;
>      vd->info.subpicture_chromas = spu_chromas;
>      vd->prepare = PictureRender;
> @@ -206,16 +212,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;
> @@ -231,12 +227,7 @@ static int Control (vout_display_t *vd, int query, va_list ap)
>        case VOUT_DISPLAY_CHANGE_DISPLAY_FILLED:
>        case VOUT_DISPLAY_CHANGE_ZOOM:
>        {
> -        vout_display_cfg_t cfg = *vd->cfg;
> -        const video_format_t *src = &vd->source;
> -
> -        FlipVerticalAlign(&cfg);
> -
> -        vout_display_PlacePicture(&sys->place, src, &cfg);
> +        vout_display_PlacePicture(&sys->place, &vd->source, vd->cfg);
>          sys->place_changed = true;
>          vlc_gl_Resize (sys->gl, vd->cfg->display.width, vd->cfg->display.height);
>          return VLC_SUCCESS;
> @@ -245,11 +236,7 @@ static int Control (vout_display_t *vd, int query, va_list ap)
>        case VOUT_DISPLAY_CHANGE_SOURCE_ASPECT:
>        case VOUT_DISPLAY_CHANGE_SOURCE_CROP:
>        {
> -        vout_display_cfg_t cfg = *vd->cfg;
> -
> -        FlipVerticalAlign(&cfg);
> -
> -        vout_display_PlacePicture(&sys->place, &vd->source, &cfg);
> +        vout_display_PlacePicture(&sys->place, &vd->source, vd->cfg);
>          sys->place_changed = true;
>          return VLC_SUCCESS;
>        }
> --
> 2.26.2
>
> _______________________________________________
> 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