Rockbox

Actually I did have coldfire in emac signed integer rounding mode at the final output stage (it uses emac to scale and perform clipping at once) but really don’t think this tiny DC bias is much of a concern for playback so I just used signed int mode. All that’s needed it to set the rounding flag back again and I guess it’s for free anyway. Also, I was getting a head for implementing a lot of ARM dsp in assembly to see if it helps things which with careful attention to avoid memory stalls, it should.

When comparing a reference decoded signal with the rockbox-output (unpatched), you will see, that there are two issues
a) DC-offset by small amount
b) white quantization noise at about -93dB average.

When using the DC-offset (which is nothing but rounding instead of truncating), the DC-offset has gone and the quantization
noise compared to reference decoder is about -107dB in average.

As all audio outputs are influenced, and if we’re talking of high precision audio output, we should correct this last stage.

On coldfire, it’s multiplying by 2^(16-scale) to cause anything > 0x7fff or < -0×8000 to get saturated by hardware and then taking the most significant 16 bits for the output sample. As a result hardware rounding won’t help for it (I just realized). But, it’s just a matter of initializing the accumulators with 2^15 to have it perform essentially (2^(16-scale) * decoder_sample + 2^15) / (2^16) = output_sample. I think I reasoned that out correctly but I’m tired. :P

As far as I understand the Coldfire solution, initializing accumulators with 2^15 is exactly what we need here.
Can you add such solution to this patch?

This adds the solution to the Colfire assembly in the output routines. No runtime comparisons performed however.

   dc_bias_dsp2.patch (8.3 KiB)

Merged all patches and diff’ed against current trunk (rev xyz). Now the final output stage (either dsp or test_codec) use same and correct rounding instead of simple truncation for all targets. This will give another few dB’s in accuray and lower the quantization noise.
No runtime comparisons performed, nevertheless any effect in runtime should at least be irrelevant (for stereo, 44.1kHz → additional 88200 adds/sec).

   dc_bias_v1.patch (10.1 KiB)

Sync’ed against 14147

Oops, now with patch file. Sync’ed against 14147

   dc_bias_v2.patch (10.1 KiB)

I still recieve an error saying

$ patch -p0 < dc_bias_v2.patch
patching file apps/dsp_cf.S
patching file apps/plugins/test_codec.c
patching file apps/dsp.c
patch unexpectedly ends in middle of line

IDK what to do about this…

Corrected. New line was missing at the end of file.

   dc_bias_v3.patch (10.1 KiB)

Looks to me like the dithering routine already has bias applied, and now gets it applied twice. Or do you see some issue here I don’t see? I’ll commit this soon.

Dithering routines remove bias because they spead the quantization errors across samples, correct? I think I agree that the dither output shouldn’t have additional rounding.

Reviewed the changes again. Now I am also not too sure about the correctness of adding the dc_bias while dithering. The error-feedback is calculated from “output &= ~mask” (which is “(output»scale)«scale”). The additional dc_bias would therefor lead to false error-feedback.
Should have taken a deeper look into it before submitting this patch…

Any new thoughts on the correctness of the dithering routine? If not, I’ll just remove the extra bias in dithering and commit.

There are no new thoughts. So, commit without extra bias in dithering.