[MPlayer-users] Sign extension bug in stream_read_qword_le

[Sean Veers]

Hello,

A bug in stream_read_qword_le causes the returned value to
unconditionally have the 32 most significant bits set if msb of the
4th byte read is set. This is a result of sign extension as a negative
signed int is promoted to a uint64_t: y|=stream_read_char(s)<<24;

A visible effect of this bug is errors in the generated keyframe
tables for certain large AVI files, making the files unseekable.

A cast to an unsigned type (uint32_t or uint64_t) fixes the bug
(compiled and verified to work):

--- mplayer-1.0-rc1svn20070812/stream/stream.h  2007-08-12
11:49:30.000000000 +0200
+++ mplayer-1.0-rc1svn20070812/stream/stream.h  2007-08-18
21:30:09.775666044 +0200
@@ -189,7 +189,7 @@ inline static uint64_t stream_read_qword_le
   y = stream_read_char(s);
   y|=stream_read_char(s)<<8;
   y|=stream_read_char(s)<<16;
-  y|=stream_read_char(s)<<24;
+  y|=(uint32_t)stream_read_char(s)<<24;
   y|=(uint64_t)stream_read_char(s)<<32;
   y|=(uint64_t)stream_read_char(s)<<40;
   y|=(uint64_t)stream_read_char(s)<<48;


$ gcc -v
Using built-in specs.
Target: x86_64-linux-gnu
Configured with: ../src/configure -v
--enable-languages=c,c++,fortran,objc,obj-c++,treelang --prefix=/usr
--enable-shared --with-system-zlib --libexecdir=/usr/lib
--without-included-gettext --enable-threads=posix --enable-nls
--with-gxx-include-dir=/usr/include/c++/4.1.3 --program-suffix=-4.1
--enable-__cxa_atexit --enable-clocale=gnu --enable-libstdcxx-debug
--enable-mpfr --enable-checking=release x86_64-linux-gnu
Thread model: posix
gcc version 4.1.3 20070812 (prerelease) (Debian 4.1.2-15)

Excerpt from -msglevel all=6, showing the error in the keyframe tables:

Before applying patch (stream not seekable):

Regenerating keyframe table for MPEG-4 video.
====== AVI Super Index Header ========
FCC (indx) dwSize (280) wLongsPerEntry(4)
bIndexSubType (0) bIndexType (0)
nEntriesInUse (16) dwChunkId (00dc)
dwReserved[0] (0) dwReserved[1] (0) dwReserved[2] (0)
===========================
ODML (00dc): [0] 0xffffffff9b37752c 0x3d7c8 31477
ODML (00dc): [1] 0xffffffff9b3b4cf4 0x3d7c8 31477
ODML (00dc): [2] 0xffffffff9b3f24bc 0x3d7c8 31477
ODML (00dc): [3] 0xffffffff9b42fc84 0x3d7c8 31477
ODML (00dc): [4] 0xffffffff9b46d44c 0x3d7c8 31477
ODML (00dc): [5] 0xffffffff9b4aac14 0x3d7c8 31477
ODML (00dc): [6] 0xffffffff9b4e83dc 0x3d7c8 31477
ODML (00dc): [7] 0xffffffff9b525ba4 0x3d7c8 31477
ODML (00dc): [8] 0xffffffff9b56336c 0x3d7c8 31477
ODML (00dc): [9] 0xffffffff9b5a0b34 0x3d7c8 31477
ODML (00dc): [10] 0xffffffff9b5de2fc 0x3d7c8 31477
ODML (00dc): [11] 0xffffffff9b61bac4 0x3d7c8 31477
ODML (00dc): [12] 0xffffffff9b65928c 0x3d7c8 31477
ODML (00dc): [13] 0xffffffff9b696a54 0x3d7c8 31477
ODML (00dc): [14] 0xffffffff9b6d421c 0x3d7c8 31477
ODML (00dc): [15] 0xffffffff9b7119e4 0x3c160 30760
list_end=0x37A

After applying patch (stream seekable):

Regenerating keyframe table for MPEG-4 video.
====== AVI Super Index Header ========
FCC (indx) dwSize (280) wLongsPerEntry(4)
bIndexSubType (0) bIndexType (0)
nEntriesInUse (16) dwChunkId (00dc)
dwReserved[0] (0) dwReserved[1] (0) dwReserved[2] (0)
===========================
ODML (00dc): [0] 0x000000039b37752c 0x3d7c8 31477
ODML (00dc): [1] 0x000000039b3b4cf4 0x3d7c8 31477
ODML (00dc): [2] 0x000000039b3f24bc 0x3d7c8 31477
ODML (00dc): [3] 0x000000039b42fc84 0x3d7c8 31477
ODML (00dc): [4] 0x000000039b46d44c 0x3d7c8 31477
ODML (00dc): [5] 0x000000039b4aac14 0x3d7c8 31477
ODML (00dc): [6] 0x000000039b4e83dc 0x3d7c8 31477
ODML (00dc): [7] 0x000000039b525ba4 0x3d7c8 31477
ODML (00dc): [8] 0x000000039b56336c 0x3d7c8 31477
ODML (00dc): [9] 0x000000039b5a0b34 0x3d7c8 31477
ODML (00dc): [10] 0x000000039b5de2fc 0x3d7c8 31477
ODML (00dc): [11] 0x000000039b61bac4 0x3d7c8 31477
ODML (00dc): [12] 0x000000039b65928c 0x3d7c8 31477
ODML (00dc): [13] 0x000000039b696a54 0x3d7c8 31477
ODML (00dc): [14] 0x000000039b6d421c 0x3d7c8 31477
ODML (00dc): [15] 0x000000039b7119e4 0x3c160 30760
list_end=0x37A

Regards,
S