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

[Bug] Audio on Planck since last breaking changes 2020/02 #8825

Open
LeReverandNox opened this issue Apr 17, 2020 · 20 comments
Open

[Bug] Audio on Planck since last breaking changes 2020/02 #8825

LeReverandNox opened this issue Apr 17, 2020 · 20 comments

Comments

@LeReverandNox
Copy link

Describe the Bug

Hi folks,

Since the breaking changes "2020 Feb 29", the Audio features on the Planck rev6 (STM32F3 mcu) are kinda messed up, the notes plays waaay too fast.
I've been testing on two Plancks, either with my personal and default keymap.

In my journey trying to understand why, I learned some interesting things on the audio features of QMK by reading the sources, especially the undocumented parts like tempo/timbre.
I was able to get a correct playing speed back by setting my tempo to ~200 (instead of the default value of 100), but the pitch (or timbre I guess) is still not right, at least not the same as before. set_timbre() makes no difference.

I think it comes from the numerous Chibios changes brought to QMK withthe last breaking changes, but unfortunately I don't have enough knowledge on such low-level MCU programming to investigate any further (almost 16 months of commits on Chibios, gosh that's at lot of changes ! ^^')
If I checkout before the merge of PR #8064 to build my firmware, it works fine.

System Information

  • Keyboard: Planck
    • Revision (if applicable): rev. 6
  • Operating system: Manjaro 19.0.2 Kyria
  • AVR GCC version: 5.4.0 (Docker build)
  • ARM GCC version: 6.3.1 (Docker bulid)
  • QMK Firmware version: 0.8.120
  • Any keyboard related software installed?
    • None
@tzarc
Copy link
Member

tzarc commented Apr 17, 2020

Can you try with #6165 applied?

@LeReverandNox
Copy link
Author

LeReverandNox commented Apr 17, 2020

Thank you,
I messed around a bit with #6165, and there's some improvements for sure. The default parameters are pretty good.

With AUDIO_DRIVER = dac_additive, the volume is pretty low. I tried to use WAVEFORM_SQUARE and adjust AUDIO_DAC_SAMPLE_MAX, no improvement.
With AUDIO_DRIVER = dac_basic and AUDIO_PIN_ALT = A4, that's a lot better, nice and loud sound.

The pitch is different, but quite enjoyable.
However, now with the TEMPO_DEFAULT set to 60, the songs plays relatively slow, and if I try to increase the tempo, they won't play at all (well, just a single note, trimmed). I'm only able to decrease it, but then the songs take forever to play, pretty unusable ^^'.

@tzarc
Copy link
Member

tzarc commented Apr 17, 2020

Thanks for the confirmation!
Might want to comment on the timing on the other PR.
At this stage I think the likely response to this issue is going to be "wait for #6165", so if you're keen to help out with the review and confirmation of what it should be doing, it'd be a massive help.

I'm not even sure how fast the audio should be, so someone with some experience with the original would be very handy. Thanks again!

@LeReverandNox
Copy link
Author

Ok, will do, thanks for the advice.
Sure, I'd be very happy to help on #6165. I maybe don't have much knowledge regarding audio chipsets or the logic behind digital sound production, but I'm really willing to provide as much assistance as I can !

TBH, I'm not sure either what the audio "should" sound like nor how fast it should be on keyboards
Planck's rev6 are my only audio featured boards, but by using them as my daily work tools for almost a year, I can't ignore the recent sound change.
Not sure which one is closer to the "right" sound though 😅

@JohSchneider
Copy link
Contributor

yeah IMHO the current/upstream-master audio implementation is... a mess
(duplicate code, dead code paths, inconsistent terminology, ...) some of the reasons a "simple add a feature" PR turned into a refactoring spree O:-)

@alexryndin
Copy link

I can confirm that olkb planck with current firmware sounds much faster and like octave upper if compared with planck-ez with default firmware.

@tzarc
Copy link
Member

tzarc commented Jul 15, 2020

@alexryndin any chance you could give a build with #6165 a run and see if it's any help?

@alexryndin
Copy link

@tzarc Certainly, I'm going to give it a try in the nearest future.

@alexryndin
Copy link

alexryndin commented Jul 15, 2020

What AUDIO_PIN_* should I use with planck rev6?

with this params and dac_basic:

#define AUDIO_DAC_SAMPLE_WAVEFORM_SQUARE

A keyboard goes unusable :). speaker just continuously makes noise...
Generally, the volume is pretty low in any dac mode, but playback speed and sound quality are much better
pwm_software gives me too low volume too, unfortunately.

I tried different values for AUDIO_DAC_SAMPLE_MAX with no success, volume only gets lower.

So I think the main problem with the PR is volume issue, I wish it would be higher.

@tzarc
Copy link
Member

tzarc commented Jul 16, 2020

Try with pwm_software, and:

#define AUDIO_PIN A5
#define AUDIO_PIN_ALT A4
#define AUDIO_PIN_ALT_AS_NEGATIVE

@alexryndin
Copy link

alexryndin commented Jul 16, 2020

@tzarc wow! So nice and loud! I like it!
And works with dac_basic too, but not with dac_additive, which gives near zero volume. Looks like additivity results in no volume :)

I don't have much to add to what @LeReverandNox already said -- sounds loud and tempo is good. Pitch is a bit different tho.
I would try to measure frequency somehow to ensure that base A is really 440 Hz.

