Closed microbit-carlos closed 2 years ago
@dpgeorge @jaustin @microbit-giles This PR is ready for review.
As we'd like not to delay the implementation too much it'd be appreciated if any feedback could be provided soonish.
Oh, and @microbit-matt-hillsdon, I think you might be interested to have a look at these as well at some point for the stubs.
My main concern is that the name Effect()
may cause confusion and problems later on, especially if we implement sound 'filters' like reverb, amplification and reverse which are known as 'Effects' in widely-used sound editing software. We also have an 'fx' parameter within Effect()
. Although longer to type, I think SoundEffect()
may be clearer and less ambiguous in future.
Oh, and @microbit-matt-hillsdon, I think you might be interested to have a look at these as well at some point for the stubs.
Thanks, yes, I think I'd like to have a stubs branch for this so we can spot any issues typing the API early.
Okay, based on @microbit-giles and @jaustin suggestion I've renamed Effect()
to SoundEffect()
in b89aa5d0f53cba0147e665da880c945f229b9246
Also updated the default values in ce19f55b4a16f2507e3d3e277a77dcfccf988892 to produce this sound: https://makecode.microbit.org/_ajAW04hDL0MP
As these are the default values, that will be the sound played with audio.play(audio.SoundEffect())
@dpgeorge I think we are at a point were we can implement the current version of the docs (minus the addition of the pre-made built-in SoundEffects), so that we can start playing with the feature and get a feeling for it.
For that I think it might be good to keep the implementation in a feature branch, so that we can still tweak it before merging it.
Also we would like to of bringing the Python Editor beta with a MicroPython build with Sound Effects for workshops at EuroPython. I think it would be really cool if we could play with the SoundEffects with people at the conference and if they'd like to contribute their sounds to an example file, we could then pick some of those to use as the pre-build SoundEffects and credit them. The conference starts on the 11th of July, do you think that is doable? Ideally with a bit of time for us to be able to make sure any workshop material we prepare works.
The parameter name
interpolation
is too long, we are still looking for alternatives
How about shape
? That matches the internal CODAL name.
The existing Sound
class lives in microbit.Sound
, and its microbit.audio.AudioFrame
. So where should SoundEffect
go, in microbit
or microbit.audio
?
The existing
Sound
class lives inmicrobit.Sound
, and itsmicrobit.audio.AudioFrame
. So where shouldSoundEffect
go, inmicrobit
ormicrobit.audio
?
The original proposal was to have these inside the audio
module, as it's related to things like the audio.AufioFrame
. So it ends up looking like this: audio.play(audio.SoundEffect())
The reason we ended up putting Sound
in the microbit
module, instead of the audio
module was:
1) We considered these sounds to be micro:bit specific, to be part of the micro:bit personality, i.e. playing the "micro:bit HAPPY sound".
2) Shorter (audio.Sound.HAPPY
vs Sound.HAPPY
) always helps
For now the test branch has it as audio.SoundEffect
, but we should reconsider if there is a good use case for having it as microbit.SoundEffect
.
Merging this as it is the current state of SoundEffect in the v2.1.0-beta.1 release. Will create a new PR to include proposals on predefined sounds.
Proposal
The current proposal adds a new class in the audio module named
SoundEffect
, which can be played using theaudio.play()
function.The user can then create new instances, configuring them via constructor arguments, and after creation they can be modified via attributes.
A collection of a Sound Effects could be played by grouping them in a list (or something more advanced like a generator, so essentially an iterable).
Preview build of the docs: https://microbit-micropython--753.org.readthedocs.build/en/753/audio.html
In progress
Things Still Under Consideration
Effect()
might not be the best name for this featureSoundEffect()
is better but also longer (and we prefer shorter names that are easier to type for young learners)SoundEffect
name as described in comment https://github.com/bbcmicrobit/micropython/pull/753#issuecomment-1172629876interpolation
is too long, we are still looking for alternatives.curve
is being considered, but one of the options is actuallyINTER_CURVE
, so we might have to rename that to something elseramp
?Alternative Proposals
A new function to play an effect instead of creating a class instance
We know that some beginner programmers struggle with the concept of instantiating classes in variables, and then using attributes to modify them. An example of this would be NeoPixels, although on the other hand things like
Images
where a new instance is immediately used in a function has generally been okay. So:So we are considering providing a function to play a single Sound Effect:
If we adopt this we might need a better name that
play_effect(...)
as it is not that different thanplay(Effect(...))
.Implementation details not shown in the docs
Image
->MicroBitImage
, so theEffect
class could have the longerMicroBitSoundEffect
internal nameimport audio
vsfrom audio import *
vsfrom microbit import *
), so their value would have to be used directlyprint(audio.Effect(vol_start=100)
->Effect(freq_start=400, freq_end=2000, duration=500, vol_start=100, vol_end=255, wave="triangle", fx="vibrato", interpolation="log")
__repr__
and keyword arguments for__str__
?Effect(None, 400, 2000, 500, 100, 255, "triangle", "vibrato", "log")
fx
has parameters that can be tweaked.audio.play()
setswait=False
so it's a blocking function. If the CODAL call async version of play is called, and multiple calls are done inmediately one after another, they will not overlap.But this will show the image while playing the sound