Skip to content
New issue

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

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

Already on GitHub? # to your account

RP2040 is out of tune #251

Closed
tomcombriat opened this issue Apr 22, 2024 · 5 comments · Fixed by #254
Closed

RP2040 is out of tune #251

tomcombriat opened this issue Apr 22, 2024 · 5 comments · Fixed by #254

Comments

@tomcombriat
Copy link
Collaborator

Making a small drone synth to play along, I noticed that a simple setup, based on a RP2040 with PWM output, seems to be out of tune (looks like something similar than #67 ).

Here are some measurements ("Sinewave" sketch, CPU speed: 133MHz (this does not seem to affect):

Asked frequency (Hz) Measured frequency (Hz) Deviation (%)
440 448 1.82
220 224 1.82
1000 1017 1.7
2000 2035 1.75
10000 10173 1.73

Looks fairly stable, hence the timer is probably off by that amount.

@tomcombriat
Copy link
Collaborator Author

Looking more closely at the code, I think the problem comes from:

micros_per_update = 1000000l / MOZZI_AUDIO_RATE;
  do {
    next_audio_update = make_timeout_time_us(micros_per_update);

(from MozziGuts_impl_RP2040.hpp l. 246). As (1000000/32768 - int(1000000/32768)) / 1000000/32768 = 1.696%

@tomcombriat
Copy link
Collaborator Author

tomcombriat commented May 6, 2024

Probable way out: https://arduino-pico.readthedocs.io/en/latest/pwm.html (using the PWM Audio class)

Note that it might remove the possibility to have an accurate EXTERNAL_TIMED output mode…

@tfry-git
Copy link
Collaborator

tfry-git commented May 7, 2024

Yes, that looks useful in its own right, but also, yes, we need to fix the timers anyway (for external timed).

One approach to fixing could be to internally keep next_audio_update at a higher resolution. IIRC, absolute_time_t is essentially just a 64 bit count of microseconds. For all practical purposes, we could turn that and micros_per_update into, say, a UFix<56,8>, and simply perform an appropriate shift when passing it to hardware_alarm_set_target().

@tomcombriat
Copy link
Collaborator Author

I am not sure I am following you on this. Whatever we do, the alarms on the RP2040 have a microsecond precision (only systick is up to the processor cycle, but the associated irq is already taken) whereas we are trying to set up something at 1/32768=30.51us… The .51 is where the shift comes from but I do not see how to get out without an precision improved alarm (or oscillating between 30 and 31…).

My plan so far is to reimplement the default output with PWM audio (which is DMA buffered and so on ;) !). I will try to keep the external timed as it is now, knowing that it is not on tune.

@tfry-git
Copy link
Collaborator

tfry-git commented May 7, 2024

My plan so far is to reimplement the default output with PWM audio

Which, again, will be useful in it's own right, so by all means, do.

Whatever we do, the alarms on the RP2040 have a microsecond precision [...] but I do not see how to get out without an precision improved alarm (or oscillating between 30 and 31…).

In effect, I'm aiming for the latter:

1/32768=30.51us

So we'll try to output the first sample at 30us, the second at 61, then 91, 122, etc, and sample no 100 at 3051us. In an ideal world we wouldn't have any jitter at all, but I do think this will be much less noticeable than being 1.6% off on the frequency.

@tomcombriat tomcombriat linked a pull request May 8, 2024 that will close this issue
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants