[FFmpeg-devel] [PATCH 2/4] lavd: make lavfi device export the metadata up to the AVFrame.

[Clément Bœsch]

On Sun, Oct 14, 2012 at 01:14:07PM +0200, Stefano Sabatini wrote:
> On date Saturday 2012-10-13 16:36:55 +0200, Clément Bœsch encoded:
> > 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)
> 
> Are these two solutions complementary or mutually exclusive?
> 

Complementary; lavd/lavfi is adding the metadata key/value from the buffer
ref into side data, so it can be accessed by decoders and then copied in
the AVFrame.

> > > 
> > 
> > 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,
> 
> Nit: I don't like METATAGS, "META" means "beyond" (like in
> "metaphyisics") or "over" (like in "metamathematics"), meta-tags is
> abusing contraction rules resulting in semantical confusion. I
> suggest one of these:
> AV_PKT_DATA_METADATA_TAGS
> AV_PKT_DATA_METADATA
> AV_PKT_DATA_TAGS
> 
> I prefer the longest/more explicit, but I'm OK with the others as
> well.
> 

I wanted to avoid "datametadata", but if metatags is a problem:
AV_PKT_DATA_METADATA, I choose you. Consider it changed locally.

[...]
> >  
> > +    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);
> > +        if (!meta)
> > +            return AVERROR(ENOMEM);
> 
> Leak meta_buf is !meta (as noted by Nicolas).
> 

Yup, will fix

[...]

-- 
Clément B.
-------------- 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/20121014/45a0ac34/attachment.asc>