diff mbox series

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

Message ID DB6PR0101MB221400A260DA0404389381348F619@DB6PR0101MB2214.eurprd01.prod.exchangelabs.com
State New
Headers show
Series [FFmpeg-devel,1/3] avcodec/nvdec: Check av_buffer_ref() | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Andreas Rheinhardt Aug. 6, 2022, 5:39 p.m. UTC
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(-)

Comments

Timo Rothenpieler Aug. 7, 2022, 8:50 a.m. UTC | #1
I'm not immediately convinced one is better than the other, but no 
strong opinion there, both approached work just fine.
diff mbox series

Patch

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)