[pulseaudio-discuss] [PATCH v7 25/33] raop: Correctly wrap RTP packet sequence number

Hajime Fujita crisp.fujita at gmail.com
Sun Nov 6 18:54:19 UTC 2016


From: Martin Blanchard <tchaik at gmx.com>

---
 src/modules/raop/raop-client.c        | 27 +++++++-------
 src/modules/raop/raop-packet-buffer.c | 70 ++++++++++++++++++++++++-----------
 src/modules/raop/raop-packet-buffer.h |  5 ++-
 3 files changed, 65 insertions(+), 37 deletions(-)

diff --git a/src/modules/raop/raop-client.c b/src/modules/raop/raop-client.c
index 574f10b..52b2dee 100644
--- a/src/modules/raop/raop-client.c
+++ b/src/modules/raop/raop-client.c
@@ -24,6 +24,7 @@
 #endif
 
 #include <stdlib.h>
+#include <stdint.h>
 #include <string.h>
 #include <errno.h>
 #include <unistd.h>
@@ -316,10 +317,10 @@ static size_t build_tcp_audio_packet(pa_raop_client *c, pa_memchunk *block, pa_m
     buffer += packet->index / sizeof(uint32_t);
     raw += block->index;
 
-    if (c->seq == 0xFFFF) {
-        pa_log_debug("wrapping sequence number");
-        c->seq = pa_raop_packet_buffer_wrap_seq(c->pbuf, c->seq);
-    } else
+    /* Wrap sequence number to 0 then UINT16_MAX is reached */
+    if (c->seq == UINT16_MAX)
+        c->seq = 0;
+    else
         c->seq++;
 
     memcpy(buffer, tcp_audio_header, sizeof(tcp_audio_header));
@@ -360,13 +361,13 @@ static ssize_t send_tcp_audio_packet(pa_raop_client *c, pa_memchunk *block, size
     ssize_t written = -1;
     size_t done = 0;
 
-    if (!(packet = pa_raop_packet_buffer_get(c->pbuf, c->seq, max)))
+    if (!(packet = pa_raop_packet_buffer_retrieve(c->pbuf, c->seq)))
         return -1;
 
     if (packet->length <= 0) {
         pa_assert(block->index == offset);
 
-        if (!(packet = pa_raop_packet_buffer_get(c->pbuf, c->seq + 1, max)))
+        if (!(packet = pa_raop_packet_buffer_prepare(c->pbuf, c->seq + 1, max)))
             return -1;
 
         packet->index = 0;
@@ -427,10 +428,10 @@ static size_t build_udp_audio_packet(pa_raop_client *c, pa_memchunk *block, pa_m
 
     c->rtptime += length / 4;
 
-    if (c->seq == 0xFFFF) {
-        pa_log_debug("wrapping sequence number");
-        c->seq = pa_raop_packet_buffer_wrap_seq(c->pbuf, c->seq);
-    } else
+    /* Wrap sequence number to 0 then UINT16_MAX is reached */
+    if (c->seq == UINT16_MAX)
+        c->seq = 0;
+    else
         c->seq++;
 
     pa_memblock_release(block->memblock);
@@ -453,11 +454,11 @@ static ssize_t send_udp_audio_packet(pa_raop_client *c, pa_memchunk *block, size
     /* UDP packet has to be sent at once ! */
     pa_assert(block->index == offset);
 
-    if (!(packet = pa_raop_packet_buffer_get(c->pbuf, c->seq, max)))
+    if (!(packet = pa_raop_packet_buffer_prepare(c->pbuf, c->seq, max)))
         return -1;
 
-    packet->length = max;
     packet->index = sizeof(udp_audio_retrans_header);
+    packet->length = max - sizeof(udp_audio_retrans_header);
     if (!build_udp_audio_packet(c, block, packet))
         return -1;
 
@@ -508,7 +509,7 @@ static ssize_t resend_udp_audio_packets(pa_raop_client *c, uint16_t seq, uint16_
         uint8_t *buffer = NULL;
         ssize_t written = -1;
 
-        if (!(packet = pa_raop_packet_buffer_get(c->pbuf, seq + i, 0)))
+        if (!(packet = pa_raop_packet_buffer_retrieve(c->pbuf, seq + i)))
             continue;
 
         if (packet->index > 0) {
diff --git a/src/modules/raop/raop-packet-buffer.c b/src/modules/raop/raop-packet-buffer.c
index 4c45d18..05f9592 100644
--- a/src/modules/raop/raop-packet-buffer.c
+++ b/src/modules/raop/raop-packet-buffer.c
@@ -25,6 +25,7 @@
 #endif
 
 #include <stdlib.h>
+#include <stdint.h>
 #include <limits.h>
 
 #include <pulse/xmalloc.h>
@@ -37,7 +38,9 @@
 struct pa_raop_packet_buffer {
     pa_memchunk *packets;
     pa_mempool *mempool;
+
     size_t size;
+    size_t count;
 
     uint16_t seq;
     size_t pos;
@@ -49,6 +52,7 @@ pa_raop_packet_buffer *pa_raop_packet_buffer_new(pa_mempool *mempool, const size
     pa_assert(mempool);
     pa_assert(size > 0);
 
+    pb->count = 0;
     pb->size = size;
     pb->mempool = mempool;
     pb->packets = pa_xnew0(pa_memchunk, size);
@@ -80,7 +84,8 @@ void pa_raop_packet_buffer_reset(pa_raop_packet_buffer *pb, uint16_t seq) {
     pa_assert(pb->packets);
 
     pb->pos = 0;
-    pb->seq = seq - 1;
+    pb->count = 0;
+    pb->seq = (!seq) ? UINT16_MAX : seq - 1;
     for (i = 0; i < pb->size; i++) {
         if (pb->packets[i].memblock)
             pa_memblock_unref(pb->packets[i].memblock);
@@ -88,23 +93,43 @@ void pa_raop_packet_buffer_reset(pa_raop_packet_buffer *pb, uint16_t seq) {
     }
 }
 
-uint16_t pa_raop_packet_buffer_wrap_seq(pa_raop_packet_buffer *pb, uint16_t seq) {
-    int seq_shift;
+pa_memchunk *pa_raop_packet_buffer_prepare(pa_raop_packet_buffer *pb, uint16_t seq, const size_t size) {
+    pa_memchunk *packet = NULL;
+    size_t i;
 
     pa_assert(pb);
+    pa_assert(pb->packets);
+
+    if (seq == 0) {
+        /* 0 means seq reached UINT16_MAX and has been wrapped... */
+        pa_assert(pb->seq == UINT16_MAX);
+        pb->seq = 0;
+    } else {
+        /* ...otherwise, seq MUST have be increased! */
+        pa_assert(seq == pb->seq + 1);
+        pb->seq++;
+    }
+
+    i = (pb->pos + 1) % pb->size;
 
-    if (seq > pb->seq)
-        seq_shift = pb->seq - 1;
-    else
-        seq_shift = seq;
+    if (pb->packets[i].memblock)
+        pa_memblock_unref(pb->packets[i].memblock);
+    pa_memchunk_reset(&pb->packets[i]);
 
-    pb->seq -= seq_shift;
+    pb->packets[i].memblock = pa_memblock_new(pb->mempool, size);
+    pb->packets[i].length = size;
+    pb->packets[i].index = 0;
 
-    return seq - seq_shift;
+    packet = &pb->packets[i];
 
+    if (pb->count < pb->size)
+        pb->count++;
+    pb->pos = i;
+
+    return packet;
 }
 
-pa_memchunk *pa_raop_packet_buffer_get(pa_raop_packet_buffer *pb, uint16_t seq, const size_t size) {
+pa_memchunk *pa_raop_packet_buffer_retrieve(pa_raop_packet_buffer *pb, uint16_t seq) {
     pa_memchunk *packet = NULL;
     size_t delta, i;
 
@@ -113,20 +138,21 @@ pa_memchunk *pa_raop_packet_buffer_get(pa_raop_packet_buffer *pb, uint16_t seq,
 
     if (seq == pb->seq)
         packet = &pb->packets[pb->pos];
-    else if (seq < pb->seq) {
-        delta = pb->seq - seq;
+    else {
+        if (seq < pb->seq) {
+            /* Regular case: pb->seq did not wrapped since seq. */
+            delta = pb->seq - seq;
+            pa_assert(delta <= pb->count);
+        } else {
+            /* Tricky case: pb->seq wrapped since seq! */
+            delta = pb->seq + (UINT16_MAX - seq);
+            pa_assert(delta <= pb->count);
+        }
+
         i = (pb->size + pb->pos - delta) % pb->size;
-        if (delta < pb->size)
+
+        if (delta < pb->size && pb->packets[i].memblock)
             packet = &pb->packets[i];
-    } else {
-        i = (pb->pos + (seq - pb->seq)) % pb->size;
-        if (pb->packets[i].memblock)
-            pa_memblock_unref(pb->packets[i].memblock);
-        pa_memchunk_reset(&pb->packets[i]);
-        pb->packets[i].memblock = pa_memblock_new(pb->mempool, size);
-        packet = &pb->packets[i];
-        pb->seq = seq;
-        pb->pos = i;
     }
 
     return packet;
diff --git a/src/modules/raop/raop-packet-buffer.h b/src/modules/raop/raop-packet-buffer.h
index cb2bfdf..c410298 100644
--- a/src/modules/raop/raop-packet-buffer.h
+++ b/src/modules/raop/raop-packet-buffer.h
@@ -33,7 +33,8 @@ pa_raop_packet_buffer *pa_raop_packet_buffer_new(pa_mempool *mempool, const size
 void pa_raop_packet_buffer_free(pa_raop_packet_buffer *pb);
 
 void pa_raop_packet_buffer_reset(pa_raop_packet_buffer *pb, uint16_t seq);
-pa_memchunk *pa_raop_packet_buffer_get(pa_raop_packet_buffer *pb, uint16_t seq, const size_t size);
 
-uint16_t pa_raop_packet_buffer_wrap_seq(pa_raop_packet_buffer *pb, uint16_t seq);
+pa_memchunk *pa_raop_packet_buffer_prepare(pa_raop_packet_buffer *pb, uint16_t seq, const size_t size);
+pa_memchunk *pa_raop_packet_buffer_retrieve(pa_raop_packet_buffer *pb, uint16_t seq);
+
 #endif
-- 
2.9.3



More information about the pulseaudio-discuss mailing list