[Bug 796963] Rework SRT plugin to unify client/server and add features

[GStreamer (GNOME Bugzilla)]

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

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

::: ext/srt/gstsrt.c
@@ +63,2 @@
+static gboolean
+g_ascii_string_to_signed (const gchar * str, guint base,

Do you want ot add a non g_ prefix? like gst_ascii...

@@ +356,3 @@
+gboolean
+gst_srt_init_params_from_uri (const GstElement * elem,
+    GstSRTParams * params, const GstUri * uri)

I think the URI parsing should be added to libsrt, that way we can ensure that
all SRT users have the same interpretation of the parameters. Maybe fill a
struct with them or something? and have a function to set them all at once on
the srt_socket..

::: ext/srt/gstsrtsrc.c
@@ +238,3 @@
+  priv->n_connect_attempts = 0;
+
+  srt_cleanup ();

What does srt_cleanup() do again? there could be multiple instances of srtsrc
in parallel...

@@ +256,3 @@
+
+  if (self->uri != NULL) {
+    // gst_srt_init_params_from_uri() will set element error as needed

Please no // style comments, on /* */ style.

::: ext/srt/gstsrtsrc.h
@@ +57,3 @@
+
+  /*< private >*/
+  gpointer _gst_reserved[GST_PADDING];

No need for reserved, this is all private.

@@ +63,3 @@
+  GstPushSrcClass parent_class;
+
+  gpointer _gst_reserved[GST_PADDING_LARGE];

No need for reserved, this is all private.

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