Rockbox

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

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#6705 - Small ARM optimizations to MPA codec

Attached to Project: Rockbox
Opened by Tomasz Malesinski (tmal) - Wednesday, 28 February 2007, 00:38 GMT+2
Last edited by Dave Chapman (linuxstb) - Saturday, 28 July 2007, 17:21 GMT+2
Task Type Patches
Category Codecs
Status Closed
Assigned To No-one
Player Type SW-codec
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

Here are some optimizations that let me play mp3 files in realtime on Iriver iFP. The iFP benefits mostly by moving things into IRAM, since its external RAM is very slow (16-bit, 70 ns, no bursts). The patch needs some cleanup, but I won't be working on it in the next weeks, so I put it here in case someone wants to look at it.

One warning: there is a bug somewhere in Makefiles and sometimes make needs to be run twice to recompile and relink the codec.

The biggest change is a different dct32 routine. It is worse in terms of number of executed instructions, but it's smaller, so it fits in IRAM and uses less cache. Compared to my last patch ( FS#6670 ), it's slightly bigger and faster. Overall gain in speed depends on speed of the external RAM, so it may differ from player to player. On Ipods libmad is currently compiled with -O instead of -O2, probably because the original dct32 is smaller and faster when compiled with -O. With the new dct32 it's worth trying again with -O2.

Rest of the changes involve using ldm and stm instructions. Some give really minor gains.

With this patch the mpegplayer plugin does not compile on Ipod because of overfull IRAM.

Some ideas for further improvements:
- integrate III_overlap into III_imdct_l.
- at the cost of accuracy, we could try changing D coefficients (used in synth_full) to use at most 24 bits. ARM multiplies faster when some of the most significant bytes (bytes, not bits) of the last operand are 0.
   mpa.patch (33.8 KiB) 
 rockbox.1/apps/codecs/libmad/bit.c            |   24 +
 rockbox.1/apps/codecs/libmad/dct32_arm.S      |  324 ++++++++++++++++++++++
 rockbox.1/apps/codecs/libmad/layer3.c         |   32 ++
 rockbox.1/apps/codecs/libmad/SOURCES          |    2 
 rockbox.1/apps/codecs/libmad/synth.c          |  371 +++++++++++++++++++++++++-
 rockbox.1/apps/codecs/libmad/synth_full_arm.S |  316 ++++++++++++++++++++++
 6 files changed, 1068 insertions(+), 1 deletion(-)

This task depends upon

Closed by  Dave Chapman (linuxstb)
Saturday, 28 July 2007, 17:21 GMT+2
Reason for closing:  Accepted
Additional comments about closing:  Finally committed to SVN - thanks.
Comment by Toni (ahellmann) - Tuesday, 20 March 2007, 20:00 GMT+2
Hi tomal,
great work.
I tested your patch on the Sansa with the following result:
~18% performance increase of the codec in 192kBit files
Some WAV output samples differ between the current and your patch.
Do you know, where this rounding error comes from?
Comment by Thom Johansen (preglow) - Friday, 23 March 2007, 00:36 GMT+2
*** Posted to wrong Flyspray entry, sorry.
Comment by Tomasz Malesinski (tmal) - Saturday, 24 March 2007, 15:11 GMT+2
Thanks for testing. dct32 in this patch uses a different algorithm, so rounding errors are different.
Comment by Douglas Valentine (Dwyloc) - Thursday, 12 April 2007, 00:24 GMT+2
Your patch works very nicely on my ipod nano with no noticeable difference in audio quality.

Music playback
normal file type: mp3 214 kbps (VBR) 44100Hz stereo boost level: 48%
patched file type: mp3 214 kbps (VBR) 44100Hz stereo boost level: 36%

Audio book playback
normal file type: mp3 81kbps (VBR) 22050Hz mono boost level: 0%
patched file type: mp3 81kbps (VBR) 22050Hz mono boost level: 0%
Comment by Chris (decayed.cell) - Tuesday, 01 May 2007, 15:26 GMT+2
Hmm I think I got a warning

synth.c: In function ‘synth_full’:
synth.c:1053: warning: unused variable ‘sb’
Comment by Dave Chapman (linuxstb) - Friday, 18 May 2007, 00:05 GMT+2
I've just done some tests of this patch on my ipod (Color/Photo) with the new test_codec plugin. My test file was a 128kbps CBR lame-encoded MP3.

