[FFmpeg-devel] [RFC] asf metadata

[Ronald S. Bultje]

Hi,

On Sun, Jan 4, 2009 at 2:07 PM, Benjamin Larsson <banan at ludd.ltu.se> wrote:
> With all the metadata stuff happening I'll just post this patch as
> reference. The added members are needed to make the wmapro spdif stream
> valid. And yes I know we don't have a spdif muxer yet.

Without looking at all code, this looks suspicious:

> @@ -125,14 +125,29 @@
>          return 0;
>  }
>
> -static int get_value(ByteIOContext *pb, int type){
> +static int get_value(ByteIOContext *pb, int type, int value_len){
> +    int rlen = 0;
> +    int val;
>      switch(type){
> -        case 2: return get_le32(pb);
> -        case 3: return get_le32(pb);
> -        case 4: return get_le64(pb);
> -        case 5: return get_le16(pb);
> -        default:return INT_MIN;
> +        case 2: // BOOL
> +        case 5: // WORD
> +            rlen = 2;
> +            val = get_le16(pb);
> +            break;
> +        case 3: // DWORD
> +            rlen = 4;
> +            val = get_le32(pb);
> +            break;
> +        case 4: // QWORD
> +            rlen = 8;
> +            val = get_le64(pb);
> +            break;
> +        default:
> +            val = INT_MIN;
>      }
> +    if (rlen < value_len)
> +        url_fskip(pb, value_len - rlen);
> +    return val;
>  }
>
>  static int asf_read_header(AVFormatContext *s, AVFormatParameters *ap)

The value for 2 switches from 32-bit to 16-bit, so you might want to
make sure that is correct...

> @@ -146,9 +161,17 @@
>      int64_t gsize;
>      AVRational dar[128];
>      uint32_t bitrate[128];
> +    uint32_t wmaDrcPeakRef[128];
> +    uint32_t wmaDrcPeakTarget[128];
> +    uint32_t wmaDrcAvgRef[128];
> +    uint32_t wmaDrcAvgTarget[128];
>
>      memset(dar, 0, sizeof(dar));
>      memset(bitrate, 0, sizeof(bitrate));
> +    memset(wmaDrcPeakRef, 0, sizeof(wmaDrcPeakRef));
> +    memset(wmaDrcPeakTarget, 0, sizeof(wmaDrcPeakTarget));
> +    memset(wmaDrcAvgRef, 0, sizeof(wmaDrcAvgRef));
> +    memset(wmaDrcAvgTarget, 0, sizeof(wmaDrcAvgTarget));
>
>      get_guid(pb, &g);
>      if (memcmp(&g, &asf_header, sizeof(GUID)))

Isn't any local variable automatically zero'ed already?

Ronald