[vlc-devel] [PATCH 1/3] nvdec: always use opaque chroma as output

Steve Lhomme robux4 at ycbcr.xyz
Fri Mar 20 09:32:30 CET 2020


Maybe that's an option we could have. But that's unlikely to happen.
Can you discuss more your usecases ? Maybe there are other ways.

Also there's one case where the opaque output is better than the CPU 
output: opengl display. So for most people on Linux. In that case the 
CUDA > texture copy is done from GPU to GPU. If we output CPU then we do 
GPU > CPU and that CPU buffer needs to be put back in the GPU.

On 2020-03-20 8:56, Quentin Chateau wrote:
> Well, my use case would benefit greatly from outputting CPU chromas, for 
> the exact reason you mention.
> 
> The idea behind this patch was rather to remove unused code...
> On Mar 20, 2020, at 08:38, Steve Lhomme <robux4 at ycbcr.xyz 
> <mailto:robux4 at ycbcr.xyz>> wrote:
> 
>     The code looks correct and mostly removes dead code, as in real life
>     there's not a case where the opaque output is refused.
> 
>     My main concern is the timing of when this conversion is done. Moving it
>     in the display thread just before displaying a picture is not optimal
>     and could result in frame drops. It's a bit different than other
>     hardware acceleration as we usually pair them with a display module that
>     can handle them with no converter. This is not possible with NVDEC.
> 
>     Until we have CUDA filters we would be better off only outputting CPU
>     chromas. The same amount of computatation would be done but it would
>     free up some time in the display thread to better drop late frames.
> 
>     In the long run this patch is the way to go and we should do chroma
>     conversions/deinterlacing outside of the display thread.
> 
>     On 2020-03-19 17:42, quentin.chateau at deepskycorp.com wrote:
> 
>         From: Quentin Chateau <quentin.chateau at deepskycorp.com>
> 
>         MapSurfaceChroma now maps cudaChroma to opaque chromas
>         ---
>         modules/hw/nvdec/nvdec.c | 95 +++
>         ------------------------------------------------------------------------
> 
>         modules/hw/nvdec/nvdec_fmt.h | 9 ----
>         modules/hw/nvdec/nvdec_gl.c | 3 --
>         3 files changed, 6 insertions(+), 101 deletions(-)
> 
>         diff --git a/modules/hw/nvdec/nvdec.c b/modules/hw/nvdec/nvdec.c
>         index 321815071d..9ee61e8c5e 100644
>         --- a/modules/hw/nvdec/nvdec.c
>         +++ b/modules/hw/nvdec/nvdec.c
>         @@ -112,14 +112,14 @@ static vlc_fourcc_t
>         MapSurfaceChroma(cudaVideoChromaFormat chroma, unsigned bitD
>         switch (chroma) {
>         case cudaVideoChromaFormat_420:
>         if (bitDepth <= 8)
>         - return VLC_CODEC_NV12;
>         + return VLC_CODEC_NVDEC_OPAQUE;
>         if (bitDepth <= 10)
>         - return VLC_CODEC_P010;
>         - return VLC_CODEC_P016;
>         + return VLC_CODEC_NVDEC_OPAQUE_10B;
>         + return VLC_CODEC_NVDEC_OPAQUE_16B;
>         case cudaVideoChromaFormat_444:
>         if (bitDepth <= 8)
>         - return VLC_CODEC_I444;
>         - return VLC_CODEC_I444_16L;
>         + return VLC_CODEC_NVDEC_OPAQUE_444;
>         + return VLC_CODEC_NVDEC_OPAQUE_444_16B;
>         default:
>         return 0;
>         }
>         @@ -130,17 +130,12 @@ static cudaVideoSurfaceFormat
>         MapSurfaceFmt(int i_vlc_fourcc)
>         switch (i_vlc_fourcc) {
>         case VLC_CODEC_NVDEC_OPAQUE_10B:
>         case VLC_CODEC_NVDEC_OPAQUE_16B:
>         - case VLC_CODEC_P010:
>         - case VLC_CODEC_P016:
>         return cudaVideoSurfaceFormat_P016;
>         case VLC_CODEC_NVDEC_OPAQUE:
>         - case VLC_CODEC_NV12:
>         return cudaVideoSurfaceFormat_NV12;
>         case VLC_CODEC_NVDEC_OPAQUE_444:
>         - case VLC_CODEC_I444:
>         return cudaVideoSurfaceFormat_YUV444;
>         case VLC_CODEC_NVDEC_OPAQUE_444_16B:
>         - case VLC_CODEC_I444_16L:
>         return cudaVideoSurfaceFormat_YUV444_16Bit;
>         default: vlc_assert_unreachable();
>         }
>         @@ -150,15 +145,6 @@ static int CUtoFMT(video_format_t *fmt,
>         const CUVIDEOFORMAT *p_format)
>         {
>         // bit depth and chroma
>         unsigned int i_bpp = p_format->bit_depth_luma_minus8 + 8;
>         - vlc_fourcc_t i_chroma;
>         - if (is_nvdec_opaque(fmt->i_chroma))
>         - i_chroma = fmt->i_chroma;
>         - else
>         - i_chroma = MapSurfaceChroma(p_format->chroma_format, i_bpp);
>         - if (i_chroma == 0)
>         - return VLC_EGENERIC;
>         -
>         - fmt->i_chroma = i_chroma;
>         // use the real padded size when we know it fmt->i_width =
>         p_format->coded_width;
>         fmt->i_height = p_format->coded_height;
>         fmt->i_x_offset = p_format->display_area.left;
>         @@ -178,8 +164,6 @@ static int CUDAAPI HandleVideoSequence(void
>         *p_opaque, CUVIDEOFORMAT *p_format)
>         nvdec_ctx_t *p_sys = p_dec->p_sys;
>         int ret;
> 
>         - 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]);
>         @@ -191,7 +175,6 @@ static int CUDAAPI HandleVideoSequence(void
>         *p_opaque, CUVIDEOFORMAT *p_format)
>         picture_pool_Release(p_sys->out_pool);
>         p_sys->out_pool = NULL;
>         }
>         - }
> 
>         // update vlc's output format using NVDEC parser's output
>         ret = CUtoFMT(&p_dec->fmt_out.video, p_format);
>         @@ -230,8 +213,6 @@ static int CUDAAPI HandleVideoSequence(void
>         *p_opaque, CUVIDEOFORMAT *p_format)
>         goto error;
> 
>         // ensure the output surfaces have the same pitch so copies can
>         work properly
>         - if ( is_nvdec_opaque(p_dec->fmt_out.video.i_chroma) )
>         - {
>         // get the real decoder pitch
>         CUdeviceptr frameDevicePtr = 0;
>         CUVIDPROCPARAMS params = {
>         @@ -297,8 +278,6 @@ clean_pics:
>         goto error;
> 
>         p_sys->out_pool = picture_pool_New(
>         ARRAY_SIZE(p_sys->outputDevicePtr), pics );
>         - }
>         -
>         p_sys->decoderHeight = p_format->coded_height;
> 
>         CALL_CUDA_DEC(cuCtxPopCurrent, NULL);
>         @@ -359,8 +338,6 @@ static int CUDAAPI HandlePictureDisplay(void
>         *p_opaque, CUVIDPARSERDISPINFO *p_d
>         };
>         int result;
> 
>         - if ( is_nvdec_opaque(p_dec->fmt_out.video.i_chroma) )
>         - {
>         p_pic = picture_pool_Wait(p_sys->out_pool);
>         if (unlikely(p_pic == NULL))
>         return 0;
>         @@ -449,49 +426,6 @@ static int CUDAAPI
>         HandlePictureDisplay(void *p_opaque, CUVIDPARSERDISPINFO *p_d
>         }
>         p_pic->context = &picctx->ctx;
>         vlc_video_context_Hold(picctx->ctx.vctx);
>         - }
>         - else
>         - {
>         - p_pic = decoder_NewPicture(p_dec);
>         - if (unlikely(p_pic == NULL))
>         - return 0;
>         -
>         - result = CALL_CUDA_DEC(cuCtxPushCurrent, p_sys->devsys->cuCtx);
>         - if (unlikely(result != VLC_SUCCESS))
>         - {
>         - picture_Release(p_pic);
>         - return 0;
>         - }
>         -
>         - unsigned int i_pitch;
>         -
>         - // Map decoded frame to a device pointer
>         - result = CALL_CUVID( cuvidMapVideoFrame, p_sys->cudecoder,
>         p_dispinfo->picture_index,
>         - &frameDevicePtr, &i_pitch, &params );
>         - if (result != VLC_SUCCESS)
>         - goto error;
>         -
>         - // Copy decoded frame into a new VLC picture
>         - size_t srcY = 0;
>         - for (int i_plane = 0; i_plane < p_pic->i_planes; i_plane++) {
>         - plane_t plane = p_pic->p[i_plane];
>         - CUDA_MEMCPY2D cu_cpy = {
>         - .srcMemoryType = CU_MEMORYTYPE_DEVICE,
>         - .srcDevice = frameDevicePtr,
>         - .srcY = srcY,
>         - .srcPitch = i_pitch,
>         - .dstMemoryType = CU_MEMORYTYPE_HOST,
>         - .dstHost = plane.p_pixels,
>         - .dstPitch = plane.i_pitch,
>         - .WidthInBytes = i_pitch,
>         - .Height = plane.i_visible_lines,
>         - };
>         - result = CALL_CUDA_DEC(cuMemcpy2D, &cu_cpy);
>         - if (result != VLC_SUCCESS)
>         - goto error;
>         - srcY += p_sys->decoderHeight;
>         - }
>         - }
> 
>         // Release surface on GPU
>         result = CALL_CUVID(cuvidUnmapVideoFrame, p_sys->cudecoder,
>         frameDevicePtr);
>         @@ -921,25 +855,8 @@ static int OpenDecoder(vlc_object_t *p_this)
>         goto error;
>         }
> 
>         - vlc_fourcc_t output_chromas[3];
>         + vlc_fourcc_t output_chromas[2];
>         size_t chroma_idx = 0;
>         - if (cudaChroma == cudaVideoChromaFormat_420)
>         - {
>         - if (i_depth_luma >= 16)
>         - output_chromas[chroma_idx++] = VLC_CODEC_NVDEC_OPAQUE_16B;
>         - else if (i_depth_luma > 8)
>         - output_chromas[chroma_idx++] = VLC_CODEC_NVDEC_OPAQUE_10B;
>         - else
>         - output_chromas[chroma_idx++] = VLC_CODEC_NVDEC_OPAQUE;
>         - }
>         - else if (cudaChroma == cudaVideoChromaFormat_444)
>         - {
>         - if (i_depth_luma > 8)
>         - output_chromas[chroma_idx++] = VLC_CODEC_NVDEC_OPAQUE_444_16B;
>         - else
>         - output_chromas[chroma_idx++] = VLC_CODEC_NVDEC_OPAQUE_444;
>         - }
>         -
>         output_chromas[chroma_idx++] = MapSurfaceChroma(cudaChroma,
>         i_depth_luma);
>         output_chromas[chroma_idx++] = 0;
> 
>         diff --git a/modules/hw/nvdec/nvdec_fmt.h
>         b/modules/hw/nvdec/nvdec_fmt.h
>         index 25784cca6d..d84672d8fb 100644
>         --- a/modules/hw/nvdec/nvdec_fmt.h
>         +++ b/modules/hw/nvdec/nvdec_fmt.h
>         @@ -51,15 +51,6 @@ static inline int CudaCheckErr(vlc_object_t
>         *obj, CudaFunctions *cudaFunctions,
>         return VLC_SUCCESS;
>         }
> 
>         -static inline bool is_nvdec_opaque(vlc_fourcc_t fourcc)
>         -{
>         - return fourcc == VLC_CODEC_NVDEC_OPAQUE ||
>         - fourcc == VLC_CODEC_NVDEC_OPAQUE_10B ||
>         - fourcc == VLC_CODEC_NVDEC_OPAQUE_16B ||
>         - fourcc == VLC_CODEC_NVDEC_OPAQUE_444 ||
>         - fourcc == VLC_CODEC_NVDEC_OPAQUE_444_16B;
>         -}
>         -
>         /* for VLC_CODEC_NVDEC_OPAQUE / VLC_CODEC_NVDEC_OPAQUE_16B */
>         typedef struct
>         {
>         diff --git a/modules/hw/nvdec/nvdec_gl.c
>         b/modules/hw/nvdec/nvdec_gl.c
>         index 137e730028..62a503a8e8 100644
>         --- a/modules/hw/nvdec/nvdec_gl.c
>         +++ b/modules/hw/nvdec/nvdec_gl.c
>         @@ -156,9 +156,6 @@ static void Close(vlc_object_t *obj)
>         static int Open(vlc_object_t *obj)
>         {
>         struct vlc_gl_interop *interop = (void *) obj;
>         - if (!is_nvdec_opaque(interop->fmt.i_chroma))
>         - return VLC_EGENERIC;
>         -
>         vlc_decoder_device *device =
>         vlc_video_context_HoldDevice(interop->vctx);
>         if (device == NULL || device->type != VLC_DECODER_DEVICE_NVDEC)
>         return VLC_EGENERIC;
>         -- 
>         2.17.1
> 
>         ------------------------------------------------------------------------
> 
>         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
> 
> 
> _______________________________________________
> 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