Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#11533 - Failing malloc in vorbis crashes clipv1

Attached to Project: Rockbox
Opened by Bertrik Sikken (bertrik) - Wednesday, 11 August 2010, 21:05 GMT+2
Last edited by Bertrik Sikken (bertrik) - Monday, 23 August 2010, 22:11 GMT+2
Task Type Bugs
Category Codecs
Status Closed
Assigned To No-one
Player Type Another
Severity Low
Priority Normal
Reported Version Release 3.6
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

When playing back certain ogg vorbis files in a sequence on the clip v1, the player crashes (with a data abort). This was observed for q=-1 48 kbps ogg vorbis files.

The failing malloc is in apps/codecs/libtremor/block.c line 168 where it tries to allocate an internal PCM buffer from IRAM (using a naive free-less allocator) of 32 kB. This allocation fails, so the buffer is allocated from regular RAM (using the TLSF allocator) instead. This buffer is never freed so eventually after playing a few of these ogg vorbis files, the TLSF pool is exhausted. This causes the player to crash with a data abort.

The vorbis codec_main function does not reinitialise the TLSF pool when playing ogg vorbis files in sequence.
There seems to be a setjmp/longjmp framework already in place to handle allocation failures, but apparently this isn't working (anymore).

More information:
http://www.anythingbutipod.com/forum/showpost.php?p=489716&postcount=581
http://www.anythingbutipod.com/forum/showpost.php?p=489750&postcount=583
This task depends upon

Closed by  Bertrik Sikken (bertrik)
Monday, 23 August 2010, 22:11 GMT+2
Reason for closing:  Accepted
Additional comments about closing:  oggmalloc.diff committed as SVN r27866.
Comment by Nils Wallménius (nls) - Thursday, 12 August 2010, 09:53 GMT+2
this should be fixed in r27776 could anyone confirm?
Comment by Bertrik Sikken (bertrik) - Thursday, 12 August 2010, 22:03 GMT+2
Seems to work fine for me on target now, thanks!
Funny enough I first wanted to confirm Nausicaa's problem with crashes with the old rockbox version still on my clip (r27639). It found it just started to skip songs in an ogg album instead of crashing, suggesting that the longjmp/setjmp mechanism for failed allocation was basically doing its job after all. I'll do some bisecting to find out if this really got broken recently.
Comment by Bertrik Sikken (bertrik) - Saturday, 14 August 2010, 10:35 GMT+2
On my clipv1, I found no crashes when running into allocation problems with ogg, even with svn r27775 (the revision just before your fix), it just starts skipping tracks once running out of memory. So I assume the setjmp/longjmp stuff is working OK, not sure why Nausicaa found crashes with this.

IMO the only question left is whether we should also move the ogg_malloc_init and ogg_malloc_destroy from outside the "next track" loop to inside the "next track" loop in vorbis.c. This should prevent any other memory leak in one track from affecting the next track. Attached patch implements this.

What do you think?
   oggmalloc.diff (0.8 KiB) 
 apps/codecs/vorbis.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comment by Nils Wallménius (nls) - Saturday, 14 August 2010, 13:40 GMT+2
looks fine to me, although it feels like a kind of brute force approach :)

Loading...