[MPlayer-dev-eng] [PATCH] Duration of VBR-encoded MP3s was wrong

[Nicolas Bonifas]

> The idea is welcome, but someone should check this and make sure it
> doesn't introduce bugs (infinite loop? etc.) or vulnerabilities with
> invalid files before committing it.

I tried to identify possible vulnerabilities:
- If the XING or VBRI header indicates that there are 0 frames, there 
will be a division by 0. The new version of patch solves this problem 
(using a > instead of a >=).
Any other strictly positive value for sh_audio->wf->nAvgBytesPerSec, 
even wrong, will not cause problems: this value is only used for 
display, not for decoding, which relies on frame headers and not on the 
MPEG header.
- There may be problems in demux_audio_open, in demux_audio.c, if the 
Xing or VBRI keyword is just at the end of the stream. I added tests in 
the new patch that prevent frames_nb from being written from 
uninitialized values of frames_in_hdr if we are at EOF.
It does not matter if we call stream_seek or stream_read with invalid 
parameters, as these functions check by themselves that we are not 
beyond EOF.
- It is necessary that sh->wf is initialized prior to calling init, in 
ad_mp3lib.c (I use the fields sh->wf->nChannels and 
sh->wf->nAvgBytesPerSec). It seems to me that it is always the case, but 
this point should be checked by someone who has got a better knowledge 
of the code than mine.

Nicolas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mplayer-260307-2108.patch
Type: text/x-diff
Size: 7940 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20070326/f8066019/attachment.patch>