[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