Rockbox

I tested the patch on r27402 … compared to how wmapro performs against ffmpeg with the current mdct and codeclib’s, the error is not bad and I think I’ll commit it, but I’ll just wait a day or two for comments.

2 comments and 1 question from my side.

Question: Is there any impact to speed?

Comment 1: You should add a comment to the “false” inversion of the sign at all places where you will change it. The comment should point out that this is mandatory to use another mdct implementation.

Comment 2: I am not sure whether it is a good idea to shift quant by (nbits-3). If I understand right nbits is 7..12, quant itself is derived from quant_tab[] which contains quite small values which you might shift to zero. It would be better imho if you would shift the result of the vector multiplication. In this case you would drop the fast asm implementation. Is there any other possibility or do you experience overflows within the mdct itself? If not, an idea could be to use wmapro_window() to scale down the pcm signal. You could simply do this by pre-shifting the window tables, as nbits describes a dedicated window size and window table.

Edit: If I read the irc logs correct, the maximum sample diff between the svn wav output and this patches output is 662. If this is true, the difference is _massive_. It might be not audible due to masking effects, but the standard quality gate for sample diff is +/- 1 (= -90dB noise). 662 equals -34 dB.

Forget comment 2. This will not work as the mdct will overflow.

But nevertheless the attached patch uses another approach that should keep the precision of the output (not tested though). It does use a different scaling right at the beginning of the decoding process. Normally we shift decoded values by «17. Now we shift by «(17-nbits+3). As nbits-3 is always >17 we do not loose any precision in this step.

   wmapro_mdct_v02.patch (3.8 KiB)

v03:
- removes wmapro’s own mdct from the build
- removes a now unused 32 KB array which will be a benefit for low memory targets

   wmapro_mdct_v03.patch (6.3 KiB)

v04 is sync’ed against r27644.

Decoding speed for wmapro_173k.wma on PP5022: 28.6 MHz (svn r27644: 29.9 MHz)

   wmapro_mdct_v04.patch (5.4 KiB)

I have attached 2 png’s with screenshots. Those show the diff-files of (svn-output) - (patched-output) with a amplification of +40 dB and with a x-scale that shows samples. As you can see the diff v04 clearly shows burst 2k-spikes and some noise-burst-like spikes with a distance of ~13,000 samples. The second diff shows the effect of a larger fract part: the 2k-spikes are reduced, the other noise-burst-spikes stay. All were measured with the file wmapro_173k.wma.

Question: Is maybe the svn-version itself buggy and adds such spikes and high noise-floor? In the attached diffs you cannot see whether svn or the patched version introduces the errors. Did you compare svn and patched version against a reference decoder?

Edit: the third file shows the diff for the decoder with larger fract part. Left channel is the diff, scaled by +40 dB, the right channel shows the signal. As you can see the noise-bursts in the diff are correlated to the signal itself.

   v04.png (60.8 KiB)
   no_2k_spikes.png (38.9 KiB)
   no_2k_spikes_error_vs_signal.... (41.3 KiB)

With r27687, and then applying patches wmapro_mdct_v04 and wmapro_v08, the error is now quite nice : http://imagebin.ca/view/e_ixUuQC.html (max = 6e-5, mean = 7e-7 [absolute] ) . The error is measured against the original ffmpeg floating point decoder.
I still want to get rid of those spikes, but I think it can go in like this now, opinions ?

EDIT:
I attached a picture for a correct difference. The previous image showed the absolute error, in the attached image, it’s clear the spikes are only in the negative the side, maybe there’s some clipping (or use unsigned in place of signed) going on somewhere ?
Anyway now the mean error is 5.8e-8 max is 3e-5.

   diff3.jpg (78.4 KiB)

Great! The difference is now in the range of +/-1 (3e-5). The max difference is only -2 (6e-5). As the noise-burst spikes disappeared this means that the the former svn implementation had a bug. Therefor the new patched implementation is better than svn anyway. The few “spikes” of -2 will not be relevant.

I say: We should submit wmapro_v08 first, but need to either change the Coldfire asm in vector_fixmul_scalar() before submission or disable the relevant asm section for Coldfire asm. Afterwards wmapro_mdct_v04 can be submitted and we’re fine :o)