[FFmpeg-devel] [PATCH] use new metadata API in realmedia
Aurelien Jacobs
aurel
Tue Feb 17 16:20:35 CET 2009
On Tue, 17 Feb 2009 14:00:23 +0100
Michael Niedermayer <michaelni at gmx.at> wrote:
> 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.
No it can't. av_metadata_get() can return NULL, so av_metadata_get()->value
will segfault.
> > 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
I generally tend to avoid seeking, so I didn't even thought about this.
I would indeed work.
> > 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 ...
Indeed, it was quick and dirty code. The size calculation can be avoided with
dyn_buf. Still I don't think we should resort to dyn_buf for such a simple
situation.
> > > > 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);
OK. Done.
> [...]
> > 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"
Not as is, due to the get_byte() or get_be16(), but OK. Done with
a slightly modified get_tag().
Aurel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: md_rm_3.diff
Type: text/x-patch
Size: 5598 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090217/2cf94036/attachment.bin>
More information about the ffmpeg-devel
mailing list