[FFmpeg-devel] [PATCH 1/3] avcodec/nvdec: Check av_buffer_ref()

[FFmpeg-devel] [PATCH 1/3] avcodec/nvdec: Check av_buffer_ref()

[Andreas Rheinhardt]

It (unfortunately) involves an allocation and can therefore fail.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
I don't have any nvidia hardware, so all these patches are untested
(apart from ensuring that they compile without creating new warnings).

 libavcodec/nvdec.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/libavcodec/nvdec.c b/libavcodec/nvdec.c
index edff46d310..15665b83bb 100644
--- a/libavcodec/nvdec.c
+++ b/libavcodec/nvdec.c
@@ -532,8 +532,11 @@ static int nvdec_retrieve_data(void *logctx, AVFrame *frame)
     }
 
     unmap_data->idx = cf->idx;
-    unmap_data->idx_ref = av_buffer_ref(cf->idx_ref);
-    unmap_data->decoder_ref = av_buffer_ref(cf->decoder_ref);
+    if (!(unmap_data->idx_ref     = av_buffer_ref(cf->idx_ref)) ||
+        !(unmap_data->decoder_ref = av_buffer_ref(cf->decoder_ref))) {
+        ret = AVERROR(ENOMEM);
+        goto copy_fail;
+    }
 
     av_pix_fmt_get_chroma_sub_sample(hwctx->sw_format, &shift_h, &shift_v);
     for (i = 0; frame->linesize[i]; i++) {
-- 
2.34.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/3] avcodec/nvdec: Use av_buffer_replace() where appropriate

[Andreas Rheinhardt]

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/nvdec.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/libavcodec/nvdec.c b/libavcodec/nvdec.c
index 15665b83bb..fbaedf0b6b 100644
--- a/libavcodec/nvdec.c
+++ b/libavcodec/nvdec.c
@@ -524,12 +524,9 @@ static int nvdec_retrieve_data(void *logctx, AVFrame *frame)
         goto copy_fail;
     }
 
-    av_buffer_unref(&frame->hw_frames_ctx);
-    frame->hw_frames_ctx = av_buffer_ref(decoder->real_hw_frames_ref);
-    if (!frame->hw_frames_ctx) {
-        ret = AVERROR(ENOMEM);
+    ret = av_buffer_replace(&frame->hw_frames_ctx, decoder->real_hw_frames_ref);
+    if (ret < 0)
         goto copy_fail;
-    }
 
     unmap_data->idx = cf->idx;
     if (!(unmap_data->idx_ref     = av_buffer_ref(cf->idx_ref)) ||
-- 
2.34.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/3] avcodec/nvdec: Use av_buffer_replace() where appropriate

[Timo Rothenpieler]

First two patches LGTM
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 3/3] avcodec/nvdec: Redo handling of failure in nvdec_retrieve_data()

[Andreas Rheinhardt]

Said function has a block intended for failure on the main code path
that is jumped over when no error happens:

goto finish;
copy_fail:
    /* cleanup code */
finish:
    /* more code */
    return ret;

This commit changes this to the more common:

finish:
   /* more code */
   return ret;
copy_fail:
   /* cleanup code */
   goto finish;

Given that the cleanup code depends upon from where one jumps to it,
it is also split into two code blocks with their own labels, thereby
avoiding the check for which case one is in.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/nvdec.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/libavcodec/nvdec.c b/libavcodec/nvdec.c
index fbaedf0b6b..0a4fb30b42 100644
--- a/libavcodec/nvdec.c
+++ b/libavcodec/nvdec.c
@@ -513,26 +513,27 @@ static int nvdec_retrieve_data(void *logctx, AVFrame *frame)
     unmap_data = av_mallocz(sizeof(*unmap_data));
     if (!unmap_data) {
         ret = AVERROR(ENOMEM);
-        goto copy_fail;
+        goto early_fail;
     }
 
     frame->buf[1] = av_buffer_create((uint8_t *)unmap_data, sizeof(*unmap_data),
                                      nvdec_unmap_mapped_frame, (void*)devptr,
                                      AV_BUFFER_FLAG_READONLY);
     if (!frame->buf[1]) {
+        av_freep(&unmap_data);
         ret = AVERROR(ENOMEM);
-        goto copy_fail;
+        goto early_fail;
     }
 
     ret = av_buffer_replace(&frame->hw_frames_ctx, decoder->real_hw_frames_ref);
     if (ret < 0)
-        goto copy_fail;
+        goto late_fail;
 
     unmap_data->idx = cf->idx;
     if (!(unmap_data->idx_ref     = av_buffer_ref(cf->idx_ref)) ||
         !(unmap_data->decoder_ref = av_buffer_ref(cf->decoder_ref))) {
         ret = AVERROR(ENOMEM);
-        goto copy_fail;
+        goto late_fail;
     }
 
     av_pix_fmt_get_chroma_sub_sample(hwctx->sw_format, &shift_h, &shift_v);
@@ -542,19 +543,16 @@ static int nvdec_retrieve_data(void *logctx, AVFrame *frame)
         offset += pitch * (frame->height >> (i ? shift_v : 0));
     }
 
-    goto finish;
-
-copy_fail:
-    if (!frame->buf[1]) {
-        CHECK_CU(decoder->cvdl->cuvidUnmapVideoFrame(decoder->decoder, devptr));
-        av_freep(&unmap_data);
-    } else {
-        av_buffer_unref(&frame->buf[1]);
-    }
-
 finish:
     CHECK_CU(decoder->cudl->cuCtxPopCurrent(&dummy));
     return ret;
+
+early_fail:
+    CHECK_CU(decoder->cvdl->cuvidUnmapVideoFrame(decoder->decoder, devptr));
+    goto finish;
+late_fail:
+    av_buffer_unref(&frame->buf[1]);
+    goto finish;
 }
 
 int ff_nvdec_start_frame(AVCodecContext *avctx, AVFrame *frame)
-- 
2.34.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 3/3] avcodec/nvdec: Redo handling of failure in nvdec_retrieve_data()

[Timo Rothenpieler]

I'm not immediately convinced one is better than the other, but no 
strong opinion there, both approached work just fine.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".