[FFmpeg-devel] [PATCH] lavc: add lavfi metadata support. [NEW ATTEMPT]
Clément Bœsch
ubitux at gmail.com
Sat Oct 20 20:55:46 CEST 2012
On Sat, Oct 20, 2012 at 08:53:54PM +0200, Clément Bœsch wrote:
> 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.
> 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(-)
>
And now with the correct patch...
--
Clément B.
-------------- next part --------------
From b893a2291f8b3d8c1d7cf04ad3553e17ae5eb6de Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux at gmail.com>
Date: Tue, 16 Oct 2012 23:49:03 +0200
Subject: [PATCH 1/3] lavc: add lavfi metadata support.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This commit introduces a new AVPacket side data type:
AV_PKT_DATA_STRINGS_METADATA. Its main goal is to provide a way to
transmit the metadata from the AVFilterBufferRef up to the AVFrame. This
is at the moment "only" useful for lavfi input from libavdevice:
lavd/lavfi only outputs packets, and the metadata from the buffer ref
kept in its context needs to be transmitted from the packet to the frame
by the decoders. The buffer ref can be destroyed at any time (along with
the metadata), and a duplication of the AVPacket needs to duplicate the
metadata as well, so the choice of using the side data to store them was
selected.
Making sure lavd/lavfi raises the metadata is useful to allow tools like
ffprobe to access the filters metadata (it is at the moment the only
way); ffprobe will now automatically show the AVFrame metadata in any
customizal output format for users. API users will also be able to
access the AVFrame->metadata pointer the same way ffprobe does
(av_frame_get_metadata).
All the changes are done in this single commit to avoid some memory
leaks: for instances, the changes in lavfi/avcodec.c are meant to
duplicate the metadata from the buffer ref into the AVFrame. Unless we
have an internal way of freeing the AVFrame->metadata automatically, it
will leak in most of the user apps. To fix this problem, we introduce
AVCodecContext->metadata and link avctx->metadata to the current
frame->metadata and free it at each decode frame call (and in the codec
closing callback for the last one). But doing this also means to update
the way the tiff decoder already handles the AVFrame->metadata (it's the
only one decoder handling metadata at the moment), by making sure it is
not trying to free a pointer already freed by the lavc internals.
The lavfi/avcodec.c buffer ref code is based on an old Thomas Kühnel
work, the rest of the code belongs to the commit author.
Signed-off-by: Thomas Kühnel <kuehnelth at googlemail.com>
Signed-off-by: Clément Bœsch <ubitux at gmail.com>
TODO: lavc and lavfi minor bump
TODO: update APIChanges
---
libavcodec/avcodec.h | 13 +++++++++++++
libavcodec/tiff.c | 7 +++----
libavcodec/utils.c | 28 ++++++++++++++++++++++++++++
libavdevice/lavfi.c | 23 +++++++++++++++++++++++
libavfilter/avcodec.c | 3 +++
libavfilter/avfilter.h | 2 ++
libavfilter/buffer.c | 8 ++++++++
7 files changed, 80 insertions(+), 4 deletions(-)
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 0b3a19a..3936d5e 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_STRINGS_METADATA,
};
typedef struct AVPacket {
@@ -3091,6 +3097,13 @@ typedef struct AVCodecContext {
int64_t pts_correction_num_faulty_dts; /// Number of incorrect DTS values so far
int64_t pts_correction_last_pts; /// PTS of the last frame
int64_t pts_correction_last_dts; /// DTS of the last frame
+
+ /**
+ * Current frame metadata.
+ * - decoding: maintained and used by libavcodec, not intended to be used by user apps
+ * - encoding: unused
+ */
+ AVDictionary *metadata;
} AVCodecContext;
AVRational av_codec_get_pkt_timebase (const AVCodecContext *avctx);
diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c
index 1f9a029..34dd42d 100644
--- a/libavcodec/tiff.c
+++ b/libavcodec/tiff.c
@@ -1018,8 +1018,9 @@ static int decode_frame(AVCodecContext *avctx,
s->compr = TIFF_RAW;
s->fill_order = 0;
free_geotags(s);
- /* free existing metadata */
- av_dict_free(&s->picture.metadata);
+ /* metadata has been destroyed from lavc internals, that pointer is not
+ * valid anymore */
+ s->picture.metadata = NULL;
// As TIFF 6.0 specification puts it "An arbitrary but carefully chosen number
// that further identifies the file as a TIFF file"
@@ -1169,8 +1170,6 @@ static av_cold int tiff_end(AVCodecContext *avctx)
TiffContext *const s = avctx->priv_data;
free_geotags(s);
- if (avctx->coded_frame && avctx->coded_frame->metadata)
- av_dict_free(&avctx->coded_frame->metadata);
ff_lzw_decode_close(&s->lzw);
if (s->picture.data[0])
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index e4b3ed7..6e83908 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -1592,6 +1592,31 @@ static void apply_param_change(AVCodecContext *avctx, AVPacket *avpkt)
}
}
+static int add_metadata_from_side_data(AVCodecContext *avctx, AVFrame *frame)
+{
+ int size, ret = 0;
+ const uint8_t *side_meta;
+ const uint8_t *end;
+
+ av_dict_free(&avctx->metadata);
+ side_meta = av_packet_get_side_data(avctx->pkt,
+ AV_PKT_DATA_STRINGS_METADATA, &size);
+ if (!side_meta)
+ goto end;
+ 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)
+ break;
+ side_meta = val + strlen(val) + 1;
+ }
+end:
+ avctx->metadata = frame->metadata;
+ return ret;
+}
+
int attribute_align_arg avcodec_decode_video2(AVCodecContext *avctx, AVFrame *picture,
int *got_picture_ptr,
const AVPacket *avpkt)
@@ -1630,6 +1655,7 @@ int attribute_align_arg avcodec_decode_video2(AVCodecContext *avctx, AVFrame *pi
if (!picture->height) picture->height = avctx->height;
if (picture->format == AV_PIX_FMT_NONE) picture->format = avctx->pix_fmt;
}
+ add_metadata_from_side_data(avctx, picture);
emms_c(); //needed to avoid an emms_c() call before every return;
@@ -1749,6 +1775,7 @@ int attribute_align_arg avcodec_decode_audio4(AVCodecContext *avctx,
if (!frame->sample_rate)
frame->sample_rate = avctx->sample_rate;
}
+ add_metadata_from_side_data(avctx, frame);
side= av_packet_get_side_data(avctx->pkt, AV_PKT_DATA_SKIP_SAMPLES, &side_size);
if(side && side_size>=10) {
@@ -1901,6 +1928,7 @@ av_cold int avcodec_close(AVCodecContext *avctx)
avctx->internal->byte_buffer_size = 0;
av_freep(&avctx->internal->byte_buffer);
av_freep(&avctx->internal);
+ av_dict_free(&avctx->metadata);
}
if (avctx->priv_data && avctx->codec && avctx->codec->priv_class)
diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c
index 944794f..555d048 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,28 @@ 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_STRINGS_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;
diff --git a/libavfilter/avcodec.c b/libavfilter/avcodec.c
index 313080d..5b5938c 100644
--- a/libavfilter/avcodec.c
+++ b/libavfilter/avcodec.c
@@ -33,6 +33,9 @@ int avfilter_copy_frame_props(AVFilterBufferRef *dst, const AVFrame *src)
dst->pos = av_frame_get_pkt_pos(src);
dst->format = src->format;
+ av_dict_free(&dst->metadata);
+ av_dict_copy(&dst->metadata, src->metadata, 0);
+
switch (dst->type) {
case AVMEDIA_TYPE_VIDEO:
dst->video->w = src->width;
diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h
index 510f28a..dccd420 100644
--- a/libavfilter/avfilter.h
+++ b/libavfilter/avfilter.h
@@ -180,6 +180,8 @@ typedef struct AVFilterBufferRef {
int perms; ///< permissions, see the AV_PERM_* flags
enum AVMediaType type; ///< media type of buffer data
+
+ AVDictionary *metadata; ///< dictionary containing metadata key=value tags
} AVFilterBufferRef;
/**
diff --git a/libavfilter/buffer.c b/libavfilter/buffer.c
index fc65b82..ae1867f 100644
--- a/libavfilter/buffer.c
+++ b/libavfilter/buffer.c
@@ -54,6 +54,10 @@ AVFilterBufferRef *avfilter_ref_buffer(AVFilterBufferRef *ref, int pmask)
if (!ret)
return NULL;
*ret = *ref;
+
+ ret->metadata = NULL;
+ av_dict_copy(&ret->metadata, ref->metadata, 0);
+
if (ref->type == AVMEDIA_TYPE_VIDEO) {
ret->video = av_malloc(sizeof(AVFilterBufferRefVideoProps));
if (!ret->video) {
@@ -172,6 +176,7 @@ void avfilter_unref_buffer(AVFilterBufferRef *ref)
av_freep(&ref->video->qp_table);
av_freep(&ref->video);
av_freep(&ref->audio);
+ av_dict_free(&ref->metadata);
av_free(ref);
}
@@ -197,6 +202,9 @@ void avfilter_copy_buffer_ref_props(AVFilterBufferRef *dst, AVFilterBufferRef *s
case AVMEDIA_TYPE_AUDIO: *dst->audio = *src->audio; break;
default: break;
}
+
+ av_dict_free(&dst->metadata);
+ av_dict_copy(&dst->metadata, src->metadata, 0);
}
AVFilterBufferRef *ff_copy_buffer_ref(AVFilterLink *outlink,
--
1.7.12.4
-------------- 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/f0606a5b/attachment.asc>
More information about the ffmpeg-devel
mailing list