Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Music playback
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Daily build (which?)
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by Andree Buschmann - 2007-06-10
Last edited by Thom Johansen - 2007-08-29

FS#7286 - correct DC-bias for output signals (dsp, test_codec)

This patch corrects the DC-bias of the PCM-signal ouput. I came around this issue when working on accuracy comparison between rockbox and reference decoder for musepack.
In trunk all signals will have DC-bias of -1/65536 which is cause by truncation of samples with more than 16 significant bits. Solution is to add 1«(scale-1) before doing the »scale shift.
The patch is available for apps/plugins/test_codec.c and apps/dsp.c. For this reason the patch is not available for coldfire targets as these use assembler code and not the changed c-code.

Closed by  Thom Johansen
2007-08-29 14:33
Reason for closing:  Accepted
Michael Sevakis commented on 2007-06-13 23:01

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.

Andree Buschmann commented on 2007-06-14 06:10

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.

Michael Sevakis commented on 2007-06-14 07:34

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

Andree Buschmann commented on 2007-06-14 15:55

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?

Michael Sevakis commented on 2007-06-15 02:00

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

Andree Buschmann commented on 2007-06-19 18:52

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).

Andree Buschmann commented on 2007-08-02 18:22

Sync’ed against 14147

Andree Buschmann commented on 2007-08-02 18:23

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

Matt M commented on 2007-08-08 05:56

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…

Andree Buschmann commented on 2007-08-09 19:21

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

Thom Johansen commented on 2007-08-24 10:37

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.

Michael Sevakis commented on 2007-08-25 04:06

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.

Andree Buschmann commented on 2007-08-25 10:02

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…

Thom Johansen commented on 2007-08-28 20:59

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

Andree Buschmann commented on 2007-08-29 07:13

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

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing