[FFmpeg-devel] [PATCH v2] avcodec/libaomenc: remove one memcpy when queueing packets

[James Almer <jamrial@gmail.com>]

Don't use an intermediary buffer. Achieve this by replacing FrameListData with
a PacketList, and by allocating and populating every packet's payload before
inserting them into the list.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/libaomenc.c | 201 +++++++++++++++--------------------------
 1 file changed, 74 insertions(+), 127 deletions(-)

diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
index a82b933c18..a422e5a4f5 100644
--- a/libavcodec/libaomenc.c
+++ b/libavcodec/libaomenc.c
@@ -38,6 +38,7 @@
 
 #include "av1.h"
 #include "avcodec.h"
+#include "bytestream.h"
 #include "bsf.h"
 #include "codec_internal.h"
 #include "encode.h"
@@ -46,24 +47,6 @@
 #include "packet_internal.h"
 #include "profiles.h"
 
-/*
- * Portion of struct aom_codec_cx_pkt from aom_encoder.h.
- * One encoded frame returned from the library.
- */
-struct FrameListData {
-    void *buf;                       /**< compressed data buffer */
-    size_t sz;                       /**< length of compressed data */
-    int64_t pts;                     /**< time stamp to show frame
-                                          (in timebase units) */
-    unsigned long duration;          /**< duration to show frame
-                                          (in timebase units) */
-    uint32_t flags;                  /**< flags for this frame */
-    uint64_t sse[4];
-    int have_sse;                    /**< true if we have pending sse[] */
-    uint64_t frame_number;
-    struct FrameListData *next;
-};
-
 typedef struct AOMEncoderContext {
     AVClass *class;
     AVBSFContext *bsf;
@@ -71,7 +54,8 @@ typedef struct AOMEncoderContext {
     struct aom_image rawimg;
     struct aom_fixed_buf twopass_stats;
     unsigned twopass_stats_size;
-    struct FrameListData *coded_frame_list;
+    PacketList coded_frame_list;
+    AVPacket *avpkt;
     int cpu_used;
     int auto_alt_ref;
     int arnr_max_frames;
@@ -283,33 +267,6 @@ static av_cold void dump_enc_cfg(AVCodecContext *avctx,
     av_log(avctx, level, "\n");
 }
 
-static void coded_frame_add(void *list, struct FrameListData *cx_frame)
-{
-    struct FrameListData **p = list;
-
-    while (*p)
-        p = &(*p)->next;
-    *p = cx_frame;
-    cx_frame->next = NULL;
-}
-
-static av_cold void free_coded_frame(struct FrameListData *cx_frame)
-{
-    av_freep(&cx_frame->buf);
-    av_freep(&cx_frame);
-}
-
-static av_cold void free_frame_list(struct FrameListData *list)
-{
-    struct FrameListData *p = list;
-
-    while (p) {
-        list = list->next;
-        free_coded_frame(p);
-        p = list;
-    }
-}
-
 static av_cold int codecctl_int(AVCodecContext *avctx,
 #ifdef UENUM1BYTE
                                 aome_enc_control_id id,
@@ -432,7 +389,8 @@ static av_cold int aom_free(AVCodecContext *avctx)
     aom_codec_destroy(&ctx->encoder);
     av_freep(&ctx->twopass_stats.buf);
     av_freep(&avctx->stats_out);
-    free_frame_list(ctx->coded_frame_list);
+    avpriv_packet_list_free(&ctx->coded_frame_list);
+    av_packet_free(&ctx->avpkt);
     av_bsf_free(&ctx->bsf);
     return 0;
 }
@@ -1042,6 +1000,10 @@ static av_cold int aom_init(AVCodecContext *avctx,
            return ret;
     }
 
+    ctx->avpkt = av_packet_alloc();
+    if (!ctx->avpkt)
+        return AVERROR(ENOMEM);
+
     if (enccfg.rc_end_usage == AOM_CBR ||
         enccfg.g_pass != AOM_RC_ONE_PASS) {
         cpb_props->max_bitrate = avctx->rc_max_rate;
@@ -1053,25 +1015,41 @@ static av_cold int aom_init(AVCodecContext *avctx,
     return 0;
 }
 
-static inline void cx_pktcpy(AOMContext *ctx,
-                             struct FrameListData *dst,
-                             const struct aom_codec_cx_pkt *src)
+static int cx_pktcpy(AVCodecContext *avctx,
+                     AVPacket *dst,
+                     const struct aom_codec_cx_pkt *src)
 {
-    dst->pts      = src->data.frame.pts;
-    dst->duration = src->data.frame.duration;
-    dst->flags    = src->data.frame.flags;
-    dst->sz       = src->data.frame.sz;
-    dst->buf      = src->data.frame.buf;
+    AOMContext *ctx = avctx->priv_data;
+    int av_unused pict_type;
+    int ret;
+
+    ret = ff_get_encode_buffer(avctx, dst, src->data.frame.sz, 0);
+    if (ret < 0) {
+        av_log(avctx, AV_LOG_ERROR,
+               "Error getting output packet of size %"SIZE_SPECIFIER".\n", src->data.frame.sz);
+        return ret;
+    }
+    memcpy(dst->data, src->data.frame.buf, src->data.frame.sz);
+    dst->pts = dst->dts = src->data.frame.pts;
+
+    if (src->data.frame.flags & AOM_FRAME_IS_KEY) {
+        dst->flags |= AV_PKT_FLAG_KEY;
 #ifdef AOM_FRAME_IS_INTRAONLY
-    dst->frame_number = ++ctx->frame_number;
-    dst->have_sse = ctx->have_sse;
+        pict_type = AV_PICTURE_TYPE_I;
+    } else if (src->data.frame.flags & AOM_FRAME_IS_INTRAONLY) {
+        pict_type = AV_PICTURE_TYPE_I;
+    } else {
+        pict_type = AV_PICTURE_TYPE_P;
+    }
+
     if (ctx->have_sse) {
-        /* associate last-seen SSE to the frame. */
-        /* Transfers ownership from ctx to dst. */
-        memcpy(dst->sse, ctx->sse, sizeof(dst->sse));
         ctx->have_sse = 0;
-    }
+        ret = ff_side_data_set_encoder_stats(dst, 0, ctx->sse + 1, 3, pict_type);
+        if (ret < 0)
+            return ret;
 #endif
+    }
+    return 0;
 }
 
 /**
@@ -1081,50 +1059,32 @@ static inline void cx_pktcpy(AOMContext *ctx,
  * @return packet data size on success
  * @return a negative AVERROR on error
  */
-static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame,
-                      AVPacket *pkt)
+static int storeframe(AVCodecContext *avctx, AVPacket *dst, AVPacket *src)
 {
     AOMContext *ctx = avctx->priv_data;
-    int av_unused pict_type;
-    int ret = ff_get_encode_buffer(avctx, pkt, cx_frame->sz, 0);
-    if (ret < 0) {
-        av_log(avctx, AV_LOG_ERROR,
-               "Error getting output packet of size %"SIZE_SPECIFIER".\n", cx_frame->sz);
-        return ret;
-    }
-    memcpy(pkt->data, cx_frame->buf, pkt->size);
-    pkt->pts = pkt->dts = cx_frame->pts;
-
-    if (!!(cx_frame->flags & AOM_FRAME_IS_KEY)) {
-        pkt->flags |= AV_PKT_FLAG_KEY;
-#ifdef AOM_FRAME_IS_INTRAONLY
-        pict_type = AV_PICTURE_TYPE_I;
-    } else if (cx_frame->flags & AOM_FRAME_IS_INTRAONLY) {
-        pict_type = AV_PICTURE_TYPE_I;
-    } else {
-        pict_type = AV_PICTURE_TYPE_P;
-    }
+    const uint8_t *sd;
+    size_t size;
+    int ret;
 
-    ff_side_data_set_encoder_stats(pkt, 0, cx_frame->sse + 1,
-                                   cx_frame->have_sse ? 3 : 0, pict_type);
+    av_packet_move_ref(dst, src);
 
-    if (cx_frame->have_sse) {
+    sd = av_packet_get_side_data(dst, AV_PKT_DATA_QUALITY_STATS, &size);
+    if (sd && size >= 4 + 4 + 8 * 3) {
         int i;
+        sd += 4 + 4;
         for (i = 0; i < 3; ++i) {
-            avctx->error[i] += cx_frame->sse[i + 1];
+            avctx->error[i] += bytestream_get_le64(&sd);
         }
-        cx_frame->have_sse = 0;
-#endif
     }
 
     if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
-        ret = av_bsf_send_packet(ctx->bsf, pkt);
+        ret = av_bsf_send_packet(ctx->bsf, dst);
         if (ret < 0) {
             av_log(avctx, AV_LOG_ERROR, "extract_extradata filter "
                    "failed to send input packet\n");
             return ret;
         }
-        ret = av_bsf_receive_packet(ctx->bsf, pkt);
+        ret = av_bsf_receive_packet(ctx->bsf, dst);
 
         if (ret < 0) {
             av_log(avctx, AV_LOG_ERROR, "extract_extradata filter "
@@ -1132,7 +1092,7 @@ static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame,
             return ret;
         }
     }
-    return pkt->size;
+    return dst->size;
 }
 
 /**
@@ -1148,16 +1108,14 @@ static int queue_frames(AVCodecContext *avctx, AVPacket *pkt_out)
     AOMContext *ctx = avctx->priv_data;
     const struct aom_codec_cx_pkt *pkt;
     const void *iter = NULL;
-    int size = 0;
+    int ret, size = 0;
 
-    if (ctx->coded_frame_list) {
-        struct FrameListData *cx_frame = ctx->coded_frame_list;
+    if (!avpriv_packet_list_get(&ctx->coded_frame_list, ctx->avpkt)) {
         /* return the leading frame if we've already begun queueing */
-        size = storeframe(avctx, cx_frame, pkt_out);
-        if (size < 0)
-            return size;
-        ctx->coded_frame_list = cx_frame->next;
-        free_coded_frame(cx_frame);
+        ret = storeframe(avctx, pkt_out, ctx->avpkt);
+        if (ret < 0)
+            goto fail;
+        size = ret;
     }
 
     /* consume all available output from the encoder before returning. buffers
@@ -1165,37 +1123,21 @@ static int queue_frames(AVCodecContext *avctx, AVPacket *pkt_out)
     while ((pkt = aom_codec_get_cx_data(&ctx->encoder, &iter))) {
         switch (pkt->kind) {
         case AOM_CODEC_CX_FRAME_PKT:
+            ret = cx_pktcpy(avctx, ctx->avpkt, pkt);
+            if (ret < 0)
+                goto fail;
             if (!size) {
-                struct FrameListData cx_frame;
-
                 /* avoid storing the frame when the list is empty and we haven't yet
                  * provided a frame for output */
-                av_assert0(!ctx->coded_frame_list);
-                cx_pktcpy(ctx, &cx_frame, pkt);
-                size = storeframe(avctx, &cx_frame, pkt_out);
-                if (size < 0)
-                    return size;
+                av_assert0(!ctx->coded_frame_list.head);
+                ret = storeframe(avctx, pkt_out, ctx->avpkt);
+                if (ret < 0)
+                    goto fail;
+                size = ret;
             } else {
-                struct FrameListData *cx_frame =
-                    av_malloc(sizeof(struct FrameListData));
-
-                if (!cx_frame) {
-                    av_log(avctx, AV_LOG_ERROR,
-                           "Frame queue element alloc failed\n");
-                    return AVERROR(ENOMEM);
-                }
-                cx_pktcpy(ctx, cx_frame, pkt);
-                cx_frame->buf = av_malloc(cx_frame->sz);
-
-                if (!cx_frame->buf) {
-                    av_log(avctx, AV_LOG_ERROR,
-                           "Data buffer alloc (%"SIZE_SPECIFIER" bytes) failed\n",
-                           cx_frame->sz);
-                    av_freep(&cx_frame);
-                    return AVERROR(ENOMEM);
-                }
-                memcpy(cx_frame->buf, pkt->data.frame.buf, pkt->data.frame.sz);
-                coded_frame_add(&ctx->coded_frame_list, cx_frame);
+                ret = avpriv_packet_list_put(&ctx->coded_frame_list, ctx->avpkt, NULL, 0);
+                if (ret < 0)
+                    goto fail;
             }
             break;
         case AOM_CODEC_STATS_PKT:
@@ -1209,7 +1151,8 @@ static int queue_frames(AVCodecContext *avctx, AVPacket *pkt_out)
                 av_freep(&stats->buf);
                 stats->sz = 0;
                 av_log(avctx, AV_LOG_ERROR, "Stat buffer realloc failed\n");
-                return AVERROR(ENOMEM);
+                ret = AVERROR(ENOMEM);
+                goto fail;
             }
             stats->buf = tmp;
             memcpy((uint8_t *)stats->buf + stats->sz,
@@ -1236,6 +1179,10 @@ static int queue_frames(AVCodecContext *avctx, AVPacket *pkt_out)
     }
 
     return size;
+fail:
+    av_packet_unref(ctx->avpkt);
+    av_packet_unref(pkt_out);
+    return ret;
 }
 
 static enum AVPixelFormat aomfmt_to_pixfmt(struct aom_image *img)
-- 
2.37.2

_______________________________________________
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".