[FFmpeg-devel] [PATCH] nvenc: Don't segfault on close if no cuda is available

[Philip Langdale]

On Wed, 30 Aug 2017 20:09:34 +0100
Mark Thompson <sw at jkqxz.net> wrote:

> ---
> """
> Cannot load libcuda.so.1
> 
> Thread 1 "ffmpeg_g" received signal SIGSEGV, Segmentation fault.
> 0x000055555657cd59 in ff_nvenc_encode_close (avctx=0x5555578ed920) at
> src/libavcodec/nvenc.c:1337 1337        cu_res =
> dl_fn->cuda_dl->cuCtxPushCurrent(ctx->cu_context); """
> 
> This only fixes the segfaults - some of the other stuff might want to
> be gated as well?
> 
> 
>  libavcodec/nvenc.c | 26 +++++++++++++++-----------
>  1 file changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
> index 8c4fd31fec..5586a8901f 100644
> --- a/libavcodec/nvenc.c
> +++ b/libavcodec/nvenc.c
> @@ -1334,10 +1334,12 @@ av_cold int
> ff_nvenc_encode_close(AVCodecContext *avctx) CUcontext dummy;
>      int i;
>  
> -    cu_res = dl_fn->cuda_dl->cuCtxPushCurrent(ctx->cu_context);
> -    if (cu_res != CUDA_SUCCESS) {
> -        av_log(avctx, AV_LOG_ERROR, "cuCtxPushCurrent failed\n");
> -        return AVERROR_EXTERNAL;
> +    if (dl_fn->cuda_dl) {
> +        cu_res = dl_fn->cuda_dl->cuCtxPushCurrent(ctx->cu_context);
> +        if (cu_res != CUDA_SUCCESS) {
> +            av_log(avctx, AV_LOG_ERROR, "cuCtxPushCurrent failed\n");
> +            return AVERROR_EXTERNAL;
> +        }
>      }
>  
>      /* the encoder has to be flushed before it can be closed */
> @@ -1381,14 +1383,16 @@ av_cold int
> ff_nvenc_encode_close(AVCodecContext *avctx)
> p_nvenc->nvEncDestroyEncoder(ctx->nvencoder); ctx->nvencoder = NULL;
>  
> -    cu_res = dl_fn->cuda_dl->cuCtxPopCurrent(&dummy);
> -    if (cu_res != CUDA_SUCCESS) {
> -        av_log(avctx, AV_LOG_ERROR, "cuCtxPopCurrent failed\n");
> -        return AVERROR_EXTERNAL;
> -    }
> +    if (dl_fn->cuda_dl) {
> +        cu_res = dl_fn->cuda_dl->cuCtxPopCurrent(&dummy);
> +        if (cu_res != CUDA_SUCCESS) {
> +            av_log(avctx, AV_LOG_ERROR, "cuCtxPopCurrent failed\n");
> +            return AVERROR_EXTERNAL;
> +        }
>  
> -    if (ctx->cu_context_internal)
> -        dl_fn->cuda_dl->cuCtxDestroy(ctx->cu_context_internal);
> +        if (ctx->cu_context_internal)
> +            dl_fn->cuda_dl->cuCtxDestroy(ctx->cu_context_internal);
> +    }
>      ctx->cu_context = ctx->cu_context_internal = NULL;
>  
>      nvenc_free_functions(&dl_fn->nvenc_dl);

It looks correct. I agree you probably want to short cut the cleanup if
you know it failed during function loading, but looking it over, the
remaining cleanup steps are safe.

--phil