From 0b508db9fc0f0bf5d1a08635a490de4ba692540a Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Thu, 20 Apr 2017 11:25:24 +0200 Subject: [PATCH] ringbuffer: simplify the API Use absolute indexes that we let wrap around. We can then easily detect how much we under of overflowed by using serial number arithmetic. Remove the Areas, we can trivially compute this ourselves, move the logic in read/write_data. --- pinos/client/loop.c | 37 ++++--- pinos/client/transport.c | 22 ++-- spa/include/spa/ringbuffer.h | 160 +++++++++++----------------- spa/lib/debug.c | 1 - spa/plugins/alsa/alsa-utils.c | 14 +-- spa/plugins/audiomixer/audiomixer.c | 11 -- 6 files changed, 107 insertions(+), 138 deletions(-) diff --git a/pinos/client/loop.c b/pinos/client/loop.c index bee850bed..a64ef7f49 100644 --- a/pinos/client/loop.c +++ b/pinos/client/loop.c @@ -182,32 +182,45 @@ loop_invoke (SpaLoop *loop, { PinosLoopImpl *impl = SPA_CONTAINER_OF (loop, PinosLoopImpl, loop); bool in_thread = pthread_equal (impl->thread, pthread_self()); - SpaRingbufferArea areas[2]; InvokeItem *item; SpaResult res; if (in_thread) { res = func (loop, false, seq, size, data, user_data); } else { - spa_ringbuffer_get_write_areas (&impl->buffer, areas); - if (areas[0].len < sizeof (InvokeItem)) { - pinos_log_warn ("data-loop %p: queue full", impl); + int32_t filled, avail; + uint32_t offset, l0; + + filled = spa_ringbuffer_get_write_index (&impl->buffer, &offset); + if (filled < 0 || filled > impl->buffer.size) { + pinos_log_warn ("data-loop %p: queue xrun %d", impl, filled); return SPA_RESULT_ERROR; } - item = SPA_MEMBER (impl->buffer_data, areas[0].offset, InvokeItem); + avail = impl->buffer.size - filled; + if (avail < sizeof (InvokeItem)) { + pinos_log_warn ("data-loop %p: queue full %d", impl, avail); + return SPA_RESULT_ERROR; + } + offset &= impl->buffer.mask; + + l0 = offset + avail; + if (l0 > impl->buffer.size) + l0 = impl->buffer.size - l0; + + item = SPA_MEMBER (impl->buffer_data, offset, InvokeItem); item->func = func; item->seq = seq; item->size = size; item->user_data = user_data; - if (areas[0].len > sizeof (InvokeItem) + size) { + if (l0 > sizeof (InvokeItem) + size) { item->data = SPA_MEMBER (item, sizeof (InvokeItem), void); item->item_size = sizeof (InvokeItem) + size; - if (areas[0].len < sizeof (InvokeItem) + item->item_size) - item->item_size = areas[0].len; + if (l0 < sizeof (InvokeItem) + item->item_size) + item->item_size = l0; } else { - item->data = SPA_MEMBER (impl->buffer_data, areas[1].offset, void); - item->item_size = areas[0].len + 1 + size; + item->data = impl->buffer_data; + item->item_size = l0 + 1 + size; } memcpy (item->data, data, size); @@ -231,8 +244,8 @@ event_func (SpaLoopUtils *utils, PinosLoopImpl *impl = data; uint32_t offset; - while (spa_ringbuffer_get_read_offset (&impl->buffer, &offset) > 0) { - InvokeItem *item = SPA_MEMBER (impl->buffer_data, offset, InvokeItem); + while (spa_ringbuffer_get_read_index (&impl->buffer, &offset) > 0) { + InvokeItem *item = SPA_MEMBER (impl->buffer_data, offset & impl->buffer.mask, InvokeItem); item->func (impl->this.loop, true, item->seq, item->size, item->data, item->user_data); spa_ringbuffer_read_advance (&impl->buffer, item->item_size); } diff --git a/pinos/client/transport.c b/pinos/client/transport.c index 5d054e84e..22d7eb99b 100644 --- a/pinos/client/transport.c +++ b/pinos/client/transport.c @@ -38,8 +38,8 @@ typedef struct { PinosMemblock mem; size_t offset; - SpaRingbufferArea areas[2]; SpaEvent current; + uint32_t current_offset; } PinosTransportImpl; static size_t @@ -206,20 +206,21 @@ pinos_transport_add_event (PinosTransport *trans, SpaEvent *event) { PinosTransportImpl *impl = (PinosTransportImpl *) trans; - SpaRingbufferArea areas[2]; - size_t avail, size; + int32_t filled, avail; + uint32_t size, index; if (impl == NULL || event == NULL) return SPA_RESULT_INVALID_ARGUMENTS; + filled = spa_ringbuffer_get_write_index (trans->output_buffer, &index); + avail = trans->output_buffer->size - filled; size = SPA_POD_SIZE (event); - avail = spa_ringbuffer_get_write_areas (trans->output_buffer, areas); if (avail < size) return SPA_RESULT_ERROR; spa_ringbuffer_write_data (trans->output_buffer, trans->output_data, - areas, + index & trans->output_buffer->mask, event, size); spa_ringbuffer_write_advance (trans->output_buffer, size); @@ -232,18 +233,21 @@ pinos_transport_next_event (PinosTransport *trans, SpaEvent *event) { PinosTransportImpl *impl = (PinosTransportImpl *) trans; - size_t avail; + int32_t avail; + uint32_t index; if (impl == NULL || event == NULL) return SPA_RESULT_INVALID_ARGUMENTS; - avail = spa_ringbuffer_get_read_areas (trans->input_buffer, impl->areas); + avail = spa_ringbuffer_get_read_index (trans->input_buffer, &index); if (avail < sizeof (SpaEvent)) return SPA_RESULT_ENUM_END; + impl->current_offset = index & trans->input_buffer->mask; + spa_ringbuffer_read_data (trans->input_buffer, trans->input_data, - impl->areas, + impl->current_offset, &impl->current, sizeof (SpaEvent)); @@ -266,7 +270,7 @@ pinos_transport_parse_event (PinosTransport *trans, spa_ringbuffer_read_data (trans->input_buffer, trans->input_data, - impl->areas, + impl->current_offset, event, size); spa_ringbuffer_read_advance (trans->input_buffer, size); diff --git a/spa/include/spa/ringbuffer.h b/spa/include/spa/ringbuffer.h index 3357bf887..abff0130a 100644 --- a/spa/include/spa/ringbuffer.h +++ b/spa/include/spa/ringbuffer.h @@ -34,24 +34,18 @@ typedef struct _SpaRingbuffer SpaRingbuffer; #include #include -typedef struct { - uint32_t offset; - uint32_t len; -} SpaRingbufferArea; - /** * SpaRingbuffer: * @readindex: the current read index * @writeindex: the current write index - * @size: the size of the ringbuffer - * @size_mask: mask if @size is power of 2 + * @size: the size of the ringbuffer must be power of 2 + * @mask: mask as @size - 1 */ struct _SpaRingbuffer { - volatile uint32_t readindex; - volatile uint32_t writeindex; - uint32_t size; - uint32_t mask; - uint32_t mask2; + uint32_t readindex; + uint32_t writeindex; + uint32_t size; + uint32_t mask; }; /** @@ -74,7 +68,6 @@ spa_ringbuffer_init (SpaRingbuffer *rbuf, rbuf->size = size; rbuf->mask = size - 1; - rbuf->mask2 = (size << 1) - 1; rbuf->readindex = 0; rbuf->writeindex = 0; @@ -94,63 +87,53 @@ spa_ringbuffer_clear (SpaRingbuffer *rbuf) rbuf->writeindex = 0; } -static inline uint32_t -spa_ringbuffer_get_read_offset (SpaRingbuffer *rbuf, - uint32_t *offset) +/** + * spa_ringbuffer_get_read_index: + * @rbuf: a #SpaRingbuffer + * @index: the value of readindex, should be masked to get the + * offset in the ringbuffer memory + * + * Returns: number of available bytes to read. values < 0 mean + * there was an underrun. values > rbuf->size means there + * was an overrun. + */ +static inline int32_t +spa_ringbuffer_get_read_index (SpaRingbuffer *rbuf, + uint32_t *index) { - uint32_t avail, r; - - r = rbuf->readindex; - *offset = r & rbuf->mask; - avail = (rbuf->writeindex - r) & rbuf->mask2; + int32_t avail; + *index = rbuf->readindex; + avail = (int32_t) (rbuf->writeindex - *index); spa_barrier_read(); return avail; } /** - * spa_ringbuffer_get_read_areas: + * spa_ringbuffer_read_data: * @rbuf: a #SpaRingbuffer - * @areas: an array of #SpaRingbufferArea + * @buffer: memory to read from + * @offset: offset in @buffer to read from + * @data: destination memory + * @len: number of bytes to read * - * Fill @areas with pointers to read from. The total amount of - * bytes that can be read can be obtained by summing the areas len fields. + * Read @len bytes from @rbuf starting @offset. @offset must be masked + * with the size of @rbuf and len should be smaller than the size. */ -static inline uint32_t -spa_ringbuffer_get_read_areas (SpaRingbuffer *rbuf, - SpaRingbufferArea areas[2]) -{ - uint32_t avail, end, r; - - avail = spa_ringbuffer_get_read_offset (rbuf, &r); - end = r + avail; - - areas[0].offset = r; - areas[1].offset = 0; - - if (SPA_UNLIKELY (end > rbuf->size)) { - areas[0].len = rbuf->size - r; - areas[1].len = end - rbuf->size; - } else { - areas[0].len = avail; - areas[1].len = 0; - } - return avail; -} - static inline void spa_ringbuffer_read_data (SpaRingbuffer *rbuf, void *buffer, - SpaRingbufferArea areas[2], + uint32_t offset, void *data, - uint32_t size) + uint32_t len) { - if (SPA_LIKELY (size < areas[0].len)) - memcpy (data, buffer + areas[0].offset, size); - else { - memcpy (data, buffer + areas[0].offset, areas[0].len); - memcpy (data + areas[0].len, buffer, size - areas[0].len); + if (SPA_LIKELY (offset + len < rbuf->size)) { + memcpy (data, buffer + offset, len); + } else { + uint32_t l0 = rbuf->size - offset; + memcpy (data, buffer + offset, l0); + memcpy (data + l0, buffer, len - l0); } } @@ -166,65 +149,46 @@ spa_ringbuffer_read_advance (SpaRingbuffer *rbuf, int32_t len) { spa_barrier_full(); - rbuf->readindex = (rbuf->readindex + len) & rbuf->mask2; -} - -static inline uint32_t -spa_ringbuffer_get_write_offset (SpaRingbuffer *rbuf, - uint32_t *offset) -{ - uint32_t avail, w; - - w = rbuf->writeindex; - *offset = w & rbuf->mask; - avail = rbuf->size - ((w - rbuf->readindex) & rbuf->mask2); - spa_barrier_full(); - - return avail; + rbuf->readindex += len; } /** - * spa_ringbuffer_get_write_areas: + * spa_ringbuffer_get_write_index: * @rbuf: a #SpaRingbuffer - * @areas: an array of #SpaRingbufferArea + * @index: the value of writeindex, should be masked to get the + * offset in the ringbuffer memory * - * Fill @areas with pointers to write to. The total amount of - * bytes that can be written can be obtained by summing the areas len fields. + * Returns: the fill level of @rbuf. values < 0 mean + * there was an underrun. values > rbuf->size means there + * was an overrun. Subsctract from the buffer size to get + * the number of bytes available for writing. */ -static inline uint32_t -spa_ringbuffer_get_write_areas (SpaRingbuffer *rbuf, - SpaRingbufferArea areas[2]) +static inline int32_t +spa_ringbuffer_get_write_index (SpaRingbuffer *rbuf, + uint32_t *index) { - uint32_t avail, end, w; + int32_t filled; - avail = spa_ringbuffer_get_write_offset (rbuf, &w); - end = w + avail; + *index = rbuf->writeindex; + filled = (int32_t) (*index - rbuf->readindex); + spa_barrier_full(); - areas[0].offset = w; - areas[1].offset = 0; - - if (SPA_UNLIKELY (end > rbuf->size)) { - areas[0].len = rbuf->size - w; - areas[1].len = end - rbuf->size; - } else { - areas[0].len = avail; - areas[1].len = 0; - } - return avail; + return filled; } static inline void spa_ringbuffer_write_data (SpaRingbuffer *rbuf, void *buffer, - SpaRingbufferArea areas[2], + uint32_t offset, void *data, - uint32_t size) + uint32_t len) { - if (SPA_LIKELY (size < areas[0].len)) - memcpy (buffer + areas[0].offset, data, size); - else { - memcpy (buffer + areas[0].offset, data, areas[0].len); - memcpy (buffer, data + areas[0].len, size - areas[0].len); + if (SPA_LIKELY (offset + len < rbuf->size)) { + memcpy (buffer + offset, data, len); + } else { + uint32_t l0 = rbuf->size - offset; + memcpy (buffer + offset, data, l0); + memcpy (buffer, data + l0, len - l0); } } @@ -241,7 +205,7 @@ spa_ringbuffer_write_advance (SpaRingbuffer *rbuf, int32_t len) { spa_barrier_write(); - rbuf->writeindex = (rbuf->writeindex + len) & rbuf->mask2; + rbuf->writeindex += len; } diff --git a/spa/lib/debug.c b/spa/lib/debug.c index a50b2ea71..34abd1805 100644 --- a/spa/lib/debug.c +++ b/spa/lib/debug.c @@ -120,7 +120,6 @@ spa_debug_buffer (const SpaBuffer *buffer) fprintf (stderr, " writeindex: %d\n", h->ringbuffer.writeindex); fprintf (stderr, " size: %d\n", h->ringbuffer.size); fprintf (stderr, " mask: %d\n", h->ringbuffer.mask); - fprintf (stderr, " mask2: %d\n", h->ringbuffer.mask2); break; } case SPA_META_TYPE_SHARED: diff --git a/spa/plugins/alsa/alsa-utils.c b/spa/plugins/alsa/alsa-utils.c index 2f62617d9..f2860a29d 100644 --- a/spa/plugins/alsa/alsa-utils.c +++ b/spa/plugins/alsa/alsa-utils.c @@ -391,8 +391,9 @@ pull_frames_ringbuffer (SpaALSAState *state, snd_pcm_uframes_t offset, snd_pcm_uframes_t frames) { - SpaRingbufferArea areas[2]; - size_t size, avail; + int32_t avail; + uint32_t index; + size_t size; SpaALSABuffer *b; uint8_t *src, *dst; @@ -401,17 +402,16 @@ pull_frames_ringbuffer (SpaALSAState *state, src = b->outbuf->datas[0].data; dst = SPA_MEMBER (my_areas[0].addr, offset * state->frame_size, uint8_t); - avail = spa_ringbuffer_get_read_areas (&b->rb->ringbuffer, areas); + avail = spa_ringbuffer_get_read_index (&b->rb->ringbuffer, &index); + size = SPA_MIN (avail, frames * state->frame_size); - spa_log_trace (state->log, "%u %u %u %u %zd %zd", - areas[0].offset, areas[0].len, - areas[1].offset, areas[1].len, offset, size); + spa_log_trace (state->log, "%u %d %zd %zd", index, avail, offset, size); if (size > 0) { spa_ringbuffer_read_data (&b->rb->ringbuffer, src, - areas, + index & b->rb->ringbuffer.mask, dst, size); spa_ringbuffer_read_advance (&b->rb->ringbuffer, size); diff --git a/spa/plugins/audiomixer/audiomixer.c b/spa/plugins/audiomixer/audiomixer.c index cbba233ab..f88fcf492 100644 --- a/spa/plugins/audiomixer/audiomixer.c +++ b/spa/plugins/audiomixer/audiomixer.c @@ -576,17 +576,6 @@ spa_audiomixer_node_port_send_command (SpaNode *node, return SPA_RESULT_NOT_IMPLEMENTED; } -static void -clear_buffer (SpaAudioMixer *this, MixerBuffer *out) -{ - int16_t *op; - size_t os; - - op = SPA_MEMBER (out->outbuf->datas[0].data, out->outbuf->datas[0].chunk->offset, void); - os = out->outbuf->datas[0].chunk->size; - memset (op, 0, os); -} - static void add_port_data (SpaAudioMixer *this, MixerBuffer *out, SpaAudioMixerPort *port, int layer) {