help with critical regions, please...

AndrewCarterHughes@gmail.com's icon

hello,

I wrote a circular buffer external, and I'm not sure if I need to do anything to make it thread safe.

The class has a buffer~ that it writes signal input to with the perform function. The buffer is implemented such that it loops around back to the beginning when it reaches the end of the buffer.

In response to a write message, the class copies the buffer to another buffer (so I can 'un-circle' the buffer and save it to a file).

Do I need to implement some type of lockout on the buffer while I am copying it so that the perform function isn't accessing the buffer while I'm copying it? Or is this taken care of for me by max?

for anyone willing to look through my code, it's listed below. I'm worried about the perform method stepping on the write method . The write method is the last method in the code.

any advice would be appreciated.

thanks - Andrew

fyi - the code seems to work fine: compiles, runs, and does what it's supposed to

/* cbuffer~.c
    todo - check on critical region of buffer copying while streaming
         - add file chooser dialog if no file name specified
         - be able to use external save buffer
*/

#include "ext.h"
#include "ext_common.h" // contains CLIP macro
#include "z_dsp.h"
#include "buffer.h"    // this defines our buffer's data structure and other goodies

#define str_eq(s1,s2) (!strcmp ((s1),(s2)))

void *cbuffer_class;

// Data structure for class
typedef struct _cbuffer
{
t_pxobject l_obj;
t_buffer *l_circular_buffer;    // pointer to circular buffer instance
    t_buffer *l_save_buffer; // pointer to save buffer instance
long l_chan;
    long cBufferIndex; // circular buffer index (in sample frames)
    long bufferSize;
} t_cbuffer;

// perform method - records audio to circular buffer
t_int *cbuffer_perform(t_int *w);

// dsp method - inserts perform method in DSP chain
void cbuffer_dsp(t_cbuffer *x, t_signal **sp);

// sets both buffers
int cbuffer_createInternalBuffers(t_cbuffer *x);

// new method
void *cbuffer_new(long n);

// saves the circular buffer to a file
void cbuffer_write(t_cbuffer *x, t_symbol *filename, t_symbol *filetype);

//void cbuffer_assist(t_cbuffer *x, void *b, long m, long a, char *s);
void cbuffer_dblclick(t_cbuffer *x);

t_symbol *ps_buffer;

//******************************************************************************************
// Setup & Initialization Methods

void main(void)
{
    setup((t_messlist **)&cbuffer_class, (method)cbuffer_new, (method)dsp_free, (short)sizeof(t_cbuffer), 0L, A_LONG, 0);

    // register the inputs and message methods
    addmess((method)cbuffer_dsp, "dsp", A_CANT, 0);
    addmess((method)cbuffer_write, "write", A_SYM, A_SYM, 0);
    //addmess((method)cbuffer_assist, "assist", A_CANT, 0);
    addmess((method)cbuffer_dblclick, "dblclick", A_CANT, 0);

    // required call
    dsp_initclass();

    // get the t_symbol for the buffer object
    ps_buffer = gensym("buffer~");
}

void cbuffer_dsp(t_cbuffer *x, t_signal **sp)
{
dsp_add(cbuffer_perform, 4, x, sp[0]->s_vec-1, sp[1]->s_vec-1, sp[0]->s_n+1);
}

