On 14.04.2014 15:49, Tanu Kaskinen wrote:
On Mon, 2014-04-14 at 14:35 +0200, Lukasz M wrote:
On 14 April 2014 09:42, Tanu Kaskinen <[email protected]> wrote:
On Mon, 2014-04-14 at 01:16 +0200, Lukasz Marek wrote:
> - pa_stream_write has parameter pa_free_cb_t to provide zero-copy.
Unfortunately it doesn't accept user data, in case of FFmpeg, the buffer
is inside AVBuffer, AVPacket, or AVFrame (AVBuffer technically).
Each of them contains a variable with a buffer that should be passed to
pa_stream_write, but free function should unref this structure, not
buffer itself.
It cannot be extracted without copying and this zero-copy is hard to
reach.
My question is: do you think it is doable/acceptable to add other
function that would accept userdata for free callback?
I know there is pa_stream_begin_write, but this is not suitable neither.
Yes, I think that would be acceptable. I wonder what would be a good
name for the function. pa_stream_write_with_free_userdata?
Maybe:
typedef void (*pa_free_ext_cb_t)(void *p, void *userdata);
Mmh, defining a new callback type is not really necessary. If you pass
the extra userdata pointer to pa_stream_write_with_free_ext() (or
whatever it will be called), the function can just pass the userdata
pointer to a regular pa_free_cb_t callback.
Yes. I was just proposing different name of the function, I didn't think
much about it.
pa_stream_write_with_free_ext(...)
I'll try to prepare a patch soon.
I have attached a patch. I tested my use case and some random file
played with paplay. It seems to be working good, but do deep review,
I've might miss something.
I don't know if I should update version or something.
--
Best Regards,
Lukasz Marek
You can avoid reality, but you cannot avoid the consequences of avoiding
reality. - Ayn Rand
>From ab9e95ab62602ed107b5d46642149de7ead15c10 Mon Sep 17 00:00:00 2001
From: Lukasz Marek <[email protected]>
Date: Tue, 15 Apr 2014 02:08:23 +0200
Subject: [PATCH] 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 | 20 ++++++++++++++++----
src/pulse/stream.h | 11 +++++++++++
src/pulsecore/memblock.c | 14 ++++++++++++--
src/pulsecore/memblock.h | 2 +-
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..2c9a777 100644
--- a/src/pulse/stream.c
+++ b/src/pulse/stream.c
@@ -1464,13 +1464,14 @@ 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,
int64_t offset,
- pa_seek_mode_t seek) {
+ pa_seek_mode_t seek,
+ const void *free_cb_data) {
pa_assert(s);
pa_assert(PA_REFCNT_VALUE(s) >= 1);
@@ -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, 1, (void*) free_cb_data);
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((void*) 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, offset, seek, data);
+}
+
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..6fbd663 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 */,
+ 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 */,
+ const void *free_cb_data /**< Argument passed to free_cb function */);
+
/** 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..3d75d99 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 */
+ pa_atomic_ptr_t 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,
+ bool read_only,
+ void *free_cb_data) {
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;
+ pa_atomic_ptr_store(&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(pa_atomic_ptr_load(&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));
+ pa_atomic_ptr_store(&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..0bfdc92 100644
--- a/src/pulsecore/memblock.h
+++ b/src/pulsecore/memblock.h
@@ -85,7 +85,7 @@ 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, bool read_only, void *free_cb_data);
/* 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)
--
1.8.3.2
_______________________________________________
pulseaudio-discuss mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
