[FFmpeg-devel] [PATCH] use new metadata API in realmedia

Michael Niedermayer michaelni
Tue Feb 17 14:00:23 CET 2009


On Tue, Feb 17, 2009 at 12:52:21PM +0100, Aurelien Jacobs wrote:
> On Tue, 17 Feb 2009 01:42:37 +0100
> Michael Niedermayer <michaelni at gmx.at> wrote:
> 
> > On Tue, Feb 17, 2009 at 01:15:04AM +0100, Aurelien Jacobs wrote:
> > > Hi,
> > > 
> > > Attached patch converts rm muxer and demuxer to the new metadata API.
> > > 
> > > Aurel
> > > Index: libavformat/rmenc.c
> > > ===================================================================
> > > --- libavformat/rmenc.c	(revision 17387)
> > > +++ libavformat/rmenc.c	(working copy)
> > > @@ -70,7 +70,18 @@
> > >      const char *desc, *mimetype;
> > >      int nb_packets, packet_total_size, packet_max_size, size, packet_avg_size, i;
> > >      int bit_rate, v, duration, flags, data_pos;
> > > +    const char *title, *author, *copyright, *comment;
> > > +    AVMetadataTag *tag;
> > >  
> > > +    tag = av_metadata_get(ctx->metadata, "title"    , NULL, 0);
> > > +    title     = tag ? tag->value : "";
> > > +    tag = av_metadata_get(ctx->metadata, "author"   , NULL, 0);
> > > +    author    = tag ? tag->value : "";
> > > +    tag = av_metadata_get(ctx->metadata, "copyright", NULL, 0);
> > > +    copyright = tag ? tag->value : "";
> > > +    tag = av_metadata_get(ctx->metadata, "comment"  , NULL, 0);
> > > +    comment   = tag ? tag->value : "";
> > > +
> > >      start_ptr = s->buf_ptr;
> > >  
> > >      put_tag(s, ".RMF");
> > > @@ -123,14 +134,14 @@
> > >      /* comments */
> > >  
> > >      put_tag(s,"CONT");
> > > -    size = strlen(ctx->title) + strlen(ctx->author) + strlen(ctx->copyright) +
> > > -        strlen(ctx->comment) + 4 * 2 + 10;
> > > +    size = strlen(title) + strlen(author) + strlen(copyright) +
> > > +        strlen(comment) + 4 * 2 + 10;
> > >      put_be32(s,size);
> > >      put_be16(s,0);
> > > -    put_str(s, ctx->title);
> > > -    put_str(s, ctx->author);
> > > -    put_str(s, ctx->copyright);
> > > -    put_str(s, ctx->comment);
> > > +    put_str(s, title);
> > > +    put_str(s, author);
> > > +    put_str(s, copyright);
> > > +    put_str(s, comment);
> > 
> > for(i=0; i<FF_ARRAY_ELEMS(meta); i++)
> >     put_str(s, av_metadata_get(ctx->metadata, meta[i], NULL, 0));
> 
> This is not exactly as simple, because:
>  - av_metadata_get() don't return a char*
>  - av_metadata_get() can return NULL
> So this would looks more like:
>   for(i=0; i<FF_ARRAY_ELEMS(meta); i++) {
>      AVMetadataTag *tag = av_metadata_get(ctx->metadata, meta[i],NULL,0);
>      put_str(s, tag ? tag->value : "");
>   }

put_str(s, av_metadata_get(ctx->metadata, meta[i],NULL,0)->value);
with a less picky put_str() could be used. Iam not saying i
suggest that just that it can be done.


> But even with this, we are still left we the size calculation above.

sizepos= url_ftell();
for()
    ...
put_be32_at(url_ftell() - sizepos, sizepos);

is one way, again not saying i suggest it just showing that it can be done


> It could be done with an additionnal loop, like:
>   size =  4 * 2 + 10;
>   for(i=0; i<FF_ARRAY_ELEMS(meta); i++) {
>      AVMetadataTag *tag = av_metadata_get(ctx->metadata, meta[i],NULL,0);
>      size += tag ? strlen(tag->value) : 0;
>   }
> So this starts looking not so much better than my initial version.
> Still, attached patch contains those modifications.
> 
> An obvious alternative would be to use a dyn_buf, so that we can
> use a single loop, but this would be even more ugly IMO:
>   char *buf;
>   int size;
>   int ret = url_open_dyn_buf(&pb);
>   if(ret < 0)
>     return ret;

>   size =  4 * 2 + 10;
>   for(i=0; i<FF_ARRAY_ELEMS(meta); i++) {
>      AVMetadataTag *tag = av_metadata_get(ctx->metadata, meta[i],NULL,0);
>      char *value = tag ? tag->value : "";
>      size += strlen(value);
>      put_str(s, value);
>   }
>   size = url_close_dyn_buf(pb, &buf);

some of your size code seems dead ...


