FS#2627 - fix to buffer overflow in dsp.c

Attached to Project: Rockbox
Opened by Frederic devernay (fredyd) - Wednesday, 10 August 2005, 13:30 GMT
Last edited by Linus Nielsen Feltzing (linusnielsen) - Wednesday, 10 August 2005, 23:18 GMT
Task Type Patches
Status Closed
Assigned To No-one
Operating System
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 0
Private No


If you happen to play 8kHz stereo files on rockbox with
the DSP enabled (e.g. on iriver), you'll clearly see a
buffer overflow on the screen (the fonts get erased!).
On Linux, it runs smoothly, and on Windows it does a
floating point exception or a segfault.

After a few hours of work on this, I finally found this
comment in dsp.c:

#define RESAMPLE_BUF_SIZE (256 * 4) /* Enough for
11,025 Hz -> 44,100 Hz*/

But in fact, the fix for this was not to change this
buffer size! Almost everything was in place in
playback.c to use several times the resample buffer if
necessary. I just had to fix a few lines in
dsp_output_size() and dsp_input_size().

I also thought carefully about what these functions
should return:
dsp_output_size should give a majorant of the output
buffer size
dsp_input_size should give a minorant of the buffer
input size.

In fact, I could rewrite dsp_input_size() so that it
gives the _exact_ output buffer size, using the
pre-computed delta used for upsampling AND
downsampling. Of course, I made sure there's no yield()
between this and dsp_process(), so that the value
cannot change.

Consequently, dsp_input_size() should ALWAYS be called
after dsp_output_size(), in case dsp_output_size()
detected an overflow.

In case there remains a bug in my code, I put some
protection code, and DEBUGF() that will make the
problem clearly visible on the simulator.

Now, you can play audio data sampled at ANY frequency
on rockbox.

Oh, the patch adds also one line to playback.c so that
rockbox doesn't crash when the metadata could not be
recovered. I already posted this on the list a while
ago, but nobody noticed it seems. The line is:
tracks[track_widx].filerem = 0;
And this line exists for all other error cases.

This task depends upon

Closed by  Linus Nielsen Feltzing (linusnielsen)
Wednesday, 10 August 2005, 23:18 GMT
Reason for closing:  Accepted
Additional comments about closing:  Logged In: YES

Thanks a lot for your work!
Comment by Linus Nielsen Feltzing (linusnielsen) - Wednesday, 10 August 2005, 23:18 GMT

Thanks a lot for your work!