[pulseaudio-commits] src/modules

[Tanu Kaskinen]

 src/modules/rtp/rtp.c |   66 +++++++++++++++++++++++++++++++++-----------------
 src/modules/rtp/rtp.h |    2 +
 2 files changed, 46 insertions(+), 22 deletions(-)

New commits:
commit 3e52972c610f0609c11e592c761379ea6ab8803f
Author: Tanu Kaskinen <tanuk at iki.fi>
Date:   Tue Sep 20 12:59:04 2016 +0300

    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

diff --git a/src/modules/rtp/rtp.c b/src/modules/rtp/rtp.c
index 17c8d3c..5ab80e7 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->memchunk.memblock)
-            pa_memblock_unref(c->memchunk.memblock);
+    if (c->recv_buf_size < (size_t) size) {
+        do
+            c->recv_buf_size *= 2;
+        while (c->recv_buf_size < (size_t) size);
 
-        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);
-
-    chunk->memblock = pa_memblock_ref(c->memchunk.memblock);
-    chunk->index = c->memchunk.index;
+    pa_assert(c->recv_buf_size >= (size_t) size);
 
-    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 bbd4278..0b877d5 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;