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

SplitterChannel::requestSampleRate doesn't work as expected #284

Closed
martinwork opened this issue Mar 7, 2023 · 5 comments
Closed

SplitterChannel::requestSampleRate doesn't work as expected #284

martinwork opened this issue Mar 7, 2023 · 5 comments
Assignees
Labels
Milestone

Comments

@martinwork
Copy link
Collaborator

martinwork commented Mar 7, 2023

Requesting a lower rate from the SplitterChannel doesn't change the reported rate. After requesting a change to a higher rate, the SplitterChannel still reports the rate of it's upstream (higher than requested) rather than the requested rate.

requestSampleRate halved from 22222, returned 11111, new rate 22222
requestSampleRate doubled from 22222, returned 44444, new rate 45454

I think the problem is this line should be this->sampleRate = sampleRate;
codal-core/source/streams/StreamSplitter.cpp#L118

Once fixed, that line might need to be moved after the check on the upstream capability, to avoid this->sampleRate being greater than the upstream sample rate, which would make the calculated step value zero.
codal-core/source/streams/StreamSplitter.cpp#L68

SplitterChannel::pull uses a truncated integer step and always picks the first item in every buffer. This will tend to generate more samples than requested.

If I keep requesting a higher rate, it eventually hangs, even though I think this->sampleRate isn't being changed. I'm guessing this is because the adc doesn't protect itself against requests for very small sample periods. SplitterChannel::pull doesn't protect itself against the possibility that someone else has changed the upstream rate, leading to zero step.

#include "MicroBit.h"

MicroBit uBit;

void onButtonA(MicroBitEvent e)
{
  float rate0 = uBit.audio.levelSPL->upstream.getSampleRate();
  float rateR = uBit.audio.levelSPL->upstream.requestSampleRate( rate0 * 2);
  float rate1 = uBit.audio.levelSPL->upstream.getSampleRate();

  ManagedString strRate0( (int) floor( 0.5 + rate0));
  ManagedString strRateR( (int) floor( 0.5 + rateR));
  ManagedString strRate1( (int) floor( 0.5 + rate1));
  uBit.serial.send( "requestSampleRate doubled from " + strRate0 + ", returned " + strRateR + ", new rate " + strRate1 + "\n");
}

void onButtonB(MicroBitEvent e)
{
  float rate0 = uBit.audio.levelSPL->upstream.getSampleRate();
  float rateR = uBit.audio.levelSPL->upstream.requestSampleRate( rate0 / 2);
  float rate1 = uBit.audio.levelSPL->upstream.getSampleRate();

  ManagedString strRate0( (int) floor( 0.5 + rate0));
  ManagedString strRateR( (int) floor( 0.5 + rateR));
  ManagedString strRate1( (int) floor( 0.5 + rate1));
  uBit.serial.send( "requestSampleRate halved from " + strRate0 + ", returned " + strRateR + ", new rate " + strRate1 + "\n");
}

void forever()
{
  while (true)
  {
    float soundLevel = uBit.audio.levelSPL->getValue();
    ManagedString strLevel( (int) floor( 0.5 + soundLevel));
    uBit.serial.send( "level:" + strLevel + "\n");
    uBit.sleep(2000);
  }
}

int main()
{
  uBit.init();
  uBit.display.disable();
  create_fiber( forever);
  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();
}
@martinwork
Copy link
Collaborator Author

If sampleRate is small relative to inRate, step (e.g. 44000/100) may be greater than inSamples, and it will generate no data.

@JohnVidler
Copy link
Collaborator

I think this is now corrected in tests/audio; although I'd appreciate the extra eyes on it...

I'll run your test code against the branch and see what we get.

@martinwork
Copy link
Collaborator Author

@JohnVidler @finneyj Here's a new test. test-adc-splitter.zip. Beware of this test code bringing it's own bugs!

When compiled with tests/audio, the mic deactivates if the test doesn't poll levelSPL->getValue() often, even though the test creates an extra channel on the audio raw splitter.

The SplitterChannel should only record the requested sample rate after the upstream has confirmed it can support it. I think that won't fix how it works with the ADC, because the ADC agrees to anything, even if it can't do it, and the actual delivery rate is lower than the agreed rate.

@JohnVidler
Copy link
Collaborator

(Reused text from another issue - there are a few here with strongly related problems)

I've given a more complete update over in #356 but the gist is that this should no longer be the case when we merge the test branch(es) into master as the SplitterChannels should now automatically resample to match the requested rates.

Thanks everyone for being patient on these issues.

@JohnVidler
Copy link
Collaborator

This should now finally be fixed with release v0.2.65

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

4 participants