microbit-foundation / micropython-microbit-v2

Temporary home for MicroPython for micro:bit v2 as we stablise it before pushing upstream
MIT License
42 stars 23 forks source link

music.pitch characteristics have changed; high pitches have background clicks #67

Closed martinwork closed 2 years ago

martinwork commented 3 years ago

Micro:bit Foundation support ticket - 43597 Thanks @whaleygeek !

mimic.zip

The zip contains two versions of a hex that produces sounds on the speaker to mimic patterns recorded at the microphone. The newer script has been updated for the new microphone event APIs but is the same for sound output via music.pitch. The old one was built with "MicroPython 93e48c3 on 2020-09-08". I have just resaved the new one with Editor Version: 2.2.0-beta.4.

I noticed 2 changes: jumps between pitches and poor quality sound for higher pitches.

The original has a great sound effect which is lost in the new version. At first, it seemed like maybe the new one was doing a better job of producing the sounds, making it sound more musical. But, with longer durations, it's clear the old one sounds like a single tone which changes pitch, whereas the new one has distinct "twiddly" changes. It's as if the old one is merging from pitch to pitch, while the new one is restarting for each change. ​ Then I tried using the REPL of each version to play simple tones (e.g. import music; music.pitch(3000)). And 2000, 1000, 440... The 440 notes are noticeably different, though I couldn't say which is correct. The old version has a purer tone for the high frequencies, with the new one having a background clicking.

dpgeorge commented 3 years ago

The older version uses direct PWM for music output. The newer version uses the CODAL mixer and virtual pin. So the difference in sound is most likely due to the CODAL mixer.

Can you provide a simpler example of a sound effect that has changed? Running the mimic example attached above, it's hard to reproduce things consistently. Maybe write some simple code that does the fast pitch change sound effect? Then we can compare that on "old" and "new" and maybe rewrite in C++ to see if it really is a CODAL issue.

whaleygeek commented 3 years ago

The sonic() function was one I wrote many years ago for the Doctor Who Mission Sonic programme.

Here is the original code, I suggest this is a good basis for sound comparisons, as the original code sounded exactly like a sonic screwdriver, using frequency modulation. (Actually a real sonic screwdriver uses both FM and AM, but it sounded close enough by just doing the FM part).

import music
from microbit import *

def sonic(f=3000, fstep=32, fdepth=100, ton=10, toff=0, duration=2.0):
    try:
        ft = f
        music.pitch(ft)
        st = running_time()
        while running_time() < st+(duration*1000):
            sleep(ton)
            if toff != 0:
                music.stop()
                sleep(toff)
            fn = ft + fstep
            if fn >= f + fdepth or fn <= f - fdepth:
                fstep *= -1
            if fn != ft:
                ft = fn
                music.pitch(ft)
    finally:
        music.stop()

while True:
    if button_a.was_pressed():
        display.show(Image.SWORD)
        sonic(f=3000)
    elif button_b.was_pressed():
        display.show(Image.SWORD)
        sonic(f=4000)
    else:
        display.show(Image.DIAMOND)
microbit-carlos commented 3 years ago

@martinwork @dpgeorge is David's example good to debug? Or do we need a smaller one to do a C++ version to check in CODAL?

martinwork commented 3 years ago

@microbit-carlos Here is a C++ sample that confirms, as @dpgeorge said, the difference is in using virtualOutputPin.

Press B for uBit.io.speaker Press A for uBit.audio.virtualOutputPin

After trying both it's necessary to reset to try again.

test-sonic-v2.zip

#include "MicroBit.h"

MicroBit uBit;

bool fVirtual = false;

codal::Pin &music_pin()
{
  return fVirtual ? (codal::Pin &) uBit.audio.virtualOutputPin : (codal::Pin &) uBit.io.speaker;
}

void music_pitch( int f)
{
  uint32_t us = floor( 0.5 + 1000000.0 / f);
  if ( music_pin().getAnalogPeriodUs() != us)
      music_pin().setAnalogPeriodUs( us);

  music_pin().setAnalogValue(512);
}

void music_stop()
{
  music_pin().setAnalogValue(0);
  music_pin().setAnalogPeriodUs( 20000);
}

uint32_t running_time()
{
  return system_timer_current_time();
}

void sleep( uint32_t ms)
{
  if (ms <= 0) {
      return;
  }
  uint32_t start = running_time();
  while (running_time() - start < ms) {
    Event(DEVICE_ID_SCHEDULER, DEVICE_SCHEDULER_EVT_IDLE);
    __WFI();
  }
}

