- Status Closed
- Percent Complete
- 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
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.
Loading...
Available keyboard shortcuts
- Alt + ⇧ Shift + l Login Dialog / Logout
- Alt + ⇧ Shift + a Add new task
- Alt + ⇧ Shift + m My searches
- Alt + ⇧ Shift + t focus taskid search
Tasklist
- o open selected task
- j move cursor down
- k move cursor up
Task Details
- n Next task
- p Previous task
- Alt + ⇧ Shift + e ↵ Enter Edit this task
- Alt + ⇧ Shift + w watch task
- Alt + ⇧ Shift + y Close Task
Task Editing
- Alt + ⇧ Shift + s save task
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.
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).
Sync’ed against 14147
Oops, now with patch file. Sync’ed against 14147
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.
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.