After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 740752 - add retransmission support
add retransmission support
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-rtsp-server
git master
Other Linux
: Normal enhancement
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-11-26 14:16 UTC by Matthew Waters (ystreet00)
Modified: 2014-12-16 15:48 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
media: implement ssrc-multiplexed retransmission support (19.74 KB, patch)
2014-11-26 14:16 UTC, Matthew Waters (ystreet00)
none Details | Review
media: implement ssrc-multiplexed retransmission support (19.76 KB, patch)
2014-12-01 12:46 UTC, Matthew Waters (ystreet00)
committed Details | Review
examples: add a retransmission example (4.53 KB, patch)
2014-12-01 12:46 UTC, Matthew Waters (ystreet00)
committed Details | Review

Description Matthew Waters (ystreet00) 2014-11-26 14:16:22 UTC
based off RFC4588
Comment 1 Matthew Waters (ystreet00) 2014-11-26 14:16:56 UTC
Created attachment 291548 [details] [review]
media: implement ssrc-multiplexed retransmission support
Comment 2 Matthew Waters (ystreet00) 2014-12-01 12:46:33 UTC
Created attachment 291884 [details] [review]
media: implement ssrc-multiplexed retransmission support

Fix a refcounting issue.
Comment 3 Matthew Waters (ystreet00) 2014-12-01 12:46:58 UTC
Created attachment 291885 [details] [review]
examples: add a retransmission example
Comment 4 Sebastian Dröge (slomo) 2014-12-14 11:32:49 UTC
Review of attachment 291884 [details] [review]:

Looks good, please push after the following few changes :)

::: gst/rtsp-server/rtsp-media.c
@@ +1272,3 @@
+    guint pt = gst_rtsp_stream_get_pt (stream);
+
+    ret = g_list_append (ret, GUINT_TO_POINTER (pt));

prepend here, and then reverse the list in the end. That's faster

@@ +1412,3 @@
+
+    if (rtx_pt == 0) {
+      /* FIXME: ran out of space of dynamic payload types */

GST_WARNING_OBJECT() maybe

::: gst/rtsp-server/rtsp-stream.c
@@ +1369,3 @@
+  if (stream->priv->rtxsend) {
+    guint pt = gst_rtsp_stream_get_pt (stream);
+    gchar *pt_s = g_strdup_printf ("%d", pt);

This string is leaked

@@ +1753,3 @@
+
+  pt = gst_rtsp_stream_get_pt (stream);
+  pt_s = g_strdup_printf ("%u", pt);

And this
Comment 5 Tim-Philipp Müller 2014-12-15 11:56:32 UTC
> +    ret = g_list_append (ret, GUINT_TO_POINTER (pt));
> 
> prepend here, and then reverse the list in the end. That's faster

That's not a good pattern, better just use a GQueue on the stack then and return queue.head when done :) </bikeshed>
Comment 6 Sebastian Dröge (slomo) 2014-12-16 15:48:00 UTC
commit fa4d8db7ea9079e3a92cdc4fcccc284e22c0df63
Author: Matthew Waters <matthew@centricular.com>
Date:   Mon Dec 1 23:42:34 2014 +1100

    examples: add a retransmisison example implementing RFC4588
    
    Currently only SSRC-multiplexed rtx streams are supported

commit a44b564f59c738f9f115df7e7995935f77d29d4b
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Tue Dec 16 16:46:15 2014 +0100

    rtsp-stream: Fix some minor memory leaks

commit 8ae35665911424eea52ba4a7444eb9d1dbff1d0c
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Tue Dec 16 16:46:06 2014 +0100

    rtsp-media: Some minor cleanup

commit 06bfc0697b8fa8d7272124868706cd896b423257
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Tue Dec 16 16:42:13 2014 +0100

    rtsp-stream: Fix compiler warnings
    
    rtsp-stream.c:1351:3: error: non-void function 'gst_rtsp_stream_get_retransmission_time' should return a value [-Wreturn-type]
      g_return_if_fail (GST_IS_RTSP_STREAM (stream));
      ^
    
    rtsp-stream.c:1384:3: error: non-void function 'gst_rtsp_stream_get_retransmission_pt' should return a value [-Wreturn-type]
      g_return_if_fail (GST_IS_RTSP_STREAM (stream));
      ^

commit 4f40781fff4f74ca80f202b3b6e435ff9a86ee1e
Author: Matthew Waters <matthew@centricular.com>
Date:   Thu Nov 27 01:12:36 2014 +1100

    media: implement ssrc-multiplexed retransmission support
    
    based off RFC 4588 and the server-rtpaux example in -good