[FFmpeg-devel] [PATCH][RFC] libavformat/mov Extend metadata handling to read in the keys from the 'keys' atom

[Michael Niedermayer]

On Thu, Sep 03, 2015 at 04:04:53PM +0100, Kevin Wheatley wrote:
> Looking for feedback on this patch, in particular the use of the
> 'keys' dictionary within the MOVContext structure.
> 
> With the patch FFmpeg can now 'find' the metadata in ARRI's sample
> footage see here:
> 
> http://www.arri.com/camera/alexa/learn/alexa_sample_footage/
> 



> Thanks
> 
> Kevin

>  isom.h |    1 +
>  mov.c  |   63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 64 insertions(+)
> db0a881fa28daf73a0f02f26fd8a0909d8caca0b  0001-RFC-Extend-metadata-handling-to-read-in-the-keys-fro.patch
> From 57feefffff09b636d8d80d3afe7dca20175ddb51 Mon Sep 17 00:00:00 2001
> From: Kevin Wheatley <kevin.j.wheatley at gmail.com>
> Date: Thu, 3 Sep 2015 15:58:49 +0100
> Subject: [PATCH] RFC: Extend metadata handling to read in the keys from the 'keys' atom
> 
> ---
>  libavformat/isom.h |    1 +
>  libavformat/mov.c  |   63 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 64 insertions(+), 0 deletions(-)
> 
> diff --git a/libavformat/isom.h b/libavformat/isom.h
> index aee9d6e..8c20ec9 100644
> --- a/libavformat/isom.h
> +++ b/libavformat/isom.h
> @@ -206,6 +206,7 @@ typedef struct MOVContext {
>      void *audible_fixed_key;
>      int audible_fixed_key_size;
>      struct AVAES *aes_decrypt;
> +    AVDictionary *keys_d; // Use a dictionary as an array a little bit ugly
>  } MOVContext;
>  
>  int ff_mp4_read_descr_len(AVIOContext *pb);
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 45367d3..1cf50bb 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -265,6 +265,7 @@ static int mov_read_udta_string(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>      uint32_t data_type = 0, str_size, str_size_alloc;
>      int (*parse)(MOVContext*, AVIOContext*, unsigned, const char*) = NULL;
>      int raw = 0;
> +    AVDictionaryEntry *entry = NULL;
>  
>      switch (atom.type) {
>      case MKTAG( '@','P','R','M'): key = "premiere_version"; raw = 1; break;
> @@ -351,6 +352,13 @@ static int mov_read_udta_string(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>      case MKTAG(0xa9,'w','r','n'): key = "warning";   break;
>      case MKTAG(0xa9,'w','r','t'): key = "composer";  break;
>      case MKTAG(0xa9,'x','y','z'): key = "location";  break;
> +    default:
> +        snprintf(key2, sizeof(key2), "%08x", atom.type);
> +        entry = av_dict_get(c->keys_d, key2, entry, 0);
> +        if (entry)
> +            key = entry->value;
> +        av_log(c->fc, AV_LOG_TRACE, "Attempting to match unknown atom type %08x %s in keys => '%s'\n", atom.type, key2, key);
> +        break;
>      }
>  retry:
>      if (c->itunes_metadata && atom.size > 8) {
> @@ -3184,6 +3192,58 @@ static int mov_read_meta(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>      return 0;
>  }
>  
> +static int mov_read_keys(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> +{
> +    AVDictionaryEntry *entry = NULL;
> +    uint32_t key_count, key_size, key_namespace, key_num;
> +    char *key_str;
> +    char key_num_str[8+1] = { 0 }; // Use hexadecimal string of a 32 bit integer + terminator
> +    int ret;
> +
> +    avio_r8(pb); /* version */
> +    avio_rb24(pb); /* flags */
> +    key_count = avio_rb32(pb);
> +    
> +    av_log(c->fc, AV_LOG_VERBOSE,
> +           "Reading keys sz: %"PRId64" %u\n", atom.size, key_count);
> +
> +    key_num = 1; // Keys are numbered from 1 and refered to in the ilst following.
> +    while (key_count--) {

> +        key_size = avio_rb32(pb) - 8;
> +        key_namespace = avio_rl32(pb);
> +        if (key_namespace == MKTAG('m','d','t','a')) {
> +            key_str = av_malloc(key_size + 1); /* Add null terminator */

missing checks for interger overflows of the addition and subtraction

also the subject says "RFC", is there a reason not to push this to
git master once it otherwise looks good ?

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150903/f93bb277/attachment.sig>