[FFmpeg-devel] [PATCH 2/4] lavd: make lavfi device export the metadata up to the AVFrame.
Nicolas George
nicolas.george at normalesup.org
Sun Oct 14 12:12:38 CEST 2012
Le duodi 22 vendémiaire, an CCXXI, Clément Bœsch a écrit :
> On Thu, Oct 11, 2012 at 11:22:53PM +0200, Clément Bœsch wrote:
> > On Wed, Oct 10, 2012 at 12:55:11AM +0200, Clément Bœsch wrote:
> > > TODO: lavc minor bump?
> > > ---
> > > libavcodec/avcodec.h | 1 +
> > > libavcodec/utils.c | 7 +++++++
> > > libavdevice/lavfi.c | 9 +++++++++
> > > 3 files changed, 17 insertions(+)
> > >
> >
> > After a discussion on IRC, we are considering something more complex but
> > more solid:
> > - in lavd/lavfi: convert each buffer ref metadata key/value into side
> > data type "meta" like let's say "key\0value\0"
> > - put some metadata dict in the raw a/v contexts (just like the tiff
> > decoder), and construct it based on the "meta" side data of each
> > packets. (AVCodecInternal was also mentioned, I need to look how it
> > helps)
> >
>
> Here we go, here is the V2. Should be better than the previous hack.
>
> --
> Clément B.
> From 5924da49cf4158f7ee4cdbfea22b822137b05be3 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 | 20 ++++++++++++++++++++
> 6 files changed, 67 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 3c3dad3..9370a56 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -941,6 +941,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_METATAGS,
We may need to add a gap in the values, to avoid ABI compatibility issues
with the fork. Unfortunately, the problem is already there with
AV_PKT_DATA_JP_DUALMONO. We may want to change that one very soon, while it
is still less than two months old and can not be considered a severe ABI
break.
> };
>
> typedef struct AVPacket {
> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> index ed38c61..60573d4 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_METATAGS 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 dd626dc..d9463e8 100644
> --- a/libavcodec/pcm.c
> +++ b/libavcodec/pcm.c
> @@ -58,8 +58,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;
> }
>
> @@ -470,6 +472,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 d426f43..2275029 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"
> @@ -258,6 +259,9 @@ static int raw_decode(AVCodecContext *avctx,
> }
> }
>
> + ff_set_metadata_from_side_data(avctx->coded_frame, avpkt);
> + frame->metadata = avctx->coded_frame->metadata;
> +
Are we to assume that the side-data metadata will only be taken into account
if it comes with PCM and rawvideo? That is not completely satisfactory, but
it is enough for the needs at hand, and it solves the risk of memory leak.
> *data_size = sizeof(AVPicture);
> return buf_size;
> }
> @@ -266,6 +270,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 183776a..d987e45 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_METATAGS, &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..faf918a 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,25 @@ 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);
> + }
> + meta = av_packet_new_side_data(pkt, AV_PKT_DATA_METATAGS, meta_buf.len);
You forgot to check that the bprint buffer was not truncated.
> + if (!meta)
> + return AVERROR(ENOMEM);
You are leaking the bprint buffer if this error happens.
> + 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;
Regards,
--
Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121014/c010ef8a/attachment.asc>
More information about the ffmpeg-devel
mailing list