From 3e52972c610f0609c11e592c761379ea6ab8803f Mon Sep 17 00:00:00 2001 From: Tanu Kaskinen Date: Tue, 20 Sep 2016 12:59:04 +0300 Subject: [PATCH] rtp: don't use memblocks for non-audio data pa_memblockq_push() expects all memchunks to be aligned to PCM frame boundaries, and that means both the index and length fields of pa_memchunk. pa_rtp_recv(), however, used a memblock for storing both the RTP packet metadata and the actual audio data. The metadata was "removed" from the audio data by setting the memchunk index appropriately, so the metadata stayed in the memblock, but it was not played back. The metadata length is not necessarily divisible by the PCM frame size, which caused pa_memblock_push() to crash in an assertion with some sample specs, because the memchunk index was not properly aligned. In my tests the metadata length was 12, so it was compatible with many configurations, but eight-channel audio didn't work. This patch adds a separate buffer for receiving the RTP packets. As a result, an extra memcpy is needed for moving the audio data from the receive buffer to the memblock buffer. BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=96612 --- src/modules/rtp/rtp.c | 66 ++++++++++++++++++++++++++++--------------- src/modules/rtp/rtp.h | 2 ++ 2 files changed, 46 insertions(+), 22 deletions(-) diff --git a/src/modules/rtp/rtp.c b/src/modules/rtp/rtp.c index 17c8d3c1b..5ab80e77a 100644 --- a/src/modules/rtp/rtp.c +++ b/src/modules/rtp/rtp.c @@ -54,6 +54,8 @@ pa_rtp_context* pa_rtp_context_init_send(pa_rtp_context *c, int fd, uint32_t ssr c->payload = (uint8_t) (payload & 127U); c->frame_size = frame_size; + c->recv_buf = NULL; + c->recv_buf_size = 0; pa_memchunk_reset(&c->memchunk); return c; @@ -155,12 +157,16 @@ pa_rtp_context* pa_rtp_context_init_recv(pa_rtp_context *c, int fd, size_t frame c->fd = fd; c->frame_size = frame_size; + c->recv_buf_size = 2000; + c->recv_buf = pa_xmalloc(c->recv_buf_size); pa_memchunk_reset(&c->memchunk); return c; } int pa_rtp_recv(pa_rtp_context *c, pa_memchunk *chunk, pa_mempool *pool, struct timeval *tstamp) { int size; + size_t audio_length; + size_t metadata_length; struct msghdr m; struct cmsghdr *cm; struct iovec iov; @@ -204,25 +210,17 @@ int pa_rtp_recv(pa_rtp_context *c, pa_memchunk *chunk, pa_mempool *pool, struct size = 1; } - if (c->memchunk.length < (unsigned) size) { - size_t l; + if (c->recv_buf_size < (size_t) size) { + do + c->recv_buf_size *= 2; + while (c->recv_buf_size < (size_t) size); - if (c->memchunk.memblock) - pa_memblock_unref(c->memchunk.memblock); - - l = PA_MAX((size_t) size, pa_mempool_block_size_max(pool)); - - c->memchunk.memblock = pa_memblock_new(pool, l); - c->memchunk.index = 0; - c->memchunk.length = pa_memblock_get_length(c->memchunk.memblock); + c->recv_buf = pa_xrealloc(c->recv_buf, c->recv_buf_size); } - pa_assert(c->memchunk.length >= (size_t) size); + pa_assert(c->recv_buf_size >= (size_t) size); - chunk->memblock = pa_memblock_ref(c->memchunk.memblock); - chunk->index = c->memchunk.index; - - iov.iov_base = pa_memblock_acquire_chunk(chunk); + iov.iov_base = c->recv_buf; iov.iov_len = (size_t) size; m.msg_name = NULL; @@ -234,7 +232,6 @@ int pa_rtp_recv(pa_rtp_context *c, pa_memchunk *chunk, pa_mempool *pool, struct m.msg_flags = 0; r = recvmsg(c->fd, &m, 0); - pa_memblock_release(chunk->memblock); if (r != size) { if (r < 0 && errno != EAGAIN && errno != EINTR) @@ -275,21 +272,42 @@ int pa_rtp_recv(pa_rtp_context *c, pa_memchunk *chunk, pa_mempool *pool, struct c->payload = (uint8_t) ((header >> 16) & 127U); c->sequence = (uint16_t) (header & 0xFFFFU); - if (12 + cc*4 > (unsigned) size) { + metadata_length = 12 + cc * 4; + + if (metadata_length > (unsigned) size) { pa_log_warn("RTP packet too short. (CSRC)"); goto fail; } - chunk->index += 12 + cc*4; - chunk->length = (size_t) size - 12 + cc*4; + audio_length = size - metadata_length; - if (chunk->length % c->frame_size != 0) { + if (audio_length % c->frame_size != 0) { pa_log_warn("Bad RTP packet size."); goto fail; } - c->memchunk.index = chunk->index + chunk->length; - c->memchunk.length = pa_memblock_get_length(c->memchunk.memblock) - c->memchunk.index; + if (c->memchunk.length < (unsigned) audio_length) { + size_t l; + + if (c->memchunk.memblock) + pa_memblock_unref(c->memchunk.memblock); + + l = PA_MAX((size_t) audio_length, pa_mempool_block_size_max(pool)); + + c->memchunk.memblock = pa_memblock_new(pool, l); + c->memchunk.index = 0; + c->memchunk.length = pa_memblock_get_length(c->memchunk.memblock); + } + + memcpy(pa_memblock_acquire_chunk(&c->memchunk), c->recv_buf + metadata_length, audio_length); + pa_memblock_release(c->memchunk.memblock); + + chunk->memblock = pa_memblock_ref(c->memchunk.memblock); + chunk->index = c->memchunk.index; + chunk->length = audio_length; + + c->memchunk.index += audio_length; + c->memchunk.length -= audio_length; if (c->memchunk.length <= 0) { pa_memblock_unref(c->memchunk.memblock); @@ -397,6 +415,10 @@ void pa_rtp_context_destroy(pa_rtp_context *c) { if (c->memchunk.memblock) pa_memblock_unref(c->memchunk.memblock); + + pa_xfree(c->recv_buf); + c->recv_buf = NULL; + c->recv_buf_size = 0; } const char* pa_rtp_format_to_string(pa_sample_format_t f) { diff --git a/src/modules/rtp/rtp.h b/src/modules/rtp/rtp.h index bbd427855..0b877d546 100644 --- a/src/modules/rtp/rtp.h +++ b/src/modules/rtp/rtp.h @@ -34,6 +34,8 @@ typedef struct pa_rtp_context { uint8_t payload; size_t frame_size; + uint8_t *recv_buf; + size_t recv_buf_size; pa_memchunk memchunk; } pa_rtp_context;