[vlc-devel] [PATCH] opengl: split interop constructor for subpictures

[Alexandre Janniaux]

Hi,

I don't have any issues with this patch being merged in order
to ease the rest of your patchset, but having a single interop
for multiple pictures (eg. a subpicture) at the same time looks
against the original design.

Regards,
--
Alexandre Janniaux
Videolabs

On Wed, Jun 17, 2020 at 09:35:18PM +0200, Romain Vimont wrote:
> Expose a separate "constructor" for creating an interop used for
> subpictures.
>
> This avoids a "subpics" flag, and make explicit that context and fmt are
> not used for SPU.
> ---
>  modules/video_output/opengl/interop.c     | 80 +++++++++++++----------
>  modules/video_output/opengl/interop.h     |  7 +-
>  modules/video_output/opengl/vout_helper.c |  4 +-
>  3 files changed, 52 insertions(+), 39 deletions(-)
>
> diff --git a/modules/video_output/opengl/interop.c b/modules/video_output/opengl/interop.c
> index 8bd71110dc73..07f8c0c950c9 100644
> --- a/modules/video_output/opengl/interop.c
> +++ b/modules/video_output/opengl/interop.c
> @@ -31,8 +31,7 @@
>
>  struct vlc_gl_interop *
>  vlc_gl_interop_New(struct vlc_gl_t *gl, const struct vlc_gl_api *api,
> -                   vlc_video_context *context, const video_format_t *fmt,
> -                   bool subpics)
> +                   vlc_video_context *context, const video_format_t *fmt)
>  {
>      struct vlc_gl_interop *interop = vlc_object_create(gl, sizeof(*interop));
>      if (!interop)
> @@ -48,46 +47,57 @@ vlc_gl_interop_New(struct vlc_gl_t *gl, const struct vlc_gl_api *api,
>      interop->api = api;
>      interop->vt = &api->vt;
>
> -    int ret;
> -    if (subpics)
> +    const vlc_chroma_description_t *desc =
> +        vlc_fourcc_GetChromaDescription(fmt->i_chroma);
> +
> +    if (desc == NULL)
>      {
> -        interop->fmt.i_chroma = VLC_CODEC_RGB32;
> -        /* Normal orientation and no projection for subtitles */
> -        interop->fmt.orientation = ORIENT_NORMAL;
> -        interop->fmt.projection_mode = PROJECTION_MODE_RECTANGULAR;
> -        interop->fmt.primaries = COLOR_PRIMARIES_UNDEF;
> -        interop->fmt.transfer = TRANSFER_FUNC_UNDEF;
> -        interop->fmt.space = COLOR_SPACE_UNDEF;
> -
> -        ret = opengl_interop_generic_init(interop, false);
> +        vlc_object_delete(interop);
> +        return NULL;
>      }
> -    else
> +    if (desc->plane_count == 0)
>      {
> -        const vlc_chroma_description_t *desc =
> -            vlc_fourcc_GetChromaDescription(fmt->i_chroma);
> +        /* Opaque chroma: load a module to handle it */
> +        interop->vctx = context;
> +        interop->module = module_need_var(interop, "glinterop", "glinterop");
> +    }
>
> -        if (desc == NULL)
> -        {
> -            vlc_object_delete(interop);
> -            return NULL;
> -        }
> -        if (desc->plane_count == 0)
> -        {
> -            /* Opaque chroma: load a module to handle it */
> -            interop->vctx = context;
> -            interop->module = module_need_var(interop, "glinterop", "glinterop");
> -        }
> +    int ret;
> +    if (interop->module != NULL)
> +        ret = VLC_SUCCESS;
> +    else
> +    {
> +        /* Software chroma or gl hw converter failed: use a generic
> +         * converter */
> +        ret = opengl_interop_generic_init(interop, true);
> +    }
>
> -        if (interop->module != NULL)
> -            ret = VLC_SUCCESS;
> -        else
> -        {
> -            /* Software chroma or gl hw converter failed: use a generic
> -             * converter */
> -            ret = opengl_interop_generic_init(interop, true);
> -        }
> +    if (ret != VLC_SUCCESS)
> +    {
> +        vlc_object_delete(interop);
> +        return NULL;
>      }
>
> +    return interop;
> +}
> +
> +struct vlc_gl_interop *
> +vlc_gl_interop_NewForSubpictures(struct vlc_gl_t *gl,
> +                                 const struct vlc_gl_api *api)
> +{
> +    struct vlc_gl_interop *interop = vlc_object_create(gl, sizeof(*interop));
> +    if (!interop)
> +        return NULL;
> +
> +    interop->init = opengl_interop_init_impl;
> +    interop->ops = NULL;
> +    interop->gl = gl;
> +    interop->api = api;
> +    interop->vt = &api->vt;
> +
> +    video_format_Init(&interop->fmt, VLC_CODEC_RGB32);
> +
> +    int ret = opengl_interop_generic_init(interop, false);
>      if (ret != VLC_SUCCESS)
>      {
>          vlc_object_delete(interop);
> diff --git a/modules/video_output/opengl/interop.h b/modules/video_output/opengl/interop.h
> index 4288da3f2581..9133b681eaa3 100644
> --- a/modules/video_output/opengl/interop.h
> +++ b/modules/video_output/opengl/interop.h
> @@ -153,8 +153,11 @@ struct vlc_gl_interop {
>
>  struct vlc_gl_interop *
>  vlc_gl_interop_New(struct vlc_gl_t *gl, const struct vlc_gl_api *api,
> -                   vlc_video_context *context, const video_format_t *fmt,
> -                   bool subpics);
> +                   vlc_video_context *context, const video_format_t *fmt);
> +
> +struct vlc_gl_interop *
> +vlc_gl_interop_NewForSubpictures(struct vlc_gl_t *gl,
> +                                 const struct vlc_gl_api *api);
>
>  void
>  vlc_gl_interop_Delete(struct vlc_gl_interop *interop);
> diff --git a/modules/video_output/opengl/vout_helper.c b/modules/video_output/opengl/vout_helper.c
> index 5e4ef90213af..526edcb23f2a 100644
> --- a/modules/video_output/opengl/vout_helper.c
> +++ b/modules/video_output/opengl/vout_helper.c
> @@ -133,7 +133,7 @@ vout_display_opengl_t *vout_display_opengl_New(video_format_t *fmt,
>          (GLint)fmt->i_height > max_tex_size)
>          ResizeFormatToGLMaxTexSize(fmt, max_tex_size);
>
> -    vgl->interop = vlc_gl_interop_New(gl, api, context, fmt, false);
> +    vgl->interop = vlc_gl_interop_New(gl, api, context, fmt);
>      if (!vgl->interop)
>      {
>          msg_Err(gl, "Could not create interop");
> @@ -157,7 +157,7 @@ vout_display_opengl_t *vout_display_opengl_New(video_format_t *fmt,
>
>      GL_ASSERT_NOERROR(vt);
>
> -    vgl->sub_interop = vlc_gl_interop_New(gl, api, NULL, fmt, true);
> +    vgl->sub_interop = vlc_gl_interop_NewForSubpictures(gl, api);
>      if (!vgl->sub_interop)
>      {
>          msg_Err(gl, "Could not create sub interop");
> --
> 2.27.0
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel