diff mbox series

[FFmpeg-devel,4/9] avcodec/libxevd: Deduplicate code

Message ID AS8P250MB074461A4FA465E6F5B5E6AC58F592@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM
State Accepted
Commit cb9fb80c7fedfadfc306060a998207622519f916
Headers show
Series [FFmpeg-devel,1/9] avcodec/libxevd: Remove FF_CODEC_CAP_SETS_PKT_DTS cap | 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 Feb. 27, 2024, 7:58 p.m. UTC
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
The ownership semantics of the pkt_au packet seem crazy;
I have no clue when ownership has actually passed to libxevd
(or rather: when one can expect to get an image with this packet
attached returned from the decoder).
Furthermore, given that libxevd sees this just as an opaque pointer
(without free callback), we will leak AVPackets still stuck
in libxevd if a user closes this decoder with the decoder not
flushed.
I don't know how libxevd behaves on errors; if it discard a frame
in case of error, the associated packet will leak.
Trying to compile doesn't work unless one overrides dangling-pointer
warnings (which are treated as errors by the build system), so I don't
expect anything good from them.

 libavcodec/libxevd.c | 160 +++++++++++++++++--------------------------
 1 file changed, 61 insertions(+), 99 deletions(-)
diff mbox series

Patch

diff --git a/libavcodec/libxevd.c b/libavcodec/libxevd.c
index 0553ebfb06..33fd18f74d 100644
--- a/libavcodec/libxevd.c
+++ b/libavcodec/libxevd.c
@@ -252,6 +252,65 @@  static av_cold int libxevd_init(AVCodecContext *avctx)
     return 0;
 }
 
+static int libxevd_return_frame(AVCodecContext *avctx, AVFrame *frame,
+                                XEVD_IMGB *imgb, AVPacket **pkt_au)
+{
+    int ret;
+
+                        AVPacket* pkt_au_imgb = (AVPacket*)imgb->pdata[0];
+                        if(!pkt_au_imgb) {
+                            av_log(avctx, AV_LOG_ERROR, "Invalid data needed to fill frame properties\n");
+
+                            if (pkt_au)
+                                av_packet_free(pkt_au);
+                            av_frame_unref(frame);
+
+                            imgb->release(imgb);
+                            imgb = NULL;
+
+                            return AVERROR_INVALIDDATA;
+                        }
+
+                        // got frame
+                        ret = libxevd_image_copy(avctx, imgb, frame);
+                        if(ret < 0) {
+                            av_log(avctx, AV_LOG_ERROR, "Image copying error\n");
+
+                            av_packet_free(&pkt_au_imgb);
+                            av_frame_unref(frame);
+
+                            imgb->release(imgb);
+                            imgb = NULL;
+
+                            return ret;
+                        }
+
+                        // use ff_decode_frame_props_from_pkt() to fill frame properties
+                        ret = ff_decode_frame_props_from_pkt(avctx, frame, pkt_au_imgb);
+                        if (ret < 0) {
+                            av_log(avctx, AV_LOG_ERROR, "ff_decode_frame_props_from_pkt error\n");
+
+                            av_packet_free(&pkt_au_imgb);
+                            av_frame_unref(frame);
+
+                            imgb->release(imgb);
+                            imgb = NULL;
+
+                            return ret;
+                        }
+
+                        frame->pkt_dts = imgb->ts[XEVD_TS_DTS];
+                        frame->pts = imgb->ts[XEVD_TS_PTS];
+
+                        av_packet_free(&pkt_au_imgb);
+
+                        // xevd_pull uses pool of objects of type XEVD_IMGB.
+                        // The pool size is equal MAX_PB_SIZE (26), so release object when it is no more needed
+                        imgb->release(imgb);
+                        imgb = NULL;
+
+    return 0;
+}
 /**
   * Decode frame with decoupled packet/frame dataflow
   *
@@ -361,56 +420,7 @@  static int libxevd_receive_frame(AVCodecContext *avctx, AVFrame *frame)
                             return  AVERROR(EAGAIN);
                         }
                     } else {
-                        // got frame
-                        AVPacket* pkt_au_imgb = (AVPacket*)imgb->pdata[0];
-                        if(!pkt_au_imgb) {
-                            av_log(avctx, AV_LOG_ERROR, "Invalid data needed to fill frame properties\n");
-
-                            av_packet_free(&pkt_au);
-                            av_frame_unref(frame);
-
-                            imgb->release(imgb);
-                            imgb = NULL;
-
-                            return AVERROR_INVALIDDATA;
-                        }
-
-                        ret = libxevd_image_copy(avctx, imgb, frame);
-                        if(ret < 0) {
-                            av_log(avctx, AV_LOG_ERROR, "Image copying error\n");
-
-                            av_packet_free(&pkt_au_imgb);
-                            av_frame_unref(frame);
-
-                            imgb->release(imgb);
-                            imgb = NULL;
-
-                            return ret;
-                        }
-
-                        // use ff_decode_frame_props_from_pkt() to fill frame properties
-                        ret = ff_decode_frame_props_from_pkt(avctx, frame, pkt_au_imgb);
-                        if (ret < 0) {
-                            av_log(avctx, AV_LOG_ERROR, "ff_decode_frame_props_from_pkt error\n");
-
-                            av_packet_free(&pkt_au_imgb);
-                            av_frame_unref(frame);
-
-                            imgb->release(imgb);
-                            imgb = NULL;
-
-                            return ret;
-                        }
-
-                        frame->pkt_dts = imgb->ts[XEVD_TS_DTS];
-                        frame->pts = imgb->ts[XEVD_TS_PTS];
-
-                        av_packet_free(&pkt_au_imgb);
-
-                        // xevd_pull uses pool of objects of type XEVD_IMGB.
-                        // The pool size is equal MAX_PB_SIZE (26), so release object when it is no more needed
-                        imgb->release(imgb);
-                        imgb = NULL;
+                        return libxevd_return_frame(avctx, frame, imgb, &pkt_au);
                     }
                 }
             }
@@ -428,61 +438,13 @@  static int libxevd_receive_frame(AVCodecContext *avctx, AVFrame *frame)
 
             return AVERROR_EXTERNAL;
         } else { // XEVD_OK
-            AVPacket* pkt_au_imgb;
             if (!imgb) {
                 av_log(avctx, AV_LOG_ERROR, "Invalid decoded image data\n");
 
                 return AVERROR_EXTERNAL;
             }
 
-            pkt_au_imgb = (AVPacket*)imgb->pdata[0];
-            if(!pkt_au_imgb) {
-                av_log(avctx, AV_LOG_ERROR, "Invalid data needed to fill frame properties\n");
-
-                imgb->release(imgb);
-                imgb = NULL;
-
-                av_frame_unref(frame);
-
-                return AVERROR_INVALIDDATA;
-            }
-
-            // got frame
-            ret = libxevd_image_copy(avctx, imgb, frame);
-            if(ret < 0) {
-                av_packet_free(&pkt_au_imgb);
-                av_frame_unref(frame);
-
-                imgb->release(imgb);
-                imgb = NULL;
-
-                return ret;
-            }
-            // use ff_decode_frame_props_from_pkt() to fill frame properties
-            ret = ff_decode_frame_props_from_pkt(avctx, frame, pkt_au_imgb);
-            if (ret < 0) {
-                av_log(avctx, AV_LOG_ERROR, "ff_decode_frame_props_from_pkt error\n");
-
-                av_packet_free(&pkt_au_imgb);
-                av_frame_unref(frame);
-
-                imgb->release(imgb);
-                imgb = NULL;
-
-                return ret;
-            }
-
-            frame->pkt_dts = imgb->ts[XEVD_TS_DTS];
-            frame->pts = imgb->ts[XEVD_TS_PTS];
-
-            av_packet_free(&pkt_au_imgb);
-
-            // xevd_pull uses pool of objects of type XEVD_IMGB.
-            // The pool size is equal MAX_PB_SIZE (26), so release object when it is no more needed
-            imgb->release(imgb);
-            imgb = NULL;
-
-            return 0;
+            return libxevd_return_frame(avctx, frame, imgb, NULL);
         }
     }