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

[Carl Eugen Hoyos]

Hi!

On 2007-03-26 19:15, Nicolas Bonifas wrote:
> > 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.

The original patch didn't compile for me (libmpdemux/muxer_mpeg.c &
libmpcodecs/ad_hwmpa.c), broke DVB playback (sh->wf is not always
initialized) and I removed one cosmetic change.

Somebody else should comment if the patch overall is a good idea. It
fixes duration of at least some of my mp3 files.

Carl Eugen
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: patchmp3duration
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20070329/ebf61345/attachment.asc>