Hi,
I've run into a bug with the jitter buffer implementation in rtpbin that
causes the jitter buffer to stop working when there is some packet loss in
the rtp stream. I have attached a simple testcase and a patch that I believe
corrects the problem.
The testcase sets up a pipeline that sends audio to the rtpbin localport for a
few seconds, then sends the rtp data to a different port to simulate some
packet loss, then resumes sending to the original port. Audio should be
heard before and after the simulated packet loss, however when the jitter
buffer is enabled, none of the data received after resuming on the correct
port is played out. The jitter buffer drops it all because the packets
are "too late".
I believe the problem is in gst_rtp_jitter_buffer_add_to_queue with the 'late
packet' case. There should be a condition that checks that the timestamp of
the newly received packet is actually less than jbuffer->min_ts otherwise the
jitter buffer can think that a packet is late just because the timestamp
delta is too large. When packets get lost, the timestamp could jump by quite
a bit but this not a case of packets arriving too late.
Anyway, please try out my patch, it has some extra debugging code to print out
the data in the queue that probably isn't needed but helped with debugging.
Without the patch you will see the jitter buffer get "stuck" when packets are
lost, but with the patch the jitter buffer is able cope with larger gaps in
rtp seqnum/timestamp caused by packet loss. I also had to fix some pretty
evil errors in priv_compare_rtp_seq_lt and priv_delta_rtp_ts. Half of 16 bit
range is 1<<15 not 1<<7 (which is only 128), and half of 32 bit range is
1<<31 not G_MAXUINT16. Looks like somebody was thinking in base 10 ;)
Regards,
~Scott
#include <stdio.h>
#include <glib.h>
#include <gst/gst.h>
#include <unistd.h>
#include <assert.h>
GstElement *gAudioDepayloader;
GstElement *gRtpDemux;
void gstNewPayloadType(GstElement *element, gint pt, GstPad *pad, gpointer data)
{
printf("Received new RTP payload type %d\n", pt);
}
void gstPayloadTypeChange(GstElement *element, gint pt, gpointer data)
{
printf("RTP payload type changed to %d\n", pt);
assert(gst_element_link_pads(gRtpDemux, "src0", gAudioDepayloader, "sink") == TRUE);
}
int main(int argc, char *argv[])
{
gst_init(NULL, NULL);
GstElement *pipeline = gst_pipeline_new("pipeline");
GstElement *rtpBin = gst_element_factory_make("rtpbin", NULL);
GstElement *rtpDemux = gst_element_factory_make("rtpdemux", NULL);
GstElement *audioSrc = gst_element_factory_make("audiotestsrc", NULL);
GstElement *audioEncoder = gst_element_factory_make("mulawenc", NULL);
GstElement *audioPayloader = gst_element_factory_make("rtppcmupay", NULL);
GstElement *audioSink = gst_element_factory_make("alsasink", NULL);
GstElement *audioDecoder = gst_element_factory_make("mulawdec", NULL);
GstElement *audioDepayloader = gst_element_factory_make("rtppcmudepay", NULL);
gst_bin_add_many(GST_BIN(pipeline), rtpBin, rtpDemux, audioSrc, audioEncoder, audioPayloader,
audioSink, audioDecoder, audioDepayloader, NULL);
gRtpDemux = rtpDemux;
gAudioDepayloader = audioDepayloader;
assert(gst_element_link(rtpBin, rtpDemux) == TRUE);
assert(gst_element_link(audioPayloader, rtpBin) == TRUE);
assert(gst_element_link(audioEncoder, audioPayloader) == TRUE);
assert(gst_element_link(audioSrc, audioEncoder) == TRUE);
assert(gst_element_link(audioDepayloader, audioDecoder) == TRUE);
assert(gst_element_link(audioDecoder, audioSink) == TRUE);
g_object_set(G_OBJECT(rtpBin), "rtcp-support", FALSE,
"localport", 6660,
"destinations", "127.0.0.1:6660",
NULL);
g_object_set(G_OBJECT(audioSrc), "is-live", TRUE, "blocksize", 320, NULL);
g_object_set(G_OBJECT(audioPayloader), "max_ptime", G_GINT64_CONSTANT(10000000), NULL);
g_object_set(G_OBJECT(audioSink), "sync", FALSE, NULL);
// Set jitter buffer size
g_object_set(G_OBJECT(rtpBin), "queue-delay", 100, NULL);
g_signal_connect(G_OBJECT(rtpDemux), "new-payload-type", G_CALLBACK(gstNewPayloadType), NULL);
g_signal_connect(G_OBJECT(rtpDemux), "payload-type-change", G_CALLBACK(gstPayloadTypeChange), NULL);
gst_element_set_state(pipeline, GST_STATE_PLAYING);
printf("\nStarting to send (should hear audio tone)...\n");
for (int i = 0; i < 5; ++i)
{
printf(".");
fflush(NULL);
sleep(1);
}
// Send to the wrong port to simulate packet loss...
g_object_set(G_OBJECT(rtpBin), "destinations", "127.0.0.1:6658", NULL);
printf("\nSending to the wrong port to simulate packet loss (silence)...\n");
for (int i = 0; i < 5; ++i)
{
printf(".");
fflush(NULL);
usleep(100000); // 100ms, same as jitter buffer size
}
// Continue sending to the original port
g_object_set(G_OBJECT(rtpBin), "destinations", "127.0.0.1:6660", NULL);
printf("\nResuming send to original port (should hear audio tone again)...\n");
for (int i = 0; i < 5; ++i)
{
printf(".");
fflush(NULL);
sleep(1);
}
printf("\ndone.\n");
return 0;
}
diff -rN -u old-gst-plugins-farsight/gst/rtpjitterbuffer/gstrtpjitterbuffer.c new-gst-plugins-farsight/gst/rtpjitterbuffer/gstrtpjitterbuffer.c
--- old-gst-plugins-farsight/gst/rtpjitterbuffer/gstrtpjitterbuffer.c 2007-01-17 23:50:19.000000000 -0700
+++ new-gst-plugins-farsight/gst/rtpjitterbuffer/gstrtpjitterbuffer.c 2007-01-17 23:50:19.000000000 -0700
@@ -96,7 +96,22 @@
static inline gboolean priv_compare_rtp_seq_lt(guint16 a, guint16 b)
{
/* check if diff more than half of the 16bit range */
- if (abs(b - a) > (1 << 7)) {
+ if ((guint16)abs(b - a) > (1 << 15)) {
+ /* one of a/b has wrapped */
+ return !(a < b);
+ }
+ else {
+ return a < b;
+ }
+}
+
+/**
+ * Performs comparison 'a < b' on 32 bit unsigned ints with check for overflows.
+ */
+static inline gboolean priv_compare_rtp_ts_lt(guint32 a, guint32 b)
+{
+ /* check if diff more than half of the 32bit range */
+ if ((guint32)labs(b - a) > (1 << 31)) {
/* one of a/b has wrapped */
return !(a < b);
}
@@ -112,7 +127,7 @@
{
/* check if diff more than half of the 32bit range */
guint32 delta = (a > b ? a - b : b - a);
- if (delta > G_MAXUINT16) {
+ if (delta > (1 << 31)) {
/* one of a/b has wrapped */
return G_MAXUINT32 - delta + 1;
}
@@ -121,6 +136,18 @@
}
}
+static void priv_print_queue(GQueue *queue)
+{
+ GList *it;
+ int i = 0;
+
+ for (it = queue->head; it; it = g_list_next(it))
+ {
+ GstBuffer *buf = (GstBuffer*)(it->data);
+ GST_DEBUG ("queue[%d]: sn %d timestamp %u", i++, gst_rtp_buffer_get_seq(buf), gst_rtp_buffer_get_timestamp(buf));
+ }
+}
+
GType
gst_rtp_jitter_buffer_get_type (void)
{
@@ -426,6 +453,7 @@
queue_len, gst_rtp_buffer_get_timestamp (in), seqnum);
QUEUE_LOCK (jbuffer);
+ priv_print_queue(queue);
if (queue_len == 0) {
/* case: our first packet, just push it */
g_queue_push_tail (queue, in);
@@ -436,8 +464,9 @@
GST_DEBUG_OBJECT (jbuffer, "buffer overflow, dropping packet#%d!", seqnum);
gst_buffer_unref (in);
}
- else if (priv_delta_rtp_ts(timestamp, jbuffer->min_ts) >
- jbtsunits * GST_RTP_JB_OVERFLOW_LIMIT) {
+ else if ((priv_compare_rtp_ts_lt(timestamp, jbuffer->min_ts) == TRUE) &&
+ (priv_delta_rtp_ts(timestamp, jbuffer->min_ts) >
+ jbtsunits * GST_RTP_JB_OVERFLOW_LIMIT)) {
/* case: late packet, drop */
GST_DEBUG_OBJECT (jbuffer,
"packet#%d too late, dropping (ts-delta=%lu, max=%u!",
@@ -589,7 +618,7 @@
headts =
gst_rtp_buffer_get_timestamp (GST_BUFFER (g_queue_peek_head (queue)));
- GST_DEBUG("maxtsunit is %u %u %u %u", maxtsunits, headts, tailts, headts - tailts);
+ GST_DEBUG("maxtsunit is %u %u %u %u", maxtsunits, headts, tailts, priv_delta_rtp_ts(tailts, headts));
if (priv_delta_rtp_ts(tailts, headts) <= maxtsunits)
break;
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Farsight-devel mailing list
Farsight-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/farsight-devel
