WINEMP3: increase factor from 4 to 12 in MPEG3_StreamSize
eric.pouech at wanadoo.fr
Tue Feb 14 04:48:29 CST 2006
Alex Villacís Lasso wrote:
> - Caller (winmm) computes required output buffer for encoded input
> buffer. This uses acmStreamSize, which in turn invokes MPEG3_StreamSize.
> - MPEG3_StreamSize return a factor 4X for the size of the output buffer
> (in my tests, a 2333-byte encoded buffer expands into (at most) 9332
> bytes of output, or so reports winemp3).
> - winmm happily calls acmStreamConvert() with a 2333 source buffer and a
> 9332 destination buffer.
> - mpglib decodes as much as it can into the 9332-byte buffer. However,
> in almost all cases, 9332 bytes is not enough to decode 2333 bytes of
> MP3 data. This is important to note, because it means a non-trivial
> amount of the MP3 data remains in the mpglib queue.
> - acmStreamConvert() is called over and over, with a 2333 source buffer
> and a 9332 destination buffer. The output buffer is always too small, so
> the undecoded data in the mpglib buffers accumulates.
> - winmm sends the last encoded buffer. Supposedly this means that
> decoding is complete, but it isn't, because mpglib has a backlog of
> undecoded data which has not been played. However, winmm stops there,
> and the user notices the cut in the middle of the song.
this explains a lot more why there's bad sound quality while playing mp3
> In the same situation, L3CODECA.ACM reports a non-integer factor for the
> output buffer, a little under 12X. Therefore L3CODECA.ACM always has
> enough space to consume the full source buffer and therefore does not
> cut the song in the middle.
> The ideal solution would be to dynamically compute the expansion factor
> based on the input and output bitrates. The attached patch raises the
> expansion factor instead to 12X to ensure that mpglib can always decode
> all the input data without running out of output space.
As you said, the factor should be computed from the input and output
formats, and shall not be a fixed factor. We need to change the factor,
but changing it for another fixed value is not the right way to go.
> + buffered_before = get_num_buffered_bytes(&amd->mp);
only compute it when you need (TRACE_ON...)
More information about the wine-devel