audio refactor: `SplitterChannel::pullInto(rawBuffer, length)` doesn't respect `length`

[microbit-carlos]

Related to issue:

• audio refactor: SplitterChannel::pullInto(rawBuffer, length) doesn't pull data #495

This covers the return value for pullInto() being larger than expected.
Issue #495 is about no data being copied into the rawBuffer.

Issue

The SplitterChannel::pullInto(uint8_t * rawBuffer, int length) was created for MicroPython to be able to pull an arbitrary number of bytes into a buffer controlled by MicroPython. If there are less bytes available than requested, then it returns as many as it has.

The return statement is a pointer to the end of the buffer, and it can be used as well to confirm how many bytes have been pulled:

uint8_t * SplitterChannel::pullInto( uint8_t * rawBuffer, int length )
{
    ManagedBuffer inData = parent->getBuffer();
    ManagedBuffer result = this->resample( inData, rawBuffer, length );

    return rawBuffer + result.length();
}

Like this:

virtual int MyDataSink::pullRequest() {
   uint8_t pull_buf[10];
   int n = 10;
   int pulled_length = upStream->pullInto(pull_buf, n) - pull_buf;
}

Before the audio refactor SplitterChannel::pullInto() respects the length value and only pulls that many samples.
After the refactor SplitterChannel::pullInto() returns the full buffer irrespectively of length and in cases like this, it overflows the buffer.

Replicate

This example programme create a DataSink that pulls data from the microphone StreamNormaliser splitter, but inside the pull request it replaces that data with a dummy value (123) that is easy to identify when printing the entire buffer to serial:

Click here to view source code

Built from codal-microbit-v2 commit 8256835, and the main branches from codal-core (a2f5051dc60f89276546517f1077e75659ae6741) and codal-nrf52 (1fbb7240290fe36a55c61378f5cdeb7640f3ec4a) at the time.

{
    "target": {
        "name": "codal-microbit-v2",
        "url": "https://github.com/lancaster-university/codal-microbit-v2",
        "branch": "master",
        "dev": true,
        "type": "git"
    },
    "config":{
        "MICROBIT_BLE_ENABLED" : 0,
        "MICROBIT_BLE_PAIRING_MODE": 0,
        "LEVEL_DETECTOR_SPL_8BIT_000_POINT": 52.0,
        "LEVEL_DETECTOR_SPL_8BIT_255_POINT": 100.0
    }
}

#include "MicroBit.h"

MicroBit uBit;

const size_t MIC_BUFFER_SIZE = 500;
const size_t MIC_BUFFER_SIZE_EXTRA = MIC_BUFFER_SIZE + 256;
uint8_t micBuffer[MIC_BUFFER_SIZE_EXTRA] = {0};


class DataSinkCapturer : public DataSink {
public:
    SplitterChannel *upstream;
    uint8_t *dest;
    int dest_pos_ptr;
    size_t dest_max;

    DataSinkCapturer(SplitterChannel *source) : upstream(source) { }
    virtual ~DataSinkCapturer() { }

    bool isCapturing() {
        return this->upstream->isConnected();
    }

    void captureAsync(uint8_t *buf, size_t max_len) {
        this->dest = buf;
        this->dest_max = max_len;
        this->dest_pos_ptr = 0;
        upstream->connect(*this);
    }

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

        if (n > 0) {
            uBit.serial.printf("Requested %d bytes, ", n);
            n = this->upstream->pullInto(pull_buf, n) - pull_buf;
            uBit.serial.printf("pulled %d bytes.\n", n);

            // fill buffer with dummy data to easily identify it
            for (size_t i = 0; i < n; i++) {
                pull_buf[i] = 123;
            }
        }

        if (n <= 0) {
            this->upstream->disconnect();
        } else {
            this->dest_pos_ptr += n;
        }

        return DEVICE_OK;
    }
};


int main() {
    uBit.init();

    uBit.serial.printf("\nMicrophone data capture Test\n");
    DataSinkCapturer *micDataCapturer = new DataSinkCapturer(uBit.audio.splitter->createChannel());
    micDataCapturer->captureAsync(micBuffer, MIC_BUFFER_SIZE);
    while (micDataCapturer->isCapturing()) {
        uBit.sleep(10);
    }
    uBit.serial.printf("\nCaptured %d samples, expected %d samples\n", micDataCapturer->dest_pos_ptr, MIC_BUFFER_SIZE);

    for (size_t i = MIC_BUFFER_SIZE - 8; i < MIC_BUFFER_SIZE + 24; i++) {
        uBit.serial.printf("micBuffer[%d] = %d\n", i, (int)micBuffer[i]);
    }
    for(;;);
}

With this programme we would expect the first 500 bytes of the buffer to be filled with value 123 and the rest filled with zeros, but we can see that instead the value 123 overflows until byte 512.

MICROBIT.hex.zip

Microphone data capture Test
Requested 500 bytes, pulled 256 bytes.
Requested 244 bytes, pulled 256 bytes.

Captured 512 samples, expected 500 samples
micBuffer[492] = 123
micBuffer[493] = 123
micBuffer[494] = 123
micBuffer[495] = 123
micBuffer[496] = 123
micBuffer[497] = 123
micBuffer[498] = 123
micBuffer[499] = 123
micBuffer[500] = 123
micBuffer[501] = 123
micBuffer[502] = 123
micBuffer[503] = 123
micBuffer[504] = 123
micBuffer[505] = 123
micBuffer[506] = 123
micBuffer[507] = 123
micBuffer[508] = 123
micBuffer[509] = 123
micBuffer[510] = 123
micBuffer[511] = 123
micBuffer[512] = 0
micBuffer[513] = 0
micBuffer[514] = 0
micBuffer[515] = 0
micBuffer[516] = 0
micBuffer[517] = 0
micBuffer[518] = 0
micBuffer[519] = 0
micBuffer[520] = 0
micBuffer[521] = 0
micBuffer[522] = 0
micBuffer[523] = 0