[gstreamer-bugs] [Bug 628214] Add support to RTSP initiation through SDP files

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=628214
  GStreamer | gst-plugins-good | git

--- Comment #9 from Marco Ballesio <gibrovacco at gmail.com> 2010-09-06 11:35:47 UTC ---
(In reply to comment #7)

Thank you for the review, my notes below.

> (From update of attachment 168963 [details])
> > static gchar **
> > gst_rtspsrc_uri_get_protocols (void)
> > {
> >-  static const gchar *protocols[] = { "rtsp", "rtspu", "rtspt", "rtsph", NULL };
> >+  static const gchar *protocols[] =
> >+      { "sdp", "rtsp", "rtspu", "rtspt", "rtsph", NULL };
> > 
> >   return (gchar **) protocols;
> > }
> 
> Please make a new rtsp-sdp:// uri which is less likely to conflict with an
> existing
> uri.
> 

Agreed.

> >+static const gchar *
> >+gst_rtspsrc_init_from_sdp (GstRTSPSrc * src, const gchar * sdpstr)
> >+{
> >+  int i, l = strlen (sdpstr);
> >+  gchar *string = g_malloc (l + 1);
> >+  const gchar *control_url = NULL;
> >+  strcpy (string, sdpstr);
> >+
> >+  for (i = 0; i < l; i++)
> >+    if (string[i] == ';' && string[i + 1] > 'a' && string[i + 1] < 'z'
> >+        && string[i + 2] == '=')
> >+      string[i] = '\n';
> 
> Why this cleanup of the SDP? I would pass a base64 encoded version of the raw
> SDP in the rtsp-sdp:// uri.

In sdpdemux the '\n' are converted with ';' . It's useful, for instance, to
easily pass the uri from command line. They're then converted back to '\n' to
let SDPMessage parse the content properly. It's trivial for me to remove all
the mangling/demangling if we so decide.

> 
> >+
> >+  if (src->sdp != NULL) {
> >+    GST_ERROR_OBJECT (src, "SDP already existing");
> >+  } else {
> 
> Setting a new rtsp-sdp:// overrides the previous one.

But it's not clear to me what would happen if we set the SDP content after the
streaming has already been initiated. I guess it would be more proper to check
the state of the element here.

> 
> >+    GST_DEBUG_OBJECT (src, "SDP protocol, extracting SDP data");
> >+    if (gst_sdp_message_new (&src->sdp) == GST_SDP_OK &&
> >+        gst_sdp_message_parse_buffer ((guchar *) string, l,
> >+            src->sdp) == GST_SDP_OK) {
> >+      GST_DEBUG_OBJECT (src, "got SDP message:\n%s", string);
> >+
> >+      for (i = 0;; i++) {
> >+        control_url =
> >+            gst_sdp_message_get_attribute_val_n (src->sdp, "control", i);
> >+        if (control_url == NULL)
> >+          break;
> >+
> >+        /* only take fully qualified urls */
> >+        if (g_str_has_prefix (control_url, "rtsp://"))
> >+          break;
> >+      }
> >+    } else {
> >+      GST_ERROR_OBJECT (src, "unable to create SDP message");
> >+    }
> >+  }
> 
> There is no need to parse the control url, this will happen later.

If the control url is not set properly the function gst_rtspsrc_open_from_sdp
will not find the control information and thus attempt to open a connection on
the 'sdp://' uri (which, of course, cannot be handled). After this fails, no
connection will be available to propagate further RTSP messages (e.g. SETUP).

> 
> > 
> >+  if (g_str_has_prefix (uri, "sdp://")) {
> >+    if ((uri = gst_rtspsrc_init_from_sdp (src, uri + 6)) == NULL)
> >+      GST_ERROR ("unable to initialize from give SDP data");
> >+  }
> >+
> >   /* try to parse */
> >   if ((res = gst_rtsp_url_parse (uri, &newurl)) < 0)
> >     goto parse_error;
> > 
> 
> There is no need to retrieve the control url from the SDP and so there is also
> no need to parse the control url.

See my comment above.

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.