[FFmpeg-devel] [PATCH 2/4] lavd: make lavfi device export the metadata up to the AVFrame.
Stefano Sabatini
stefasab at gmail.com
Sat Oct 20 16:05:32 CEST 2012
On date Tuesday 2012-10-16 21:36:05 +0200, Clément Bœsch encoded:
> On Sun, Oct 14, 2012 at 02:34:34PM +0200, Clément Bœsch wrote:
> [...]
> > > > + 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.
> > >
> >
> > Mmh I never used av_bprint_is_complete()... 'will fix soon.
> >
> > > > + if (!meta)
> > > > + return AVERROR(ENOMEM);
> > >
> > > You are leaking the bprint buffer if this error happens.
> > >
> >
> > I'll send a new patch in a while, thanks
> >
>
> Both fixed, new patch attached.
>
> BTW, I've kept AV_PKT_DATA_METADATA for now. Anyone wants something else
> before I push?
>
> --
> 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(-)
>
> 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;
Nit: _metadata ("meta" is not a noun)
> + 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;
Again, s/meta/metadata/, calling a variable "meta" is the same as
calling it "super".
> + AVDictionaryEntry *e = NULL;
> + AVBPrint meta_buf;
> +
> + av_bprint_init(&meta_buf, 0, AV_BPRINT_SIZE_UNLIMITED);
Note: we may add an init macro, in order to avoid the av_bprint_init()
call (which is somehow awkward).
> + 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);
what about:
av_bprintf(&buf, "%s%c%s%c", e->key, 0, e->value, 0);
?
> + }
> + 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
Looks good otherwise, thanks for working on this.
--
FFmpeg = Formidable and Fabulous Mastering Portentous Enigmatic Gladiator
More information about the ffmpeg-devel
mailing list