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

@@ -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);
+    int32_t filled, avail;
-    if (areas[0].len < sizeof (InvokeItem)) {
+    uint32_t offset, l0;
-      pinos_log_warn ("data-loop %p: queue full", impl);
+
+    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)
+      if (l0 < sizeof (InvokeItem) + item->item_size)
-        item->item_size = areas[0].len;
+        item->item_size = l0;
     } else {
-      item->data = SPA_MEMBER (impl->buffer_data, areas[1].offset, void);
+      item->data = impl->buffer_data;
-      item->item_size = areas[0].len + 1 + size;
+      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) {
+  while (spa_ringbuffer_get_read_index (&impl->buffer, &offset) > 0) {
-    InvokeItem *item = SPA_MEMBER (impl->buffer_data, offset, InvokeItem);
+    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);
   }

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];
+  int32_t filled, avail;
-  size_t avail, size;
+  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);

spa/include/spa/ringbuffer.h

@@ -34,24 +34,18 @@ typedef struct _SpaRingbuffer SpaRingbuffer;
 #include <spa/defs.h>
 #include <spa/barrier.h>
 
-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: the size of the ringbuffer must be power of 2
- * @size_mask: mask if @size is power of 2
+ * @mask: mask as @size - 1
  */
 struct _SpaRingbuffer {
-  volatile uint32_t     readindex;
+  uint32_t     readindex;
-  volatile uint32_t     writeindex;
+  uint32_t     writeindex;
-  uint32_t              size;
+  uint32_t     size;
-  uint32_t              mask;
+  uint32_t     mask;
-  uint32_t              mask2;
 };
 
 /**
@@ -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,
+ * spa_ringbuffer_get_read_index:
-                                uint32_t      *offset)
+ * @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;
+  int32_t avail;
-
-  r = rbuf->readindex;
-  *offset = r & rbuf->mask;
-  avail = (rbuf->writeindex - r) & rbuf->mask2;
 
+  *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
+ * Read @len bytes from @rbuf starting @offset. @offset must be masked
- * bytes that can be read can be obtained by summing the areas len fields.
+ * 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))
+  if (SPA_LIKELY (offset + len < rbuf->size)) {
-    memcpy (data, buffer + areas[0].offset, size);
+    memcpy (data, buffer + offset, len);
-  else {
+  } else {
-    memcpy (data, buffer + areas[0].offset, areas[0].len);
+    uint32_t l0 = rbuf->size - offset;
-    memcpy (data + areas[0].len, buffer, size - areas[0].len);
+    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;
+  rbuf->readindex += len;
-}
-
-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;
 }
 
 /**
- * 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
+ * Returns: the fill level of @rbuf. values < 0 mean
- * bytes that can be written can be obtained by summing the areas len fields.
+ *          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
+static inline int32_t
-spa_ringbuffer_get_write_areas (SpaRingbuffer      *rbuf,
+spa_ringbuffer_get_write_index (SpaRingbuffer *rbuf,
-                                SpaRingbufferArea   areas[2])
+                                uint32_t      *index)
 {
-  uint32_t avail, end, w;
+  int32_t filled;
 
-  avail = spa_ringbuffer_get_write_offset (rbuf, &w);
+  *index = rbuf->writeindex;
-  end = w + avail;
+  filled = (int32_t) (*index - rbuf->readindex);
+  spa_barrier_full();
 
-  areas[0].offset = w;
+  return filled;
-  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;
 }
 
 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))
+  if (SPA_LIKELY (offset + len < rbuf->size)) {
-    memcpy (buffer + areas[0].offset, data, size);
+    memcpy (buffer + offset, data, len);
-  else {
+  } else {
-    memcpy (buffer + areas[0].offset, data, areas[0].len);
+    uint32_t l0 = rbuf->size - offset;
-    memcpy (buffer, data + areas[0].len, size - areas[0].len);
+    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;
 }
 
 

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:

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];
+  int32_t avail;
-  size_t size, 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",
+  spa_log_trace (state->log, "%u %d %zd %zd", index, avail, offset, size);
-      areas[0].offset, areas[0].len,
-      areas[1].offset, areas[1].len, 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);

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)
 {