[FFmpeg-devel] [PATCH] simplify and fix ebml_read_num

Aurelien Jacobs aurel
Wed Sep 1 23:20:38 CEST 2010


On Wed, Sep 01, 2010 at 08:13:07PM +0200, Reimar D?ffinger wrote:
> Hello,
> this obviously needs to be split before applying.
> It simplifies the code by using ff_log2_tab and it
> fixes recognizing unknown length values (e.g. a encoded
> value of 0xff also means unknown length).

> Index: libavformat/matroskadec.c
> ===================================================================
> --- libavformat/matroskadec.c	(revision 25017)
> +++ libavformat/matroskadec.c	(working copy)
> @@ -538,8 +538,8 @@
>  static int ebml_read_num(MatroskaDemuxContext *matroska, ByteIOContext *pb,
>                           int max_size, uint64_t *number)
>  {
> -    int len_mask = 0x80, read = 1, n = 1;
> -    int64_t total = 0;
> +    int read = 1, n = 1;
> +    uint64_t total = 0;
>  
>      /* The first byte tells us the length in bytes - get_byte() can normally
>       * return 0, but since that's not a valid first ebmlID byte, we can
> @@ -556,11 +556,8 @@
>      }
>  
>      /* get the length of the EBML number */
> -    while (read <= max_size && !(total & len_mask)) {
> -        read++;
> -        len_mask >>= 1;
> -    }
> -    if (read > max_size) {
> +    read = 8 - ff_log2_tab[total];
> +    if (!total || read > max_size) {

!total is already checked a few line above, so it can't be true here.

>          int64_t pos = url_ftell(pb) - 1;
>          av_log(matroska->ctx, AV_LOG_ERROR,
>                 "Invalid EBML number size tag 0x%02x at pos %"PRIu64" (0x%"PRIx64")\n",
> @@ -569,10 +566,14 @@
>      }
>  
>      /* read out length */
> -    total &= ~len_mask;
> +    total ^= 1 << (8 - read);

Here I'm not sure, but it might be more readable this way:
    total ^= 1 << ff_log2_tab[total];
But I don't really mind. Pick the way you prefer.

Except those remarks, this part of the patch (simplification) is OK.

>      while (n++ < read)
>          total = (total << 8) | get_byte(pb);
>  
> +    /* unknown length */
> +    if (total + 1 == 1ULL << (7 * read))
> +        total = 0xffffffffffffff;

I'm not sure if this is strictly correct. The matroska spec is not a
100% clear, but I seem to understand that this rule don't apply to all
kind of ebml number. It seems to only apply to element size.
So maybe we need a new ebml_read_length() function, similar to this
ebml_read_num(), but with this additionnal "unknown length" rule ?

Aurel



More information about the ffmpeg-devel mailing list