Current SVN decoded at 164% realtime.
With this patch applied - 175% realtime
With the two .S files and the synth_full function removed from IRAM - 183% realtime

Removing those functions from IRAM also means that mpegplayer compiles fine.
Comment by Bobby Graese (TrueJournals) - Tuesday, 22 May 2007, 00:36 GMT+2
Just tested this plugin, and this plugin with linuxstb's suggestion using the test_codec plugin. Here are the results:

UNPATCHED BUILD:
128kbps MP3: 292.70% realtime
VBR MP3: 150.89% realtime
PATCHED WITH MPA.PATCH:
128kbps MP3: 331.01% realtime
VBR MP3: 169.95% realtime
PATCHED AND REMOVED IRAM:
128kbps MP3: 345.93% realtime
VBR MP3: 180.56% realtime

Both MP3 files were about four minutes in length. These tests were done on an iriver H10. For me, mpegplayer did not compile after I removed the two .S files from IRAM. Attached is the patch file with the stuff removed from IRAM for those interested. I remember someone saying on IRC that we could also get mpegplayer to compile if we added a special conditional for mpegplayer so that it wouldn't use the IRAM changes. However, I have no idea how to do this, so I'll leave it to someone else...
   mpa_removed_iram.patch (18 KiB) 
 apps/codecs/libmad/layer3.c                    |   32 ++
 apps/codecs/libmad/SOURCES                     |    2 
 apps/codecs/libmad/synth.c                     |  371 ++++++++++++++++++++++++-
 apps/codecs/libmad/bit.c                       |   24 +
 firmware/target/arm/iriver/h10/button-target.h |    1 
 5 files changed, 429 insertions(+), 1 deletion(-)

Comment by Bobby Graese (TrueJournals) - Tuesday, 22 May 2007, 00:39 GMT+2
Err... forget the file I attached above... it didn't include the .S files for some reason. If you want to remove the added stuff from IRAM, simply change .icode to .text in the two S files.
Comment by Bobby Graese (TrueJournals) - Tuesday, 22 May 2007, 03:43 GMT+2
Turns ou that linuxstb also did another little modification... Here are the results for the complete modifications of the attached patch:

128kbps MP3: 345.93% realtime
VBR MP3: 180.73% realtime

So it seems to be about the same. Attached is the patch with the modifications linuxstb did.
   mpa_removed_iram.patch (35.4 KiB) 
 apps/codecs/libmad/layer3.c         |   32 +++
 apps/codecs/libmad/dct32_arm.S      |  324 +++++++++++++++++++++++++++++++
 apps/codecs/libmad/SOURCES          |    2 
 apps/codecs/libmad/synth.c          |  373 +++++++++++++++++++++++++++++++++++-
 apps/codecs/libmad/synth_full_arm.S |  316 ++++++++++++++++++++++++++++++
 apps/codecs/libmad/bit.c            |   24 ++
 6 files changed, 1070 insertions(+), 1 deletion(-)

Comment by Alexander Papst (DerPapst) - Tuesday, 22 May 2007, 13:39 GMT+2
Tested on a 3G iPod:

Song: 193,12s, 192kBit/s

Recent SVN: 117,79% (163,94s)
MPA Patch: 154,50% (124,99s)
MPA no IRAM: 130,19% (148,33s)

Forgot to test a VBR mp3. I can do a test with one in a few hours if needed.
Comment by MichaelGiacomelli (saratoga) - Tuesday, 24 July 2007, 03:40 GMT+2
I tested this on my Sansa today with no issues. mpegplayer and mp3 playback work normally.

Is there something else holding up this patch?
Comment by MichaelGiacomelli (saratoga) - Friday, 27 July 2007, 02:11 GMT+2
I've tested a few files on my Sansa with and without this patch, and the difference between the two seems very small. Most samples are identical, and those that are off seem to be off by only 1 bit. I computed the RMS difference (sum((SVN-patched)^2)/length)^.5 for one 128k file and another lame -v2 vbr file at each less then ~-120dB.

I've also been listening to a my albums for the last couple days and not noticed any issues. Unless anyone else has objections, perhaps we could commit this patch and see if any problems are reported?

Loading...