void *cbuffer_new(long n)
{

    post("creating a cbuffer...");

    // create a new instance of the external class
    t_cbuffer *x = (t_cbuffer *)newobject(cbuffer_class);

    // reality check
    if (n
    {
        error("cbuffer~: buffer size
        return(0);
    }

    // set buffer
    x->bufferSize = n;

    // set number of channels
    x->l_chan = 2;

    // call dsp_setup to insert perform function into DSP chain
    dsp_setup((t_pxobject *)x, 2);

    // set buffers
    if ( cbuffer_createInternalBuffers(x) == 0 ) return 0;

    // initialize circular buffer index to 0
    x->cBufferIndex = 0;

    return (x);
}

//******************************************************************************************

//******************************************************************************************
// Perform Method

t_int *cbuffer_perform(t_int *w)
{
t_cbuffer *x = (t_cbuffer *)(w[1]); // pointer to instance
t_float *in1 = (t_float *)(w[2]); // input
t_float *in2 = (t_float *)(w[3]); // output
int n = (int)(w[4]);    // number of sample frames to process per call to perform
    t_buffer *b = x->l_circular_buffer; // get a reference to the buffer

    float *tab;
    long chan,frames,nc;
    long saveinuse;

    // if object is disabled, skip processing
    if (x->l_obj.z_disabled)
        goto out;

    // if buffer doesn't exist, output zero
    if (!b)
        goto out;

    // if buffer is invalid, output zero
    if (!b->b_valid)
        goto out;

    saveinuse = b->b_inuse; // saves the buffers inuse status for reset when done
    b->b_inuse = true; // set inuse to true

    tab = b->b_samples; // pointer to sample data
    chan = x->l_chan; // number of channels
    frames = b->b_frames; // number of sample frames
    nc = b->b_nchans; // number of channels

    // process all of the samples in the input buffer
    while (--n)
    {

        // copy channel1 to buffer
        tab[x->cBufferIndex] = *++in1;

        // copy channel2 to buffer
        tab[x->cBufferIndex+1] = *++in2;

        // advance buffer index one frame
        x->cBufferIndex = x->cBufferIndex + 2;

        // restart frame if necessary
        if ( x->cBufferIndex >= b->b_size ) x->cBufferIndex = 0;

    }
    b->b_inuse = saveinuse;
    return w + 5;

out:
    return w + 5;
}

//******************************************************************************************

//******************************************************************************************
// Create internal buffers

// creates both buffers
int cbuffer_createInternalBuffers(t_cbuffer *x)
{

    t_atom bufferArgs[3];

    bufferArgs[0].a_type        = A_LONG;
    bufferArgs[1].a_w.w_long    = x->bufferSize;
    bufferArgs[2].a_type        = A_LONG;
    bufferArgs[2].a_w.w_long    = x->bufferSize;
    bufferArgs[2].a_type        = A_LONG;
    bufferArgs[2].a_w.w_long    = 2;

    x->l_circular_buffer    = (t_buffer*)newinstance(gensym("buffer~"), 3, bufferArgs);
    if (x->l_circular_buffer == 0)
    {
        error("cbuffer~ : failed to create internal circular buffer~");
        return(0);
    }

    x->l_save_buffer        = (t_buffer*)newinstance(gensym("buffer~"), 3, bufferArgs);
    if (x->l_circular_buffer == 0)
    {
        error("cbuffer~ : failed to create internal save buffer~");
        return(0);
    }

}

//******************************************************************************************

//******************************************************************************************
// Other User-defined methods

// this lets us double-click on cbuffer~ to open up the buffer~ it references
void cbuffer_dblclick(t_cbuffer *x)
{
    mess0((t_object *)x->l_circular_buffer,gensym("dblclick"));
    mess0((t_object *)x->l_save_buffer,gensym("dblclick"));
}

/*
void cbuffer_assist(t_cbuffer *x, void *b, long m, long a, char *s)
{
    if (m == ASSIST_OUTLET)
        sprintf(s,"(signal) Sample Value at Index");
    else {
        switch (a) {    
        case 0:
            sprintf(s,"(signal) Sample Index");
            break;
        case 1:
            sprintf(s,"Audio Channel In buffer~");
            break;
        }
    }
}
*/

// saves the cbuffer to a file
void cbuffer_write(t_cbuffer *x, t_symbol *filename, t_symbol *filetype)
{

    post("verifying buffers...");

    t_buffer *cBuffer = x->l_circular_buffer;
    t_buffer *sBuffer = x->l_save_buffer;

    // make sure buffers are set
    if (cBuffer == 0)
    {
        error("cbuffer~: no circular buffer to save!");
        return;
    }
    if (sBuffer == 0)
    {
        error("cbuffer~: no save buffer to save to!");
        return;
    }

    post("copying buffers...");

    long c_index = x->cBufferIndex; // circular buffer index
    long s_index = 0;
    float *c_tab = cBuffer->b_samples;    // pointer to cbuffer data
    float *s_tab = sBuffer->b_samples;    // pointer to sbuffer data

    long size = cBuffer->b_size; // buffer size in frames

    //post("c buffer size is %ld", size);
    //post("s buffer size is %ld", sBuffer->b_size);
    //post("c buffer index is %ld", c_index);

    while(s_index < size)
    {            

        s_tab[s_index] = c_tab[c_index];
        c_index++; s_index++;

        s_tab[s_index] = c_tab[c_index];
        c_index++; s_index++;

        // restart circular index at 0 if necessary
        if ( c_index >= size )
        {
            c_index = 0;
        }

    }

    post("writing buffer...");

    if (filetype == gensym("aiff")) mess1(x->l_save_buffer,gensym("writeaiff"), filename);
    else if (filetype == gensym("wav")) mess1(x->l_save_buffer,gensym("writewav"), filename);
    else
    {
        post("trying to save %s, type %s", filename, filetype);
        error("cbuffer~ : invalid file type for save");
    }

}

projects's icon

Hi Andrew,

You don't have to worry about reading data from a shared resource -
this is safe. If both threads were writing to a shared memory
location then you'd have to think about protecting the memory.

Ben