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

Default sampling rates when using StreamRecording don't match #311

Closed
microbit-carlos opened this issue May 4, 2023 · 4 comments
Closed
Assignees
Labels
Milestone

Comments

@microbit-carlos
Copy link
Collaborator

microbit-carlos commented May 4, 2023

With this example, pressing down button A records sound and plays it back as soon as the button is released.
In this case, if the sampling rate is not set in uBit.audio.mixer.addChannel() the playback is speeded up.

#include "MicroBit.h"
#include "StreamRecording.h"

MicroBit uBit;

static void onButtonPress(MicroBitEvent) {
    static SplitterChannel *splitterChannel = uBit.audio.splitter->createChannel();
    static StreamRecording *recording = new StreamRecording(*splitterChannel);

    // The defaults here don't match
    static MixerChannel *channel = uBit.audio.mixer.addChannel(*recording);
    // This version works
    // static MixerChannel *channel = uBit.audio.mixer.addChannel(*recording, 11000);

    uBit.audio.processor->setGain(0.08);
    channel->setVolume(100);
    uBit.audio.mixer.setVolume(1023);
    MicroBitAudio::requestActivation();

    recording->record();
    while (uBit.buttonA.isPressed()) {
        uBit.sleep(5);
    }
    recording->stop();
    uBit.audio.mic->disable();
    uBit.audio.deactivateMic();

    recording->play();
    while (recording->isPlaying()) {
        uBit.sleep(20);
    }
    recording->erase();
}

int main() {
    uBit.init();

    uBit.messageBus.listen(MICROBIT_ID_BUTTON_A, MICROBIT_BUTTON_EVT_DOWN, onButtonPress);

    while (true) {
        uBit.sleep(1000);
    }
}
@microbit-carlos
Copy link
Collaborator Author

@JohnVidler I've tested this with the latest CODAL and still sounds speed up if no sampling rate is specified in the user code.

@microbit-carlos microbit-carlos modified the milestones: v0.2.52, v0.2.53, v0.2.54 May 18, 2023
@microbit-carlos microbit-carlos modified the milestones: v0.2.54, v0.2.57, v0.2.56 Jun 15, 2023
@microbit-carlos
Copy link
Collaborator Author

microbit-carlos commented Jun 29, 2023

I can confirm this is still replicable with CODAL v0.2.55.

Updated, smaller example to replicate:
MICROBIT.hex.zip

#include "MicroBit.h"
#include "StreamRecording.h"

MicroBit uBit;

static void onButtonPress(MicroBitEvent) {
    static SplitterChannel *splitterChannel = uBit.audio.splitter->createChannel();
    static StreamRecording *recording = new StreamRecording(*splitterChannel);

    // The defaults here don't match
    static MixerChannel *channel = uBit.audio.mixer.addChannel(*recording);
    // This version works
    //static MixerChannel *channel = uBit.audio.mixer.addChannel(*recording, 11000);

    channel->setVolume(100);
    MicroBitAudio::requestActivation();

    recording->recordAsync();
    while (uBit.buttonA.isPressed()) {
        uBit.sleep(5);
    }
    recording->stop();

    recording->play();
    while (recording->isPlaying()) {
        uBit.sleep(20);
    }
    recording->erase();
}

int main() {
    uBit.init();

    uBit.messageBus.listen(MICROBIT_ID_BUTTON_A, MICROBIT_BUTTON_EVT_DOWN, onButtonPress);

    while (true) {
        uBit.sleep(1000);
    }
}

@microbit-carlos
Copy link
Collaborator Author

I've checked again, ensuring I'm using the latest commits from all dependencies, I can still replicate this issue.

@microbit-carlos microbit-carlos modified the milestones: v0.2.56, v0.2.57 Jul 14, 2023
@microbit-carlos microbit-carlos modified the milestones: v0.2.57, v0.2.58 Jul 25, 2023
@microbit-carlos microbit-carlos added p0 and removed p2 labels Aug 3, 2023
@microbit-carlos microbit-carlos modified the milestones: v0.2.58, v0.2.59 Aug 3, 2023
@JohnVidler
Copy link
Collaborator

So technically this is intended behaviour, as the addChannel method defaulted to setting the sample rate to zero, which caused the mixer channel to use the mixer sample rate (44k) which is far higher than the default mic sample rate (11k), and I agree that this is weird default behaviour here.

I've added a new CONFIG_MIXER_DEFAULT_CHANNEL_SAMPLERATE constant that sets both the mic default rate and the new mixer channel rate to 11k (both of which can be configured by the user at call-time, so anyone who cares to set a value will see no change here).

I'm not thrilled by that config name, but it aligns with others we have in the mixer, but I'm open to creating a distinct CONFIG_MIC_DEFAULT_SAMPLE_SAMPLERATE which defaults to whatever the CONFIG_MIXER_DEFAULT_CHANNEL_SAMPLERATE is, so these align by default, but can be set independently, but honestly as these can be set by the user at runtime anyway, this only matters in the one case where no preference at either end is expressed, so having just the one is fine, barring any specific requests for two.

See this commit for the patch; d54ce9b and I'm marking this as 'complete' for now, but feel free to reopen if major changes are needed (like the addition of the extra mic-specific config var)

@microbit-carlos microbit-carlos modified the milestones: v0.2.60, v0.2.64 Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants