[vlc-devel] [PATCH 03/17] opengl: simplify renderer format

[Alexandre Janniaux]

Hi,

I'm not sure what the comment means:

> from the core point of view, it does not change.

The common usage of this flag is that if you enforce a value,
the core will instantiate what is needed to transform the
input picture. So it doesn't seem to match your commit
message.

Imho the comments should explain why there is the
intermediate orientation value, ie. why the orientation
might be modified by the interop, and mention why it should
not be modified (ie. that opengl renderer supports any input
rotation).

But I'm not really sure why the input format is modified in
the first place for OpenGL. I don't recall what we did in the
interop that needed that so it might help to improve the
commit message.

Regards,
--
Alexandre Janniaux
Videolabs

On Thu, Apr 02, 2020 at 02:24:16PM +0200, Romain Vimont wrote:
> The interop may modify its own copy of the video_format_t.
>
> These changes must be reported to the core (by writing to the provided
> video_format_t), except for the orientation: it is managed internally by
> OpenGL, so from the core point of view, the orientation does not change.
>
> Handle the format to report to the core outside of the renderer. This
> paves the way to pass only the interop to the renderer, without an
> additional video_format_t instance.
> ---
>  modules/video_output/opengl/renderer.c    | 5 +----
>  modules/video_output/opengl/vout_helper.c | 7 ++++++-
>  2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/modules/video_output/opengl/renderer.c b/modules/video_output/opengl/renderer.c
> index 7b1cca6a1d..ded06dd500 100644
> --- a/modules/video_output/opengl/renderer.c
> +++ b/modules/video_output/opengl/renderer.c
> @@ -490,10 +490,7 @@ vlc_gl_renderer_New(vlc_gl_t *gl, const struct vlc_gl_api *api,
>                                    sampler->var.OrientationMatrix);
>      getViewpointMatrixes(renderer, interop->fmt.projection_mode);
>
> -    /* Update the fmt to main program one */
> -    renderer->fmt = interop->fmt;
> -    /* The orientation is handled by the orientation matrix */
> -    renderer->fmt.orientation = fmt->orientation;
> +    renderer->fmt = *fmt;
>
>      /* Texture size */
>      for (unsigned j = 0; j < interop->tex_count; j++) {
> diff --git a/modules/video_output/opengl/vout_helper.c b/modules/video_output/opengl/vout_helper.c
> index 1945f01e71..31b828d64a 100644
> --- a/modules/video_output/opengl/vout_helper.c
> +++ b/modules/video_output/opengl/vout_helper.c
> @@ -187,7 +187,12 @@ vout_display_opengl_t *vout_display_opengl_New(video_format_t *fmt,
>          return NULL;
>      }
>
> -    *fmt = renderer->fmt;
> +    video_orientation_t orientation = fmt->orientation;
> +    *fmt = vgl->interop->fmt;
> +    /* The orientation is handled by the orientation matrix: from the core
> +     * point of view, it does not change. */
> +    fmt->orientation = orientation;
> +
>      if (subpicture_chromas) {
>          *subpicture_chromas = gl_subpicture_chromas;
>      }
> --
> 2.26.0
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel