Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cps1: adpcm filter #559

Closed
jotego opened this issue Feb 21, 2024 · 20 comments
Closed

cps1: adpcm filter #559

jotego opened this issue Feb 21, 2024 · 20 comments
Assignees
Labels
enhancement New feature or request

Comments

@jotego
Copy link
Owner

jotego commented Feb 21, 2024

First spectrum comparison for ADPCM, on commit fbb89f0

  • the ac coupling in the PCB has a high pass effect visible in the spectrum, with 10Hz signals more attenuated than in the core. Not critical as these are not really audible
  • the low-pass filter on the board has a clear effect and it is needed for sound accuracy (compare red curve with blue one)
  • the current low-pass filter in the MiSTer core is too steep and it comes too late (see yellow curve)

imagen

@jotego jotego added the enhancement New feature or request label Feb 21, 2024
@jotego jotego self-assigned this Feb 21, 2024
@jotego
Copy link
Owner Author

jotego commented Feb 21, 2024

The steep attenuation comes from the interpolation filter inside jt6295. Using the soft filter with INTERPOLATOR=2, results in the same profile (error in the coefficients?)

jotego added a commit that referenced this issue Feb 21, 2024
@paulb-nl
Copy link
Contributor

Which pcb are you targeting for ADPCM? Artemio showed that there big differences with older (GNG) vs newer pcb's (SF2).

@jotego
Copy link
Owner Author

jotego commented Feb 22, 2024

Which pcb are you targeting for ADPCM? Artemio showed that there big differences with older (GNG) vs newer pcb's (SF2).

I need to collect more information. Ideally, I will match each game to its PCB. If anyone has a list of PCB ID code vs games, please share it.

@ArtemioUrbina
Copy link

ArtemioUrbina commented Feb 22, 2024

Hello @jotego, here is the Z80 code that loops between:
20 Hz
200 Hz
1000 Hz
2000 Hz

CPS1-Z80loop.zip

Took me longer than expected since MAME doesn't handle the Busy status properly. Tested on real hardware
Take care

@jotego
Copy link
Owner Author

jotego commented Feb 23, 2024

It looks like the data in that file is not correctly encoded as there are some HF oscillations in it. I did my own ADPCM rom and plugged it with your Z80 code. patches.bin.zip. But I still got waveforms that didn't look great.

The problem is that looking at the ADPCM alone, with no filtering, brings so much aliasing that signals look very distorted. When I add a 1st order low-pass filter, I can start seeing the sine waveforms appears as I pull the cut-off frequency lower and lower.

This makes the comparisons a trickier than expected.

I did find a regression error I introduced recently and could fix that. I also understand now that the interpolator I was using was removing aliasing too well in comparison with the real PCB. So we're getting results, but we're not quite there yet.

@jotego
Copy link
Owner Author

jotego commented Feb 23, 2024

In case you want to try, using this file jtcps1.rbf.zip you can vary the ADPCM gain and the filter cut-off frequency.

Connecting a keyboard to the MiSTer, you can control the numbers visible at the bottom. By pressing shift and the numbers from 1 to 8, you will be able to alter each one of the bits making up that number.

  • Top four bits affect the filter cut-off frequency. The higher the number, the stronger the filtering
  • Lower four bits set the gain. Zero is no ADPCM, 1111 is maximum gain

Thank you

@ArtemioUrbina
Copy link

It looks like the data in that file is not correctly encoded as there are some HF oscillations in it. I did my own ADPCM rom and plugged it with your Z80 code. patches.bin.zip. But I still got waveforms that didn't look great.

Could you share which encoder you used so I can refine the one in the SDK?

The problem is that looking at the ADPCM alone, with no filtering, brings so much aliasing that signals look very distorted.

Indeed

In case you want to try, using this file jtcps1.rbf.zip you can vary the ADPCM gain and the filter cut-off frequency.

Will try out that core. Thank you

@ArtemioUrbina
Copy link

In case you want to try, using this file jtcps1.rbf.zip you can vary the ADPCM gain and the filter cut-off frequency.

Connecting a keyboard to the MiSTer, you can control the numbers visible at the bottom. By pressing shift and the numbers from 1 to 8, you will be able to alter each one of the bits making up that number.

* Top four bits affect the filter cut-off frequency. The higher the number, the stronger the filtering

* Lower four bits set the gain. Zero is no ADPCM, 1111 is maximum gain

Thank you

Here are my results with the test core, excellent idea. Sorry I didn't have these sooner, but I was fixing some issues I had with the SDK. I made a test ROM that can run my regular MDFourier and an ADPCM only version, here are the results:

5F:
DA__ALL_0-CPS1-88617A-7b-MagicSw_vs_5F_0000

6F:
DA__ALL_0-CPS1-88617A-7b-MagicSw_vs_6F_0000

7F:
DA__ALL_0-CPS1-88617A-7b-MagicSw_vs_7F_0000

8F:
DA__ALL_0-CPS1-88617A-7b-MagicSw_vs_8F_0000

9F:
DA__ALL_0-CPS1-88617A-7b-MagicSw_vs_9F_0000