void sonic( int f=3000, int fstep=32, int fdepth=100, int ton=10, int toff=0, float duration=2.0)
{
    int ft = f;
    music_pitch(ft);
    uint32_t st = running_time();
    while ( running_time() < st + ( duration * 1000))
    {
        sleep(ton);
        if ( toff != 0)
        {
            music_stop();
            sleep(toff);
        }
        int fn = ft + fstep;
        if ( fn >= f + fdepth || fn <= f - fdepth)
        {
            fstep *= -1;
        }
        if ( fn != ft)
        {
            ft = fn;
            music_pitch(ft);
        }
    }

    music_stop();
}

void onButtonA(MicroBitEvent e)
{
    fVirtual = true;
    sonic( 3000);
}

void onButtonB(MicroBitEvent e)
{
    fVirtual = false;
    sonic( 3000);
}

int  main()
{
    uBit.init();

    uBit.messageBus.listen( MICROBIT_ID_BUTTON_A,  MICROBIT_BUTTON_EVT_CLICK, onButtonA);
    uBit.messageBus.listen( MICROBIT_ID_BUTTON_B,  MICROBIT_BUTTON_EVT_CLICK, onButtonB);

    release_fiber();
}
dpgeorge commented 3 years ago

@microbit-carlos where shall we go from here, is it important to make the sonic() code from v1 sound the same on v2? If volume is set at 100% then it should be technically possible to make it sound the same using virtualOutputPin (because with max volume there's no higher frequency PWM modulating the volume, the waveform should be the same as a normal PWM output on a pin). But I suspect it'd be a lot of work to make the timings the same as v1.

martinwork commented 3 years ago

The following works, but not music.pitch() in the same program, which is the same problem I had in C++.

import math
from microbit import *

def sonic_music_pitch( f):
    us = math.floor( 0.5 + 1000000.0 / f)
    pin_speaker.set_analog_period_microseconds( us)
    pin_speaker.write_analog(512)

def sonic_music_stop():
    pin_speaker.write_analog(0)
    pin_speaker.set_analog_period_microseconds( 20000)

def sonic(f=3000, fstep=32, fdepth=100, ton=10, toff=0, duration=2.0):
    try:
        ft = f
        sonic_music_pitch(ft)
        st = running_time()
        while running_time() < st+(duration*1000):
            sleep(ton)
            if toff != 0:
                sonic_music_stop()
                sleep(toff)
            fn = ft + fstep
            if fn >= f + fdepth or fn <= f - fdepth:
                fstep *= -1
            if fn != ft:
                ft = fn
                sonic_music_pitch(ft)
    finally:
        sonic_music_stop()

while True:
    if button_a.was_pressed():
        display.show(Image.SWORD)
        sonic(f=3000)
    elif button_b.was_pressed():
        display.show(Image.SWORD)
        sonic(f=4000)
    else:
        display.show(Image.DIAMOND)
microbit-carlos commented 3 years ago

Thanks @martinwork, we should take this to CODAL to be investigated over there. Could you open an issue in the codal-microbit-v2 repo and summarise all the finding listed here?

martinwork commented 3 years ago

@microbit-carlos We already have https://github.com/lancaster-university/codal-microbit-v2/issues/42, that I started looking at yesterday.

microbit-mark commented 2 years ago

Is the fix from https://github.com/lancaster-university/codal-microbit-v2/commit/6e880bc3d10df1a585bb9046ee4ab02bdcab1f34 now applied here?

microbit-carlos commented 2 years ago

The latest tag, v0.2.33, was released in July and that commit was added in August, so it's not in a CODAL release yet.

dpgeorge commented 2 years ago

Is the fix from lancaster-university/codal-microbit-v2@6e880bc now applied here?

Yes it is now applied in this repo, CODAL was updated to v0.2.35 in 76c1b3153970e967401dee6a2566fbfec36d8fe2

jaustin commented 2 years ago

@martinwork could you please test with the version of MicroPython in https://python.microbit.org/v/beta and close this if the issue is now resolved?

martinwork commented 2 years ago

There are two hexes in the original mimic.zip. The older one doesn't load into Python v2 or beta. This is not a new problem, but maybe worth noting that the error messages are different. v2 says Could not find valid Python code in the hex file. beta says Cannot load file Invalid record type 0x0A at record 2 (should be between 0x00 and 0x05)

I loaded and saved the newer one in beta and made a new zip: mimic03Aug22.zip

The behaviour of the program and the microphone sensitivity seems different, compared to the original hex.

The sound is OK, though very slightly different, I think.