[Bug 771525] gst-rtsp-server: Poor performance with interleaved RTSP due to missing buffer list support and merging of all memories in buffers

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=771525

Tim-Philipp Müller <t.i.m at zen.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #373684|none                        |reviewed
             status|                            |

--- Comment #29 from Tim-Philipp Müller <t.i.m at zen.co.uk> ---
Comment on attachment 373684
  --> https://bugzilla.gnome.org/attachment.cgi?id=373684
rtsp-server: Add support for buffer lists

Looks fine to me by and large.

Some nitpicks:

>--- a/gst/rtsp-server/rtsp-stream.c
>+++ b/gst/rtsp-server/rtsp-stream.c
>@@ -2394,6 +2394,8 @@ send_tcp_message (GstRTSPStream * stream, gint idx)
>   GList *walk;
>   GstSample *sample;
>   GstBuffer *buffer;
>+  GstBufferList *buffer_list;
>+  guint n_messages = 0;
>   gboolean is_rtp;
> 
>   if (priv->n_outstanding > 0 || !priv->have_buffer[idx]) {
>@@ -2414,6 +2416,14 @@ send_tcp_message (GstRTSPStream * stream, gint idx)
>   }
> 
>   buffer = gst_sample_get_buffer (sample);
>+  buffer_list = gst_sample_get_buffer_list (sample);
>+
>+  /* We will get one message-sent notification per message,
>+   * i.e. per buffer that is actually sent out */
>+  if (buffer)
>+    n_messages += 1;
>+  if (buffer_list)
>+    n_messages += gst_buffer_list_length (buffer_list);

Why are we doing "+=" here and not "=" and should it be "else if" here for
clarity or can we get both a buffer and a buffer list? (don't think so?)

"else if" also a few times in the code below.


>--- a/tests/check/gst/rtspserver.c
>+++ b/tests/check/gst/rtspserver.c
>@@ -2117,7 +2117,7 @@ GST_START_TEST (test_record_tcp)
> 
>   mfactory =
>       start_record_server
>-      ("( rtppcmadepay name=depay0 ! appsink name=sink async=false )");
>+      ("( rtppcmadepay name=depay0 ! appsink name=sink buffer-list=true async=false )");
> 
>   g_signal_connect (mfactory, "media-constructed",
>       G_CALLBACK (media_constructed_cb), &server_sink);

Does this (buffer-list=true) here actually do anything? Is it needed?

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.