>   put_be32(s,size);
>   put_be16(s,0);
>   put_buffer(s, buf, size);
>   av_free(buf);
> 
> > >      for(i=0;i<ctx->nb_streams;i++) {
> > >          int codec_data_size;
> > > Index: libavformat/rmdec.c
> > > ===================================================================
> > > --- libavformat/rmdec.c	(revision 17387)
> > > +++ libavformat/rmdec.c	(working copy)
> > > @@ -72,6 +72,13 @@
> > >      get_strl(pb, buf, buf_size, get_byte(pb));
> > >  }
> > >  
> > > +static void get_tag(AVFormatContext *s, int len, const char *name)
> > > +{
> > > +    char buf[1024];
> > > +    get_strl(s->pb, buf, sizeof(buf), len);
> > > +    av_metadata_set(&s->metadata, name, buf);
> > > +}
> > > +
> > >  RMStream *ff_rm_alloc_rmstream (void)
> > >  {
> > >      RMStream *rms = av_mallocz(sizeof(RMStream));
> > > @@ -95,10 +102,10 @@
> > >      if (((version >> 16) & 0xff) == 3) {
> > >          int64_t startpos = url_ftell(pb);
> > >          url_fskip(pb, 14);
> > > -        get_str8(pb, s->title, sizeof(s->title));
> > > -        get_str8(pb, s->author, sizeof(s->author));
> > > -        get_str8(pb, s->copyright, sizeof(s->copyright));
> > > -        get_str8(pb, s->comment, sizeof(s->comment));
> > > +        get_tag(s, get_byte(pb), "title");
> > > +        get_tag(s, get_byte(pb), "author");
> > > +        get_tag(s, get_byte(pb), "copyright");
> > > +        get_tag(s, get_byte(pb), "comment");
> > 
> > for()
> >     get_tag(s, ...meta_tag_name_whatever[i]);
> 
> OK.
> 
> Updated patch attached.
[...]
> @@ -123,14 +124,17 @@
>      /* comments */
>  
>      put_tag(s,"CONT");
> -    size = strlen(ctx->title) + strlen(ctx->author) + strlen(ctx->copyright) +
> -        strlen(ctx->comment) + 4 * 2 + 10;
> +    size =  4 * 2 + 10;
> +    for(i=0; i<FF_ARRAY_ELEMS(ff_rm_metadata); i++) {
> +        tag = av_metadata_get(ctx->metadata, ff_rm_metadata[i], NULL, 0);

> +        size += tag ? strlen(tag->value) : 0;

if(tag) size+= strlen(tag->value);


[...]
> Index: libavformat/rmdec.c
> ===================================================================
> --- libavformat/rmdec.c	(r?vision 17390)
> +++ libavformat/rmdec.c	(copie de travail)
> @@ -72,6 +72,13 @@
>      get_strl(pb, buf, buf_size, get_byte(pb));
>  }
>  
> +static void get_tag(AVFormatContext *s, int len, const char *name)
> +{
> +    char buf[1024];
> +    get_strl(s->pb, buf, sizeof(buf), len);
> +    av_metadata_set(&s->metadata, name, buf);
> +}
> +
>  RMStream *ff_rm_alloc_rmstream (void)
>  {
>      RMStream *rms = av_mallocz(sizeof(RMStream));
> @@ -89,16 +96,15 @@
>  {
>      char buf[256];
>      uint32_t version;
> +    int i;
>  
>      /* ra type header */
>      version = get_be32(pb); /* version */
>      if (((version >> 16) & 0xff) == 3) {
>          int64_t startpos = url_ftell(pb);
>          url_fskip(pb, 14);
> -        get_str8(pb, s->title, sizeof(s->title));
> -        get_str8(pb, s->author, sizeof(s->author));
> -        get_str8(pb, s->copyright, sizeof(s->copyright));
> -        get_str8(pb, s->comment, sizeof(s->comment));
> +        for (i=0; i<FF_ARRAY_ELEMS(ff_rm_metadata); i++)
> +            get_tag(s, get_byte(pb), ff_rm_metadata[i]);
>          if ((startpos + (version & 0xffff)) >= url_ftell(pb) + 2) {
>              // fourcc (should always be "lpcJ")
>              get_byte(pb);
> @@ -214,10 +220,8 @@
>              get_byte(pb);
>              get_byte(pb);
>  
> -            get_str8(pb, s->title, sizeof(s->title));
> -            get_str8(pb, s->author, sizeof(s->author));
> -            get_str8(pb, s->copyright, sizeof(s->copyright));
> -            get_str8(pb, s->comment, sizeof(s->comment));
> +            for (i=0; i<FF_ARRAY_ELEMS(ff_rm_metadata); i++)
> +                get_tag(s, get_byte(pb), ff_rm_metadata[i]);
>          }
>      }
>      return 0;
> @@ -317,6 +321,7 @@
>      unsigned int start_time, duration;
>      char buf[128];
>      int flags = 0;
> +    int i;
>  
>      tag = get_le32(pb);
>      if (tag == MKTAG('.', 'r', 'a', 0xfd)) {
> @@ -364,10 +369,8 @@
>              flags = get_be16(pb); /* flags */
>              break;
>          case MKTAG('C', 'O', 'N', 'T'):
> -            get_str16(pb, s->title, sizeof(s->title));
> -            get_str16(pb, s->author, sizeof(s->author));
> -            get_str16(pb, s->copyright, sizeof(s->copyright));
> -            get_str16(pb, s->comment, sizeof(s->comment));
> +            for (i=0; i<FF_ARRAY_ELEMS(ff_rm_metadata); i++)
> +                get_tag(s, get_be16(pb), ff_rm_metadata[i]);

these for loops can be factorized into "get_tag"

[..]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090217/f528ac79/attachment.pgp>



More information about the ffmpeg-devel mailing list