[FFmpeg-devel] [PATCH] lavc: add lavfi metadata support. [NEW ATTEMPT]

Clément Bœsch ubitux at gmail.com
Sat Oct 20 20:53:54 CEST 2012


On Wed, Oct 17, 2012 at 11:39:25AM +0200, Nicolas George wrote:
> Le sextidi 26 vendémiaire, an CCXXI, Clément Bœsch a écrit :
> > @@ -1572,6 +1595,7 @@ int attribute_align_arg avcodec_decode_video2(AVCodecContext *avctx, AVFrame *pi
> >          int did_split = av_packet_split_side_data(&tmp);
> >          apply_param_change(avctx, &tmp);
> >          avctx->pkt = &tmp;
> > +        set_metadata_from_side_data(avctx, picture);
> >          if (HAVE_THREADS && avctx->active_thread_type & FF_THREAD_FRAME)
> >              ret = ff_thread_decode_frame(avctx, picture, got_picture_ptr,
> >                                           &tmp);
> > @@ -1588,7 +1612,9 @@ int attribute_align_arg avcodec_decode_video2(AVCodecContext *avctx, AVFrame *pi
> >              if (!picture->width)                   picture->width               = avctx->width;
> >              if (!picture->height)                  picture->height              = avctx->height;
> >              if (picture->format == AV_PIX_FMT_NONE)   picture->format              = avctx->pix_fmt;
> > +
> >          }
> > +        avctx->metadata = picture->metadata;
> 
> It looks quite smart, and it seems it will work even with B-frames.
> 
> Unfortunately, I am afraid it will leak the dictionary if the codec is
> closed while it still has delayed frames that were not flushed.
> 

Argl and again I messed up and replied in private. New patch attached
following Nicolas' "private" comments and Stefano in the old thread.

Here is some context about the exchanges Nicolas & I got, and so how the
patch got changed:

> > > Can you be a bit more specific about that situation?
> > >
> > > To me it looks like every AVFrame metadata pointer will be saved in the
> > > avctx, and it can only be kept if the old one is destroyed (see the top of
> > > set_metadata_from_side_data). I'd be pretty happy to fix the problem
> > > you're talking about but I would need a bit more information...
> >
> > Actually, I may have read it wrong, but on second thought, does it work at
> > all? A lot of codecs, in fact more or less all non-trivial codecs, finish
> > their work by:
> >
> >     *data_size = sizeof(AVFrame);
> >     *(AVFrame*)data = *frame;
> >
> > which will undo any initialization done on the AVFrame structure before the
> > call to the decoder.
> >
> 
> Arhem, I feel stupid now. Yeah it worked with my test case but obviously
> not all the time. Code simplified and fixed: now the metadata are *added*
> to the AVFrame after the decode (and the rest with the avctx local
> metadata automatically deleted is kept unchanged).
> 
> Note: I also renamed AV_PKT_DATA_METADATA to AV_PKT_DATA_STRINGS_METADATA
> in this version because I'm quite worried about Libav introducing a
> AV_PKT_DATA_METADATA "soon" (and so leading to a name conflict).
> 
> I tested if there was any leak with tiff, video filter metadata (scene
> detect) and audio filter metadata (silence detect) and it seems fine.


-- 
Clément B.
-------------- next part --------------
From 36057e1ca33763537e0123a9dd88b4a46e491ca7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux at gmail.com>
Date: Tue, 9 Oct 2012 23:01:07 +0200
Subject: [PATCH 2/4] lavd: make lavfi device export the metadata up to the
 AVFrame.

TODO: lavc minor bump + APIChanges
---
 libavcodec/avcodec.h  |  6 ++++++
 libavcodec/internal.h |  7 +++++++
 libavcodec/pcm.c      |  8 ++++++--
 libavcodec/rawdec.c   |  6 ++++++
 libavcodec/utils.c    | 22 ++++++++++++++++++++++
 libavdevice/lavfi.c   | 22 ++++++++++++++++++++++
 6 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 0b3a19a..6c00634 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -942,6 +942,12 @@ enum AVPacketSideDataType {
      * @endcode
      */
     AV_PKT_DATA_JP_DUALMONO,
+
+    /**
+     * A list of zero terminated key/value strings. There is no end marker for
+     * the list, so it is required to rely on the side data size to stop.
+     */
+    AV_PKT_DATA_METADATA,
 };
 
 typedef struct AVPacket {
diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index ed38c61..0193be2 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -178,4 +178,11 @@ int ff_get_logical_cpus(AVCodecContext *avctx);
 
 int avpriv_h264_has_num_reorder_frames(AVCodecContext *avctx);
 
+/**
+ * Construct a new metadata dictionary in the given frame from a
+ * AV_PKT_DATA_METADATA side data of the provided packet.
+ * @note the frame->metadata is always destroyed
+ */
+int ff_set_metadata_from_side_data(AVFrame *frame, AVPacket *pkt);
+
 #endif /* AVCODEC_INTERNAL_H */
diff --git a/libavcodec/pcm.c b/libavcodec/pcm.c
index e2ae9f9..277e523 100644
--- a/libavcodec/pcm.c
+++ b/libavcodec/pcm.c
@@ -59,8 +59,10 @@ static av_cold int pcm_encode_init(AVCodecContext *avctx)
 
 static av_cold int pcm_encode_close(AVCodecContext *avctx)
 {
-    av_freep(&avctx->coded_frame);
-
+    if (avctx->coded_frame) {
+        av_dict_free(&avctx->coded_frame->metadata);
+        av_freep(&avctx->coded_frame);
+    }
     return 0;
 }
 
@@ -471,6 +473,8 @@ static int pcm_decode_frame(AVCodecContext *avctx, void *data,
         return -1;
     }
 
+    ff_set_metadata_from_side_data(&s->frame, avpkt);
+
     *got_frame_ptr   = 1;
     *(AVFrame *)data = s->frame;
 
diff --git a/libavcodec/rawdec.c b/libavcodec/rawdec.c
index eb96deb..db16e47 100644
--- a/libavcodec/rawdec.c
+++ b/libavcodec/rawdec.c
@@ -25,6 +25,7 @@
  */
 
 #include "avcodec.h"
+#include "internal.h"
 #include "raw.h"
 #include "libavutil/avassert.h"
 #include "libavutil/common.h"
@@ -265,6 +266,9 @@ static int raw_decode(AVCodecContext *avctx,
         }
     }
 
+    ff_set_metadata_from_side_data(avctx->coded_frame, avpkt);
+    frame->metadata = avctx->coded_frame->metadata;
+
     *data_size = sizeof(AVPicture);
     return buf_size;
 }
@@ -273,6 +277,8 @@ static av_cold int raw_close_decoder(AVCodecContext *avctx)
 {
     RawVideoContext *context = avctx->priv_data;
 
+    if (avctx->coded_frame)
+        av_dict_free(&avctx->coded_frame->metadata);
     av_freep(&context->buffer);
     return 0;
 }
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index d64de0e..d780132 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -2590,3 +2590,25 @@ int avcodec_is_open(AVCodecContext *s)
 {
     return !!s->internal;
 }
+
+int ff_set_metadata_from_side_data(AVFrame *frame, AVPacket *pkt)
+{
+    int size;
+    const uint8_t *side_meta;
+    const uint8_t *end;
+
+    av_dict_free(&frame->metadata);
+    side_meta = av_packet_get_side_data(pkt, AV_PKT_DATA_METADATA, &size);
+    if (!side_meta)
+        return 0;
+    end = side_meta + size;
+    while (side_meta < end) {
+        const uint8_t *key = side_meta;
+        const uint8_t *val = side_meta + strlen(key) + 1;
+        int ret = av_dict_set(&frame->metadata, key, val, 0);
+        if (ret < 0)
+            return ret;
+        side_meta = val + strlen(val) + 1;
+    }
+    return 0;
+}
diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c
index 944794f..e476262 100644
--- a/libavdevice/lavfi.c
+++ b/libavdevice/lavfi.c
@@ -27,6 +27,7 @@
 
 #include "float.h"              /* DBL_MIN, DBL_MAX */
 
+#include "libavutil/bprint.h"
 #include "libavutil/log.h"
 #include "libavutil/mem.h"
 #include "libavutil/opt.h"
@@ -339,6 +340,27 @@ static int lavfi_read_packet(AVFormatContext *avctx, AVPacket *pkt)
         memcpy(pkt->data, ref->data[0], size);
     }
 
+    if (ref->metadata) {
+        uint8_t *meta;
+        AVDictionaryEntry *e = NULL;
+        AVBPrint meta_buf;
+
+        av_bprint_init(&meta_buf, 0, AV_BPRINT_SIZE_UNLIMITED);
+        while ((e = av_dict_get(ref->metadata, "", e, AV_DICT_IGNORE_SUFFIX))) {
+            av_bprintf(&meta_buf, "%s", e->key);
+            av_bprint_chars(&meta_buf, '\0', 1);
+            av_bprintf(&meta_buf, "%s", e->value);
+            av_bprint_chars(&meta_buf, '\0', 1);
+        }
+        if (!av_bprint_is_complete(&meta_buf) ||
+            !(meta = av_packet_new_side_data(pkt, AV_PKT_DATA_METADATA, meta_buf.len))) {
+            av_bprint_finalize(&meta_buf, NULL);
+            return AVERROR(ENOMEM);
+        }
+        memcpy(meta, meta_buf.str, meta_buf.len);
+        av_bprint_finalize(&meta_buf, NULL);
+    }
+
     pkt->stream_index = stream_idx;
     pkt->pts = ref->pts;
     pkt->pos = ref->pos;
-- 
1.7.12.2

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121020/d703b644/attachment.asc>


More information about the ffmpeg-devel mailing list