qmk / qmk_firmware

Open-source keyboard firmware for Atmel AVR and Arm USB families
https://qmk.fm
GNU General Public License v2.0
18.09k stars 38.89k forks source link

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

Open LeReverandNox opened 4 years ago

LeReverandNox commented 4 years ago

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

tzarc commented 4 years ago

Can you try with #6165 applied?

LeReverandNox commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 :sweat_smile:

JohSchneider commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

alexryndin commented 4 years ago

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

alexryndin commented 4 years ago

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 commented 4 years ago

Try with pwm_software, and:

#define AUDIO_PIN A5
#define AUDIO_PIN_ALT A4
#define AUDIO_PIN_ALT_AS_NEGATIVE
alexryndin commented 4 years ago

@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 commented 4 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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:

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 commented 3 years ago

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.

lja83 commented 3 years ago

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 commented 3 years ago

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 commented 6 months ago

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.