On 2014-11-05 00:26, Peter Meerwald wrote:
From: Peter Meerwald <[email protected]>
item_info has per-type fields which should be within a union to
save space
I think this is mostly a bikeshed patch, because the memory saved can't
be significant, can it?
If the memory saved is insignificant I'd say that just longer code
(added "per_type.memblock_info" here and there) is a slight drawback and
hence this patch could be skipped. But it's not a strong opinion.
Signed-off-by: Peter Meerwald <[email protected]>
---
src/pulsecore/pstream.c | 117 +++++++++++++++++++++++++++---------------------
1 file changed, 65 insertions(+), 52 deletions(-)
diff --git a/src/pulsecore/pstream.c b/src/pulsecore/pstream.c
index 96ee247..f8217b3 100644
--- a/src/pulsecore/pstream.c
+++ b/src/pulsecore/pstream.c
@@ -92,21 +92,26 @@ struct item_info {
PA_PSTREAM_ITEM_SHMREVOKE
} type;
- /* packet info */
- pa_packet *packet;
+ union {
+ /* packet info */
+ pa_packet *packet;
+
+ /* release/revoke info */
+ uint32_t block_id;
+
+ /* memblock info */
+ struct {
+ pa_memchunk chunk;
+ uint32_t channel;
+ int64_t offset;
+ pa_seek_mode_t seek_mode;
+ } memblock_info;
+ } per_type;
+
#ifdef HAVE_CREDS
bool with_ancil_data;
pa_cmsg_ancil_data ancil_data;
#endif
-
- /* memblock info */
- pa_memchunk chunk;
- uint32_t channel;
- int64_t offset;
- pa_seek_mode_t seek_mode;
-
- /* release/revoke info */
- uint32_t block_id;
};
struct pstream_read {
@@ -285,11 +290,11 @@ static void item_free(void *item) {
pa_assert(i);
if (i->type == PA_PSTREAM_ITEM_MEMBLOCK) {
- pa_assert(i->chunk.memblock);
- pa_memblock_unref(i->chunk.memblock);
+ pa_assert(i->per_type.memblock_info.chunk.memblock);
+ pa_memblock_unref(i->per_type.memblock_info.chunk.memblock);
} else if (i->type == PA_PSTREAM_ITEM_PACKET) {
- pa_assert(i->packet);
- pa_packet_unref(i->packet);
+ pa_assert(i->per_type.packet);
+ pa_packet_unref(i->per_type.packet);
}
if (pa_flist_push(PA_STATIC_FLIST_GET(items), i) < 0)
@@ -324,25 +329,19 @@ static void pstream_free(pa_pstream *p) {
pa_xfree(p);
}
-void pa_pstream_send_packet(pa_pstream*p, pa_packet *packet, const
pa_cmsg_ancil_data *ancil_data) {
- struct item_info *i;
-
+static void pa_pstream_send_item(pa_pstream*p, struct item_info *item, const
pa_cmsg_ancil_data *ancil_data) {
It looks like pa_pstream_send_item is only used for
pa_pstream_send_packet and not for pa_pstream_send_memblock (etc), how come?
pa_assert(p);
pa_assert(PA_REFCNT_VALUE(p) > 0);
- pa_assert(packet);
+ pa_assert(item);
- if (p->dead)
+ if (p->dead) {
+ item_free(item);
return;
-
- if (!(i = pa_flist_pop(PA_STATIC_FLIST_GET(items))))
- i = pa_xnew(struct item_info, 1);
-
- i->type = PA_PSTREAM_ITEM_PACKET;
- i->packet = pa_packet_ref(packet);
+ }
#ifdef HAVE_CREDS
- if ((i->with_ancil_data = !!ancil_data)) {
- i->ancil_data = *ancil_data;
+ if ((item->with_ancil_data = !!ancil_data)) {
+ item->ancil_data = *ancil_data;
if (ancil_data->creds_valid)
pa_assert(ancil_data->nfd == 0);
else
@@ -350,11 +349,25 @@ void pa_pstream_send_packet(pa_pstream*p, pa_packet
*packet, const pa_cmsg_ancil
}
#endif
- pa_queue_push(p->send_queue, i);
+ pa_queue_push(p->send_queue, item);
p->mainloop->defer_enable(p->defer_event, 1);
}
+void pa_pstream_send_packet(pa_pstream*p, pa_packet *packet, const
pa_cmsg_ancil_data *ancil_data) {
+ struct item_info *i;
+
+ pa_assert(packet);
+
+ if (!(i = pa_flist_pop(PA_STATIC_FLIST_GET(items))))
+ i = pa_xnew(struct item_info, 1);
+
+ i->type = PA_PSTREAM_ITEM_PACKET;
+ i->per_type.packet = pa_packet_ref(packet);
+
+ pa_pstream_send_item(p, i, ancil_data);
+}
+
void pa_pstream_send_memblock(pa_pstream*p, uint32_t channel, int64_t offset,
pa_seek_mode_t seek_mode, const pa_memchunk *chunk) {
size_t length, idx;
size_t bsm;
@@ -381,13 +394,13 @@ void pa_pstream_send_memblock(pa_pstream*p, uint32_t
channel, int64_t offset, pa
i->type = PA_PSTREAM_ITEM_MEMBLOCK;
n = PA_MIN(length, bsm);
- i->chunk.index = chunk->index + idx;
- i->chunk.length = n;
- i->chunk.memblock = pa_memblock_ref(chunk->memblock);
+ i->per_type.memblock_info.chunk.index = chunk->index + idx;
+ i->per_type.memblock_info.chunk.length = n;
+ i->per_type.memblock_info.chunk.memblock =
pa_memblock_ref(chunk->memblock);
- i->channel = channel;
- i->offset = offset;
- i->seek_mode = seek_mode;
+ i->per_type.memblock_info.channel = channel;
+ i->per_type.memblock_info.offset = offset;
+ i->per_type.memblock_info.seek_mode = seek_mode;
#ifdef HAVE_CREDS
i->with_ancil_data = false;
#endif
@@ -414,7 +427,7 @@ void pa_pstream_send_release(pa_pstream *p, uint32_t
block_id) {
if (!(item = pa_flist_pop(PA_STATIC_FLIST_GET(items))))
item = pa_xnew(struct item_info, 1);
item->type = PA_PSTREAM_ITEM_SHMRELEASE;
- item->block_id = block_id;
+ item->per_type.block_id = block_id;
#ifdef HAVE_CREDS
item->with_ancil_data = false;
#endif
@@ -451,7 +464,7 @@ void pa_pstream_send_revoke(pa_pstream *p, uint32_t
block_id) {
if (!(item = pa_flist_pop(PA_STATIC_FLIST_GET(items))))
item = pa_xnew(struct item_info, 1);
item->type = PA_PSTREAM_ITEM_SHMREVOKE;
- item->block_id = block_id;
+ item->per_type.block_id = block_id;
#ifdef HAVE_CREDS
item->with_ancil_data = false;
#endif
@@ -495,9 +508,9 @@ static void prepare_next_write_item(pa_pstream *p) {
if (p->write.current->type == PA_PSTREAM_ITEM_PACKET) {
size_t plen;
- pa_assert(p->write.current->packet);
+ pa_assert(p->write.current->per_type.packet);
- p->write.data = (void *) pa_packet_data(p->write.current->packet,
&plen);
+ p->write.data = (void *) pa_packet_data(p->write.current->per_type.packet,
&plen);
p->write.descriptor[PA_PSTREAM_DESCRIPTOR_LENGTH] = htonl((uint32_t)
plen);
if (plen <= MINIBUF_SIZE - PA_PSTREAM_DESCRIPTOR_SIZE) {
@@ -508,32 +521,32 @@ static void prepare_next_write_item(pa_pstream *p) {
} else if (p->write.current->type == PA_PSTREAM_ITEM_SHMRELEASE) {
p->write.descriptor[PA_PSTREAM_DESCRIPTOR_FLAGS] =
htonl(PA_FLAG_SHMRELEASE);
- p->write.descriptor[PA_PSTREAM_DESCRIPTOR_OFFSET_HI] =
htonl(p->write.current->block_id);
+ p->write.descriptor[PA_PSTREAM_DESCRIPTOR_OFFSET_HI] =
htonl(p->write.current->per_type.block_id);
} else if (p->write.current->type == PA_PSTREAM_ITEM_SHMREVOKE) {
p->write.descriptor[PA_PSTREAM_DESCRIPTOR_FLAGS] =
htonl(PA_FLAG_SHMREVOKE);
- p->write.descriptor[PA_PSTREAM_DESCRIPTOR_OFFSET_HI] =
htonl(p->write.current->block_id);
+ p->write.descriptor[PA_PSTREAM_DESCRIPTOR_OFFSET_HI] =
htonl(p->write.current->per_type.block_id);
} else {
uint32_t flags;
bool send_payload = true;
pa_assert(p->write.current->type == PA_PSTREAM_ITEM_MEMBLOCK);
- pa_assert(p->write.current->chunk.memblock);
+ pa_assert(p->write.current->per_type.memblock_info.chunk.memblock);
- p->write.descriptor[PA_PSTREAM_DESCRIPTOR_CHANNEL] =
htonl(p->write.current->channel);
- p->write.descriptor[PA_PSTREAM_DESCRIPTOR_OFFSET_HI] = htonl((uint32_t)
(((uint64_t) p->write.current->offset) >> 32));
- p->write.descriptor[PA_PSTREAM_DESCRIPTOR_OFFSET_LO] = htonl((uint32_t)
((uint64_t) p->write.current->offset));
+ p->write.descriptor[PA_PSTREAM_DESCRIPTOR_CHANNEL] =
htonl(p->write.current->per_type.memblock_info.channel);
+ p->write.descriptor[PA_PSTREAM_DESCRIPTOR_OFFSET_HI] = htonl((uint32_t)
(((uint64_t) p->write.current->per_type.memblock_info.offset) >> 32));
+ p->write.descriptor[PA_PSTREAM_DESCRIPTOR_OFFSET_LO] = htonl((uint32_t)
((uint64_t) p->write.current->per_type.memblock_info.offset));
- flags = (uint32_t) (p->write.current->seek_mode & PA_FLAG_SEEKMASK);
+ flags = (uint32_t) (p->write.current->per_type.memblock_info.seek_mode
& PA_FLAG_SEEKMASK);
if (p->use_shm) {
uint32_t block_id, shm_id;
size_t offset, length;
uint32_t *shm_info = (uint32_t *)
&p->write.minibuf[PA_PSTREAM_DESCRIPTOR_SIZE];
size_t shm_size = sizeof(uint32_t) * PA_PSTREAM_SHM_MAX;
- pa_mempool *current_pool =
pa_memblock_get_pool(p->write.current->chunk.memblock);
+ pa_mempool *current_pool =
pa_memblock_get_pool(p->write.current->per_type.memblock_info.chunk.memblock);
pa_memexport *current_export;
if (p->mempool == current_pool)
@@ -542,7 +555,7 @@ static void prepare_next_write_item(pa_pstream *p) {
pa_assert_se(current_export = pa_memexport_new(current_pool,
memexport_revoke_cb, p));
if (pa_memexport_put(current_export,
- p->write.current->chunk.memblock,
+
p->write.current->per_type.memblock_info.chunk.memblock,
&block_id,
&shm_id,
&offset,
@@ -555,8 +568,8 @@ static void prepare_next_write_item(pa_pstream *p) {
shm_info[PA_PSTREAM_SHM_BLOCKID] = htonl(block_id);
shm_info[PA_PSTREAM_SHM_SHMID] = htonl(shm_id);
- shm_info[PA_PSTREAM_SHM_INDEX] = htonl((uint32_t) (offset +
p->write.current->chunk.index));
- shm_info[PA_PSTREAM_SHM_LENGTH] = htonl((uint32_t)
p->write.current->chunk.length);
+ shm_info[PA_PSTREAM_SHM_INDEX] = htonl((uint32_t) (offset +
p->write.current->per_type.memblock_info.chunk.index));
+ shm_info[PA_PSTREAM_SHM_LENGTH] = htonl((uint32_t)
p->write.current->per_type.memblock_info.chunk.length);
p->write.descriptor[PA_PSTREAM_DESCRIPTOR_LENGTH] =
htonl(shm_size);
p->write.minibuf_validsize = PA_PSTREAM_DESCRIPTOR_SIZE +
shm_size;
@@ -569,8 +582,8 @@ static void prepare_next_write_item(pa_pstream *p) {
}
if (send_payload) {
- p->write.descriptor[PA_PSTREAM_DESCRIPTOR_LENGTH] = htonl((uint32_t)
p->write.current->chunk.length);
- p->write.memchunk = p->write.current->chunk;
+ p->write.descriptor[PA_PSTREAM_DESCRIPTOR_LENGTH] = htonl((uint32_t)
p->write.current->per_type.memblock_info.chunk.length);
+ p->write.memchunk = p->write.current->per_type.memblock_info.chunk;
pa_memblock_ref(p->write.memchunk.memblock);
p->write.data = NULL;
}
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
_______________________________________________
pulseaudio-discuss mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
