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

[DO NOT MERGE] Sim changes for forthcoming beta MicroPython release #113

Draft
wants to merge 36 commits into
base: main
Choose a base branch
from

Conversation

microbit-matt-hillsdon
Copy link
Contributor

@microbit-matt-hillsdon microbit-matt-hillsdon commented Mar 21, 2024

See microbit-foundation/python-editor-v3#1155

Closes microbit-foundation/python-editor-v3#1082
Closes #110
Closes #101

Browser support issues

Firefox

Regression playing AudioFrame even at default rate

There's a 4x reduction in sample rate is due to the removal of buffer expansion in 2c67ed68a15bb64949e7825eaba46acb57f680b2 which drops the sample rate out of Firefox's supported range (older Safari will likely have issues too as we saw previously for speech).

Presumably user supplied rates can cause issues even if we fix this with interpolation, so we need to determine the range we must support and write resampling code, even if we can skip it on some browsers/at some rates.

Recording sample rate issue

Uncaught (in promise) DOMException: AudioContext.createMediaStreamSource: Connecting AudioNodes from AudioContexts with different sample-rate is currently not supported.
    startRecording http://localhost:3000/build/simulator.js:1541
    _mp_js_hal_microphone_start_recording http://localhost:3000/build/firmware.js:1781

Seems that Firefox just doesn't support using web audio to resample. We've also found you can't ask media recorder for arbitrary sample rates. So that leaves us with manually resampling, at least for Firefox. We could use something like https://github.com/rochars/wave-resampler (or at least the same techniques, not clear how maintained that is).

Safari

  • Safari 13 (desktop & iOS)
    • Requires prefix for OfflineAudioContext and doesn't support the object constructor (use (channels, length, rate))
    • Worse: doesn't support sample rates below 44100 so OfflineAudioContext is useless for resampling - could use same approach as Firefox?
  • Safari 14 (desktop & iOS)
    • Non-prefixed. Seems to work with lower sample rates but we can't actually get the mic to work via Browserstack so need a real old device to check which might not happen...

Perhaps we can accept some limitations here. Need to check whether the Firefox playback issue above applies here.

Misc quality issues

  • Can we make the browser tab microphone indicator go away when we stop recording? 4ed8c93
  • Do we do anything about clicks/pops at the end of the sample?
  • Do we want to make playback worse to match the device speaker? How?

Work to do if/when implemented in MicroPython

microbit-matt-hillsdon and others added 4 commits March 21, 2024 10:37
On the basis that's as close as we can get to the beta for now.
Still has HAL issues.

One local change to MicroPython was needed to include mphal.h in
modspeech - need to review how this works for the non-sim build as it
seems like it should be included.
Copy link

Preview build will be at
https://review-python-simulator.usermbit.org/beta-updates/

Copy link

netlify bot commented Mar 21, 2024

Deploy Preview for distracted-dubinsky-fd8a42 ready!

Name Link
🔨 Latest commit aea7937
🔍 Latest deploy log https://app.netlify.com/sites/distracted-dubinsky-fd8a42/deploys/66180155f649210008be1348
😎 Deploy Preview https://deploy-preview-113--distracted-dubinsky-fd8a42.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Comment on lines 197 to 199
// TODO: consider AudioWorklet - worth it? Browser support?
// consider alternative resampling approaches
// what sample rates are actually supported this way?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ScriptProcessorNode support vs AudioWorklet support so we'd lose early Safari 14.x and Safari 13 (for 13 I think limited support is OK).

See also the techniques used in this project.

@@ -9,6 +9,8 @@

extern ringbuf_t stdin_ringbuf;

#define mp_hal_ticks_cpu() (0)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to understand this / investigate new time API.
We need to do the stubs for that too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some discussion on the stubs PR too

- Fix missing typescript types
- Use Safari 13 compatible OfflineAudioContext constructor - this may
  prove pointless due to lack of < 44100 sample rate support, but is an
  easier starting point to try things out.
@jaustin
Copy link

jaustin commented Apr 10, 2024

Having used this a little, I've noticed the microphone LED isn't coming on when using the computer mic.

by stopping micStream tracks
Passing this.microphone.microphoneOn/Off functions as BoardAudio constructors in  board/index.ts didn't work for some reasons. The microphone element was flagged as being undefined. So instead I passed the element directly into the BoardAudio constructor, which worked.
Copy link

cloudflare-workers-and-pages bot commented May 3, 2024

Deploying micropython-microbit-v2-simulator with  Cloudflare Pages  Cloudflare Pages

Latest commit: ab07528
Status: ✅  Deploy successful!
Preview URL: https://81027e5a.micropython-microbit-v2-simulator.pages.dev
Branch Preview URL: https://beta-updates.micropython-microbit-v2-simulator.pages.dev

View logs

Enable a larger audio buffer
- Use smaller buffer to avoid too much audio after wait=True
- Don't reset nextStartTime to avoid overlapping audio
- Add indirection to callback so we can clear it properly to avoid
  calling it when audio finishes after MicroPython has terminated
@microbit-matt-hillsdon
Copy link
Contributor Author

microbit-matt-hillsdon commented May 24, 2024

Re the Firefox regression the sample rate seems to have changed.

Main, working:
image

Branch, not working:
image

This 4x reduction in sample rate is due to the removal of buffer expansion in 2c67ed68a15bb64949e7825eaba46acb57f680b2 which drops the sample rate out of Firefox's supported range (older Safari will have issues too as we saw previously for speech).

I guess we need to look at resampling in the sim for multiple reasons:

  1. increasing the audio frame playback rate
  2. downsampling the media recorder rate so we can get data at the requested rate

@microbit-matt-hillsdon
Copy link
Contributor Author

microbit-matt-hillsdon and others added 3 commits August 19, 2024 17:29
Adjust build and samples for record/playback
No HAL changes but the API has changed
We need to do this for recording as we can't specify a rate.

We need to do this for playback for sample rates outside of the
browser's supported range. That includes the current default rate for
Firefox and older Safari.

Uses a resampler build that doesn't support medium/best to save on
bundle size.

I've also increased the buffer size so I can play a 44k sample in an
OK-ish way. Maybe 256 will be needed.

We know Firefox on Windows still performs poorly. It did badly with
audio before these changes.

Another significant issue is that it can take 3 seconds to call getUserMedia on Desktop Safari.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants