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

[Reimar Döffinger]

On Wed, Sep 01, 2010 at 11:20:38PM +0200, Aurelien Jacobs wrote:
> Except those remarks, this part of the patch (simplification) is OK.

Applied with changes.

> >      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 ?

Easy enough to do, and fixes parsing of test case 4 up to that point,
however I have no idea how one is supposed to parse the file from
there (except wildly trying to parse until it works).

Index: libavformat/matroskadec.c
===================================================================
--- libavformat/matroskadec.c   (revision 25026)
+++ libavformat/matroskadec.c   (working copy)
@@ -575,6 +575,20 @@
     return read;
 }
 
+/**
+ * Read a EBML length value.
+ * This needs special handling for the "unknown length" case which has multiple
+ * encodings.
+ */
+static int ebml_read_length(MatroskaDemuxContext *matroska, ByteIOContext *pb,
+                            uint64_t *number)
+{
+    int res = ebml_read_num(matroska, pb, 8, number);
+    if (res > 0 && *number + 1 == 1ULL << (7 * res))
+        *number = 0xffffffffffffffULL;
+    return res;
+}
+
 /*
  * Read the next element as an unsigned int.
  * 0 is success, < 0 is failure.
@@ -780,7 +794,7 @@
 
     if (syntax->type != EBML_PASS && syntax->type != EBML_STOP) {
         matroska->current_id = 0;
-        if ((res = ebml_read_num(matroska, pb, 8, &length)) < 0)
+        if ((res = ebml_read_length(matroska, pb, &length)) < 0)
             return res;
     }