rtsp-client: always allocate both IPV4 and IPV6 sockets
authorMathieu Duponchelle <mathieu@centricular.com>
Wed, 25 Jul 2018 17:54:55 +0000 (19:54 +0200)
committerMathieu Duponchelle <mathieu@centricular.com>
Wed, 1 Aug 2018 18:42:34 +0000 (20:42 +0200)
multiudpsink does not support setting the socket* properties
after it has started, which meant that rtsp-server could no
longer serve on both IPV4 and IPV6 sockets since the patches
from https://bugzilla.gnome.org/show_bug.cgi?id=757488 were
merged.

When first connecting an IPV6 client then an IPV4 client,
multiudpsink fell back to using the IPV6 socket.

When first connecting an IPV4 client, then an IPV6 client,
multiudpsink errored out, released the IPV4 socket, then
crashed when trying to send a message on NULL nevertheless,
that is however a separate issue.

This could probably be fixed by handling the setting of
sockets in multiudpsink after it has started, that will
however be a much more significant effort.

For now, this commit simply partially reverts the behaviour
of rtsp-stream: it will continue to only create the udpsinks
when needed, as was the case since the patches were merged,
it will however when creating them, always allocate both
sockets and set them on the sink before it starts, as was
the case prior to the patches.

Transport configuration will only error out if the allocation
of UDP sockets fails for the actual client's family, this
also downgrades the GST_ERRORs in alloc_ports_one_family
to GST_WARNINGs, as failing to allocate is no longer
necessarily fatal.

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

gst/rtsp-server/rtsp-client.c
gst/rtsp-server/rtsp-stream.c

index 1b33f03..92135f2 100644 (file)
@@ -1964,19 +1964,29 @@ default_configure_client_transport (GstRTSPClient * client,
   /* we have a valid transport now, set the destination of the client. */
   if (ct->lower_transport == GST_RTSP_LOWER_TRANS_UDP_MCAST ||
       ct->lower_transport == GST_RTSP_LOWER_TRANS_UDP) {
-
     /* allocate UDP ports */
     GSocketFamily family;
     gboolean use_client_settings = FALSE;
 
     family = priv->is_ipv6 ? G_SOCKET_FAMILY_IPV6 : G_SOCKET_FAMILY_IPV4;
+
     if ((ct->lower_transport == GST_RTSP_LOWER_TRANS_UDP_MCAST) &&
         gst_rtsp_auth_check (GST_RTSP_AUTH_CHECK_TRANSPORT_CLIENT_SETTINGS) &&
         (ct->destination != NULL))
       use_client_settings = TRUE;
 
-    if (!gst_rtsp_stream_allocate_udp_sockets (ctx->stream, family, ct,
-            use_client_settings))
+    /* We need to allocate the sockets for both families before starting
+     * multiudpsink, otherwise multiudpsink won't accept new clients with
+     * a different family.
+     */
+    /* FIXME: could be more adequately solved by making it possible
+     * to set a socket on multiudpsink after it has already been started */
+    if (!gst_rtsp_stream_allocate_udp_sockets (ctx->stream, G_SOCKET_FAMILY_IPV4, ct,
+            use_client_settings) && family == G_SOCKET_FAMILY_IPV4)
+      goto error_allocating_ports;
+
+    if (!gst_rtsp_stream_allocate_udp_sockets (ctx->stream, G_SOCKET_FAMILY_IPV6, ct,
+            use_client_settings) && family == G_SOCKET_FAMILY_IPV6)
       goto error_allocating_ports;
 
     if (ct->lower_transport == GST_RTSP_LOWER_TRANS_UDP_MCAST) {
index 4aa06dd..3439ea3 100644 (file)
@@ -1469,28 +1469,28 @@ again:
   /* ERRORS */
 no_udp_protocol:
   {
-    GST_ERROR_OBJECT (stream, "failed to allocate UDP ports: protocol error");
+    GST_WARNING_OBJECT (stream, "failed to allocate UDP ports: protocol error");
     goto cleanup;
   }
 no_pool:
   {
-    GST_ERROR_OBJECT (stream,
+    GST_WARNING_OBJECT (stream,
         "failed to allocate UDP ports: no address pool specified");
     goto cleanup;
   }
 no_address:
   {
-    GST_ERROR_OBJECT (stream, "failed to acquire address from pool");
+    GST_WARNING_OBJECT (stream, "failed to acquire address from pool");
     goto cleanup;
   }
 no_ports:
   {
-    GST_ERROR_OBJECT (stream, "failed to allocate UDP ports: no ports");
+    GST_WARNING_OBJECT (stream, "failed to allocate UDP ports: no ports");
     goto cleanup;
   }
 socket_error:
   {
-    GST_ERROR_OBJECT (stream, "failed to allocate UDP ports: socket error");
+    GST_WARNING_OBJECT (stream, "failed to allocate UDP ports: socket error");
     goto cleanup;
   }
 cleanup: