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

Submitted by Mark Thompson on Aug. 30, 2017, 7:09 p.m.

Details

Message ID 19d1faff-6281-f56c-ed31-86f8c09d8ed4@jkqxz.net
State New
Headers show

Commit Message

Mark Thompson Aug. 30, 2017, 7:09 p.m.
---
"""
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(-)

Comments

Philip Langdale Sept. 2, 2017, 2:58 a.m.
On Wed, 30 Aug 2017 20:09:34 +0100
Mark Thompson <sw@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
Timo Rothenpieler Sept. 2, 2017, 9:53 a.m.
This should already be fixed in master and release/3.3

Patch hide | download patch | download mbox

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);