upd: I found some very noticeable issues with dac_basic. Sometimes (very often right after flashing) the music can interrupt on its own, sometimes volume noticeably steps down, and right now, when I type this message, the keyboard became unresponsible :(

upd 2: I've measured A4 frequency with dac_basic and with current audio implementation and got interesting results! The PR with dac_basic gives exactly 440 Hz, whereas the current master code gives me only 220Hz :(. I think that's quite noticeable because TEMPO also needs to be doubled to get good playback speed... pwm_software gives 440Hz too.

upd 3: I gave another try to #6165 with dac_basic. I find sound it prodices very nice -- very clean, loud enough, nice pitch, but at some point the keyboard becomes unresponsible, and I need to replug it :(.

upd 4: Another bug with #6165 and dac_basic is that Audio Click mode misses some keypresses. I get click on about 49 of 50 keypresses, but 1 silent of 50 is hard to ignore.

@alexryndin
Copy link

I found out that STANDARD_PITCH_A has nothing to do with songs and notes pitch, but rather adjusts default pitch in music mode. Is there a way to adjust standard note frequency?

@jukkakoskinen
Copy link

Planck rev6 owner here. I have the same problem with songs playing too fast. Is there a specific commit I can go back to, or is the more recommended solution to compile against #6165?

@shinmai
Copy link
Contributor

shinmai commented Oct 26, 2020

Planck rev6.1 and Preonic rev3 here, same issue on both. Tried #6165, and the only noticeable difference is audio is quieter and even MORE high-pithced. Some of my layer-change sounds also now result in a low buzzing (w/o #6165) or an ear-piercing shrill whine (with the changes from the PR) until the layer is switched off.

As such I'm turning audio features off for now, since I already spent one evening syncing with latest master, merging to my personal branch and updating mingw and msys to get the new and "improved" qmk tool installed, so don't feel like spending any more time on QMK update struggles for now. Really hope there's some kind of proper fix to this soon, as using my leader combinations without sound feedback will be really annoying.

@tzarc
Copy link
Member

tzarc commented Oct 27, 2020

@shinmai thanks for the update. These are the sorts of issues that we'd had reported, but without enough detail for us to be able to reproduce.

I'll likely be taking over work on #6165 in the next week or so, given that it seems to have stalled -- I'll be looking at the outputs with an oscilloscope so actual timing and the like should be relatively easy to verify.

It'd be great if you could outline your exact usage patterns to cause this, specifically:

  • Your fork of QMK with your keymap (if it's not in the main repo as yet)
  • Your reproduction steps for when you encounter the mismatching outputs

Some of the "buzzing" has previously been attributed to specific output drivers (there are now 4), so it'd be great if I could dissect your current settings to try and reproduce locally on a breadboard.
I did note that there was a weird low-level static in some scenarios, but never tracked down a set of reproduction steps.

As for the timing -- before the breaking changes update, timing under ChibiOS was plain wrong, so tempo and the like were also known to be problematic. In the past I'd put it down to people being familiar with the bad timing functionality, but I think we've finally reached a critical mass where it warrants actual verification with oscilloscopes and the like.

@shinmai
Copy link
Contributor

shinmai commented Oct 27, 2020

Alright, nailed the "buzzing"/wheezing down. If you don't have enough entries in your DEFAULT_LAYER_SONGS for all your layers, unlike the old code which played a default song for the rest, the new code plays a continuous high tone (I suspect it's just pulsing the speaker and the actual "tone" of the sound is down to some internal frequency, since without the #6165 code it's a slow/low ticking/buzzing and with the PR code it's a high tone) until another layer song/sound is played. This happens with all drivers, so doesn't seem related to that.

My actual keymap has a bunch of NDAd stuff in it that I can't publish, but my mess of a QMK fork has an up-to-date-ish sanitised version. That being said, I've been testing this issue with the default Planck and Preonic keymaps from the main QMK repo, so my keymap and configuration changes shouldn't play into it.

The timings aren't an issue, that's just down to updating songs, better to have consistent timings and have people migrate.

Like I said I've been using the default keymaps with minimal changes (literally just adding a few simple songs to config.h and mapping them to keys in keymap.c) to test, but I can publish a branch with the exact keymaps I've been testing with if that's helpful.

My current issue is that I can't seem to be able to control audio volume with DAC_SAMPLE_MAX any more, which is another deal-breaker sinceI work from home and need to keep an amicable relationship with my family.

@jpskenn jpskenn mentioned this issue Feb 24, 2021
14 tasks
@lja83
Copy link

lja83 commented May 12, 2021

Has there been any update to this? I seem to still have the issue where using dac_additive results in extremely quiet sound on a planck rev6

@hyperbolist
Copy link

hyperbolist commented Aug 29, 2021

Up until breaking changes 2021Aug28 I was able to use BOOTMAGIC_ENABLE = no to get the startup song, but now I'm not getting the startup song anymore. The reset sound still plays though. (I guess problems began before the latest breaking changes update because if I use the immediate prior commit 29ec2d8 I still get the same no-startup-song behavior.)

@tamasd
Copy link

tamasd commented Mar 23, 2024

I think this issue also exists on Planck rev4. I have no startup song, but the music mode works.

I used an older version (2019 I believe) of QMK on my previous PC, and on my current machine I pulled the latest version to make some changes on my keymap.

@drashna
Copy link
Member

drashna commented Oct 13, 2024

#define AUDIO_INIT_DELAY may fix the startup issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants