FS#10140 - Simple tremor optimisations

Attached to Project: Rockbox
Opened by Dave Hooper (stripwax) - Saturday, 18 April 2009, 11:11 GMT
Last edited by Steve Bavin (pondlife) - Saturday, 25 April 2009, 11:43 GMT
Task Type Patches
Category Codecs
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Version 3.2
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


Two simple optimisation patches, both applicable independently or together: one that fixes alignment checks on Coldfire, one that removes unnecessary memset on all targets.

Effect of alignment fix on coldfire (tested on h120): about 1Mhz improvement according to test_codec on 96, 350, and 500 bitrate samples; roughly unchanged at 128, 192 and 256 bitrate samples. Will have no effect on non-coldfire targets

Effect of removing redundant memsets (tested on h120): incrementally additional ~1.5Mhz improvement at 192 bitrates and above. Patch will definitely benefit other targets too (although I've not measured other targets here)
This task depends upon

Closed by  Steve Bavin (pondlife)
Saturday, 25 April 2009, 11:43 GMT
Reason for closing:  Accepted
Additional comments about closing:  Closed upon request of OP.
Comment by Dave Hooper (stripwax) - Saturday, 18 April 2009, 12:14 GMT
On a rerun of the timings for the memset removal patch, I get much smaller improvement (which seems more realistic) - more like 0.1Mhz rather than 1.5Mhz. Rerunning a few times seems to consistently give the attached rather than the previous VORBIS_h120_r20728_alignment_redundantmemset
Benefit of the memset removal patch on its own is therefore very small on coldfire h120 - but still it really is a redundant memset, and other targets might see greater improvements in any case
Comment by Nils Wallménius (nls) - Saturday, 18 April 2009, 12:38 GMT
just a small nitpick, please use /* comments */ intead of // as per docs/CONTRIBUTING
otherwise, great to see someone improving the tremor codec! :)
Comment by Dave Hooper (stripwax) - Saturday, 18 April 2009, 13:19 GMT
nls - sure. memset patch updated.
Comment by Dave Hooper (stripwax) - Saturday, 25 April 2009, 11:38 GMT
This can be closed now, as it has been incorporated into  FS#9882  and committed