[Bug 771525] gst-rtsp-server: poor performance while streaming RTP over RTSP

[GStreamer (GNOME Bugzilla)]

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

Olivier Crête <olivier.crete at ocrete.ca> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #356397|none                        |needs-work
             status|                            |

--- Comment #12 from Olivier Crête <olivier.crete at ocrete.ca> ---
Review of attachment 356397:
 --> (https://bugzilla.gnome.org/review?bug=771525&attachment=356397)

::: gst/rtsp-server/rtsp-client.c
@@ +44,3 @@
 #include <stdio.h>
 #include <string.h>
+#include <sys/socket.h>

Why do you include this? arent all socket operations abstracted through GIO ?

@@ +1037,3 @@
+    data_header += 4;
+  }
+  GST_DEBUG (" mem:%d, pos: %d", mem, pos);

This should be a GST_LOG, it's on every buffer.

@@ +4085,3 @@
+      num_written =
+          g_socket_send_message (socket, NULL, &vecs[i], send_len, NULL, 0,
+          MSG_DONTWAIT, NULL, &err);

You shouldn't pull out the socket from here.. Instead you should add the proper
API to gst_rtsp_connection_..()

And you sholdn't use MSG_DONTWAIT, the parameter here is a GSocketMsgFlags

@@ +4087,3 @@
+          MSG_DONTWAIT, NULL, &err);
+      if (err != NULL)
+        GST_ERROR ("error: %s", err->message);

You end up re-printing the error down there, so this one can be downgraded or
removed.

@@ +4088,3 @@
+      if (err != NULL)
+        GST_ERROR ("error: %s", err->message);
+    } while (num_written == -1 && (errno == EINTR || errno == EAGAIN));

Instead of looking at errno, you want to look at the err with
g_error_matches(). And if you want to g_clear_error(&err) if you decide to
ignore the error.

@@ +4091,3 @@
+
+    if (num_written == -1) {
+      GST_ERROR ("failed to send message: %s", err->message);

While we're at it, maybe you should pass the client object here so you can do
GST_ERROR_OBJECT()..

@@ +4133,3 @@
+      total_mems = gst_buffer_n_memory (buffer);
+      map_infos = g_new (GstMapInfo, total_mems);
+      vecs = g_new (GOutputVector, total_mems + num_buffers);

Why not g_newa() here?

@@ +4155,3 @@
+      map_infos = g_new (GstMapInfo, total_mems);
+      vecs = g_new (GOutputVector, total_mems + num_buffers);
+      data_header = g_malloc (4 * num_buffers);

Any reason not to use g_newa() here too ?

@@ +4183,3 @@
+    GST_ERROR_OBJECT (client, "waiting for backlog");
+    ret = gst_rtsp_watch_wait_backlog (priv->watch, &time);
+    GST_ERROR_OBJECT (client, "Resend due to backlog full");

Those are not really errors, should be at DEBUG level.

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