Rockbox

Tasklist

FS#11709 - ARMv5E iQMF for libatrac

Attached to Project: Rockbox
Opened by MichaelGiacomelli (saratoga) - Monday, 01 November 2010, 01:16 GMT
Last edited by MichaelGiacomelli (saratoga) - Wednesday, 10 November 2010, 18:30 GMT
Task Type Patches
Category Codecs
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

Work in progress iQMF using the ARMv5E 32x16 multiply instructions. So far this converts libatrac to using 16 bit iQMF coefficients, and then prescales everything going into the iQMF by 8 bits, then the output another 7 bits. Unfortunately, the sim audio is somewhat garbled because the prescaling isn't performed correctly (scaling in the fixmul15 function works, but not prescaling the output of the imdct for some reason). I'm obviously missing something about how data is used in the codec, and either not scaling all the samples, or double scaling some of them.

I haven't tested the asm code, but its fairly simple so it shouldn't be too hard to fix once the sim is working. The ASM code is also not properly scheduled. Once its working it can be arranged to avoid pipeline stalls.
This task depends upon

Closed by  MichaelGiacomelli (saratoga)
Wednesday, 10 November 2010, 18:30 GMT
Reason for closing:  Accepted
Additional comments about closing:  Accepted in r28549.
Comment by Andree Buschmann (Buschel) - Monday, 01 November 2010, 09:18 GMT
The updated patch corrects scaling for simulation and preserves higher precision of the windowing coefficients.

I have defined a fixmul32x16(int16, int) which needs to do >>=15 to receive a 32bit result. THis was the main fault in your scaling. All the other pre-/post-scaling could be removed -- except the windowing coefficients themselves, of course.

Difference to svn decoding is max +/-4 of +/-32768 (~ -78 dB), and average ~ -110 dB. This is quite few.
Comment by MichaelGiacomelli (saratoga) - Monday, 01 November 2010, 13:56 GMT
>I have defined a fixmul32x16(int16, int) which needs to do >>=15 to receive a 32bit result. THis was the main fault in your scaling.

thanks, but I had that working, and am now trying to get it to work without a shift. I did some tests yesterday with the c code, and prescaling everything so that each multiply is a single smlaw<x> instruction gives acceptable results. The problem is I just can't seem to get it work once I put it all together :)
Comment by MichaelGiacomelli (saratoga) - Tuesday, 09 November 2010, 22:22 GMT
Updated patch to fix various mistakes in the ARM asm pointed out by buschel. Also cleaned up comments. Still doesn't work.
Comment by MichaelGiacomelli (saratoga) - Tuesday, 09 November 2010, 23:02 GMT
Thanks again buschel for spotting a stupid bug in the ASM. This version works. Further clean up and scheduling is still needed before this can be committed.

Speed up is quite large, 26% on my test file.
Comment by MichaelGiacomelli (saratoga) - Tuesday, 09 November 2010, 23:21 GMT
Restoring buschel's rounding when truncating to 16 bit.
Comment by MichaelGiacomelli (saratoga) - Wednesday, 10 November 2010, 00:09 GMT
* Fix building on armv4, coldfire and sim
* Disable 16 bit optimizations on these targets because theres no performance benefit

TODO:

* Improve scheduling
* Port this to mp3 filterbank ...
Comment by MichaelGiacomelli (saratoga) - Wednesday, 10 November 2010, 00:45 GMT
* Cleaned up useless changes
* Scheduled for arm11 by overlapping memory accesses with multiply. 2 out of every 3 arm11 loads should incur no stall at all.

Interestingly the improved scheduling speeds up arm9e by 0.5MHz, presumably because some of the memory latency on a cache miss is now hidden.

If anyone has an arm11 target handy it would be interesting to see v7 verses v8 comparisons. Test file here: http://duke.edu/~mgg6/rockbox/girls-just-wanna-atrc.rm

I think this is ready to commit.
Comment by MichaelGiacomelli (saratoga) - Wednesday, 10 November 2010, 15:42 GMT
* Add cache alignment for the Beast.

Comment by MichaelGiacomelli (saratoga) - Wednesday, 10 November 2010, 17:03 GMT
* Improve scheduling on arm11

arm11 iQMF goes from 15 -> 12MHz runtime. No speed up on arm9e.

On arm11 this version uses 2.8 cycles/tap, verses the ideal (zero wait state memory) 1.75 cycles/tap. However I don't see anyway to improve memory performance. Forcing 32 byte alignment of all buffers does not improve cache performance measurably. It may just be that the Beast ram is fairly slow.

Loading...