Skip to content
This repository has been archived by the owner on Sep 4, 2023. It is now read-only.

Fix clipping/scaling issue #71

Merged
merged 1 commit into from Aug 15, 2019
Merged

Conversation

greyrogue
Copy link
Contributor

The accumulator is adding operators, not channels (up to 24, not 6), which means it can reach max value (although the code in the jt12_single_acc is preventing it from actually overflowing).
Also, I believe this should be adding pre_left and pre_right, not left and right.

The accumulator is adding operators, not channels (up to 24, not 6), which means it can reach max value (although the code in the jt12_single_acc is preventing it from actually overflowing).
Also, I believe this should be adding pre_left and pre_right, not left and right.
@greyrogue
Copy link
Contributor Author

#28 #54

@sorgelig
Copy link
Member

i think commented out part is plain wrong. there must be pre_left/pre_right, not left/right. So it seems a typo.

@sorgelig sorgelig merged commit 610a515 into MiSTer-devel:master Aug 15, 2019
@jotego
Copy link
Member

jotego commented Aug 16, 2019

I see several issues open and from what I hear it looks like this is pointing to clipping in the accumulator. If the clipping is exclusively FM (and not FM+PSG), and you're sure there should be no clipping; then it is very easy to add more bits to the accumulator:

Change the wout parameter to 13, and you will gain 3dB of dynamic range.

jt12_single_acc #(.win(9),.wout(12)) u_right(
.clk ( clk ),
.clk_en ( clk_en ),
.op_result ( acc_input ),
.sum_en ( sum_or_pcm & right_en ),
.zero ( zero ),
.snd ( pre_right )
);

Then on the output assignment to left, just take the MSB bits.

The original chip had a multiplexed output and it was impossible that some loud channels could cause clipping when combined with the other channels. The down side was that multiplexed output is equivalent to 6-fold attenuation in the signal plus some artifacts.

You can also leave away the pre_left to left amplification, by just assigning left <= pre_left. But this will have no effect unless the clipping problem comes in the sum of PSG plus FM sound. If pre_left is saturated, then you cannot reverse that.

Also note that this JT12's repository is actually here. This is being used in many other cores, not just MiSTer's FPGAgen core. NeoGeo, MiST's FPGAgen, 1943, Ghosts'n Goblins, Commando, ZX Spectrum expansion... all use this core so it is important to keep the central repo sync'ed.

@sorgelig
Copy link
Member

Fixes in cores are mostly experimental and may not be applicable to original repo due to different reasons. So it's up to you to pick the change or not. Probably it needs to be fixed by some other, better way.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
3 participants