Rockbox

  • Status Closed
  • Percent Complete
    0%
  • Task Type Patches
  • Category
  • Assigned To No-one
  • Operating System
  • Severity Low
  • Priority Very Low
  • Reported Version
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by fredyd - 2005-08-10
Last edited by linusnielsen - 2005-08-10

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

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.

Fred

Closed by  linusnielsen
2005-08-10 23:18
Reason for closing:  Accepted
Additional comments about closing:  

Logged In: YES
user_id=259137

Thanks a lot for your work!

Project Manager

Thanks a lot for your work!

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing