[vlc-devel] [PATCH 1/2] nvdec: use a reference-counted device picture pool
quentin.chateau at deepskycorp.com
quentin.chateau at deepskycorp.com
Tue Mar 24 15:00:45 CET 2020
From: Quentin Chateau <quentin.chateau at deepskycorp.com>
This commit fixes an issue where CUDA device memory
could be used after the picture pool was freed by the
decoder.
The picture_pool_t and array of CUdeviceptr is replaced
by a new reference-counted pool: nvdec_pool_t
All pool-related code is moved to a new nvdec_pool.c file.
The picture context is generated by the pool.
The new picture pool holds a reference to the decoder
device to make sure the CUDA functions and context are
not destroyed before the pool.
In the picture context, bufferHeight changed: it used
to contain the number of pixel lines.
It now contains the height of the buffer such as
bufferHeight * bufferPitch = bufferSize
---
modules/hw/nvdec/Makefile.am | 3 +-
modules/hw/nvdec/nvdec.c | 125 +++++-----------------
modules/hw/nvdec/nvdec_fmt.h | 12 +--
modules/hw/nvdec/nvdec_pool.c | 193 ++++++++++++++++++++++++++++++++++
modules/hw/nvdec/nvdec_pool.h | 46 ++++++++
5 files changed, 268 insertions(+), 111 deletions(-)
create mode 100644 modules/hw/nvdec/nvdec_pool.c
create mode 100644 modules/hw/nvdec/nvdec_pool.h
diff --git a/modules/hw/nvdec/Makefile.am b/modules/hw/nvdec/Makefile.am
index de5fba03ab..5c2af212e4 100644
--- a/modules/hw/nvdec/Makefile.am
+++ b/modules/hw/nvdec/Makefile.am
@@ -5,7 +5,8 @@ libnvdec_plugin_la_SOURCES = \
packetizer/hxxx_nal.h packetizer/hxxx_nal.c \
packetizer/h264_nal.c packetizer/h264_nal.h \
packetizer/hevc_nal.c packetizer/hevc_nal.h \
- hw/nvdec/nvdec_fmt.h
+ hw/nvdec/nvdec_fmt.h hw/nvdec/nvdec_pool.c \
+ hw/nvdec/nvdec_pool.h
libnvdec_plugin_la_LIBADD = $(LIBDL)
if HAVE_NVDEC
codec_LTLIBRARIES += libnvdec_plugin.la
diff --git a/modules/hw/nvdec/nvdec.c b/modules/hw/nvdec/nvdec.c
index 76aec6a246..8bfea6aa4a 100644
--- a/modules/hw/nvdec/nvdec.c
+++ b/modules/hw/nvdec/nvdec.c
@@ -36,6 +36,7 @@
#include <ffnvcodec/dynlink_loader.h>
#include "../../codec/hxxx_helper.h"
#include "nvdec_fmt.h"
+#include "nvdec_pool.h"
static int OpenDecoder(vlc_object_t *);
static void CloseDecoder(vlc_object_t *);
@@ -73,11 +74,11 @@ vlc_module_end ()
/* */
#define MAX_HXXX_SURFACES (16 + 1)
#define NVDEC_DISPLAY_SURFACES 1
-#define MAX_POOL_SIZE 4 // number of in-flight buffers, if more are needed the decoder waits
-
#define OUTPUT_WIDTH_ALIGN 16
+
typedef struct nvdec_ctx {
+ vlc_decoder_device *dec_device;
decoder_device_nvdec_t *devsys;
CuvidFunctions *cuvidFunctions;
CUVIDDECODECAPS selectedDecoder;
@@ -96,9 +97,8 @@ typedef struct nvdec_ctx {
bool b_nvparser_success;
size_t decoderHeight;
- CUdeviceptr outputDevicePtr[MAX_POOL_SIZE];
unsigned int outputPitch;
- picture_pool_t *out_pool;
+ nvdec_pool_t *nvdec_pool;
vlc_video_context *vctx_out;
} nvdec_ctx_t;
@@ -180,16 +180,10 @@ static int CUDAAPI HandleVideoSequence(void *p_opaque, CUVIDEOFORMAT *p_format)
if ( is_nvdec_opaque(p_dec->fmt_out.video.i_chroma) )
{
- for (size_t i=0; i < ARRAY_SIZE(p_sys->outputDevicePtr); i++)
- {
- CALL_CUDA_DEC(cuMemFree, p_sys->outputDevicePtr[i]);
- p_sys->outputDevicePtr[i] = 0;
- }
-
- if (p_sys->out_pool)
+ if (p_sys->nvdec_pool)
{
- picture_pool_Release(p_sys->out_pool);
- p_sys->out_pool = NULL;
+ nvdec_pool_Release(p_sys->nvdec_pool);
+ p_sys->nvdec_pool = NULL;
}
}
@@ -259,44 +253,12 @@ static int CUDAAPI HandleVideoSequence(void *p_opaque, CUVIDEOFORMAT *p_format)
vlc_assert_unreachable();
}
- picture_t *pics[ARRAY_SIZE(p_sys->outputDevicePtr)];
- for (size_t i=0; i < ARRAY_SIZE(p_sys->outputDevicePtr); i++)
- {
- ret = CALL_CUDA_DEC(cuMemAlloc, &p_sys->outputDevicePtr[i], ByteWidth * Height);
- if (ret != VLC_SUCCESS || p_sys->outputDevicePtr[i] == 0)
- goto clean_pics;
- picture_resource_t res = {
- .p_sys = (void*)(uintptr_t)i,
- };
- pics[i] = picture_NewFromResource( &p_dec->fmt_out.video, &res );
- if (unlikely(pics[i] == NULL))
- {
- msg_Dbg(p_dec, "failed to get a picture for the buffer");
- ret = VLC_ENOMEM;
- goto clean_pics;
- }
- continue;
-clean_pics:
- if (p_sys->outputDevicePtr[i])
- {
- CALL_CUDA_DEC(cuMemFree, p_sys->outputDevicePtr[i]);
- p_sys->outputDevicePtr[i] = 0;
- }
- if (i > 0)
- {
- while (i--)
- {
- picture_Release(pics[i]);
- CALL_CUDA_DEC(cuMemFree, p_sys->outputDevicePtr[i]);
- p_sys->outputDevicePtr[i] = 0;
- }
- }
- break;
- }
- if (ret != VLC_SUCCESS)
+ p_sys->nvdec_pool = nvdec_pool_Create(p_sys->dec_device,
+ &p_dec->fmt_out.video,
+ ByteWidth,
+ Height);
+ if (p_sys->nvdec_pool == NULL)
goto cuda_error;
-
- p_sys->out_pool = picture_pool_New( ARRAY_SIZE(p_sys->outputDevicePtr), pics );
}
p_sys->decoderHeight = p_format->coded_height;
@@ -329,25 +291,6 @@ static int CUDAAPI HandlePictureDecode(void *p_opaque, CUVIDPICPARAMS *p_picpara
return (ret == VLC_SUCCESS);
}
-static void NVDecCtxDestroy(struct picture_context_t *picctx)
-{
- pic_context_nvdec_t *srcpic = container_of(picctx, pic_context_nvdec_t, ctx);
- free(srcpic);
-}
-
-static struct picture_context_t *NVDecCtxClone(struct picture_context_t *srcctx)
-{
- pic_context_nvdec_t *clonectx = malloc(sizeof(*clonectx));
- if (unlikely(clonectx == NULL))
- return NULL;
- pic_context_nvdec_t *srcpic = container_of(srcctx, pic_context_nvdec_t, ctx);
-
- *clonectx = *srcpic;
- vlc_video_context_Hold(clonectx->ctx.vctx);
- return &clonectx->ctx;
-}
-
-
static int CUDAAPI HandlePictureDisplay(void *p_opaque, CUVIDPARSERDISPINFO *p_dispinfo)
{
decoder_t *p_dec = (decoder_t *) p_opaque;
@@ -364,9 +307,10 @@ static int CUDAAPI HandlePictureDisplay(void *p_opaque, CUVIDPARSERDISPINFO *p_d
if ( is_nvdec_opaque(p_dec->fmt_out.video.i_chroma) )
{
- p_pic = picture_pool_Wait(p_sys->out_pool);
+ p_pic = nvdec_pool_Wait(p_sys->nvdec_pool);
if (unlikely(p_pic == NULL))
return 0;
+ pic_context_nvdec_t *picctx = nvdec_pool_GetPictureContext(p_pic);
result = CALL_CUDA_DEC(cuCtxPushCurrent, p_sys->devsys->cuCtx);
if (unlikely(result != VLC_SUCCESS))
@@ -383,19 +327,6 @@ static int CUDAAPI HandlePictureDisplay(void *p_opaque, CUVIDPARSERDISPINFO *p_d
if (result != VLC_SUCCESS)
goto error;
- // put a new context in the output picture
- pic_context_nvdec_t *picctx = malloc(sizeof(*picctx));
- if (unlikely(picctx == NULL))
- goto error;
- picctx->ctx = (picture_context_t) {
- NVDecCtxDestroy, NVDecCtxClone,
- p_sys->vctx_out,
- };
- uintptr_t pool_idx = (uintptr_t)p_pic->p_sys;
- picctx->devicePtr = p_sys->outputDevicePtr[pool_idx];
- picctx->bufferPitch = p_sys->outputPitch;
- picctx->bufferHeight = p_sys->decoderHeight;
-
size_t srcY = 0;
size_t dstY = 0;
if (p_pic->format.i_chroma == VLC_CODEC_NVDEC_OPAQUE_444 || p_pic->format.i_chroma == VLC_CODEC_NVDEC_OPAQUE_444_16B)
@@ -415,10 +346,8 @@ static int CUDAAPI HandlePictureDisplay(void *p_opaque, CUVIDPARSERDISPINFO *p_d
};
result = CALL_CUDA_DEC(cuMemcpy2DAsync, &cu_cpy, 0);
if (unlikely(result != VLC_SUCCESS))
- {
- free(picctx);
goto error;
- }
+
srcY += picctx->bufferHeight;
dstY += p_sys->decoderHeight;
}
@@ -442,16 +371,12 @@ static int CUDAAPI HandlePictureDisplay(void *p_opaque, CUVIDPARSERDISPINFO *p_d
cu_cpy.Height >>= 1;
result = CALL_CUDA_DEC(cuMemcpy2DAsync, &cu_cpy, 0);
if (unlikely(result != VLC_SUCCESS))
- {
- free(picctx);
goto error;
- }
+
srcY += picctx->bufferHeight;
dstY += p_sys->decoderHeight;
}
}
- p_pic->context = &picctx->ctx;
- vlc_video_context_Hold(picctx->ctx.vctx);
}
else
{
@@ -771,17 +696,16 @@ static int OpenDecoder(vlc_object_t *p_this)
return VLC_EGENERIC;
}
- vlc_decoder_device *dec_device = decoder_GetDecoderDevice( p_dec );
- if (dec_device == NULL)
+ p_sys->dec_device = decoder_GetDecoderDevice( p_dec );
+ if (p_sys->dec_device == NULL)
return VLC_ENOOBJ;
- p_sys->devsys = GetNVDECOpaqueDevice(dec_device);
+ p_sys->devsys = GetNVDECOpaqueDevice(p_sys->dec_device);
if (p_sys->devsys == NULL)
{
- vlc_decoder_device_Release(dec_device);
+ vlc_decoder_device_Release(p_sys->dec_device);
return VLC_ENOOBJ;
}
- p_sys->vctx_out = vlc_video_context_Create( dec_device, VLC_VIDEO_CONTEXT_NVDEC, 0, NULL );
- vlc_decoder_device_Release(dec_device);
+ p_sys->vctx_out = vlc_video_context_Create( p_sys->dec_device, VLC_VIDEO_CONTEXT_NVDEC, 0, NULL );
if (unlikely(p_sys->vctx_out == NULL))
{
msg_Err(p_dec, "failed to create a video context");
@@ -998,10 +922,8 @@ static void CloseDecoder(vlc_object_t *p_this)
CALL_CUDA_DEC(cuCtxPushCurrent, p_sys->devsys->cuCtx);
CALL_CUDA_DEC(cuCtxPopCurrent, NULL);
- for (size_t i=0; i < ARRAY_SIZE(p_sys->outputDevicePtr); i++)
- CALL_CUDA_DEC(cuMemFree, p_sys->outputDevicePtr[i]);
- if (p_sys->out_pool)
- picture_pool_Release(p_sys->out_pool);
+ if (p_sys->nvdec_pool)
+ nvdec_pool_Release(p_sys->nvdec_pool);
if (p_sys->cudecoder)
CALL_CUVID(cuvidDestroyDecoder, p_sys->cudecoder);
if (p_sys->cuparser)
@@ -1011,6 +933,7 @@ static void CloseDecoder(vlc_object_t *p_this)
cuvid_free_functions(&p_sys->cuvidFunctions);
if (p_sys->b_is_hxxx)
hxxx_helper_clean(&p_sys->hh);
+ vlc_decoder_device_Release(p_sys->dec_device);
}
/** Decoder Device **/
diff --git a/modules/hw/nvdec/nvdec_fmt.h b/modules/hw/nvdec/nvdec_fmt.h
index 25784cca6d..d8598f7f7a 100644
--- a/modules/hw/nvdec/nvdec_fmt.h
+++ b/modules/hw/nvdec/nvdec_fmt.h
@@ -23,8 +23,11 @@
#ifndef VLC_VIDEOCHROMA_NVDEC_FMT_H_
#define VLC_VIDEOCHROMA_NVDEC_FMT_H_
+#include <vlc_codec.h>
#include <ffnvcodec/dynlink_loader.h>
+#include "nvdec_pool.h"
+
typedef struct {
CudaFunctions *cudaFunctions;
@@ -60,13 +63,4 @@ static inline bool is_nvdec_opaque(vlc_fourcc_t fourcc)
fourcc == VLC_CODEC_NVDEC_OPAQUE_444_16B;
}
-/* for VLC_CODEC_NVDEC_OPAQUE / VLC_CODEC_NVDEC_OPAQUE_16B */
-typedef struct
-{
- picture_context_t ctx;
- CUdeviceptr devicePtr;
- unsigned int bufferPitch;
- unsigned int bufferHeight;
-} pic_context_nvdec_t;
-
#endif /* include-guard */
diff --git a/modules/hw/nvdec/nvdec_pool.c b/modules/hw/nvdec/nvdec_pool.c
new file mode 100644
index 0000000000..e989c06207
--- /dev/null
+++ b/modules/hw/nvdec/nvdec_pool.c
@@ -0,0 +1,193 @@
+/*****************************************************************************
+ * nvdec_pool.c: NVDEC memory pool
+ *****************************************************************************
+ * Copyright (C) 2020 VLC authors and VideoLAN
+ *
+ * Authors: Quentin Chateau <quentin.chateau at gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
+ *****************************************************************************/
+#ifdef HAVE_CONFIG_H
+# include "config.h"
+#endif
+
+#include <vlc_common.h>
+#include <vlc_atomic.h>
+#include <vlc_picture_pool.h>
+
+#include <ffnvcodec/dynlink_loader.h>
+
+#include "nvdec_fmt.h"
+#include "nvdec_pool.h"
+
+#define CALL_CUDA_POOL(func, ...) CudaCheckErr(VLC_OBJECT(pool->dec_dev), pool->nvdec_dev->cudaFunctions, pool->nvdec_dev->cudaFunctions->func(__VA_ARGS__), #func)
+#define MAX_POOL_SIZE 4 // number of in-flight buffers, if more are needed the decoder waits
+
+struct nvdec_pool_t {
+ vlc_decoder_device *dec_dev;
+ decoder_device_nvdec_t *nvdec_dev;
+
+ size_t buffer_pitch;
+ size_t buffer_height;
+ CUdeviceptr device_ptrs[MAX_POOL_SIZE];
+ picture_pool_t *picture_pool;
+
+ vlc_atomic_rc_t rc;
+};
+
+static void nvdec_pool_Destroy(nvdec_pool_t *pool)
+{
+ CALL_CUDA_POOL(cuCtxPushCurrent, pool->nvdec_dev->cuCtx);
+ for (size_t i=0; i < ARRAY_SIZE(pool->device_ptrs); i++)
+ CALL_CUDA_POOL(cuMemFree, pool->device_ptrs[i]);
+ CALL_CUDA_POOL(cuCtxPopCurrent, NULL);
+
+ picture_pool_Release(pool->picture_pool);
+ vlc_decoder_device_Release(pool->dec_dev);
+}
+
+void nvdec_pool_AddRef(nvdec_pool_t *pool)
+{
+ vlc_atomic_rc_inc(&pool->rc);
+}
+
+void nvdec_pool_Release(nvdec_pool_t *pool)
+{
+ if (!vlc_atomic_rc_dec(&pool->rc))
+ return;
+
+ nvdec_pool_Destroy(pool);
+}
+
+nvdec_pool_t* nvdec_pool_Create(vlc_decoder_device *dec_dev,
+ const video_format_t *fmt,
+ size_t buffer_pitch,
+ size_t buffer_height)
+{
+ nvdec_pool_t *pool = calloc(1, sizeof(*pool));
+ if (!pool)
+ return NULL;
+
+ vlc_decoder_device_Hold(dec_dev);
+ pool->buffer_pitch = buffer_pitch;
+ pool->buffer_height = buffer_height;
+ pool->dec_dev = dec_dev;
+ pool->nvdec_dev = GetNVDECOpaqueDevice(dec_dev);
+ if (pool->nvdec_dev == NULL)
+ goto error;
+
+ int ret = CALL_CUDA_POOL(cuCtxPushCurrent, pool->nvdec_dev->cuCtx);
+ if (ret != VLC_SUCCESS)
+ goto error;
+
+ picture_t *pics[ARRAY_SIZE(pool->device_ptrs)] = {0};
+ for (size_t i=0; i < ARRAY_SIZE(pool->device_ptrs); i++)
+ {
+ ret = CALL_CUDA_POOL(cuMemAlloc,
+ &pool->device_ptrs[i],
+ buffer_pitch * buffer_height);
+ if (ret != VLC_SUCCESS || pool->device_ptrs[i] == 0)
+ goto free_pool;
+
+ picture_resource_t res = {
+ .p_sys = (void*)(uintptr_t)i,
+ };
+ pics[i] = picture_NewFromResource(fmt, &res);
+ if (!pics[i])
+ goto free_pool;
+ }
+
+ CALL_CUDA_POOL(cuCtxPopCurrent, NULL);
+
+ pool->picture_pool = picture_pool_New(ARRAY_SIZE(pool->device_ptrs), pics);
+ if (!pool->picture_pool)
+ goto free_pool;
+
+ vlc_atomic_rc_init(&pool->rc);
+ return pool;
+
+free_pool:
+ for (size_t i=0; i < ARRAY_SIZE(pool->device_ptrs); i++)
+ {
+ if (pool->device_ptrs[i] != 0)
+ CALL_CUDA_POOL(cuMemFree, pool->device_ptrs[i]);
+ if (pics[i] != NULL)
+ picture_Release(pics[i]);
+ }
+ CALL_CUDA_POOL(cuCtxPopCurrent, NULL);
+error:
+ vlc_decoder_device_Release(dec_dev);
+ free(pool);
+ return NULL;
+}
+
+static void nvdec_picture_CtxDestroy(struct picture_context_t *pic_ctx)
+{
+ pic_context_nvdec_t *nvdec_pic_ctx =
+ container_of(pic_ctx, pic_context_nvdec_t, ctx);
+ nvdec_pool_Release(nvdec_pic_ctx->pool);
+ free(nvdec_pic_ctx);
+}
+
+static struct picture_context_t *nvdec_picture_CtxClone(struct picture_context_t *src_ctx)
+{
+ pic_context_nvdec_t *nvdec_clone_ctx = malloc(sizeof(*nvdec_clone_ctx));
+ if (!nvdec_clone_ctx)
+ return NULL;
+
+ pic_context_nvdec_t *nvdec_src_ctx =
+ container_of(src_ctx, pic_context_nvdec_t, ctx);
+
+ *nvdec_clone_ctx = *nvdec_src_ctx;
+ nvdec_pool_AddRef(nvdec_clone_ctx->pool);
+
+ return &nvdec_clone_ctx->ctx;
+}
+
+picture_t* nvdec_pool_Wait(nvdec_pool_t *pool)
+{
+ picture_t *pic = picture_pool_Wait(pool->picture_pool);
+ if (!pic)
+ return NULL;
+
+ pic_context_nvdec_t *picctx = malloc(sizeof(*picctx));
+ if (!picctx)
+ goto error;
+
+ picctx->ctx = (picture_context_t) {
+ nvdec_picture_CtxDestroy,
+ nvdec_picture_CtxClone,
+ NULL,
+ };
+
+ uintptr_t pool_idx = (uintptr_t)pic->p_sys;
+ picctx->devicePtr = pool->device_ptrs[pool_idx];
+ picctx->bufferPitch = pool->buffer_pitch;
+ picctx->bufferHeight = pool->buffer_height;
+ picctx->pool = pool;
+ nvdec_pool_AddRef(picctx->pool);
+
+ pic->context = &picctx->ctx;
+ return pic;
+
+error:
+ picture_Release(pic);
+ return NULL;
+}
+
+pic_context_nvdec_t* nvdec_pool_GetPictureContext(picture_t *picture)
+{
+ return container_of(picture->context, pic_context_nvdec_t, ctx);
+}
\ No newline at end of file
diff --git a/modules/hw/nvdec/nvdec_pool.h b/modules/hw/nvdec/nvdec_pool.h
new file mode 100644
index 0000000000..b48a9b283a
--- /dev/null
+++ b/modules/hw/nvdec/nvdec_pool.h
@@ -0,0 +1,46 @@
+/*****************************************************************************
+ * nvdec_pool.h: NVDEC memory pool
+ *****************************************************************************
+ * Copyright (C) 2020 VLC authors and VideoLAN
+ *
+ * Authors: Quentin Chateau <quentin.chateau at gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
+ *****************************************************************************/
+
+#ifndef VLC_VIDEOCHROMA_NVDEC_POOL_H_
+#define VLC_VIDEOCHROMA_NVDEC_POOL_H_
+
+#include <vlc_codec.h>
+
+typedef struct nvdec_pool_t nvdec_pool_t;
+typedef struct {
+ picture_context_t ctx;
+ CUdeviceptr devicePtr;
+ unsigned int bufferPitch;
+ unsigned int bufferHeight;
+ nvdec_pool_t *pool;
+} pic_context_nvdec_t;
+
+void nvdec_pool_AddRef(nvdec_pool_t *pool);
+void nvdec_pool_Release(nvdec_pool_t *pool);
+nvdec_pool_t* nvdec_pool_Create(vlc_decoder_device *dec_dev,
+ const video_format_t *fmt,
+ size_t buffer_pitch,
+ size_t buffer_height);
+picture_t* nvdec_pool_Wait(nvdec_pool_t *pool);
+pic_context_nvdec_t* nvdec_pool_GetPictureContext(picture_t *picture);
+
+#endif /* include-guard */
--
2.17.1
More information about the vlc-devel
mailing list