From 27acab7532e8870d6921c31c359ef8628e8f0008 Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Tue, 20 Sep 2016 19:52:05 +0200 Subject: [PATCH] Improve memory handling Reserve 0,0 for stack allocated mem that we never free Improve allocation in the link Parse format and properties in-line, there is no need to allocate memory. Improve buffer cleanup in alsa Add timestamps and offset to alsa --- pinos/gst/gstpinossrc.c | 4 ++ pinos/server/link.c | 48 ++++++++++++----- spa/lib/control.c | 39 +++++--------- spa/lib/memory.c | 20 +++---- spa/plugins/alsa/alsa-source.c | 97 +++++++++++++++++++++++++++------- spa/plugins/alsa/alsa-utils.c | 13 ++++- spa/plugins/alsa/alsa-utils.h | 3 +- 7 files changed, 155 insertions(+), 69 deletions(-) diff --git a/pinos/gst/gstpinossrc.c b/pinos/gst/gstpinossrc.c index 25d677d0b..6f8bd1daf 100644 --- a/pinos/gst/gstpinossrc.c +++ b/pinos/gst/gstpinossrc.c @@ -403,6 +403,10 @@ on_add_buffer (GObject *gobject, GstMemory *gmem; mem = spa_memory_find (&d->mem.mem); + if (mem == NULL) { + g_warning ("failed to get buffer memory"); + continue; + } if (mem->fd) { gmem = gst_fd_allocator_alloc (pinossrc->fd_allocator, dup (mem->fd), diff --git a/pinos/server/link.c b/pinos/server/link.c index 572d2d596..9309d895d 100644 --- a/pinos/server/link.c +++ b/pinos/server/link.c @@ -34,6 +34,8 @@ #define PINOS_LINK_GET_PRIVATE(obj) \ (G_TYPE_INSTANCE_GET_PRIVATE ((obj), PINOS_TYPE_LINK, PinosLinkPrivate)) +#define MAX_BUFFERS 64 + struct _PinosLinkPrivate { PinosDaemon *daemon; @@ -46,9 +48,9 @@ struct _PinosLinkPrivate PinosLinkState state; GError *error; - SpaBuffer *in_buffers[16]; + SpaBuffer *in_buffers[MAX_BUFFERS]; unsigned int n_in_buffers; - SpaBuffer *out_buffers[16]; + SpaBuffer *out_buffers[MAX_BUFFERS]; unsigned int n_out_buffers; }; @@ -359,6 +361,18 @@ error: } } +static void * +find_param (const SpaPortInfo *info, SpaAllocParamType type) +{ + unsigned int i; + + for (i = 0; i < info->n_params; i++) { + if (info->params[i]->type == type) + return info->params[i]; + } + return NULL; +} + static SpaResult do_allocation (PinosLink *this, SpaNodeState in_state, SpaNodeState out_state) { @@ -367,6 +381,8 @@ do_allocation (PinosLink *this, SpaNodeState in_state, SpaNodeState out_state) const SpaPortInfo *iinfo, *oinfo; SpaPortInfoFlags in_flags, out_flags; GError *error = NULL; + unsigned int max_buffers = MAX_BUFFERS; + SpaAllocParamBuffers *in_alloc, *out_alloc; if (in_state != SPA_NODE_STATE_READY && out_state != SPA_NODE_STATE_READY) return SPA_RESULT_OK; @@ -392,6 +408,14 @@ do_allocation (PinosLink *this, SpaNodeState in_state, SpaNodeState out_state) in_flags = iinfo->flags; out_flags = oinfo->flags; + max_buffers = MAX_BUFFERS; + in_alloc = find_param (iinfo, SPA_ALLOC_PARAM_TYPE_BUFFERS); + if (in_alloc) + max_buffers = in_alloc->max_buffers == 0 ? max_buffers : SPA_MIN (in_alloc->max_buffers, max_buffers); + out_alloc = find_param (oinfo, SPA_ALLOC_PARAM_TYPE_BUFFERS); + if (out_alloc) + max_buffers = out_alloc->max_buffers == 0 ? max_buffers : SPA_MIN (out_alloc->max_buffers, max_buffers); + if (out_flags & SPA_PORT_INFO_FLAG_LIVE) { this->output_node->live = true; this->input_node->live = true; @@ -408,7 +432,7 @@ do_allocation (PinosLink *this, SpaNodeState in_state, SpaNodeState out_state) in_flags = SPA_PORT_INFO_FLAG_CAN_ALLOC_BUFFERS; } else if ((out_flags & SPA_PORT_INFO_FLAG_CAN_USE_BUFFERS) && (in_flags & SPA_PORT_INFO_FLAG_CAN_USE_BUFFERS)) { - priv->n_in_buffers = 16; + priv->n_in_buffers = max_buffers; out_flags = SPA_PORT_INFO_FLAG_CAN_USE_BUFFERS; in_flags = SPA_PORT_INFO_FLAG_CAN_USE_BUFFERS; @@ -448,8 +472,8 @@ do_allocation (PinosLink *this, SpaNodeState in_state, SpaNodeState out_state) spa_debug_port_info (oinfo); spa_debug_port_info (iinfo); - if (in_flags & SPA_PORT_INFO_FLAG_CAN_ALLOC_BUFFERS) { - priv->n_in_buffers = 16; + if (in_flags & SPA_PORT_INFO_FLAG_CAN_ALLOC_BUFFERS && priv->n_in_buffers == 0) { + priv->n_in_buffers = max_buffers; if ((res = spa_node_port_alloc_buffers (this->input_node->node, this->input_port, oinfo->params, oinfo->n_params, priv->in_buffers, &priv->n_in_buffers)) < 0) { @@ -459,10 +483,10 @@ do_allocation (PinosLink *this, SpaNodeState in_state, SpaNodeState out_state) "error alloc input buffers: %d", res); goto error; } - g_debug ("allocated in_buffers %p", priv->in_buffers); + g_debug ("allocated in_buffers %p from input port", priv->in_buffers); } - else if (out_flags & SPA_PORT_INFO_FLAG_CAN_ALLOC_BUFFERS) { - priv->n_out_buffers = 16; + else if (out_flags & SPA_PORT_INFO_FLAG_CAN_ALLOC_BUFFERS && priv->n_out_buffers == 0) { + priv->n_out_buffers = max_buffers; if ((res = spa_node_port_alloc_buffers (this->output_node->node, this->output_port, iinfo->params, iinfo->n_params, priv->out_buffers, &priv->n_out_buffers)) < 0) { @@ -472,10 +496,10 @@ do_allocation (PinosLink *this, SpaNodeState in_state, SpaNodeState out_state) "error alloc output buffers: %d", res); goto error; } - g_debug ("allocated out_buffers %p", priv->out_buffers); + g_debug ("allocated out_buffers %p from output port", priv->out_buffers); } - if (in_flags & SPA_PORT_INFO_FLAG_CAN_USE_BUFFERS) { - g_debug ("using out_buffers %p", priv->out_buffers); + else if (in_flags & SPA_PORT_INFO_FLAG_CAN_USE_BUFFERS) { + g_debug ("using out_buffers %p on input port", priv->out_buffers); if ((res = spa_node_port_use_buffers (this->input_node->node, this->input_port, priv->out_buffers, priv->n_out_buffers)) < 0) { g_set_error (&error, @@ -486,7 +510,7 @@ do_allocation (PinosLink *this, SpaNodeState in_state, SpaNodeState out_state) } } else if (out_flags & SPA_PORT_INFO_FLAG_CAN_USE_BUFFERS) { - g_debug ("using in_buffers %p", priv->out_buffers); + g_debug ("using in_buffers %p on output port", priv->in_buffers); if ((res = spa_node_port_use_buffers (this->output_node->node, this->output_port, priv->in_buffers, priv->n_in_buffers)) < 0) { g_set_error (&error, diff --git a/spa/lib/control.c b/spa/lib/control.c index 519da9a73..81aa463fc 100644 --- a/spa/lib/control.c +++ b/spa/lib/control.c @@ -341,7 +341,7 @@ spa_control_iter_get_data (SpaControlIter *iter, size_t *size) } static SpaProps * -parse_props (SpaMemory *mem, void *p, off_t offset) +parse_props (void *p, off_t offset) { SpaProps *tp; unsigned int i, j; @@ -375,7 +375,7 @@ parse_props (SpaMemory *mem, void *p, off_t offset) } static SpaFormat * -parse_format (SpaMemory *mem, void *p, off_t offset) +parse_format (void *p, size_t size, off_t offset) { SpaFormat *f; @@ -383,11 +383,12 @@ parse_format (SpaMemory *mem, void *p, off_t offset) return NULL; f = SPA_MEMBER (p, offset, SpaFormat); - f->mem.mem = mem->mem; + f->mem.mem.pool_id = 0; + f->mem.mem.id = 0; f->mem.offset = offset; - f->mem.size = mem->size - offset; + f->mem.size = size - offset; - parse_props (mem, f, offsetof (SpaFormat, props)); + parse_props (f, offsetof (SpaFormat, props)); return f; } @@ -395,40 +396,31 @@ parse_format (SpaMemory *mem, void *p, off_t offset) static void iter_parse_node_update (struct stack_iter *si, SpaControlCmdNodeUpdate *nu) { - void *p; - SpaMemory *mem; - memcpy (nu, si->data, sizeof (SpaControlCmdNodeUpdate)); - - mem = spa_memory_alloc_size (SPA_MEMORY_POOL_LOCAL, si->data, si->size); - p = spa_memory_ensure_ptr (mem); - if (nu->props) - nu->props = parse_props (mem, p, SPA_PTR_TO_INT (nu->props)); + nu->props = parse_props (si->data, SPA_PTR_TO_INT (nu->props)); } static void iter_parse_port_update (struct stack_iter *si, SpaControlCmdPortUpdate *pu) { void *p; - SpaMemory *mem; unsigned int i; memcpy (pu, si->data, sizeof (SpaControlCmdPortUpdate)); - mem = spa_memory_alloc_size (SPA_MEMORY_POOL_LOCAL, si->data, si->size); - p = spa_memory_ensure_ptr (mem); + p = si->data; if (pu->possible_formats) pu->possible_formats = SPA_MEMBER (p, SPA_PTR_TO_INT (pu->possible_formats), SpaFormat *); for (i = 0; i < pu->n_possible_formats; i++) { - pu->possible_formats[i] = parse_format (mem, p, + pu->possible_formats[i] = parse_format (p, si->size, SPA_PTR_TO_INT (pu->possible_formats[i])); } if (pu->props) - pu->props = parse_props (mem, p, SPA_PTR_TO_INT (pu->props)); + pu->props = parse_props (p, SPA_PTR_TO_INT (pu->props)); if (pu->info) { SpaPortInfo *pi; @@ -445,15 +437,8 @@ iter_parse_port_update (struct stack_iter *si, SpaControlCmdPortUpdate *pu) static void iter_parse_set_format (struct stack_iter *si, SpaControlCmdSetFormat *cmd) { - void *p; - SpaMemory *mem; - memcpy (cmd, si->data, sizeof (SpaControlCmdSetFormat)); - - mem = spa_memory_alloc_size (SPA_MEMORY_POOL_LOCAL, si->data, si->size); - p = spa_memory_ensure_ptr (mem); - - cmd->format = parse_format (mem, p, SPA_PTR_TO_INT (cmd->format)); + cmd->format = parse_format (si->data, si->size, SPA_PTR_TO_INT (cmd->format)); } static void @@ -473,7 +458,7 @@ iter_parse_use_buffers (struct stack_iter *si, SpaControlCmdUseBuffers *cmd) mc = SPA_MEMBER (p, SPA_PTR_TO_INT (cmd->buffers[i]), SpaMemoryChunk); mem = spa_memory_find (&mc->mem); - cmd->buffers[i] = SPA_MEMBER (spa_memory_ensure_ptr (mem), mc->offset, SpaBuffer); + cmd->buffers[i] = mem ? SPA_MEMBER (spa_memory_ensure_ptr (mem), mc->offset, SpaBuffer) : NULL; } } diff --git a/spa/lib/memory.c b/spa/lib/memory.c index e91856d6c..415c70182 100644 --- a/spa/lib/memory.c +++ b/spa/lib/memory.c @@ -33,7 +33,7 @@ #undef USE_MEMFD -#if 0 +#if 1 #define SPA_DEBUG_MEMORY(format,args...) fprintf(stderr,format,##args) #else #define SPA_DEBUG_MEMORY(format,args...) @@ -60,7 +60,7 @@ spa_memory_pool_init (SpaMemoryPool *pool, uint32_t id) memset (pool, 0, sizeof (SpaMemoryPool)); for (i = 0; i < MAX_MEMORIES; i++) pool->free_mem[i] = MAX_MEMORIES - 1 - i; - pool->n_free = MAX_MEMORIES; + pool->n_free = MAX_MEMORIES - 1; pool->id = id; pool->valid = true; } @@ -129,7 +129,7 @@ spa_memory_alloc (uint32_t pool_id) mem->mem.pool_id = pool_id; mem->mem.id = id; - SPA_DEBUG_MEMORY ("mem %p: alloc\n", mem); + SPA_DEBUG_MEMORY ("mem %p: alloc %u:%u\n", mem, pool_id, id); return mem; } @@ -254,7 +254,8 @@ spa_memory_ref (SpaMemoryRef *ref) if (!(mem = spa_memory_find (ref))) return SPA_RESULT_ERROR; - mem->refcount++; + if (mem->mem.id > 0) + mem->refcount++; return SPA_RESULT_OK; } @@ -264,7 +265,7 @@ spa_memory_free (SpaMemory *mem) { SpaMemoryPool *pool; - SPA_DEBUG_MEMORY ("mem %p: free\n", mem); + SPA_DEBUG_MEMORY ("mem %p: free %u:%u\n", mem, mem->mem.pool_id, mem->mem.id); if (mem->fd != -1) { if (mem->ptr) @@ -287,7 +288,7 @@ spa_memory_unref (SpaMemoryRef *ref) if (!(mem = spa_memory_find (ref))) return SPA_RESULT_ERROR; - if (--mem->refcount == 0) { + if (mem->mem.id > 0 && --mem->refcount == 0) { if (mem->notify) mem->notify (mem); @@ -303,13 +304,14 @@ spa_memory_find (SpaMemoryRef *ref) { SpaMemoryPool *pool; - if (ref == NULL || ref->pool_id >= MAX_POOLS || !pools[ref->pool_id].valid) + if (ref == NULL || ref->pool_id >= MAX_POOLS || !pools[ref->pool_id].valid || ref->id <= 0) return NULL; pool = &pools[ref->pool_id]; - if (ref->id >= MAX_MEMORIES || pool->memories[ref->id].refcount <= 0) - return NULL; + if (ref->id >= MAX_MEMORIES || pool->memories[ref->id].refcount <= 0) { + SPA_DEBUG_MEMORY ("can't find mem %u:%u\n", ref->pool_id, ref->id); + } return &pool->memories[ref->id]; } diff --git a/spa/plugins/alsa/alsa-source.c b/spa/plugins/alsa/alsa-source.c index e2baccff4..89383b573 100644 --- a/spa/plugins/alsa/alsa-source.c +++ b/spa/plugins/alsa/alsa-source.c @@ -48,8 +48,8 @@ update_state (SpaALSASource *this, SpaNodeState state) } static const char default_device[] = "hw:0"; -static const uint32_t default_buffer_time = 1000; -static const uint32_t default_period_time = 100; +static const uint32_t default_buffer_time = 100000; +static const uint32_t default_period_time = 10000; static const bool default_period_event = 0; static void @@ -310,6 +310,49 @@ spa_alsa_source_node_port_enum_formats (SpaNode *node, return SPA_RESULT_OK; } +static void +recycle_buffer (SpaALSASource *this, uint32_t buffer_id) +{ + SpaALSABuffer *b; + + b = &this->alloc_buffers[buffer_id]; + if (!b->outstanding) + return; + + b->outstanding = false; + SPA_QUEUE_PUSH_TAIL (&this->free, SpaALSABuffer, next, b); +} + +static SpaResult +spa_alsa_clear_buffers (SpaALSASource *this) +{ + int i; + + if (!this->have_buffers) + return SPA_RESULT_OK; + + for (i = 0; i < this->n_buffers; i++) { + SpaALSABuffer *b; + + b = &this->alloc_buffers[i]; + if (b->outstanding) { + fprintf (stderr, "queueing outstanding buffer %p\n", b); + recycle_buffer (this, i); + } + } + if (this->alloc_bufmem) + spa_memory_unref (&this->alloc_bufmem->mem); + if (this->alloc_mem) + spa_memory_unref (&this->alloc_mem->mem); + + this->alloc_mem = NULL; + this->alloc_bufmem = NULL; + this->n_buffers = 0; + this->have_buffers = false; + + return SPA_RESULT_OK; +} + static SpaResult spa_alsa_source_node_port_set_format (SpaNode *node, uint32_t port_id, @@ -329,6 +372,7 @@ spa_alsa_source_node_port_set_format (SpaNode *node, if (format == NULL) { this->have_format = false; + spa_alsa_clear_buffers (this); update_state (this, SPA_NODE_STATE_CONFIGURE); return SPA_RESULT_OK; } @@ -351,7 +395,7 @@ spa_alsa_source_node_port_set_format (SpaNode *node, this->param_buffers.minsize = this->period_frames * this->frame_size; this->param_buffers.stride = 0; this->param_buffers.min_buffers = 1; - this->param_buffers.max_buffers = 16; + this->param_buffers.max_buffers = 32; this->param_buffers.align = 16; this->info.features = NULL; @@ -423,25 +467,39 @@ spa_alsa_source_node_port_set_props (SpaNode *node, static SpaResult spa_alsa_source_node_port_use_buffers (SpaNode *node, - uint32_t port_id, - SpaBuffer **buffers, - uint32_t n_buffers) + uint32_t port_id, + SpaBuffer **buffers, + uint32_t n_buffers) { - return SPA_RESULT_NOT_IMPLEMENTED; + SpaALSASource *this; + SpaResult res; + + if (node == NULL || node->handle == NULL) + return SPA_RESULT_INVALID_ARGUMENTS; + + if (port_id != 0) + return SPA_RESULT_INVALID_PORT; + + this = (SpaALSASource *) node->handle; + + if (!this->have_format) + return SPA_RESULT_NO_FORMAT; + + if (this->have_buffers) { + if ((res = spa_alsa_clear_buffers (this)) < 0) + return res; + } + if (buffers != NULL) + return SPA_RESULT_NOT_IMPLEMENTED; + + if (this->have_buffers) + update_state (this, SPA_NODE_STATE_PAUSED); + else + update_state (this, SPA_NODE_STATE_READY); + + return SPA_RESULT_OK; } -static void -recycle_buffer (SpaALSASource *this, uint32_t buffer_id) -{ - SpaALSABuffer *b; - - b = &this->alloc_buffers[buffer_id]; - if (!b->outstanding) - return; - - b->outstanding = false; - SPA_QUEUE_PUSH_TAIL (&this->free, SpaALSABuffer, next, b); -} static SpaResult spa_alsa_source_node_port_alloc_buffers (SpaNode *node, @@ -521,6 +579,7 @@ spa_alsa_source_node_port_alloc_buffers (SpaNode *node, b->datas[0].mem.size = buffer_size; b->datas[0].stride = 0; b->ptr = bufmem + buffer_size * i; + b->h = &b->header; buffers[i] = b->outbuf = &b->buffer; diff --git a/spa/plugins/alsa/alsa-utils.c b/spa/plugins/alsa/alsa-utils.c index 66a966e4f..ec88a3a3f 100644 --- a/spa/plugins/alsa/alsa-utils.c +++ b/spa/plugins/alsa/alsa-utils.c @@ -318,15 +318,25 @@ mmap_read (SpaALSAState *state) avail = snd_pcm_status_get_avail (status); snd_pcm_status_get_htstamp (status, &htstamp); - now = (int64_t)htstamp.tv_sec * 1000000000ll + (int64_t)htstamp.tv_nsec; + now = (int64_t)htstamp.tv_sec * SPA_NSEC_PER_SEC + (int64_t)htstamp.tv_nsec; + + state->last_ticks = state->sample_count * SPA_USEC_PER_SEC / state->rate; + state->last_monotonic = now; SPA_QUEUE_POP_HEAD (&state->free, SpaALSABuffer, next, b); if (b == NULL) { fprintf (stderr, "no more buffers\n"); } else { dest = b->ptr; + if (b->h) { + b->h->seq = state->sample_count; + b->h->pts = state->last_monotonic; + b->h->dts_offset = 0; + } } + state->sample_count += avail; + size = avail; while (size > 0) { frames = size; @@ -358,6 +368,7 @@ mmap_read (SpaALSAState *state) size -= frames; } + if (b) { SpaNodeEvent event; SpaNodeEventHaveOutput ho; diff --git a/spa/plugins/alsa/alsa-utils.h b/spa/plugins/alsa/alsa-utils.h index af2347ff1..cb549be97 100644 --- a/spa/plugins/alsa/alsa-utils.h +++ b/spa/plugins/alsa/alsa-utils.h @@ -52,6 +52,7 @@ struct _SpaALSABuffer { SpaMetaRingbuffer ringbuffer; SpaData datas[1]; SpaBuffer *outbuf; + SpaMetaHeader *h; void *ptr; bool outstanding; SpaALSABuffer *next; @@ -89,7 +90,6 @@ struct _SpaALSAState { SpaPortStatus status; bool have_buffers; - SpaMemory *alloc_bufmem; SpaMemory *alloc_mem; SpaALSABuffer *alloc_buffers; @@ -102,6 +102,7 @@ struct _SpaALSAState { SpaPollFd fds[16]; SpaPollItem poll; + int64_t sample_count; int64_t last_ticks; int64_t last_monotonic; };