[vlc-devel] [PATCH 13/26] video context: make the video context structure private
Thomas Guillem
thomas at gllm.fr
Mon Sep 23 15:48:11 CEST 2019
One question.
Is it useful to have both decoder device and video context holdable ?
>From what I can see (from the outside), it seems that we can remove ref counting on the decode device.
Indeed, the decoder device is held by the video context, that is held by vouts/converter.
Maybe the filters need them ?
On Fri, Sep 20, 2019, at 16:28, Steve Lhomme wrote:
> The video context will be passed to various modules up to the display where it
> will be ultimately released when the last associated picture will be released.
> It needs to be refcounted and we need a destructor to do this final release.
>
> By default the video context holds a reference to the matching decoder device.
> This reference is released with the video context.
> ---
> include/vlc_picture.h | 23 ++++++++--
> modules/hw/nvdec/nvdec_gl.c | 3 +-
> modules/video_chroma/d3d11_fmt.h | 3 +-
> modules/video_chroma/d3d9_fmt.h | 3 +-
> modules/video_output/opengl/converter_vaapi.c | 17 ++++++-
> modules/video_output/opengl/converter_vdpau.c | 19 ++++++--
> modules/video_output/splitter.c | 6 ++-
> src/input/decoder_helpers.c | 45 +++++++++++++++++--
> src/libvlccore.sym | 3 ++
> src/video_output/display.c | 2 +-
> 10 files changed, 105 insertions(+), 19 deletions(-)
>
> diff --git a/include/vlc_picture.h b/include/vlc_picture.h
> index 99495ca43a4..9238e098956 100644
> --- a/include/vlc_picture.h
> +++ b/include/vlc_picture.h
> @@ -80,14 +80,29 @@ typedef struct picture_buffer_t
> } picture_buffer_t;
>
> typedef struct vlc_decoder_device vlc_decoder_device;
> -typedef struct vlc_video_context
> +typedef struct vlc_video_context vlc_video_context;
> +
> +struct vlc_video_context_operations
> {
> - vlc_decoder_device *device;
> -} vlc_video_context;
> + void (*destroy)(void *priv);
> +};
>
> -VLC_API vlc_video_context *
> vlc_video_context_Create(vlc_decoder_device *);
> +VLC_API vlc_video_context *
> vlc_video_context_Create(vlc_decoder_device *,
> + size_t private_size,
> + const struct
> vlc_video_context_operations *);
> VLC_API void vlc_video_context_Release(vlc_video_context *);
>
> +VLC_API void *vlc_video_context_GetPrivate(vlc_video_context *);
> +VLC_API vlc_video_context *vlc_video_context_Hold(vlc_video_context *);
> +
> +/**
> + * Get the decoder device used by the device context.
> + *
> + * This will increment the refcount of the decoder device.
> + */
> +VLC_API vlc_decoder_device *vlc_video_context_HoldDevice(vlc_video_context *);
> +
> +
> /**
> * Video picture
> */
> diff --git a/modules/hw/nvdec/nvdec_gl.c b/modules/hw/nvdec/nvdec_gl.c
> index f3d5dfaf910..e819c510b8a 100644
> --- a/modules/hw/nvdec/nvdec_gl.c
> +++ b/modules/hw/nvdec/nvdec_gl.c
> @@ -153,10 +153,9 @@ static int Open(vlc_object_t *obj)
> if (!is_nvdec_opaque(tc->fmt.i_chroma))
> return VLC_EGENERIC;
>
> - vlc_decoder_device *device = tc->vctx->device;
> + vlc_decoder_device *device = vlc_video_context_HoldDevice(tc->vctx);
> if (device == NULL || device->type != VLC_DECODER_DEVICE_NVDEC)
> return VLC_EGENERIC;
> - device = vlc_decoder_device_Hold(device);
>
> converter_sys_t *p_sys = vlc_obj_malloc(VLC_OBJECT(tc),
> sizeof(*p_sys));
> if (unlikely(p_sys == NULL))
> diff --git a/modules/video_chroma/d3d11_fmt.h
> b/modules/video_chroma/d3d11_fmt.h
> index 8afe82bd801..fea8064f690 100644
> --- a/modules/video_chroma/d3d11_fmt.h
> +++ b/modules/video_chroma/d3d11_fmt.h
> @@ -107,7 +107,7 @@ static inline d3d11_decoder_device_t
> *GetD3D11OpaqueDevice(vlc_decoder_device *d
>
> static inline d3d11_decoder_device_t
> *GetD3D11OpaqueContext(vlc_video_context *vctx)
> {
> - vlc_decoder_device *device = vctx ? vctx->device : NULL;
> + vlc_decoder_device *device = vctx ?
> vlc_video_context_HoldDevice(vctx) : NULL;
> if (unlikely(device == NULL))
> return NULL;
> d3d11_decoder_device_t *res = NULL;
> @@ -116,6 +116,7 @@ static inline d3d11_decoder_device_t
> *GetD3D11OpaqueContext(vlc_video_context *v
> assert(device->opaque != NULL);
> res = GetD3D11OpaqueDevice(device);
> }
> + vlc_decoder_device_Release(device);
> return res;
> }
>
> diff --git a/modules/video_chroma/d3d9_fmt.h
> b/modules/video_chroma/d3d9_fmt.h
> index eef89fa3c45..f52068439a1 100644
> --- a/modules/video_chroma/d3d9_fmt.h
> +++ b/modules/video_chroma/d3d9_fmt.h
> @@ -94,7 +94,7 @@ static inline d3d9_decoder_device_t
> *GetD3D9OpaqueDevice(vlc_decoder_device *dev
>
> static inline d3d9_decoder_device_t
> *GetD3D9OpaqueContext(vlc_video_context *vctx)
> {
> - vlc_decoder_device *device = vctx ? vctx->device : NULL;
> + vlc_decoder_device *device = vctx ?
> vlc_video_context_HoldDevice(vctx) : NULL;
> if (unlikely(device == NULL))
> return NULL;
> d3d9_decoder_device_t *res = NULL;
> @@ -103,6 +103,7 @@ static inline d3d9_decoder_device_t
> *GetD3D9OpaqueContext(vlc_video_context *vct
> assert(device->opaque != NULL);
> res = GetD3D9OpaqueDevice(device);
> }
> + vlc_decoder_device_Release(device);
> return res;
> }
>
> diff --git a/modules/video_output/opengl/converter_vaapi.c
> b/modules/video_output/opengl/converter_vaapi.c
> index 9f6dde70b5d..98ae4a13960 100644
> --- a/modules/video_output/opengl/converter_vaapi.c
> +++ b/modules/video_output/opengl/converter_vaapi.c
> @@ -224,11 +224,12 @@ tc_vaegl_get_pool(const opengl_tex_converter_t
> *tc, unsigned requested_count)
> vlc_object_t *o = VLC_OBJECT(tc->gl);
> struct priv *priv = tc->priv;
>
> - vlc_decoder_device *dec_device = tc->vctx->device;
> + vlc_decoder_device *dec_device = vlc_video_context_HoldDevice(tc->vctx);
> picture_pool_t *pool =
> vlc_vaapi_PoolNew(VLC_OBJECT(tc->gl), dec_device, priv->vadpy,
> requested_count, &priv->va_surface_ids, &tc->fmt,
> true);
> + vlc_decoder_device_Release(dec_device);
> if (!pool)
> return NULL;
>
> @@ -333,20 +334,29 @@ Open(vlc_object_t *obj)
>
> if (tc->vctx == NULL)
> return VLC_EGENERIC;
> - vlc_decoder_device *dec_device = tc->vctx->device;
> + vlc_decoder_device *dec_device = vlc_video_context_HoldDevice(tc->vctx);
> if (dec_device->type != VLC_DECODER_DEVICE_VAAPI
> || !vlc_vaapi_IsChromaOpaque(tc->fmt.i_chroma)
> || tc->gl->ext != VLC_GL_EXT_EGL
> || tc->gl->egl.createImageKHR == NULL
> || tc->gl->egl.destroyImageKHR == NULL)
> + {
> + vlc_decoder_device_Release(dec_device);
> return VLC_EGENERIC;
> + }
>
> if (!vlc_gl_StrHasToken(tc->glexts, "GL_OES_EGL_image"))
> + {
> + vlc_decoder_device_Release(dec_device);
> return VLC_EGENERIC;
> + }
>
> const char *eglexts = tc->gl->egl.queryString(tc->gl,
> EGL_EXTENSIONS);
> if (eglexts == NULL || !vlc_gl_StrHasToken(eglexts,
> "EGL_EXT_image_dma_buf_import"))
> + {
> + vlc_decoder_device_Release(dec_device);
> return VLC_EGENERIC;
> + }
>
> struct priv *priv = tc->priv = calloc(1, sizeof(struct priv));
> if (unlikely(tc->priv == NULL))
> @@ -391,8 +401,11 @@ Open(vlc_object_t *obj)
> tc->pf_update = tc_vaegl_update;
> tc->pf_get_pool = tc_vaegl_get_pool;
>
> + vlc_decoder_device_Release(dec_device);
> +
> return VLC_SUCCESS;
> error:
> + vlc_decoder_device_Release(dec_device);
> free(priv);
> return VLC_EGENERIC;
> }
> diff --git a/modules/video_output/opengl/converter_vdpau.c
> b/modules/video_output/opengl/converter_vdpau.c
> index 296c47adaf0..06596a490bb 100644
> --- a/modules/video_output/opengl/converter_vdpau.c
> +++ b/modules/video_output/opengl/converter_vdpau.c
> @@ -62,10 +62,13 @@ static picture_pool_t *
> tc_vdpau_gl_get_pool(opengl_tex_converter_t const *tc,
> unsigned int requested_count)
> {
> - vlc_decoder_device *dec_device = tc->vctx->device;
> - return vlc_vdp_output_pool_create(dec_device->opaque,
> + vlc_decoder_device *dec_device =
> vlc_video_context_HoldDevice(tc->vctx);
> + picture_pool_t *pool;
> + pool = vlc_vdp_output_pool_create(dec_device->opaque,
> VDP_RGBA_FORMAT_B8G8R8A8,
> &tc->fmt, requested_count);
> + vlc_decoder_device_Release(dec_device);
> + return pool;
> }
>
> static int
> @@ -113,8 +116,9 @@ Close(vlc_object_t *obj)
> {
> opengl_tex_converter_t *tc = (void *)obj;
> _glVDPAUFiniNV(); assert(tc->vt->GetError() == GL_NO_ERROR);
> - vlc_decoder_device *dec_device = tc->vctx->device;
> + vlc_decoder_device *dec_device = vlc_video_context_HoldDevice(tc->vctx);
> vdp_release_x11(dec_device->opaque);
> + vlc_decoder_device_Release(dec_device);
> }
>
> static int
> @@ -123,14 +127,17 @@ Open(vlc_object_t *obj)
> opengl_tex_converter_t *tc = (void *) obj;
> if (tc->vctx == NULL)
> return VLC_EGENERIC;
> - vlc_decoder_device *dec_device = tc->vctx->device;
> + vlc_decoder_device *dec_device = vlc_video_context_HoldDevice(tc->vctx);
> if (dec_device->type != VLC_DECODER_DEVICE_VDPAU
> || (tc->fmt.i_chroma != VLC_CODEC_VDPAU_VIDEO_420
> && tc->fmt.i_chroma != VLC_CODEC_VDPAU_VIDEO_422
> && tc->fmt.i_chroma != VLC_CODEC_VDPAU_VIDEO_444)
> || !vlc_gl_StrHasToken(tc->glexts, "GL_NV_vdpau_interop")
> || tc->gl->surface->type != VOUT_WINDOW_TYPE_XID)
> + {
> + vlc_decoder_device_Release(dec_device);
> return VLC_EGENERIC;
> + }
>
> tc->fmt.i_chroma = VLC_CODEC_VDPAU_OUTPUT;
>
> @@ -143,6 +150,7 @@ Open(vlc_object_t *obj)
> VDP_FUNC_ID_GET_PROC_ADDRESS, &vdp_gpa)
> != VDP_STATUS_OK)
> {
> + vlc_decoder_device_Release(dec_device);
> vdp_release_x11(vdp);
> return VLC_EGENERIC;
> }
> @@ -151,6 +159,7 @@ Open(vlc_object_t *obj)
> _##fct = vlc_gl_GetProcAddress(tc->gl, #fct); \
> if (!_##fct) \
> { \
> + vlc_decoder_device_Release(dec_device); \
> vdp_release_x11(vdp); \
> return VLC_EGENERIC; \
> }
> @@ -167,6 +176,8 @@ Open(vlc_object_t *obj)
>
> INTEROP_CALL(glVDPAUInitNV, (void *)(uintptr_t)device, vdp_gpa);
>
> + vlc_decoder_device_Release(dec_device);
> +
> tc->fshader = opengl_fragment_shader_init(tc, GL_TEXTURE_2D,
> VLC_CODEC_RGB32,
> COLOR_SPACE_UNDEF);
> diff --git a/modules/video_output/splitter.c b/modules/video_output/splitter.c
> index 4cebe79ebbb..f1ca7fdf36b 100644
> --- a/modules/video_output/splitter.c
> +++ b/modules/video_output/splitter.c
> @@ -31,6 +31,7 @@
> #include <vlc_common.h>
> #include <vlc_modules.h>
> #include <vlc_plugin.h>
> +#include <vlc_codec.h>
> #include <vlc_vout_display.h>
> #include <vlc_video_splitter.h>
>
> @@ -289,8 +290,11 @@ static int vlc_vidsplit_Open(vout_display_t *vd,
> }
>
> vdcfg.window = part->window;
> + vlc_decoder_device *dec_device = vlc_video_context_HoldDevice(ctx);
> vout_display_t *display = vout_display_New(obj, &output->fmt, &vdcfg,
> - ctx->device, modname, NULL);
> + dec_device, modname, NULL);
> + if (dec_device)
> + vlc_decoder_device_Release(dec_device);
> if (display == NULL) {
> vout_window_Disable(part->window);
> vout_window_Delete(part->window);
> diff --git a/src/input/decoder_helpers.c b/src/input/decoder_helpers.c
> index 6a9a106aefa..e12fd836a3d 100644
> --- a/src/input/decoder_helpers.c
> +++ b/src/input/decoder_helpers.c
> @@ -31,6 +31,7 @@
> #include <vlc_atomic.h>
> #include <vlc_meta.h>
> #include <vlc_modules.h>
> +#include <vlc_picture.h>
> #include "libvlc.h"
>
> void decoder_Init( decoder_t *p_dec, const es_format_t *restrict p_fmt )
> @@ -193,15 +194,53 @@ vlc_decoder_device_Release(vlc_decoder_device *device)
>
> /* video context */
>
> -vlc_video_context * vlc_video_context_Create(vlc_decoder_device
> *device)
> +struct vlc_video_context
> {
> - vlc_video_context *vctx = malloc(sizeof(*vctx));
> + vlc_atomic_rc_t rc;
> + vlc_decoder_device *device;
> + const struct vlc_video_context_operations *ops;
> + size_t private_size;
> + uint8_t private[];
> +};
> +
> +vlc_video_context * vlc_video_context_Create(vlc_decoder_device
> *device,
> + size_t private_size,
> + const struct
> vlc_video_context_operations *ops)
> +{
> + vlc_video_context *vctx = malloc(sizeof(*vctx) + private_size);
> if (unlikely(vctx == NULL))
> return NULL;
> + vlc_atomic_rc_init( &vctx->rc );
> + vctx->private_size = private_size;
> vctx->device = device;
> + vctx->ops = ops;
> return vctx;
> }
> +
> +void *vlc_video_context_GetPrivate(vlc_video_context *vctx)
> +{
> + return &vctx->private;
> +}
> +
> +vlc_video_context *vlc_video_context_Hold(vlc_video_context *vctx)
> +{
> + vlc_atomic_rc_inc( &vctx->rc );
> + return vctx;
> +}
> +
> void vlc_video_context_Release(vlc_video_context *vctx)
> {
> - free(vctx);
> + if ( vlc_atomic_rc_dec( &vctx->rc ) )
> + {
> + if (vctx->device)
> + vlc_decoder_device_Release( vctx->device );
> + if ( vctx->ops && vctx->ops->destroy )
> + vctx->ops->destroy( vlc_video_context_GetPrivate(vctx) );
> + free(vctx);
> + }
> }
> +
> +vlc_decoder_device* vlc_video_context_HoldDevice(vlc_video_context
> *vctx)
> +{
> + return vlc_decoder_device_Hold( vctx->device );
> +}
> \ No newline at end of file
> diff --git a/src/libvlccore.sym b/src/libvlccore.sym
> index ab065f94bf4..610f98bfcaa 100644
> --- a/src/libvlccore.sym
> +++ b/src/libvlccore.sym
> @@ -943,3 +943,6 @@ vlc_media_tree_Preparse
> vlc_viewpoint_to_4x4
> vlc_video_context_Create
> vlc_video_context_Release
> +vlc_video_context_GetPrivate
> +vlc_video_context_Hold
> +vlc_video_context_HoldDevice
> diff --git a/src/video_output/display.c b/src/video_output/display.c
> index 7b550334dba..9ae040039cf 100644
> --- a/src/video_output/display.c
> +++ b/src/video_output/display.c
> @@ -770,7 +770,7 @@ vout_display_t *vout_display_New(vlc_object_t
> *parent,
> if (owner)
> vd->owner = *owner;
>
> - vlc_video_context *fake_vctx =
> vlc_video_context_Create(dec_device);
> + vlc_video_context *fake_vctx =
> vlc_video_context_Create(dec_device, 0, NULL);
>
> if (vlc_module_load(vd, "vout display", module, module && *module != '\0',
> vout_display_start, vd, &osys->cfg, &vd->fmt,
> --
> 2.17.1
>
> _______________________________________________
> 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