Skip to main content
Added naive inplementation of FIFO_pop without memmove.
Source Link
harald
  • 696
  • 5
  • 12

Here's a few suggestions in addition to what Keith mentions.

I would try to avoid the memory copy and move operations. You mention a circular buffer, which is one way of doing it, another would be to have the fifo hold discrete buffers and pop each buffer from the front in it's entirety. Avoiding the copy and move operations should speed up the code significantly, which I assume is of concern when processing audio data :)

Also, I would drop the typedef, just use struct fifo instead. This makes it easier to convert this into a full ADT (Abstract Data Type) later, and just forward declare the struct to users while keeping the actual struct private.

As a final note, are you absolutely sure that the restrict keyword is the proper thing to use here?

Edit: Here's one naive way of getting rid of the memove operation:

typedef struct {
    float *buffer;
    float *start;
    float *end;
    size_t max_length;
} FIFO;


bool FIFO_pop(FIFO *restrict fifo, float *restrict data, size_t count) {
    if (!fifo || !data) return false;
    if (count == 0 || FIFO_available(fifo) < count) return false;

    // move count items from the buffer
    memcpy(data, fifo->start, count*sizeof(float));
    fifo->start += count;
    
    return true;
}

This will only work if you know the max sized buffer that you need, but it should give you an idea.

Here's a few suggestions in addition to what Keith mentions.

I would try to avoid the memory copy and move operations. You mention a circular buffer, which is one way of doing it, another would be to have the fifo hold discrete buffers and pop each buffer from the front in it's entirety. Avoiding the copy and move operations should speed up the code significantly, which I assume is of concern when processing audio data :)

Also, I would drop the typedef, just use struct fifo instead. This makes it easier to convert this into a full ADT (Abstract Data Type) later, and just forward declare the struct to users while keeping the actual struct private.

As a final note, are you absolutely sure that the restrict keyword is the proper thing to use here?

Here's a few suggestions in addition to what Keith mentions.

I would try to avoid the memory copy and move operations. You mention a circular buffer, which is one way of doing it, another would be to have the fifo hold discrete buffers and pop each buffer from the front in it's entirety. Avoiding the copy and move operations should speed up the code significantly, which I assume is of concern when processing audio data :)

Also, I would drop the typedef, just use struct fifo instead. This makes it easier to convert this into a full ADT (Abstract Data Type) later, and just forward declare the struct to users while keeping the actual struct private.

As a final note, are you absolutely sure that the restrict keyword is the proper thing to use here?

Edit: Here's one naive way of getting rid of the memove operation:

typedef struct {
    float *buffer;
    float *start;
    float *end;
    size_t max_length;
} FIFO;


bool FIFO_pop(FIFO *restrict fifo, float *restrict data, size_t count) {
    if (!fifo || !data) return false;
    if (count == 0 || FIFO_available(fifo) < count) return false;

    // move count items from the buffer
    memcpy(data, fifo->start, count*sizeof(float));
    fifo->start += count;
    
    return true;
}

This will only work if you know the max sized buffer that you need, but it should give you an idea.

Source Link
harald
  • 696
  • 5
  • 12

Here's a few suggestions in addition to what Keith mentions.

I would try to avoid the memory copy and move operations. You mention a circular buffer, which is one way of doing it, another would be to have the fifo hold discrete buffers and pop each buffer from the front in it's entirety. Avoiding the copy and move operations should speed up the code significantly, which I assume is of concern when processing audio data :)

Also, I would drop the typedef, just use struct fifo instead. This makes it easier to convert this into a full ADT (Abstract Data Type) later, and just forward declare the struct to users while keeping the actual struct private.

As a final note, are you absolutely sure that the restrict keyword is the proper thing to use here?