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

AudioFrame behaviour is different for V1/V2 #47

Closed
microbit-mark opened this issue Dec 15, 2020 · 22 comments
Closed

AudioFrame behaviour is different for V1/V2 #47

microbit-mark opened this issue Dec 15, 2020 · 22 comments
Assignees
Milestone

Comments

@microbit-mark
Copy link

The example given behaves differently on V1 and V2

import audio
import math
def repeated_frame(frame, count):
    for i in range(count):
        yield frame
frame = audio.AudioFrame()
 
while 1:
    for i in range(len(frame)):
        frame[i] = int(math.sin(math.pi*i/16)*124+128.5)
    audio.play(repeated_frame(frame, 100),wait=True)

And the one in https://microbit-micropython.readthedocs.io/en/v2-docs/audio.html# seems to have another behaviour where the square wave is combined with the sawtooth one rather than being distinct from it.

From user:

I tried the previous program which can write 32 points of a beep that is repeated.
I saw that in your document, these 32 points were sent over a period of 4ms.
So with the following program we should hear a sound of 250Hz. One period on 32 points of the frame.

If instead of dividing by 16, we divide by 8, we should with a sound at 500Hz.

Using a smartphone application that does the FFT, I can't see the spectral line at 250Hz or 500Hz in my spectrum? A priori the sound intensity of your speaker is far too low. Obviously we can only generate multiple sounds of 250Hz. Obviously it will be difficult to go beyond 1000Hz a priori, because the sinusoid on 32 points is then too distorted.

I made a very simple little program with the V1 and with an external speaker grove. This program allows me to generate sound at the exact frequency. This program works perfectly

@microbit-mark
Copy link
Author

@martinwork can you take a look if you are able to this week.

@martinwork
Copy link
Collaborator

@microbit-mark Will do!

@martinwork
Copy link
Collaborator

The code that plays the AudioFrame in V2 is completely different to the V1 code. I don't know enough about the CODAL audio pipeline right now to be able to tell if the V2 problem is in CODAL or micropython.

The data supplied to uBit.audio.mixer by the AudioSource class is the AudioFrame expanded BUFFER_EXPANSION times by linear interpolation, but the sample rate is multiplied by BUFFER_EXPANSION / 4.

I've experimented in C++ with supplying similar data to MemorySource (which is similar to AudioSource) and that doesn't sound right either.

AudioSource

class AudioSource : public DataSource {

audio_init

static void audio_init(uint32_t sample_rate) {

linear interpolation

microbit_audio_frame_obj_t *buffer = (microbit_audio_frame_obj_t *)buffer_obj;

@jaustin
Copy link
Contributor

jaustin commented Jan 20, 2021

@dpgeorge I think this is related to the reproducing case you are building for Joe?
This would be great to fix because there are a number of fun tutorials around for how to play wav from a micro:bit with MicroPython and they don't, at present, work well on V2.

@jaustin jaustin added this to the 2.0.0-beta.4 milestone Feb 12, 2021
@dpgeorge
Copy link
Collaborator

I have been trying to reproduce the "buffering" problem in pure C++ but I can not do it. So at this stage I'd suggest working around any buffering issues by just making it work correctly from the MicroPython side.

@microbit-carlos
Copy link
Contributor

I'm not too familiar with this buffering issue, is there a CODAL ticket present already, or were we pending the C++ reproducer?

Is this something is still worthing opening in a ticket over there even if it is worked around in the MicroPython side?

@dpgeorge
Copy link
Collaborator

is there a CODAL ticket present already, or were we pending the C++ reproducer?

AFAIK there is no CODAL ticket for this, mainly because I cannot pin point it enough to describe it in technical detail. This was first seen with the initial issues with speech when moving it over to use the mixer, and fixed only by adding some extra buffering to the speech output. I'm still not sure why it doesn't work without the buffering, but the plan is to do a similar buffering for audio.play.

@microbit-carlos
Copy link
Contributor

Okay, thanks for the clarification Damien.
We are expecting a CODAL tag at the end of today and probably do a MicroPython release tomorrow. Will this work fit for the beta.5 or do we need to move it to beta.6?

@dpgeorge
Copy link
Collaborator

Will this work fit for the beta.5 or do we need to move it to beta.6?

I will try!

@dpgeorge
Copy link
Collaborator

Audio output should now be improved, see commit 9679263

I'm not sure if it's exactly the same as v1, it will need to be tested on a variety of examples.

@dpgeorge
Copy link
Collaborator

CODAL v0.2.25 was updated in 76054c2 and that improved the sampling rate accuracy of audio.play.

@microbit-carlos
Copy link
Contributor

Great, thanks Damien!
@microbit-mark would you be able to test this issue again with the hex from https://github.com/microbit-foundation/micropython-microbit-v2/actions/runs/655946723 ?

@microbit-mark
Copy link
Author

The example in https://microbit-micropython.readthedocs.io/en/v2-docs/audio.html# still sounds different on V1 and V2. On V2 There is an audible click between samples that is less evident on V1.

I will ask the person on the support ticket to also test.

@kevinjwalters
Copy link

@microbit-mark If the PWM output is well behaved and the samples start and end at the midpoint you shouldn't get any clicks/pops at start/end?

@microbit-carlos microbit-carlos modified the milestones: 2.0.0-rc.1, 2.1 Jun 7, 2021
@dpgeorge
Copy link
Collaborator

This repo was updated to use CODAL v0.2.35 in 76c1b31, which changed the audio sampling time from 50ms to 5ms, to make V2 behave more like V1.

@jaustin
Copy link
Contributor

jaustin commented Aug 3, 2022

@martinwork as with #67 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
Copy link
Collaborator

@jaustin I loaded the original python sample into beta and WebUSB downloaded it.

V1 makes a tone with lots of crackles, and clicks between.
V2 makes a very quiet tone with a machine noise and stops after a few seconds with an error: usually "line 5 ValueError: generator already executing"; occasionally "line 11 ValueError: generator already executing"

Is it already known that switching to the beta editor tab resets micro:bit?

@microbit-carlos
Copy link
Contributor

I can confirm that with the latest MicroPython (2b57ddd, which has been updated to CODAL v0.2.40), this script still throws the ValueError: generator already executing errors.

dpgeorge added a commit that referenced this issue Aug 21, 2022
Otherwise audio_data_fetcher may be called while it's running, if the CODAL
just finished playing another sound and schedules audio_data_fetcher.

See issue #47.

Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge
Copy link
Collaborator

The ValueError: generator already executing problem should be fixed by 3b56528.

@microbit-carlos microbit-carlos modified the milestones: 2.1-beta.1, 2.1.0 Aug 26, 2022
@microbit-carlos
Copy link
Contributor

As difference in V1 and V2 was shipped in v2.0.0, and we have tight deadline for v2.1.0, we'll push this one to v2.2.0.

@microbit-carlos microbit-carlos modified the milestones: 2.1.0, 2.2 Sep 23, 2022
@microbit-carlos microbit-carlos modified the milestones: 2.1.1, 2.1.2 Oct 24, 2022
dpgeorge added a commit that referenced this issue Aug 22, 2023
Changes to codal-core:
- added a lock to the initial access/events for the level detector
- setGain and getGain now return floats, as they use fractional values
  internally
- StreamRecording objects now use a linked list of buffers for storage
- StreamSplitter now correctly connects to its upstream
- added virtual to isConnected on DataSource to prevent internal misrouting
- extended DMESG with optional Fiber and Time markers
- updated CodalFiber to correctly behave as a counting semaphore
- corrected locking behaviour on the LevelDetectorSPL to avoid race conditions
- refactored assert to remove assert_true and assert_false
- set the default FiberLock count to 1
- added a "fast path" route to reduce delays on getValue calls
- added a new arg to FiberLock to run in legacy MUTEX mode, or SEMAPHORE
  mode with corrected semantics
- TouchSensor: added missing public as part of declaration
- Compass::clearCalibration() resets calibration member variable
- update touch code to release/reconnect on pin reuse (codal-microbit-v2#345)

Changes to codal-microbit-v2:
- fix BLE panic 071 (#334)
- remove FXOS vestiges
- removed unused include for the old LevelDetector, updated event handling
  for demand activation
- MicroBitUtilityService - replaces PR 178 (#287)
- added isMicrophoneEnabled() to MicroBitAudio to match isSpeakerEnabled
- removed spurious duplicate documentation, added support for
  isMicrophoneEnabled
- MicroBitCompassCalibrator avoid using max(int,int) (#290)
- disbled the MICROBIT_BLE_UTILITY_SERVICE_PAIRING configuration by
  default, enabled on BETA
- MicroBitBLEManager - Move MICROBIT_BLE_MAXIMUM_BONDS to MicroBitConfig (#299)

Changes to codal-nrf52:
- fix NeoPixel/WS2812B timing (#47)
- update touch code to release/reconnect on pin reuse (codal-microbit-v2#345)

Signed-off-by: Damien George <damien@micropython.org>
@microbit-carlos
Copy link
Contributor

microbit-carlos commented Aug 6, 2024

Some of the differences in the original code were due to the CPU speed change between V1 and V2, as V2 can process the sine wave quicker and constantly entering and exiting audio.play in an infinite loop.

This modified example better shows the output sound difference between V1 and V2 when outputting a sine wave on pin0, which needs headphones.

import audio
import math
import microbit


def repeated_frame():
    frame = audio.AudioFrame()
    for i in range(len(frame)):
        frame[i] = int(math.sin(math.pi*i/16)*124+128.5)
    while True:
        yield frame


if hasattr(microbit, 'speaker'):
    microbit.speaker.off() 

audio.play(repeated_frame(), wait=True)

audioframe-play-diff-v1-v2 (5).hex.zip

This video shows how they sound with an external speaker, using headphone the sound is very similar.
The beginning with a V1 is a bit quiet, but hopefully still good enough to hear the difference

20240806_161813.mp4

@microbit-carlos
Copy link
Contributor

Okay, the test above on a V2 was with MicroPython v2.1.2 from the Python Editor.
Using the latest build from the recording and playback branch and the sine wave on the V2 sounds much much better.
Maybe sounded a bit more triangular than the V1, as it was louder as well, but close enough.

I think this is as close as we are gonna get, so we can close this as completed.

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

No branches or pull requests

6 participants