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

Romain Vimont rom1v at videolabs.io
Thu May 14 12:10:05 CEST 2020



On Wed, May 13, 2020 at 05:56:14PM +0200, Alexandre Janniaux wrote:
> Hi,
> 
> I'm not sure what the comment means:
> 
> > from the core point of view, it does not change.

It means that after the Open(), fmt->orientation is the same as before
the call.

> 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.

This orientation overwriting was a hack for the Android interop, which I
fixed and explained in a future commit:
https://code.videolan.org/rom1v/vlc/-/commit/62b19673c2423b4b6d564444af85c513a4adaa92

What this current patch changes is the fact that the renderer is no
longer involved in this "hack". This paves the way to make the renderer
an independant filter.

I just added a commit to remove the unnecessary hack:
https://code.videolan.org/rom1v/vlc/-/commit/6901a8f4cb848238192f39ddc35a987799589ee8

But this is after this patchset. For this patch, I edited the commit
message:

    opengl: simplify renderer format

    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).

    But as a hack, the Android interop currently flips the orientation to
    avoid a vertical flip in the renderer, since it provides its own
    transform matrix. This change must not be reported to the core (the core
    must not flip the input), so the orientation change is reverted. A
    further refactor will remove this hack later.

    Meanwhile, handle the format to report to the core outside of the
    renderer, so that the renderer is not involved in this hack. This paves
    the way to pass only the interop to the renderer, without an additional
    video_format_t instance.

https://code.videolan.org/rom1v/vlc/-/commit/28bd9dfe022a825180732389db33527079d2295c

I hope this clarifies.

Regards

> 
> 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
> _______________________________________________
> 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