On 10 June 2014 12:19, David Henningsson <[email protected]> wrote:
> Thanks for the patch. Overall I think this is good new functionality. > > Here's a review, which is mostly about details: > I've attached updated patch. All your remarks are fixed. I also fixed pa_memblock_new_malloced define. It was not present before or I missed it.
From 74aed2a944efa50ec80fdf5001a40f4e5955d9f4 Mon Sep 17 00:00:00 2001 From: Lukasz Marek <[email protected]> Date: Tue, 15 Apr 2014 02:08:23 +0200 Subject: [PATCH] Client API: Add pa_stream_write_ext_free() function. New function allows to pass data pointer that is a member of the outer structure that need to be freed too when data is not needed anymore. Signed-off-by: Lukasz Marek <[email protected]> --- src/map-file | 1 + src/pulse/stream.c | 18 +++++++++++++++--- src/pulse/stream.h | 11 +++++++++++ src/pulsecore/memblock.c | 14 ++++++++++++-- src/pulsecore/memblock.h | 4 ++-- 5 files changed, 41 insertions(+), 7 deletions(-) diff --git a/src/map-file b/src/map-file index dc36fdc..5159829 100644 --- a/src/map-file +++ b/src/map-file @@ -330,6 +330,7 @@ pa_stream_update_sample_rate; pa_stream_update_timing_info; pa_stream_writable_size; pa_stream_write; +pa_stream_write_ext_free; pa_strerror; pa_sw_cvolume_divide; pa_sw_cvolume_divide_scalar; diff --git a/src/pulse/stream.c b/src/pulse/stream.c index 8e35c29..3c04c42 100644 --- a/src/pulse/stream.c +++ b/src/pulse/stream.c @@ -1464,11 +1464,12 @@ int pa_stream_cancel_write( return 0; } -int pa_stream_write( +int pa_stream_write_ext_free( pa_stream *s, const void *data, size_t length, pa_free_cb_t free_cb, + void *free_cb_data, int64_t offset, pa_seek_mode_t seek) { @@ -1519,7 +1520,7 @@ int pa_stream_write( chunk.index = 0; if (free_cb && !pa_pstream_get_shm(s->context->pstream)) { - chunk.memblock = pa_memblock_new_user(s->context->mempool, (void*) t_data, t_length, free_cb, 1); + chunk.memblock = pa_memblock_new_user(s->context->mempool, (void*) t_data, t_length, free_cb, free_cb_data, 1); chunk.length = t_length; } else { void *d; @@ -1544,7 +1545,7 @@ int pa_stream_write( } if (free_cb && pa_pstream_get_shm(s->context->pstream)) - free_cb((void*) data); + free_cb(free_cb_data); } /* This is obviously wrong since we ignore the seeking index . But @@ -1591,6 +1592,17 @@ int pa_stream_write( return 0; } +int pa_stream_write( + pa_stream *s, + const void *data, + size_t length, + pa_free_cb_t free_cb, + int64_t offset, + pa_seek_mode_t seek) { + + return pa_stream_write_ext_free(s, data, length, free_cb, (void*) data, offset, seek); +} + int pa_stream_peek(pa_stream *s, const void **data, size_t *length) { pa_assert(s); pa_assert(PA_REFCNT_VALUE(s) >= 1); diff --git a/src/pulse/stream.h b/src/pulse/stream.h index 7ceb569..bc17143 100644 --- a/src/pulse/stream.h +++ b/src/pulse/stream.h @@ -554,6 +554,17 @@ int pa_stream_write( int64_t offset, /**< Offset for seeking, must be 0 for upload streams */ pa_seek_mode_t seek /**< Seek mode, must be PA_SEEK_RELATIVE for upload streams */); +/** Function does exactly the same as pa_stream_write() with the difference + * that free_cb_data is passed to free_cb instead of data. \since 6.0 */ +int pa_stream_write_ext_free( + pa_stream *p /**< The stream to use */, + const void *data /**< The data to write */, + size_t nbytes /**< The length of the data to write in bytes */, + pa_free_cb_t free_cb /**< A cleanup routine for the data or NULL to request an internal copy */, + void *free_cb_data /**< Argument passed to free_cb function */, + int64_t offset /**< Offset for seeking, must be 0 for upload streams */, + pa_seek_mode_t seek /**< Seek mode, must be PA_SEEK_RELATIVE for upload streams */); + /** Read the next fragment from the buffer (for recording streams). * If there is data at the current read index, \a data will point to * the actual data and \a nbytes will contain the size of the data in diff --git a/src/pulsecore/memblock.c b/src/pulsecore/memblock.c index 9cc02c1..2d4752b 100644 --- a/src/pulsecore/memblock.c +++ b/src/pulsecore/memblock.c @@ -83,6 +83,8 @@ struct pa_memblock { struct { /* If type == PA_MEMBLOCK_USER this points to a function for freeing this memory block */ pa_free_cb_t free_cb; + /* If type == PA_MEMBLOCK_USER this is passed as free_cb argument */ + void *free_cb_data; } user; struct { @@ -387,7 +389,13 @@ pa_memblock *pa_memblock_new_fixed(pa_mempool *p, void *d, size_t length, bool r } /* No lock necessary */ -pa_memblock *pa_memblock_new_user(pa_mempool *p, void *d, size_t length, pa_free_cb_t free_cb, bool read_only) { +pa_memblock *pa_memblock_new_user( + pa_mempool *p, + void *d, + size_t length, + pa_free_cb_t free_cb, + void *free_cb_data, + bool read_only) { pa_memblock *b; pa_assert(p); @@ -410,6 +418,7 @@ pa_memblock *pa_memblock_new_user(pa_mempool *p, void *d, size_t length, pa_free pa_atomic_store(&b->please_signal, 0); b->per_type.user.free_cb = free_cb; + b->per_type.user.free_cb_data = free_cb_data; stat_add(b); return b; @@ -513,7 +522,7 @@ static void memblock_free(pa_memblock *b) { switch (b->type) { case PA_MEMBLOCK_USER : pa_assert(b->per_type.user.free_cb); - b->per_type.user.free_cb(pa_atomic_ptr_load(&b->data)); + b->per_type.user.free_cb(b->per_type.user.free_cb_data); /* Fall through */ @@ -647,6 +656,7 @@ static void memblock_make_local(pa_memblock *b) { /* Humm, not enough space in the pool, so lets allocate the memory with malloc() */ b->per_type.user.free_cb = pa_xfree; pa_atomic_ptr_store(&b->data, pa_xmemdup(pa_atomic_ptr_load(&b->data), b->length)); + b->per_type.user.free_cb_data = pa_atomic_ptr_load(&b->data); b->type = PA_MEMBLOCK_USER; b->read_only = false; diff --git a/src/pulsecore/memblock.h b/src/pulsecore/memblock.h index 502f207..54778ac 100644 --- a/src/pulsecore/memblock.h +++ b/src/pulsecore/memblock.h @@ -85,10 +85,10 @@ pa_memblock *pa_memblock_new(pa_mempool *, size_t length); pa_memblock *pa_memblock_new_pool(pa_mempool *, size_t length); /* Allocate a new memory block of type PA_MEMBLOCK_USER */ -pa_memblock *pa_memblock_new_user(pa_mempool *, void *data, size_t length, pa_free_cb_t free_cb, bool read_only); +pa_memblock *pa_memblock_new_user(pa_mempool *, void *data, size_t length, pa_free_cb_t free_cb, void *free_cb_data, bool read_only); /* A special case of pa_memblock_new_user: take a memory buffer previously allocated with pa_xmalloc() */ -#define pa_memblock_new_malloced(p,data,length) pa_memblock_new_user(p, data, length, pa_xfree, 0) +#define pa_memblock_new_malloced(p,data,length) pa_memblock_new_user(p, data, length, pa_xfree, data, 0) /* Allocate a new memory block of type PA_MEMBLOCK_FIXED */ pa_memblock *pa_memblock_new_fixed(pa_mempool *, void *data, size_t length, bool read_only); -- 1.7.9.5
_______________________________________________ pulseaudio-discuss mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
