Rockbox

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

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#6848 - Optimizations to Vorbis codec

Attached to Project: Rockbox
Opened by Tomasz Malesinski (tmal) - Sunday, 18 March 2007, 15:05 GMT+2
Last edited by Tomasz Malesinski (tmal) - Tuesday, 25 September 2007, 23:02 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

I made some optimizations to the Vorbis codec. vector.patch is only for ARM (it shouldn't change anything for Coldfire), others should make the codec slightly faster on all targets. I would like someone to check how these patches affect CPU boost ratio on Ipod and Coldfire based targets.

Please note bug  FS#6847  when testing this (make sometimes needs to be run twice to relink the codec).
   codebook.patch (4.6 KiB) 
 new-rockbox.1/apps/codecs/Tremor/codebook.c |  103 ++++++++++++++++++++++++----
 new-rockbox.1/apps/codecs/Tremor/misc.h     |   10 ++
 2 files changed, 101 insertions(+), 12 deletions(-)

   coupling.patch (0.9 KiB) 
 new-rockbox.1/apps/codecs/Tremor/mapping0.c |   24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

   vector.patch (9.3 KiB) 
 new-rockbox.1/apps/codecs/Tremor/asm_arm.h     |  106 +++++++++++++++++++++++++
 new-rockbox.1/apps/codecs/Tremor/asm_mcf5249.h |   13 ++-
 new-rockbox.1/apps/codecs/Tremor/block.c       |   43 +---------
 new-rockbox.1/apps/codecs/Tremor/misc.h        |   45 ++++++++++
 new-rockbox.1/apps/codecs/Tremor/window.c      |   20 ----
 5 files changed, 168 insertions(+), 59 deletions(-)

This task depends upon

Closed by  Tomasz Malesinski (tmal)
Tuesday, 25 September 2007, 23:02 GMT+2
Reason for closing:  Accepted
Additional comments about closing:  Two of the patches were committed. I'll take a look at coupling.patch and possibly resubmit it later. Meanwhile I have some new patches.
Comment by Dominik Riebeling (bluebrother) - Sunday, 18 March 2007, 16:21 GMT+2
I just did a short comparison on my mini. I looked at the boost ratio in the audio thread debug screen, and after a while it settled at around 46% with patch as well as without the path. File is -q 5 and I waited for the buffer to get filled completely before I looked at the screen. Playback was running for about 5 minutes.
Comment by Dominik Riebeling (bluebrother) - Sunday, 18 March 2007, 16:32 GMT+2
D'oh! I got the rebuild issue of  FS#6847  and didn't notice it :(
Ok, ran the test again and boost ration now drops to 36%. Nice improvement!
Comment by Nils Wallménius (nls) - Sunday, 18 March 2007, 23:29 GMT+2
I tested the coupling and codebook patches on my h300, the coupling patch did not affect boost ratio at all (i think i saw a slight decrease but it could just as easily have been my imagination) i verified that the patch had taken by diffing the vorbis.codec files between testing and did reboot my player.
The codebook patch had a problem, all music had turned into horrible noise, but the boost dropped about 10% on one test track with both patches :-)

edit: it works fine in the sim, maybe an endian issue?
Comment by Thom Johansen (preglow) - Friday, 23 March 2007, 01:13 GMT+2
Adding endian swap to both reads from ptr[] seems to fix it. Gains also for Coldfire, but not so much as for ARM.
Comment by Sylvain Fourmanoit (syfou) - Friday, 23 March 2007, 06:38 GMT+2
coupling.patch seems to have an adverse effect on decoding of very low quality (-q -1) vorbis encoded with both libvorbis or libvorbis + aotuv; once applied, output is more choppy, and channel discontinuity more noticeable (at least on the Sansa E200 target).
Comment by Nils Wallménius (nls) - Friday, 23 March 2007, 16:00 GMT+2
Here is an updated version of the codebook patch that works on coldfire, I used
the letoh32() macro when ptr[] is accessed like preglow suggested so it should
still work fine on little endian devices too.

I did a little testing on my h300.
The firs number is boost ratio with a current svn build and the second is with the codebook patch.
Jewel of the summertime (q4) 16% 11%
Sabbath bloody sabbath (q6) 21% 12%
Ölstugan som inte finns (q4) 10% 6%
   codebook2.diff (4.4 KiB) 
 apps/codecs/Tremor/misc.h     |   10 ++++
 apps/codecs/Tremor/codebook.c |  103 +++++++++++++++++++++++++++++++++++++-----
 2 files changed, 101 insertions(+), 12 deletions(-)

Comment by Tomasz Malesinski (tmal) - Saturday, 24 March 2007, 16:16 GMT+2
Thanks for catching and correcting endianess issue. I commited codebook and vector patches.

Sylvain, did you notice the difference with only the coupling patch applied? I looked at the patch and could not find any bug, I also checked the output with and without the patch and could not see the difference. Could you provide me with a file that exposes the problem?
Comment by Sylvain Fourmanoit (syfou) - Sunday, 25 March 2007, 10:23 GMT+2
Tomasz,

applying only codebook.patch and vector.patch does not seem to affect the audio output quality; applying coupling.patch, alone or at the same time than the two others had a significant impact in my case. Here are a few clips for you to listen to:

http://www.filesend.net/download.php?f=0cd14e2767ea406ba54d45e4bcad96dc

I am not sure how much of the real hardware rockboxui emulates or simulates (I am pretty new to Rockbox), but the audiodump it generates is flawless, unlike what I hear on the real thing unfortunately, so I am not sure any of your code is even wrong... I should probably point out that the decoding of a vorbis stream encoded at higher quality (-q 3 for instance) is not affected by your patch from an audio standpoint... I wish I could help you more.
Comment by Dave Hooper (stripwax) - Tuesday, 03 April 2007, 01:12 GMT+2
What happens if mag>0 and ang= -2^31? Won't d=-d result in overflow? It looks like the original code would not overflow in this case. Maybe Sylvain's bug report results from the different ways the target and the sim behave in this situation.

I wonder if using ARM's conditional instructions could result in smaller/faster coupling code. (I haven't seen the output of gcc for the coupling code though)
Comment by Dominik Riebeling (bluebrother) - Wednesday, 01 August 2007, 17:19 GMT+2
afaict this has been submitted to svn except the coupling stuff. Does that still apply or can this task be closed?
Comment by Sylvain Fourmanoit (syfou) - Wednesday, 01 August 2007, 17:52 GMT+2
I just applied coupling.patch against rockbox trunk rev. 14120: on my sansa e280, the chopiness it initially caused (audible, vinyl-like scratches) is gone, but channel loss still occurs a few times per song on low quality (-q -1) libvorbis+aotuv.

Loading...