AF:
DA__ALL_0-CPS1-88617A-7b-MagicSw_vs_AF_0000

BF:
DA__ALL_0-CPS1-88617A-7b-MagicSw_vs_BF_0000

CF:
DA__ALL_0-CPS1-88617A-7b-MagicSw_vs_CF_0000

DF:
DA__ALL_0-CPS1-88617A-7b-MagicSw_vs_DF_0000

EF:
DA__ALL_0-CPS1-88617A-7b-MagicSw_vs_EF_0000

FF:
DA__ALL_0-CPS1-88617A-7b-MagicSw_vs_FF_0000

I ignored the amplitude parameter since it made no difference (here's FF vs F7):

DA__ALL_AVG_FF_vs_F7_0000

Here's the recordings and profile: ADPCM_Only.zip

To my eyes AF and BF look the best from these, so here's how the spectrograms look for AF

PCB:
SP__ALL_A_0-CPS1-88617A-7b-MagicSw

AF:
SP__ALL_B_AF

PCB:
T_SP_A_0-CPS1-88617A-7b-MagicSw

AF:
T_SP_B_AF

Please let me know what you think

@ArtemioUrbina
Copy link

PCB:
SP__ALL_A_0-CPS1-88617A-7b-MagicSw
8F:
SP__ALL_B_8F

@jotego
Copy link
Owner Author

jotego commented Feb 24, 2024

Thank you very much. We are still not getting the right shape. I reviewed the signal chain again and made two changes:

  • The low-pass filter must run at the full sampling rate (48kHz) instead of the ADPCM sampling rate in order to have the same effect as the PCB
  • I changed the core so all 8-bits of the debug value go to the filter. The expected correct value is E7, which should give you a Fc of 770 Hz, matching Forgotten Worlds schematics. Use this file jtcps1.rbf.zip. Amplitude is fixed.

The algorithm I used is here. This comes originally from an old telephony book (page 275). You can browse it here

@ArtemioUrbina
Copy link

I believe you'll like this

* I changed the core so all 8-bits of the debug value go to the filter. The expected correct value is **E7**, which should give you a Fc of **770 Hz**, matching _Forgotten Worlds_ schematics. Use this file [jtcps1.rbf.zip](https://github.com/jotego/jtcores/files/14391995/jtcps1.rbf.zip). Amplitude is fixed.

Here are E6, E7 and E8. I didn't record others further on either side since it is way too late here, I can do that tomorrow for completeness. I download them to the same folder and compare them quickly changing between them with an image viewer

E6:
DA__ALL_0-CPS1-88617A-7b-MagicSw_vs_E6_0000

E7:
DA__ALL_0-CPS1-88617A-7b-MagicSw_vs_E7_0000

E8:
DA__ALL_0-CPS1-88617A-7b-MagicSw_vs_E8_0000

I like E8 better, but they are very similar:

PCB:

PCB:
SP__ALL_A_0-CPS1-88617A-7b-MagicSw
E6:
SP__ALL_B_E6

PCB:
SP__ALL_A_0-CPS1-88617A-7b-MagicSw
E7:
SP__ALL_B_E7

PCB:
SP__ALL_A_0-CPS1-88617A-7b-MagicSw
E8:
SP__ALL_B_E8

The algorithm I used is here. This comes originally from an old telephony book (page 275). You can browse it here

Thank you

@jotego
Copy link
Owner Author

jotego commented Feb 24, 2024

Thank you very much for staying late to take the measurements. It looks like we're almost there. There is no need to take other measurements.

I think at this stage we can focus at a single SS value, to make things clearer, as we understand its effect well.

I am fine with the discrepancies at low frequencies as any ac coupling capacitor in the signal flow -and there will be a few of them- will alter the spectrum there without our control. But I am surprised to see so much random variation in the error plots. It call my attention that you name it "amplitude". We should be comparing power and not amplitudes, otherwise the signal phase will cause funny effects. Maybe you're already comparing power and it is just an improper name in the difference plots.

@ArtemioUrbina
Copy link

ArtemioUrbina commented Feb 24, 2024

I think at this stage we can focus at a single SS value, to make things clearer, as we understand its effect well.

Understood

I am surprised to see so much random variation in the error plots. It call my attention that you name it "amplitude". We should be comparing power and not amplitudes, otherwise the signal phase will cause funny effects. Maybe you're already comparing power and it is just an improper name in the difference plots.

I am using sqrt(r1r1 + i1i1)) to calculate the values for each bin from the DFT. I am indeed a software engineer implementing these, out of my element. Any corrections are more than welcome to improve the work. So please feel free to comment, I won't be offended.

Regarding the random variation I attribute it to the aliasing and distortion caused by the ADPCM compression. Yes, they should match but they don't. On MiSTer you get more of them, here si what MDfourier tells us.

The "Missing" PNG gives us the frequency data that was in the reference (PCB in this case) but not matched with any coincidence in the Comparision (MiSTer). Here it is, and it is basically empty:

MISSING-A-T_SP_0-CPS1-88617A-7b-MagicSw_S

i.e. it could find a matching frequency on the comparision, even if the amplitude was way different

The "Extra" png gives us the frequencies that were in the Comparison (MiSTer) but were not in the reference (PCB recording)

MISSING-EXTRA_T_SP_E7_S

As you can see, there's a bunch of that that can come from the differences in nature of the filters, I honestly don't know just speculating.

And here are the clean plots for each:

PCB:
T_SP_A_0-CPS1-88617A-7b-MagicSw

MiSTer:
T_SP_B_E7

@jotego
Copy link
Owner Author

jotego commented Feb 28, 2024

It looks like the IIR filter causes some ringing. I will reimplement the filter as a FIR one. I am doing some preparatory works in other cores at the moment so I can apply a global solution to all cores. That is why I am quiet. I will report back in a few days

@jotego
Copy link
Owner Author

jotego commented Mar 1, 2024

I moved to a FIR implementation. Could you please measure this one jtcps1.rbf.zip ?

This is the spectrum. It should match the analog filter at 7700Hz, although at 770Hz the analog one has more attenuation. The analog one will have more attenuation at 14kHz too. I used this online tool to design it.

imagen

@ArtemioUrbina
Copy link

I moved to a FIR implementation. Could you please measure this one jtcps1.rbf.zip ?

Here are the results against the CPS1 88617A-7b (Magic Sword) PCB we were targeting:

DA__ALL_0-CPS1-88617A-7b-MagicSw_vs_MiSTerFIRFilter-01032024_0000

It is interesting to note the comparison between 88617A-7b (ref, older and 89626A-4 (Comp, newer):

DA__ALL_0-CPS1-88617A-7b-MagicSw_vs_0-CPS1-89626A-4-SF2Board_0000

So, here is the 89626A-4 (Ref/SF2) and the new Core:

DA__ALL_0-CPS1-89626A-4-SF2Board_vs_MiSTerFIRFilter-01032024_0000

For reference here are the recordings:

resultsFIR.zip

And the ROMs used (use button 3, HP for ADPCM only)

sf2ud.zip

And these:

SP__ALL_A_0-CPS1-89626A-4-SF2Board

SP__ALL_B_MiSTerFIRFilter-01032024

Thanks for all your great work!

@jotego
Copy link
Owner Author

jotego commented Mar 2, 2024

what is the frequency range you are sweeping over? I wonder if you are going above the Nyquist frequency for the sampling rate and that is making the ADPCM algorithm do funny things... The sweep should go from 20Hz to 2 or 3 kHz. We will still get signal for higher frequencies because of quantization noise, aliasing and ADPCM artifacts.

@ArtemioUrbina
Copy link

20 to 3787hz since the sampling is 7575 to my knowledge

@jotego jotego closed this as completed in 22b08c0 Mar 3, 2024
@jotego
Copy link
Owner Author

jotego commented Mar 3, 2024

I have finally settled for the single pole at 770Hz, as schematics suggest for the 88617A board, and a 2nd order Butterworth filter at 4kHz for the 89626A. I have made it selectable via the DIP switches menu. I have enabled the FX volume setting in the OSD for user flexibility.

This is the core file: jtcps1.rbf.zip

The ideal filter responses are below. The single pole works quite well in the core but the Butterworth has worse performance in the fixed point math version (i.e. the core). This is often the case when going from float to fixed point, but it is a bit worse than expected.

The FM/PCM sound balance might not be exact. I find it difficult to derive from the recordings, to be honest as too many things are moving. That's one of the reasons I left open the user option to adjust it.

Let me know what you think and thanks again for all the support!

all
core 4k
core 770

@ArtemioUrbina
Copy link

I have finally settled for the single pole at 770Hz, as schematics suggest for the 88617A board, and a 2nd order Butterworth filter at 4kHz for the 89626A. I have made it selectable via the DIP switches menu. I have enabled the FX volume setting in the OSD for user flexibility.

Selectable is the perfect optio0n in my opinion. I couldn't find it in the file linked above yet, but did find the FX volume setting.

The FM/PCM sound balance might not be exact. I find it difficult to derive from the recordings, to be honest as too many things are moving. That's one of the reasons I left open the user option to adjust it.

I used MDfourier to measure it against both PCB setups. And high (the default) seems to be the most appropriate

88617A-7b vs Highest:

88617A-7b-highest

88617A-7b vs High:

88617A-7b-high

88617A-7b vs Low:

88617A-7b-low

89626A-4 vs Highest

89626A-4-highest

89626A-4 vs High

89626A-4-high

89626A-4 vs Low

89626A-4-low

Source recordings:

PCBs:

CPS1-89626A-4-SF2Board-ADPCM-Full.zip

CPS1-88617A-7b-MagicSword-ADPCM-Full.zip

Mister:
MisterFPGA-CPS1-04032024-hi-lo.zip

Let me know what you think and thanks again for all the support!

No thank you for caring about audio and your great work! Let me know if I can be of further assistance

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: ✅ Done
Development

No branches or pull requests

3 participants