Bug#198883: Suggestion for solving the bug

[Ulrich Eckhardt]

Hi!

The reporter writes:
   Total data length: 272544776 bytes
   Playback length: 517m:00s
   Average bitrate: -68.169146 kbps

If you multiply the 273MB by the average eight bit bytesize, you end up with a 
number that can't be represented as a signed 32 bit integer. Rather, this 
wraps around, causing undefined behaviour and allowing this equation:

   272544776 * 8 = -2114609088

If you now divide this by 517 minutes and 60 seconds per minute you end up 
with the -68.something kbps. I'm not exactly sure where the culprit is, but 
the calculations in vorbis_end() surely seem suspicious to me. My suggestion 
for that is to use floating point arithmetic as long as possible, because 
with them big numbers only cause loss of precision which can usually be 
tolerated. In particular I suggest this micro patch...

-    bitrate = inf->bytes*8 / time / 1000.0;
+    bitrate = inf->bytes*8.0 / time / 1000.0;

...along with an audit for similar cases. Note: I'm looking at the upstream 
1.1.1 code, not the patched version of Debian at the moment, so actual code 
there may or may not differ. ;)


BTW: looking for the relevant code I came across a very similar piece of code, 
only that it was written not for Vorbis but Theora, you might want to check 
that one, too!

cheers

Uli