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

Missing sound data between the first and second data buffer from the microphone #446

Open
microbit-carlos opened this issue Sep 10, 2024 · 2 comments
Assignees
Milestone

Comments

@microbit-carlos
Copy link
Collaborator

microbit-carlos commented Sep 10, 2024

Using a custom DataSink, as done by MicroPython, we can see that when pulling from the splitter channel, we end up with what it looks like missing data between the first and second buffer from the ADC.

This test was carried out with the current CODAL configuration where the ADC is set to 45454 Hz, and the example code then sets the splitter channel sample rate to 7812.
As the ADC DMA buffers are 512 bytes, and we have 14-bits per sample, we get 256 samples per buffer. Downsampling that buffer from 45454 to 7812, and we get 44 bytes left (256 / (45454 / 7812) ≈ 44).

This example code is configured use a custom DataSink to pull 128 bytes 10 times in a row (each time connecting and disconnecting from the pipeline) and the data can be printed to serial.

Then I've played a 382 Hz sine wave (just picked a random frequency that easily illustrates this issue) with my phone next to the micro:bit microphone and plotted the data. We can see that in all of the plots the wave glitches at the 44th byte.
Interestingly we don't see the same at the 88th byte, so I wonder if this only affects the boundary between the 1st and 2nd byte from the moment the splitter channel is connected?

MICROBIT.hex.zip

Source code

Press button A to record, and button B to print data to serial.

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

MicroBit uBit;

const int SAMPLE_RATE = 7812;
const int BUFFERS_LEN = 10;
const int BUFFERS_SIZE = 128;
uint8_t buffers[BUFFERS_LEN][BUFFERS_SIZE];

class MyStreamRecording : public DataSink {
public:
    SplitterChannel *upStream;
    uint8_t *dest;
    int dest_pos_ptr;
    size_t dest_max;
    bool request_stop;

    MyStreamRecording(SplitterChannel *source) : upStream(source) { }
    virtual ~MyStreamRecording() { }

    float getSampleRate() {
        return this->upStream->getSampleRate();
    }
    bool isRecording() {
        return upStream->isConnected();
    }
    void stopRecording() {
        this->request_stop = true;
    }
    void recordAsync(uint8_t *buf, size_t max_len) {
        this->dest = buf;
        this->dest_max = max_len;
        this->dest_pos_ptr = 0;
        this->request_stop = false;

        upStream->connect(*this);
    }
    virtual int pullRequest() {
        uint8_t *pull_buf = this->dest + this->dest_pos_ptr;
        size_t n = this->dest_max - this->dest_pos_ptr;

        if (n > 0) {
            n = this->upStream->pullInto(pull_buf, n) - pull_buf;
        }

        if (n == 0 || this->request_stop) {
            this->upStream->disconnect();
            this->request_stop = false;
        } else {
            // Convert signed 8-bit to unsigned 8-bit data.
            for (size_t i = 0; i < n; ++i) {
                pull_buf[i] += 128;
            }
            this->dest_pos_ptr += n;
        }

        return DEVICE_OK;
    }
};

int main() {
    uBit.init();

    SplitterChannel *splitterChannel = uBit.audio.splitter->createChannel();
    splitterChannel->requestSampleRate(SAMPLE_RATE);
    MyStreamRecording *recording = new MyStreamRecording(splitterChannel);

    memset(buffers, 128, sizeof(buffers));

    while (true) {
        if (uBit.buttonA.isPressed()) {
            // Record data into buffers
            uBit.display.print("R");
            uBit.sleep(500);
            for (int i = 0; i < BUFFERS_LEN; ++i) {
                recording->recordAsync(buffers[i], BUFFERS_SIZE);
                while (recording->isRecording()) {
                    uBit.sleep(1);
                }
            }
        }
        if (uBit.buttonB.isPressed()) {
            // Print buffers to serial in CSV format
            uBit.display.print("P");
            for (int i = 0; i < BUFFERS_SIZE; ++i) {
                for (int j = 0; j < BUFFERS_LEN - 1; ++j) {
                    uBit.serial.printf("%d, ", buffers[j][i]);
                }
                uBit.serial.printf("%d\n", buffers[BUFFERS_LEN - 1][i]);
            }
        }
        uBit.display.clear();
        uBit.sleep(100);
    }
}

(I've omitted the first two buffers, as they contain bad data as caused by issue #442)

image image image
@microbit-carlos
Copy link
Collaborator Author

To see if we could capture more than one glitch per "pipeline stream" (as the example code connects and disconnects the stream for each of the 128 byte plots), I've tried increasing each block of pulled data from 128 bytes to 256 bytes, and a few different sound frequencies (in case small offsets made it more obvious in the plot).

But, it looks like we really are only seeing the issue between the 1st and 2nd pull, at byte 44:
image

@martinwork
Copy link
Collaborator

martinwork commented Sep 11, 2024

@microbit-carlos The first packet in each recording is an old packet, and the latest packet is dropped.

Based on a previous look, I think... When the StreamRecorder connects, the next generated ADC packet will prompt the pull request, but the buffer pulled from mic->output will be whatever it was holding before the packet generation, and the buffer the ADC just sent will be dropped.

The mic->output DataStream holds each buffer it receives until it is pulled, and drops newer packets. I don’t understand why it does this. I expected the opposite. I think perhaps it needs a double buffer system, like the ADC hardware uses, with the event handler switching the buffer for the ADC interrupt to use before it forwards the last buffer received.
https://github.com/lancaster-university/codal-core/blob/509086cc8590465041b15493ab52b56e7071c110/source/streams/DataStream.cpp#L188

By adding a serial number in the first byte of each ADC packet, and preserving it in StreamNormalizer, the packet numbers in each recording can be dumped. See below, two A presses.

It's clear that a packet is dropped in each recording, and the second batch starts with the old packet 39 that was not pulled through in the last batch. In fact, I think the first packet in each recording is an old one from just after the previous recording.

For the output below, I made the serial number a static. With the serial number as a member variable in NRF52ADCChannel, the numbers restart for the second batch (the first recording is 39 4 5), because the channel is released and recreated during the pause between A presses.

0 1 2
3 5 6
7 9 10
11 13 14
15 17 18
19 21 22
23 25 26
27 29 30
31 33 34
35 37 38

39 183 184
185 187 188
189 191 192
193 195 196
197 199 200
201 203 204
205 207 208
209 211 212
213 215 216
217 219 220

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

No branches or pull requests

3 participants