[FFmpeg-devel] [PATCH 5/6] MOV: episode and season number metadata

[Baptiste Coudurier]

On 11/22/09 8:14 PM, David Conrad wrote:
> ---
>   libavformat/mov.c    |   14 ++++++++++++++
>   libavformat/movenc.c |   25 +++++++++++++++++++++++++
>   2 files changed, 39 insertions(+), 0 deletions(-)
>
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 8c42a7c..dee251d 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -93,6 +93,16 @@ static int mov_metadata_trkn(MOVContext *c, ByteIOContext *pb, unsigned len, con
>       return 0;
>   }
>
> +static int mov_metadata_be32(MOVContext *c, ByteIOContext *pb, unsigned len, const char *key)
> +{

I think mov won't never use le32. I feel mov_metadata_int32 is better, 
what do you think ?

> +    char buf[16];
> +
> +    snprintf(buf, sizeof(buf), "%d", get_be32(pb));
> +    av_metadata_set(&c->fc->metadata, key, buf);
> +
> +    return 0;
> +

I really think av_metadata_set_int could be introduced to factorize this 
code from all demuxers.

>   static int mov_read_udta_string(MOVContext *c, ByteIOContext *pb, MOVAtom atom)
>   {
>   #ifdef MOV_EXPORT_ALL_METADATA
> @@ -122,6 +132,10 @@ static int mov_read_udta_string(MOVContext *c, ByteIOContext *pb, MOVAtom atom)
>       case MKTAG( 't','v','s','h'): key = "show";      break;
>       case MKTAG( 't','v','e','n'): key = "episode_id";break;
>       case MKTAG( 't','v','n','n'): key = "network";   break;
> +    case MKTAG( 't','v','s','n'): key = "season";
> +        parse = mov_metadata_be32; break;
> +    case MKTAG( 't','v','e','s'): key = "episode";
> +        parse = mov_metadata_be32; break;
>       case MKTAG( 't','r','k','n'): key = "track";
>           parse = mov_metadata_trkn; break;
>       }

Humm, the switch is becoming huge, what about adding a table ?

> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index ac6378c..c2b2154 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -1361,6 +1361,29 @@ static int mov_write_trkn_tag(ByteIOContext *pb, MOVMuxContext *mov,
>       return size;
>   }
>
> +static int mov_write_be32_metadata(AVFormatContext *s, ByteIOContext *pb,
> +                                   const char *name, const char *tag)
> +{
> +    AVMetadataTag *t = av_metadata_get(s->metadata, tag, NULL, 0);
> +    int size = 0;
> +    if (t) {
> +        int64_t pos = url_ftell(pb);
> +        put_be32(pb, 0); /* size */
> +        put_tag(pb, name);
> +        {
> +            int64_t pos = url_ftell(pb);
> +            put_be32(pb, 0); /* size */
> +            put_tag(pb, "data");
> +            put_be32(pb, 0);        // 8 bytes empty
> +            put_be32(pb, 0);
> +            put_be32(pb, atoi(t->value));
> +            updateSize(pb, pos);
> +        }
> +        size = updateSize(pb, pos);
> +    }
> +    return size;
> +}

This code is ugly in the first place (where it was copied/pasted from), 
it has to be simplified first.

[...]

-- 
Baptiste COUDURIER
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                                  http://www.ffmpeg.org