Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH 1/8] lavu/frame: improve AVFrame.opaque[_ref] documentation
@ 2023-02-28 12:00 Anton Khirnov
  2023-02-28 12:00 ` [FFmpeg-devel] [PATCH 2/8] lavc/libvpxenc: drop frame_number Anton Khirnov
                   ` (6 more replies)
  0 siblings, 7 replies; 18+ messages in thread
From: Anton Khirnov @ 2023-02-28 12:00 UTC (permalink / raw)
  To: ffmpeg-devel

Make them match each other, mention interaction with
AV_CODEC_FLAG_COPY_OPAQUE.
---
 libavutil/frame.h | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/libavutil/frame.h b/libavutil/frame.h
index 2580269549..4ed27cf43f 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -470,7 +470,18 @@ typedef struct AVFrame {
     int quality;
 
     /**
-     * for some private data of the user
+     * Frame owner's private data.
+     *
+     * This field may be set by the code that allocates/owns the frame data.
+     * It is then not touched by any library functions, except:
+     * - it is copied to other references by av_frame_copy_props() (and hence by
+     *   av_frame_ref());
+     * - it is set to NULL when the frame is cleared by av_frame_unref()
+     * - on the caller's explicit request. E.g. libavcodec encoders/decoders
+     *   will copy this field to/from @ref AVPacket "AVPackets" if the caller sets
+     *   @ref AV_CODEC_FLAG_COPY_OPAQUE.
+     *
+     * @see opaque_ref the reference-counted analogue
      */
     void *opaque;
 
@@ -678,13 +689,18 @@ typedef struct AVFrame {
     AVBufferRef *hw_frames_ctx;
 
     /**
-     * AVBufferRef for free use by the API user. FFmpeg will never check the
-     * contents of the buffer ref. FFmpeg calls av_buffer_unref() on it when
-     * the frame is unreferenced. av_frame_copy_props() calls create a new
-     * reference with av_buffer_ref() for the target frame's opaque_ref field.
+     * Frame owner's private data.
+     *
+     * This field may be set by the code that allocates/owns the frame data.
+     * It is then not touched by any library functions, except:
+     * - a new reference to the underlying buffer is propagated by
+     *   av_frame_copy_props() (and hence by av_frame_ref());
+     * - it is unreferenced in av_frame_unref();
+     * - on the caller's explicit request. E.g. libavcodec encoders/decoders
+     *   will propagate a new reference to/from @ref AVPacket "AVPackets" if the
+     *   caller sets @ref AV_CODEC_FLAG_COPY_OPAQUE.
      *
-     * This is unrelated to the opaque field, although it serves a similar
-     * purpose.
+     * @see opaque the plain pointer analogue
      */
     AVBufferRef *opaque_ref;
 
-- 
2.39.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".

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [FFmpeg-devel] [PATCH 2/8] lavc/libvpxenc: drop frame_number
  2023-02-28 12:00 [FFmpeg-devel] [PATCH 1/8] lavu/frame: improve AVFrame.opaque[_ref] documentation Anton Khirnov
@ 2023-02-28 12:00 ` Anton Khirnov
  2023-02-28 20:50   ` James Zern
  2023-02-28 12:00 ` [FFmpeg-devel] [PATCH 3/8] lavc/libvpxenc: reindent Anton Khirnov
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Anton Khirnov @ 2023-02-28 12:00 UTC (permalink / raw)
  To: ffmpeg-devel

It is not used, except to check whether the packet is valid before
writing HDR metadata to the packet in storeframe(). However, that check
serves no purpose, as the encoded packet is already treated as valid
higher up in this function.
---
 libavcodec/libvpxenc.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
index 339d4d8146..eaa4ad8f25 100644
--- a/libavcodec/libvpxenc.c
+++ b/libavcodec/libvpxenc.c
@@ -63,7 +63,6 @@ struct FrameListData {
     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;
 };
 
@@ -84,7 +83,6 @@ typedef struct VPxEncoderContext {
     int deadline; //i.e., RT/GOOD/BEST
     uint64_t sse[4];
     int have_sse; /**< true if we have pending sse[] */
-    uint64_t frame_number;
     struct FrameListData *coded_frame_list;
     struct FrameListData *alpha_coded_frame_list;
 
@@ -1220,9 +1218,8 @@ static inline void cx_pktcpy(struct FrameListData *dst,
     dst->sz       = src->data.frame.sz;
     dst->buf      = src->data.frame.buf;
     dst->have_sse = 0;
-    /* For alt-ref frame, don't store PSNR or increment frame_number */
+    /* For alt-ref frame, don't store PSNR */
     if (!(dst->flags & VPX_FRAME_IS_INVISIBLE)) {
-        dst->frame_number = ++ctx->frame_number;
         dst->have_sse = ctx->have_sse;
         if (ctx->have_sse) {
             /* associate last-seen SSE to the frame. */
@@ -1232,8 +1229,6 @@ static inline void cx_pktcpy(struct FrameListData *dst,
             memcpy(dst->sse, ctx->sse, sizeof(dst->sse));
             ctx->have_sse = 0;
         }
-    } else {
-        dst->frame_number = -1;   /* sanity marker */
     }
 }
 
@@ -1289,13 +1284,11 @@ static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame,
         AV_WB64(side_data, 1);
         memcpy(side_data + 8, alpha_cx_frame->buf, alpha_cx_frame->sz);
     }
-    if (cx_frame->frame_number != -1) {
         if (ctx->hdr10_plus_fifo) {
             int err = copy_hdr10_plus_to_pkt(ctx->hdr10_plus_fifo, pkt);
             if (err < 0)
                 return err;
         }
-    }
 
     return pkt->size;
 }
-- 
2.39.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".

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [FFmpeg-devel] [PATCH 3/8] lavc/libvpxenc: reindent
  2023-02-28 12:00 [FFmpeg-devel] [PATCH 1/8] lavu/frame: improve AVFrame.opaque[_ref] documentation Anton Khirnov
  2023-02-28 12:00 ` [FFmpeg-devel] [PATCH 2/8] lavc/libvpxenc: drop frame_number Anton Khirnov
@ 2023-02-28 12:00 ` Anton Khirnov
  2023-02-28 20:50   ` James Zern
  2023-02-28 12:01 ` [FFmpeg-devel] [PATCH 4/8] lavc/libvpxenc: rename hdr10_plus_fifo and related objects Anton Khirnov
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Anton Khirnov @ 2023-02-28 12:00 UTC (permalink / raw)
  To: ffmpeg-devel

---
 libavcodec/libvpxenc.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
index eaa4ad8f25..abaa8c3513 100644
--- a/libavcodec/libvpxenc.c
+++ b/libavcodec/libvpxenc.c
@@ -1284,11 +1284,11 @@ static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame,
         AV_WB64(side_data, 1);
         memcpy(side_data + 8, alpha_cx_frame->buf, alpha_cx_frame->sz);
     }
-        if (ctx->hdr10_plus_fifo) {
-            int err = copy_hdr10_plus_to_pkt(ctx->hdr10_plus_fifo, pkt);
-            if (err < 0)
-                return err;
-        }
+    if (ctx->hdr10_plus_fifo) {
+        int err = copy_hdr10_plus_to_pkt(ctx->hdr10_plus_fifo, pkt);
+        if (err < 0)
+            return err;
+    }
 
     return pkt->size;
 }
-- 
2.39.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".

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [FFmpeg-devel] [PATCH 4/8] lavc/libvpxenc: rename hdr10_plus_fifo and related objects
  2023-02-28 12:00 [FFmpeg-devel] [PATCH 1/8] lavu/frame: improve AVFrame.opaque[_ref] documentation Anton Khirnov
  2023-02-28 12:00 ` [FFmpeg-devel] [PATCH 2/8] lavc/libvpxenc: drop frame_number Anton Khirnov
  2023-02-28 12:00 ` [FFmpeg-devel] [PATCH 3/8] lavc/libvpxenc: reindent Anton Khirnov
@ 2023-02-28 12:01 ` Anton Khirnov
  2023-02-28 20:55   ` James Zern
  2023-02-28 12:01 ` [FFmpeg-devel] [PATCH 5/8] lavc/libvpxenc: handle frame durations and AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE Anton Khirnov
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Anton Khirnov @ 2023-02-28 12:01 UTC (permalink / raw)
  To: ffmpeg-devel

This AVFifo is used to propagate HDR metadata from input frames to
output packets, since libvpx does not allow passing through arbitrary
user data.

It will be extended to pass through other kinds of data in future
commits, so give it a more generic name.
---
 libavcodec/libvpxenc.c | 51 +++++++++++++++++++++---------------------
 1 file changed, 26 insertions(+), 25 deletions(-)

diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
index abaa8c3513..77921badba 100644
--- a/libavcodec/libvpxenc.c
+++ b/libavcodec/libvpxenc.c
@@ -66,10 +66,10 @@ struct FrameListData {
     struct FrameListData *next;
 };
 
-typedef struct FrameHDR10Plus {
+typedef struct FrameData {
     int64_t pts;
     AVBufferRef *hdr10_plus;
-} FrameHDR10Plus;
+} FrameData;
 
 typedef struct VPxEncoderContext {
     AVClass *class;
@@ -130,7 +130,9 @@ typedef struct VPxEncoderContext {
     int corpus_complexity;
     int tpl_model;
     int min_gf_interval;
-    AVFifo *hdr10_plus_fifo;
+
+    // This FIFO is used to propagate various properties from frames to packets.
+    AVFifo *fifo;
     /**
      * If the driver does not support ROI then warn the first time we
      * encounter a frame with ROI side data.
@@ -327,32 +329,32 @@ static av_cold void free_frame_list(struct FrameListData *list)
     }
 }
 
-static av_cold void free_hdr10_plus_fifo(AVFifo **fifo)
+static av_cold void fifo_free(AVFifo **fifo)
 {
-    FrameHDR10Plus frame_hdr10_plus;
-    while (av_fifo_read(*fifo, &frame_hdr10_plus, 1) >= 0)
-        av_buffer_unref(&frame_hdr10_plus.hdr10_plus);
+    FrameData fd;
+    while (av_fifo_read(*fifo, &fd, 1) >= 0)
+        av_buffer_unref(&fd.hdr10_plus);
     av_fifo_freep2(fifo);
 }
 
-static int copy_hdr10_plus_to_pkt(AVFifo *fifo, AVPacket *pkt)
+static int frame_data_apply(AVFifo *fifo, AVPacket *pkt)
 {
-    FrameHDR10Plus frame_hdr10_plus;
+    FrameData fd;
     uint8_t *data;
-    if (!pkt || av_fifo_peek(fifo, &frame_hdr10_plus, 1, 0) < 0)
+    if (!pkt || av_fifo_peek(fifo, &fd, 1, 0) < 0)
         return 0;
-    if (!frame_hdr10_plus.hdr10_plus || frame_hdr10_plus.pts != pkt->pts)
+    if (!fd.hdr10_plus || fd.pts != pkt->pts)
         return 0;
     av_fifo_drain2(fifo, 1);
 
-    data = av_packet_new_side_data(pkt, AV_PKT_DATA_DYNAMIC_HDR10_PLUS, frame_hdr10_plus.hdr10_plus->size);
+    data = av_packet_new_side_data(pkt, AV_PKT_DATA_DYNAMIC_HDR10_PLUS, fd.hdr10_plus->size);
     if (!data) {
-        av_buffer_unref(&frame_hdr10_plus.hdr10_plus);
+        av_buffer_unref(&fd.hdr10_plus);
         return AVERROR(ENOMEM);
     }
 
-    memcpy(data, frame_hdr10_plus.hdr10_plus->data, frame_hdr10_plus.hdr10_plus->size);
-    av_buffer_unref(&frame_hdr10_plus.hdr10_plus);
+    memcpy(data, fd.hdr10_plus->data, fd.hdr10_plus->size);
+    av_buffer_unref(&fd.hdr10_plus);
     return 0;
 }
 
@@ -447,8 +449,8 @@ static av_cold int vpx_free(AVCodecContext *avctx)
     av_freep(&avctx->stats_out);
     free_frame_list(ctx->coded_frame_list);
     free_frame_list(ctx->alpha_coded_frame_list);
-    if (ctx->hdr10_plus_fifo)
-        free_hdr10_plus_fifo(&ctx->hdr10_plus_fifo);
+    if (ctx->fifo)
+        fifo_free(&ctx->fifo);
     return 0;
 }
 
@@ -919,9 +921,8 @@ static av_cold int vpx_init(AVCodecContext *avctx,
         // Keep HDR10+ if it has bit depth higher than 8 and
         // it has PQ trc (SMPTE2084).
         if (enccfg.g_bit_depth > 8 && avctx->color_trc == AVCOL_TRC_SMPTE2084) {
-            ctx->hdr10_plus_fifo = av_fifo_alloc2(1, sizeof(FrameHDR10Plus),
-                                                  AV_FIFO_FLAG_AUTO_GROW);
-            if (!ctx->hdr10_plus_fifo)
+            ctx->fifo = av_fifo_alloc2(1, sizeof(FrameData), AV_FIFO_FLAG_AUTO_GROW);
+            if (!ctx->fifo)
                 return AVERROR(ENOMEM);
         }
     }
@@ -1284,8 +1285,8 @@ static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame,
         AV_WB64(side_data, 1);
         memcpy(side_data + 8, alpha_cx_frame->buf, alpha_cx_frame->sz);
     }
-    if (ctx->hdr10_plus_fifo) {
-        int err = copy_hdr10_plus_to_pkt(ctx->hdr10_plus_fifo, pkt);
+    if (ctx->fifo) {
+        int err = frame_data_apply(ctx->fifo, pkt);
         if (err < 0)
             return err;
     }
@@ -1702,18 +1703,18 @@ static int vpx_encode(AVCodecContext *avctx, AVPacket *pkt,
             }
         }
 
-        if (ctx->hdr10_plus_fifo) {
+        if (ctx->fifo) {
             AVFrameSideData *hdr10_plus_metadata;
             // Add HDR10+ metadata to queue.
             hdr10_plus_metadata = av_frame_get_side_data(frame, AV_FRAME_DATA_DYNAMIC_HDR_PLUS);
             if (hdr10_plus_metadata) {
                 int err;
-                struct FrameHDR10Plus data;
+                FrameData data;
                 data.pts = frame->pts;
                 data.hdr10_plus = av_buffer_ref(hdr10_plus_metadata->buf);
                 if (!data.hdr10_plus)
                     return AVERROR(ENOMEM);
-                err = av_fifo_write(ctx->hdr10_plus_fifo, &data, 1);
+                err = av_fifo_write(ctx->fifo, &data, 1);
                 if (err < 0) {
                     av_buffer_unref(&data.hdr10_plus);
                     return err;
-- 
2.39.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".

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [FFmpeg-devel] [PATCH 5/8] lavc/libvpxenc: handle frame durations and AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE
  2023-02-28 12:00 [FFmpeg-devel] [PATCH 1/8] lavu/frame: improve AVFrame.opaque[_ref] documentation Anton Khirnov
                   ` (2 preceding siblings ...)
  2023-02-28 12:01 ` [FFmpeg-devel] [PATCH 4/8] lavc/libvpxenc: rename hdr10_plus_fifo and related objects Anton Khirnov
@ 2023-02-28 12:01 ` Anton Khirnov
  2023-02-28 21:11   ` James Zern
  2023-02-28 23:30   ` James Almer
  2023-02-28 12:01 ` [FFmpeg-devel] [PATCH 6/8] lavc/libvpxenc: drop a useless condition Anton Khirnov
                   ` (2 subsequent siblings)
  6 siblings, 2 replies; 18+ messages in thread
From: Anton Khirnov @ 2023-02-28 12:01 UTC (permalink / raw)
  To: ffmpeg-devel

---
 libavcodec/libvpxenc.c | 139 +++++++++++++++++++++++++++++------------
 libavcodec/version.h   |   2 +-
 2 files changed, 100 insertions(+), 41 deletions(-)

diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
index 77921badba..af16e53deb 100644
--- a/libavcodec/libvpxenc.c
+++ b/libavcodec/libvpxenc.c
@@ -68,6 +68,14 @@ struct FrameListData {
 
 typedef struct FrameData {
     int64_t pts;
+    int64_t duration;
+
+#if FF_API_REORDERED_OPAQUE
+    int64_t      reordered_opaque;
+#endif
+    void        *frame_opaque;
+    AVBufferRef *frame_opaque_ref;
+
     AVBufferRef *hdr10_plus;
 } FrameData;
 
@@ -329,32 +337,101 @@ static av_cold void free_frame_list(struct FrameListData *list)
     }
 }
 
+static void frame_data_uninit(FrameData *fd)
+{
+    av_buffer_unref(&fd->frame_opaque_ref);
+    av_buffer_unref(&fd->hdr10_plus);
+}
+
 static av_cold void fifo_free(AVFifo **fifo)
 {
     FrameData fd;
     while (av_fifo_read(*fifo, &fd, 1) >= 0)
-        av_buffer_unref(&fd.hdr10_plus);
+        frame_data_uninit(&fd);
     av_fifo_freep2(fifo);
 }
 
-static int frame_data_apply(AVFifo *fifo, AVPacket *pkt)
+static int frame_data_submit(AVCodecContext *avctx, AVFifo *fifo,
+                             const AVFrame *frame)
+{
+    VPxContext *ctx = avctx->priv_data;
+    const struct vpx_codec_enc_cfg *enccfg = ctx->encoder.config.enc;
+
+    FrameData        fd = { .pts = frame->pts };
+
+    AVFrameSideData *av_uninit(sd);
+    int ret;
+
+#if CONFIG_LIBVPX_VP9_ENCODER
+    // Keep HDR10+ if it has bit depth higher than 8 and
+    // it has PQ trc (SMPTE2084).
+    sd = av_frame_get_side_data(frame, AV_FRAME_DATA_DYNAMIC_HDR_PLUS);
+    if (avctx->codec_id == AV_CODEC_ID_VP9 && sd &&
+        enccfg->g_bit_depth > 8 && avctx->color_trc == AVCOL_TRC_SMPTE2084) {
+        fd.hdr10_plus = av_buffer_ref(sd->buf);
+        if (!fd.hdr10_plus)
+            return AVERROR(ENOMEM);
+    }
+#endif
+
+    fd.duration     = frame->duration;
+    fd.frame_opaque = frame->opaque;
+    if (avctx->flags & AV_CODEC_FLAG_COPY_OPAQUE && frame->opaque_ref) {
+        ret = av_buffer_replace(&fd.frame_opaque_ref, frame->opaque_ref);
+        if (ret < 0)
+            goto fail;
+    }
+#if FF_API_REORDERED_OPAQUE
+FF_DISABLE_DEPRECATION_WARNINGS
+    fd.reordered_opaque = frame->reordered_opaque;
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
+
+    ret = av_fifo_write(fifo, &fd, 1);
+    if (ret < 0)
+        goto fail;
+
+    return 0;
+fail:
+    frame_data_uninit(&fd);
+    return ret;
+}
+
+static int frame_data_apply(AVCodecContext *avctx, AVFifo *fifo, AVPacket *pkt)
 {
     FrameData fd;
     uint8_t *data;
     if (!pkt || av_fifo_peek(fifo, &fd, 1, 0) < 0)
         return 0;
-    if (!fd.hdr10_plus || fd.pts != pkt->pts)
+    if (fd.pts != pkt->pts)
         return 0;
     av_fifo_drain2(fifo, 1);
 
-    data = av_packet_new_side_data(pkt, AV_PKT_DATA_DYNAMIC_HDR10_PLUS, fd.hdr10_plus->size);
-    if (!data) {
+#if FF_API_REORDERED_OPAQUE
+FF_DISABLE_DEPRECATION_WARNINGS
+    avctx->reordered_opaque = fd.reordered_opaque;
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
+
+    pkt->duration = fd.duration;
+    if (avctx->flags & AV_CODEC_FLAG_COPY_OPAQUE) {
+        pkt->opaque         = fd.frame_opaque;
+        pkt->opaque_ref     = fd.frame_opaque_ref;
+        fd.frame_opaque_ref = NULL;
+    }
+    av_buffer_unref(&fd.frame_opaque_ref);
+
+    if (fd.hdr10_plus) {
+        data = av_packet_new_side_data(pkt, AV_PKT_DATA_DYNAMIC_HDR10_PLUS, fd.hdr10_plus->size);
+        if (!data) {
+            av_buffer_unref(&fd.hdr10_plus);
+            return AVERROR(ENOMEM);
+        }
+
+        memcpy(data, fd.hdr10_plus->data, fd.hdr10_plus->size);
         av_buffer_unref(&fd.hdr10_plus);
-        return AVERROR(ENOMEM);
     }
 
-    memcpy(data, fd.hdr10_plus->data, fd.hdr10_plus->size);
-    av_buffer_unref(&fd.hdr10_plus);
     return 0;
 }
 
@@ -914,17 +991,14 @@ static av_cold int vpx_init(AVCodecContext *avctx,
         return AVERROR(EINVAL);
     }
 
+    ctx->fifo = av_fifo_alloc2(1, sizeof(FrameData), AV_FIFO_FLAG_AUTO_GROW);
+    if (!ctx->fifo)
+        return AVERROR(ENOMEM);
+
 #if CONFIG_LIBVPX_VP9_ENCODER
     if (avctx->codec_id == AV_CODEC_ID_VP9) {
         if (set_pix_fmt(avctx, codec_caps, &enccfg, &flags, &img_fmt))
             return AVERROR(EINVAL);
-        // Keep HDR10+ if it has bit depth higher than 8 and
-        // it has PQ trc (SMPTE2084).
-        if (enccfg.g_bit_depth > 8 && avctx->color_trc == AVCOL_TRC_SMPTE2084) {
-            ctx->fifo = av_fifo_alloc2(1, sizeof(FrameData), AV_FIFO_FLAG_AUTO_GROW);
-            if (!ctx->fifo)
-                return AVERROR(ENOMEM);
-        }
     }
 #endif
 
@@ -1285,11 +1359,9 @@ static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame,
         AV_WB64(side_data, 1);
         memcpy(side_data + 8, alpha_cx_frame->buf, alpha_cx_frame->sz);
     }
-    if (ctx->fifo) {
-        int err = frame_data_apply(ctx->fifo, pkt);
-        if (err < 0)
-            return err;
-    }
+    ret = frame_data_apply(avctx, ctx->fifo, pkt);
+    if (ret < 0)
+        return ret;
 
     return pkt->size;
 }
@@ -1703,24 +1775,9 @@ static int vpx_encode(AVCodecContext *avctx, AVPacket *pkt,
             }
         }
 
-        if (ctx->fifo) {
-            AVFrameSideData *hdr10_plus_metadata;
-            // Add HDR10+ metadata to queue.
-            hdr10_plus_metadata = av_frame_get_side_data(frame, AV_FRAME_DATA_DYNAMIC_HDR_PLUS);
-            if (hdr10_plus_metadata) {
-                int err;
-                FrameData data;
-                data.pts = frame->pts;
-                data.hdr10_plus = av_buffer_ref(hdr10_plus_metadata->buf);
-                if (!data.hdr10_plus)
-                    return AVERROR(ENOMEM);
-                err = av_fifo_write(ctx->fifo, &data, 1);
-                if (err < 0) {
-                    av_buffer_unref(&data.hdr10_plus);
-                    return err;
-                }
-            }
-        }
+        res = frame_data_submit(avctx, ctx->fifo, frame);
+        if (res < 0)
+            return res;
     }
 
     // this is for encoding with preset temporal layering patterns defined in
@@ -1953,7 +2010,8 @@ const FFCodec ff_libvpx_vp8_encoder = {
     .p.type         = AVMEDIA_TYPE_VIDEO,
     .p.id           = AV_CODEC_ID_VP8,
     .p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY |
-                      AV_CODEC_CAP_OTHER_THREADS,
+                      AV_CODEC_CAP_OTHER_THREADS            |
+                      AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE,
     .priv_data_size = sizeof(VPxContext),
     .init           = vp8_init,
     FF_CODEC_ENCODE_CB(vpx_encode),
@@ -1986,7 +2044,8 @@ FFCodec ff_libvpx_vp9_encoder = {
     .p.type         = AVMEDIA_TYPE_VIDEO,
     .p.id           = AV_CODEC_ID_VP9,
     .p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY |
-                      AV_CODEC_CAP_OTHER_THREADS,
+                      AV_CODEC_CAP_OTHER_THREADS            |
+                      AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE,
     .p.profiles     = NULL_IF_CONFIG_SMALL(ff_vp9_profiles),
     .p.priv_class   = &class_vp9,
     .p.wrapper_name = "libvpx",
diff --git a/libavcodec/version.h b/libavcodec/version.h
index 06631ffa8c..789d9047c2 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -30,7 +30,7 @@
 #include "version_major.h"
 
 #define LIBAVCODEC_VERSION_MINOR   4
-#define LIBAVCODEC_VERSION_MICRO 100
+#define LIBAVCODEC_VERSION_MICRO 101
 
 #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
                                                LIBAVCODEC_VERSION_MINOR, \
-- 
2.39.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".

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [FFmpeg-devel] [PATCH 6/8] lavc/libvpxenc: drop a useless condition
  2023-02-28 12:00 [FFmpeg-devel] [PATCH 1/8] lavu/frame: improve AVFrame.opaque[_ref] documentation Anton Khirnov
                   ` (3 preceding siblings ...)
  2023-02-28 12:01 ` [FFmpeg-devel] [PATCH 5/8] lavc/libvpxenc: handle frame durations and AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE Anton Khirnov
@ 2023-02-28 12:01 ` Anton Khirnov
  2023-02-28 21:12   ` James Zern
  2023-02-28 12:01 ` [FFmpeg-devel] [PATCH 7/8] lavc/libvpxenc: handle queue desync more gracefully Anton Khirnov
  2023-02-28 12:01 ` [FFmpeg-devel] [PATCH 8/8] lavc/libvpxenc: drop FrameListData.duration Anton Khirnov
  6 siblings, 1 reply; 18+ messages in thread
From: Anton Khirnov @ 2023-02-28 12:01 UTC (permalink / raw)
  To: ffmpeg-devel

A non-NULL packet is always passed to frame_data_apply().
---
 libavcodec/libvpxenc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
index af16e53deb..33f35bbaf4 100644
--- a/libavcodec/libvpxenc.c
+++ b/libavcodec/libvpxenc.c
@@ -401,7 +401,7 @@ static int frame_data_apply(AVCodecContext *avctx, AVFifo *fifo, AVPacket *pkt)
 {
     FrameData fd;
     uint8_t *data;
-    if (!pkt || av_fifo_peek(fifo, &fd, 1, 0) < 0)
+    if (av_fifo_peek(fifo, &fd, 1, 0) < 0)
         return 0;
     if (fd.pts != pkt->pts)
         return 0;
-- 
2.39.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".

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [FFmpeg-devel] [PATCH 7/8] lavc/libvpxenc: handle queue desync more gracefully
  2023-02-28 12:00 [FFmpeg-devel] [PATCH 1/8] lavu/frame: improve AVFrame.opaque[_ref] documentation Anton Khirnov
                   ` (4 preceding siblings ...)
  2023-02-28 12:01 ` [FFmpeg-devel] [PATCH 6/8] lavc/libvpxenc: drop a useless condition Anton Khirnov
@ 2023-02-28 12:01 ` Anton Khirnov
  2023-02-28 12:01 ` [FFmpeg-devel] [PATCH 8/8] lavc/libvpxenc: drop FrameListData.duration Anton Khirnov
  6 siblings, 0 replies; 18+ messages in thread
From: Anton Khirnov @ 2023-02-28 12:01 UTC (permalink / raw)
  To: ffmpeg-devel

If the packets returned by libvpx and our internal frame properties
queue get desynchronized for some reason (should not happen, but it is
not clear libvpx API guarantees this), we will keep adding to the queue
indefinitely and never remove anything.

Change the code to drain the queue even if timestamps do not match.
---
 libavcodec/libvpxenc.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
index 33f35bbaf4..aeeaa0e681 100644
--- a/libavcodec/libvpxenc.c
+++ b/libavcodec/libvpxenc.c
@@ -401,11 +401,16 @@ static int frame_data_apply(AVCodecContext *avctx, AVFifo *fifo, AVPacket *pkt)
 {
     FrameData fd;
     uint8_t *data;
+    int ret = 0;
+
     if (av_fifo_peek(fifo, &fd, 1, 0) < 0)
         return 0;
-    if (fd.pts != pkt->pts)
-        return 0;
-    av_fifo_drain2(fifo, 1);
+    if (fd.pts != pkt->pts) {
+        av_log(avctx, AV_LOG_WARNING,
+               "Mismatching timestamps: libvpx %"PRId64" queued %"PRId64"; "
+               "this is a bug, please report it\n", pkt->pts, fd.pts);
+        goto skip;
+    }
 
 #if FF_API_REORDERED_OPAQUE
 FF_DISABLE_DEPRECATION_WARNINGS
@@ -419,20 +424,22 @@ FF_ENABLE_DEPRECATION_WARNINGS
         pkt->opaque_ref     = fd.frame_opaque_ref;
         fd.frame_opaque_ref = NULL;
     }
-    av_buffer_unref(&fd.frame_opaque_ref);
 
     if (fd.hdr10_plus) {
         data = av_packet_new_side_data(pkt, AV_PKT_DATA_DYNAMIC_HDR10_PLUS, fd.hdr10_plus->size);
         if (!data) {
-            av_buffer_unref(&fd.hdr10_plus);
-            return AVERROR(ENOMEM);
+            ret = AVERROR(ENOMEM);
+            goto skip;
         }
 
         memcpy(data, fd.hdr10_plus->data, fd.hdr10_plus->size);
-        av_buffer_unref(&fd.hdr10_plus);
     }
 
-    return 0;
+skip:
+    av_fifo_drain2(fifo, 1);
+    frame_data_uninit(&fd);
+
+    return ret;
 }
 
 static av_cold int codecctl_int(AVCodecContext *avctx,
-- 
2.39.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".

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [FFmpeg-devel] [PATCH 8/8] lavc/libvpxenc: drop FrameListData.duration
  2023-02-28 12:00 [FFmpeg-devel] [PATCH 1/8] lavu/frame: improve AVFrame.opaque[_ref] documentation Anton Khirnov
                   ` (5 preceding siblings ...)
  2023-02-28 12:01 ` [FFmpeg-devel] [PATCH 7/8] lavc/libvpxenc: handle queue desync more gracefully Anton Khirnov
@ 2023-02-28 12:01 ` Anton Khirnov
  2023-02-28 21:16   ` James Zern
  6 siblings, 1 reply; 18+ messages in thread
From: Anton Khirnov @ 2023-02-28 12:01 UTC (permalink / raw)
  To: ffmpeg-devel

It is write-only.
---
 libavcodec/libvpxenc.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
index aeeaa0e681..3d1abb7df9 100644
--- a/libavcodec/libvpxenc.c
+++ b/libavcodec/libvpxenc.c
@@ -58,8 +58,6 @@ struct FrameListData {
     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[] */
@@ -1295,7 +1293,6 @@ static inline void cx_pktcpy(struct FrameListData *dst,
                              VPxContext *ctx)
 {
     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;
-- 
2.39.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".

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/8] lavc/libvpxenc: drop frame_number
  2023-02-28 12:00 ` [FFmpeg-devel] [PATCH 2/8] lavc/libvpxenc: drop frame_number Anton Khirnov
@ 2023-02-28 20:50   ` James Zern
  0 siblings, 0 replies; 18+ messages in thread
From: James Zern @ 2023-02-28 20:50 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Tue, Feb 28, 2023 at 4:02 AM Anton Khirnov <anton@khirnov.net> wrote:
>
> It is not used, except to check whether the packet is valid before
> writing HDR metadata to the packet in storeframe(). However, that check
> serves no purpose, as the encoded packet is already treated as valid
> higher up in this function.

lgtm. I agree, since the HDR path is VP9 only, there's no possibility
of having a separate alt-ref packet. The libaom wrapper could probably
use a similar update.

> ---
>  libavcodec/libvpxenc.c | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
> index 339d4d8146..eaa4ad8f25 100644
> --- a/libavcodec/libvpxenc.c
> +++ b/libavcodec/libvpxenc.c
> @@ -63,7 +63,6 @@ struct FrameListData {
>      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;
>  };
>
> @@ -84,7 +83,6 @@ typedef struct VPxEncoderContext {
>      int deadline; //i.e., RT/GOOD/BEST
>      uint64_t sse[4];
>      int have_sse; /**< true if we have pending sse[] */
> -    uint64_t frame_number;
>      struct FrameListData *coded_frame_list;
>      struct FrameListData *alpha_coded_frame_list;
>
> @@ -1220,9 +1218,8 @@ static inline void cx_pktcpy(struct FrameListData *dst,
>      dst->sz       = src->data.frame.sz;
>      dst->buf      = src->data.frame.buf;
>      dst->have_sse = 0;
> -    /* For alt-ref frame, don't store PSNR or increment frame_number */
> +    /* For alt-ref frame, don't store PSNR */
>      if (!(dst->flags & VPX_FRAME_IS_INVISIBLE)) {
> -        dst->frame_number = ++ctx->frame_number;
>          dst->have_sse = ctx->have_sse;
>          if (ctx->have_sse) {
>              /* associate last-seen SSE to the frame. */
> @@ -1232,8 +1229,6 @@ static inline void cx_pktcpy(struct FrameListData *dst,
>              memcpy(dst->sse, ctx->sse, sizeof(dst->sse));
>              ctx->have_sse = 0;
>          }
> -    } else {
> -        dst->frame_number = -1;   /* sanity marker */
>      }
>  }
>
> @@ -1289,13 +1284,11 @@ static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame,
>          AV_WB64(side_data, 1);
>          memcpy(side_data + 8, alpha_cx_frame->buf, alpha_cx_frame->sz);
>      }
> -    if (cx_frame->frame_number != -1) {
>          if (ctx->hdr10_plus_fifo) {
>              int err = copy_hdr10_plus_to_pkt(ctx->hdr10_plus_fifo, pkt);
>              if (err < 0)
>                  return err;
>          }
> -    }
>
>      return pkt->size;
>  }
> --
> 2.39.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 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".

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [FFmpeg-devel] [PATCH 3/8] lavc/libvpxenc: reindent
  2023-02-28 12:00 ` [FFmpeg-devel] [PATCH 3/8] lavc/libvpxenc: reindent Anton Khirnov
@ 2023-02-28 20:50   ` James Zern
  0 siblings, 0 replies; 18+ messages in thread
From: James Zern @ 2023-02-28 20:50 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Tue, Feb 28, 2023 at 4:01 AM Anton Khirnov <anton@khirnov.net> wrote:
>
> ---
>  libavcodec/libvpxenc.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>

lgtm.

> diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
> index eaa4ad8f25..abaa8c3513 100644
> --- a/libavcodec/libvpxenc.c
> +++ b/libavcodec/libvpxenc.c
> @@ -1284,11 +1284,11 @@ static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame,
>          AV_WB64(side_data, 1);
>          memcpy(side_data + 8, alpha_cx_frame->buf, alpha_cx_frame->sz);
>      }
> -        if (ctx->hdr10_plus_fifo) {
> -            int err = copy_hdr10_plus_to_pkt(ctx->hdr10_plus_fifo, pkt);
> -            if (err < 0)
> -                return err;
> -        }
> +    if (ctx->hdr10_plus_fifo) {
> +        int err = copy_hdr10_plus_to_pkt(ctx->hdr10_plus_fifo, pkt);
> +        if (err < 0)
> +            return err;
> +    }
>
>      return pkt->size;
>  }
> --
> 2.39.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 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".

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [FFmpeg-devel] [PATCH 4/8] lavc/libvpxenc: rename hdr10_plus_fifo and related objects
  2023-02-28 12:01 ` [FFmpeg-devel] [PATCH 4/8] lavc/libvpxenc: rename hdr10_plus_fifo and related objects Anton Khirnov
@ 2023-02-28 20:55   ` James Zern
  0 siblings, 0 replies; 18+ messages in thread
From: James Zern @ 2023-02-28 20:55 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Tue, Feb 28, 2023 at 4:02 AM Anton Khirnov <anton@khirnov.net> wrote:
>
> This AVFifo is used to propagate HDR metadata from input frames to
> output packets, since libvpx does not allow passing through arbitrary
> user data.
>
> It will be extended to pass through other kinds of data in future
> commits, so give it a more generic name.
> ---
>  libavcodec/libvpxenc.c | 51 +++++++++++++++++++++---------------------
>  1 file changed, 26 insertions(+), 25 deletions(-)
>

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [FFmpeg-devel] [PATCH 5/8] lavc/libvpxenc: handle frame durations and AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE
  2023-02-28 12:01 ` [FFmpeg-devel] [PATCH 5/8] lavc/libvpxenc: handle frame durations and AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE Anton Khirnov
@ 2023-02-28 21:11   ` James Zern
  2023-03-01 12:01     ` Anton Khirnov
  2023-02-28 23:30   ` James Almer
  1 sibling, 1 reply; 18+ messages in thread
From: James Zern @ 2023-02-28 21:11 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Tue, Feb 28, 2023 at 4:01 AM Anton Khirnov <anton@khirnov.net> wrote:
>
> ---
>  libavcodec/libvpxenc.c | 139 +++++++++++++++++++++++++++++------------
>  libavcodec/version.h   |   2 +-
>  2 files changed, 100 insertions(+), 41 deletions(-)
>

lgtm

> diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
> index 77921badba..af16e53deb 100644
> --- a/libavcodec/libvpxenc.c
> +++ b/libavcodec/libvpxenc.c
> @@ -68,6 +68,14 @@ struct FrameListData {
>
>  typedef struct FrameData {
>      int64_t pts;
> +    int64_t duration;
> +
> +#if FF_API_REORDERED_OPAQUE
> +    int64_t      reordered_opaque;
> +#endif
> +    void        *frame_opaque;
> +    AVBufferRef *frame_opaque_ref;
> +
>      AVBufferRef *hdr10_plus;
>  } FrameData;
>
> @@ -329,32 +337,101 @@ static av_cold void free_frame_list(struct FrameListData *list)
>      }
>  }
>
> +static void frame_data_uninit(FrameData *fd)
> +{
> +    av_buffer_unref(&fd->frame_opaque_ref);
> +    av_buffer_unref(&fd->hdr10_plus);
> +}
> +
>  static av_cold void fifo_free(AVFifo **fifo)
>  {
>      FrameData fd;
>      while (av_fifo_read(*fifo, &fd, 1) >= 0)
> -        av_buffer_unref(&fd.hdr10_plus);
> +        frame_data_uninit(&fd);
>      av_fifo_freep2(fifo);
>  }
>
> -static int frame_data_apply(AVFifo *fifo, AVPacket *pkt)
> +static int frame_data_submit(AVCodecContext *avctx, AVFifo *fifo,
> +                             const AVFrame *frame)
> +{
> +    VPxContext *ctx = avctx->priv_data;
> +    const struct vpx_codec_enc_cfg *enccfg = ctx->encoder.config.enc;
> +
> +    FrameData        fd = { .pts = frame->pts };
> +

The alignment of this declaration looks strange.

> +    AVFrameSideData *av_uninit(sd);
> +    int ret;
> +
> +#if CONFIG_LIBVPX_VP9_ENCODER
> +    // Keep HDR10+ if it has bit depth higher than 8 and
> +    // it has PQ trc (SMPTE2084).

Out of curiosity are there any HDR10+ files in fate?

> [...]
> diff --git a/libavcodec/version.h b/libavcodec/version.h
> index 06631ffa8c..789d9047c2 100644
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -30,7 +30,7 @@
>  #include "version_major.h"
>
>  #define LIBAVCODEC_VERSION_MINOR   4
> -#define LIBAVCODEC_VERSION_MICRO 100
> +#define LIBAVCODEC_VERSION_MICRO 101
>

This needs a rebase to apply cleanly.
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [FFmpeg-devel] [PATCH 6/8] lavc/libvpxenc: drop a useless condition
  2023-02-28 12:01 ` [FFmpeg-devel] [PATCH 6/8] lavc/libvpxenc: drop a useless condition Anton Khirnov
@ 2023-02-28 21:12   ` James Zern
  0 siblings, 0 replies; 18+ messages in thread
From: James Zern @ 2023-02-28 21:12 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Tue, Feb 28, 2023 at 4:02 AM Anton Khirnov <anton@khirnov.net> wrote:
>
> A non-NULL packet is always passed to frame_data_apply().
> ---
>  libavcodec/libvpxenc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

lgtm

> diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
> index af16e53deb..33f35bbaf4 100644
> --- a/libavcodec/libvpxenc.c
> +++ b/libavcodec/libvpxenc.c
> @@ -401,7 +401,7 @@ static int frame_data_apply(AVCodecContext *avctx, AVFifo *fifo, AVPacket *pkt)
>  {
>      FrameData fd;
>      uint8_t *data;
> -    if (!pkt || av_fifo_peek(fifo, &fd, 1, 0) < 0)
> +    if (av_fifo_peek(fifo, &fd, 1, 0) < 0)
>          return 0;
>      if (fd.pts != pkt->pts)
>          return 0;
> --
> 2.39.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 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".

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [FFmpeg-devel] [PATCH 8/8] lavc/libvpxenc: drop FrameListData.duration
  2023-02-28 12:01 ` [FFmpeg-devel] [PATCH 8/8] lavc/libvpxenc: drop FrameListData.duration Anton Khirnov
@ 2023-02-28 21:16   ` James Zern
  2023-03-01 12:07     ` Anton Khirnov
  0 siblings, 1 reply; 18+ messages in thread
From: James Zern @ 2023-02-28 21:16 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Tue, Feb 28, 2023 at 4:02 AM Anton Khirnov <anton@khirnov.net> wrote:
>
> It is write-only.
> ---
>  libavcodec/libvpxenc.c | 3 ---
>  1 file changed, 3 deletions(-)
>

libaomenc.c transfers this to AVPacket. You added this in:
  7cf161abe5 lavc/libaomenc: pass through frame durations to encoded packets
Should the same be done here?
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [FFmpeg-devel] [PATCH 5/8] lavc/libvpxenc: handle frame durations and AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE
  2023-02-28 12:01 ` [FFmpeg-devel] [PATCH 5/8] lavc/libvpxenc: handle frame durations and AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE Anton Khirnov
  2023-02-28 21:11   ` James Zern
@ 2023-02-28 23:30   ` James Almer
  2023-03-01 12:04     ` Anton Khirnov
  1 sibling, 1 reply; 18+ messages in thread
From: James Almer @ 2023-02-28 23:30 UTC (permalink / raw)
  To: ffmpeg-devel

On 2/28/2023 9:01 AM, Anton Khirnov wrote:
> +#if FF_API_REORDERED_OPAQUE
> +FF_DISABLE_DEPRECATION_WARNINGS
> +    avctx->reordered_opaque = fd.reordered_opaque;
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif

If this was not being set before this patch, does it make sense at all 
to set it considering it's a deprecated field? I remember for example we 
would not fill avctx->coded_frame on new encoders after it was deprecated.
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [FFmpeg-devel] [PATCH 5/8] lavc/libvpxenc: handle frame durations and AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE
  2023-02-28 21:11   ` James Zern
@ 2023-03-01 12:01     ` Anton Khirnov
  0 siblings, 0 replies; 18+ messages in thread
From: Anton Khirnov @ 2023-03-01 12:01 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Quoting James Zern (2023-02-28 22:11:29)
> On Tue, Feb 28, 2023 at 4:01 AM Anton Khirnov <anton@khirnov.net> wrote:
> > diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
> > index 77921badba..af16e53deb 100644
> > --- a/libavcodec/libvpxenc.c
> > +++ b/libavcodec/libvpxenc.c
> > @@ -68,6 +68,14 @@ struct FrameListData {
> >
> >  typedef struct FrameData {
> >      int64_t pts;
> > +    int64_t duration;
> > +
> > +#if FF_API_REORDERED_OPAQUE
> > +    int64_t      reordered_opaque;
> > +#endif
> > +    void        *frame_opaque;
> > +    AVBufferRef *frame_opaque_ref;
> > +
> >      AVBufferRef *hdr10_plus;
> >  } FrameData;
> >
> > @@ -329,32 +337,101 @@ static av_cold void free_frame_list(struct FrameListData *list)
> >      }
> >  }
> >
> > +static void frame_data_uninit(FrameData *fd)
> > +{
> > +    av_buffer_unref(&fd->frame_opaque_ref);
> > +    av_buffer_unref(&fd->hdr10_plus);
> > +}
> > +
> >  static av_cold void fifo_free(AVFifo **fifo)
> >  {
> >      FrameData fd;
> >      while (av_fifo_read(*fifo, &fd, 1) >= 0)
> > -        av_buffer_unref(&fd.hdr10_plus);
> > +        frame_data_uninit(&fd);
> >      av_fifo_freep2(fifo);
> >  }
> >
> > -static int frame_data_apply(AVFifo *fifo, AVPacket *pkt)
> > +static int frame_data_submit(AVCodecContext *avctx, AVFifo *fifo,
> > +                             const AVFrame *frame)
> > +{
> > +    VPxContext *ctx = avctx->priv_data;
> > +    const struct vpx_codec_enc_cfg *enccfg = ctx->encoder.config.enc;
> > +
> > +    FrameData        fd = { .pts = frame->pts };
> > +
> 
> The alignment of this declaration looks strange.

Ah yes, it's a remnant from before I moved it to a separate function.
Fixed locally.

> 
> > +    AVFrameSideData *av_uninit(sd);
> > +    int ret;
> > +
> > +#if CONFIG_LIBVPX_VP9_ENCODER
> > +    // Keep HDR10+ if it has bit depth higher than 8 and
> > +    // it has PQ trc (SMPTE2084).
> 
> Out of curiosity are there any HDR10+ files in fate?

Yes, there is hevc/hdr10_plus_h265_sample.hevc. I tested that the side
data does appear on output packets.

> 
> > [...]
> > diff --git a/libavcodec/version.h b/libavcodec/version.h
> > index 06631ffa8c..789d9047c2 100644
> > --- a/libavcodec/version.h
> > +++ b/libavcodec/version.h
> > @@ -30,7 +30,7 @@
> >  #include "version_major.h"
> >
> >  #define LIBAVCODEC_VERSION_MINOR   4
> > -#define LIBAVCODEC_VERSION_MICRO 100
> > +#define LIBAVCODEC_VERSION_MICRO 101
> >
> 
> This needs a rebase to apply cleanly.

Rebased locally.

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [FFmpeg-devel] [PATCH 5/8] lavc/libvpxenc: handle frame durations and AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE
  2023-02-28 23:30   ` James Almer
@ 2023-03-01 12:04     ` Anton Khirnov
  0 siblings, 0 replies; 18+ messages in thread
From: Anton Khirnov @ 2023-03-01 12:04 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Quoting James Almer (2023-03-01 00:30:25)
> On 2/28/2023 9:01 AM, Anton Khirnov wrote:
> > +#if FF_API_REORDERED_OPAQUE
> > +FF_DISABLE_DEPRECATION_WARNINGS
> > +    avctx->reordered_opaque = fd.reordered_opaque;
> > +FF_ENABLE_DEPRECATION_WARNINGS
> > +#endif
> 
> If this was not being set before this patch, does it make sense at all 
> to set it considering it's a deprecated field? I remember for example we 
> would not fill avctx->coded_frame on new encoders after it was deprecated.

AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE currently guarantees that the
encoder will set reordered_opaque. The users might rely on it, so we
should keep the behavior until reordered_opaque is gone.

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [FFmpeg-devel] [PATCH 8/8] lavc/libvpxenc: drop FrameListData.duration
  2023-02-28 21:16   ` James Zern
@ 2023-03-01 12:07     ` Anton Khirnov
  0 siblings, 0 replies; 18+ messages in thread
From: Anton Khirnov @ 2023-03-01 12:07 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Quoting James Zern (2023-02-28 22:16:39)
> On Tue, Feb 28, 2023 at 4:02 AM Anton Khirnov <anton@khirnov.net> wrote:
> >
> > It is write-only.
> > ---
> >  libavcodec/libvpxenc.c | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> 
> libaomenc.c transfers this to AVPacket. You added this in:
>   7cf161abe5 lavc/libaomenc: pass through frame durations to encoded packets
> Should the same be done here?

We could do that, but the duration types do not match (we use int64_t,
libvpx has unsigned long), which adds corner cases that need to be
handled. Since I need the machinery for opaque[_ref] anyway, might as
well use it for duration.
I'll do the same for aomenc when I get to implementing opaque
passthrough for it.

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2023-03-01 12:07 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-28 12:00 [FFmpeg-devel] [PATCH 1/8] lavu/frame: improve AVFrame.opaque[_ref] documentation Anton Khirnov
2023-02-28 12:00 ` [FFmpeg-devel] [PATCH 2/8] lavc/libvpxenc: drop frame_number Anton Khirnov
2023-02-28 20:50   ` James Zern
2023-02-28 12:00 ` [FFmpeg-devel] [PATCH 3/8] lavc/libvpxenc: reindent Anton Khirnov
2023-02-28 20:50   ` James Zern
2023-02-28 12:01 ` [FFmpeg-devel] [PATCH 4/8] lavc/libvpxenc: rename hdr10_plus_fifo and related objects Anton Khirnov
2023-02-28 20:55   ` James Zern
2023-02-28 12:01 ` [FFmpeg-devel] [PATCH 5/8] lavc/libvpxenc: handle frame durations and AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE Anton Khirnov
2023-02-28 21:11   ` James Zern
2023-03-01 12:01     ` Anton Khirnov
2023-02-28 23:30   ` James Almer
2023-03-01 12:04     ` Anton Khirnov
2023-02-28 12:01 ` [FFmpeg-devel] [PATCH 6/8] lavc/libvpxenc: drop a useless condition Anton Khirnov
2023-02-28 21:12   ` James Zern
2023-02-28 12:01 ` [FFmpeg-devel] [PATCH 7/8] lavc/libvpxenc: handle queue desync more gracefully Anton Khirnov
2023-02-28 12:01 ` [FFmpeg-devel] [PATCH 8/8] lavc/libvpxenc: drop FrameListData.duration Anton Khirnov
2023-02-28 21:16   ` James Zern
2023-03-01 12:07     ` Anton Khirnov

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git