Search code examples
caudiomemory-managementdeep-copyportaudio

Memory problems with continuously recording audio


Here I am trying to write some code for a continuously recording audio system. I am then attempting to record the audio for a certain amount of time when a certain amplitude threshold is broken.

#include <stdio.h>
#include <stdlib.h>
#include <math.h>
#include <string.h>
#include <time.h>
#include <portaudio.h>
#include <sndfile.h>

#define FRAMES_PER_BUFFER (1024)
#define SAMPLE_SIZE (4)

typedef struct
{
    uint16_t formatType;
    uint16_t numberOfChannels;
    uint32_t sampleRate;
    float* recordedSamples;
} AudioData;

AudioData initAudioData(uint32_t sampleRate, uint16_t channels, int type)
{
    AudioData data;
    data.formatType = type;
    data.numberOfChannels = channels;
    data.sampleRate = sampleRate;
    return data;
}

float avg(float *data)
{
    int elems = sizeof(data) / sizeof(data[0]);
    float sum = 0;
    for (int i = 0; i < elems; i++)
    {
        sum += fabs(*(data + i));
    }
    return (float) sum / elems;
}

int main(void)
{
    AudioData data = initAudioData(44100, 2, paFloat32);
    PaStream *stream = NULL;
    PaError err = paNoError;
    int size = FRAMES_PER_BUFFER * data.numberOfChannels * SAMPLE_SIZE;
    float *sampleBlock = malloc(size);
    float *recordedSamples = NULL;
    time_t talking = 0;
    time_t silence = 0;

    if((err = Pa_Initialize())) goto done;
    PaStreamParameters inputParameters =
    {
        .device = Pa_GetDefaultInputDevice(),
        .channelCount = data.numberOfChannels,
        .sampleFormat = data.formatType,
        .suggestedLatency = Pa_GetDeviceInfo(Pa_GetDefaultInputDevice())->defaultHighInputLatency,
        .hostApiSpecificStreamInfo = NULL
    };
    if((err = Pa_OpenStream(&stream, &inputParameters, NULL, data.sampleRate, FRAMES_PER_BUFFER, paClipOff, NULL, NULL))) goto done;
    if((err = Pa_StartStream(stream))) goto done;
    for(int i = 0;;)
    {
        err = Pa_ReadStream(stream, sampleBlock, FRAMES_PER_BUFFER);
        if(avg(sampleBlock) > 0.000550) // talking
        {
            printf("You're talking! %d\n", i);
            i++;
            time(&talking);
            recordedSamples = realloc(recordedSamples, size * i);
            if (recordedSamples) memcpy(recordedSamples + ((i - 1) * size), sampleBlock, size); // problem here writing to memory at i = 16?
            else free(recordedSamples);
        }
        else //silence
        {
            double test = difftime(time(&silence), talking);
            printf("Time diff: %g\n", test);
            if (test >= 1.5)
            {
                // TODO: finish code processing audio snippet
                talking = 0;
                free(recordedSamples); // problem freeing memory?
            }
        }
    }

done:
    free(sampleBlock);
    Pa_Terminate();
    return err;
}

However, the code is being somewhat finicky. Sometimes when I run my program in Xcode, I get the following output:

Time diff: 1.4218e+09
You're talking! 0
You're talking! 1
You're talking! 2
You're talking! 3
You're talking! 4
You're talking! 5
You're talking! 6
You're talking! 7
You're talking! 8
You're talking! 9
You're talking! 10
You're talking! 11
You're talking! 12
You're talking! 13
You're talking! 14
You're talking! 15
(lldb)

With Xcode pointing to this line being the problem:

if (recordedSamples) memcpy(recordedSamples + ((i - 1) * size), sampleBlock, size); // problem here writing to memory at i = 16?

Other times I run the code, I get this error:

Time diff: 1.4218e+09
You're talking! 0
Time diff: 0
Time diff: 0
Time diff: 0
Time diff: 0
Time diff: 0
Time diff: 0
Time diff: 0
Time diff: 0
Time diff: 0
Time diff: 0
Time diff: 0
Time diff: 0
Time diff: 0
Time diff: 0
Time diff: 0
Time diff: 0
Time diff: 0
Time diff: 0
Time diff: 0
Time diff: 0
Time diff: 0
Time diff: 0
Time diff: 0
Time diff: 0
Time diff: 0
Time diff: 0
Time diff: 0
Time diff: 0
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 1
Time diff: 2
Time diff: 1.4218e+09
CTestEnvironment(55085,0x7fff7938e300) malloc: *** error for object 0x10081ea00: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug

Both errors are somewhat confusing me... any suggestions?


Solution

  • You are writing out of bounds of the allocated buffer:

    recordedSamples = realloc(recordedSamples, size * i);
    memcpy(recordedSamples + ((i - 1) * size), sampleBlock, size);
    

    realloc() allocates a certain number of bytes, here size * i. The resulting pointer is stored in recordedSamples, which has type float*.

    The memcpy() then tries to write data to recordedSamples + ((i - 1) * size. Pointer arithmetic is used to determine the location that should be written to. Since recordedSamples is of type float*, recordedSample + X points to a offset of X float values (not X bytes).

    In other words, recordedSamples + ((i - 1) * size points to the memory location ((i - 1) * size * sizeof(float) bytes after recordedSamples. This is usually not inside the allocated buffer, since floats are larger than a single byte.

    To fix this, the big question is if size is supposed to be a number of bytes or a number of floats. This depends on the API functions you are using, I haven't looked into it in detail.

    If it's a number of floats, then you have to adjust the calls to basic memory management functions like malloc, realloc and memcpy, because the all operate on bytes. To instead of malloc(size) you would call malloc(size * sizeof(float)).

    If size is indeed a number of bytes, then it would be more logical to make recordedSamples a char*, or at least cast it before doing pointer arithmetic with byte offsets, like memcpy((char*)recordedSamples + ...).