After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 487488 - udpsrc errors
udpsrc errors
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal normal
: 0.10.7
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2007-10-17 13:02 UTC by Laurent Glayal
Modified: 2007-10-18 17:04 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Laurent Glayal 2007-10-17 13:02:46 UTC
Context :

- udpsrc is used with a file descriptor (option sokfd)
- This file descriptor is a CONNECTED UDP socket (a connect has been issued on the fd to peer), so packets are filtered AND ICMP errors may be received by fd.

- When receiving ICMP error, the select() call at gstudpsrc.c:422 returns indicating something happended on fd, exiting the loop and calling 

  readsize = 0;
  if ((ret = IOCTL_SOCKET (udpsrc->sock, FIONREAD, &readsize)) < 0)
    goto ioctl_failed;

  if (!readsize) 
         goto nothing_to_read;

As the return from select was due to icmp message on network the FIONREAD returns a redsize of 0, issuing a nothing_to_read => GST_FLOW_ERROR on pipeline.

I think a readsize of 0 should retry a select on the fd, for ex :
Index: gstudpsrc.c
===================================================================
RCS file: /cvs/gstreamer/gst-plugins-good/gst/udp/gstudpsrc.c,v
retrieving revision 1.78
diff -u -r1.78 gstudpsrc.c
--- gstudpsrc.c 26 Sep 2007 14:28:20 -0000      1.78
+++ gstudpsrc.c 17 Oct 2007 12:48:06 -0000
@@ -377,6 +377,7 @@

   udpsrc = GST_UDPSRC (psrc);

+retry :
   /* quick check, avoid going in select when we already have data */
   readsize = 0;
   if ((ret = IOCTL_SOCKET (udpsrc->sock, FIONREAD, &readsize)) < 0)
@@ -450,8 +451,8 @@
   if ((ret = IOCTL_SOCKET (udpsrc->sock, FIONREAD, &readsize)) < 0)
     goto ioctl_failed;

-  if (!readsize)
-    goto nothing_to_read;
+  if (!readsize)
+           goto retry;

 no_select:
   GST_LOG_OBJECT (udpsrc, "ioctl says %d bytes available", (int) readsize);
@@ -514,13 +515,6 @@
         ("ioctl failed %d: %s (%d)", ret, g_strerror (errno), errno));
     return GST_FLOW_ERROR;
   }
-nothing_to_read:
-  {
-    GST_ELEMENT_ERROR (udpsrc, RESOURCE, READ, (NULL),
-        ("ioctl returned readsize 0 %d: %s (%d)", ret, g_strerror (errno),
-            errno));
-    return GST_FLOW_ERROR;
-  }
 receive_error:
   {
     g_free (pktdata);
Comment 1 Wim Taymans 2007-10-17 14:17:01 UTC
Nothing against this patch. Is the app reading on the socket for ICMP errors? 
Comment 2 Laurent Glayal 2007-10-17 14:31:33 UTC
Yes, the sockfd is used in our app to receive RTP (using udpsrc) and send UDP (using a raw sendto()), the sendto() gets return code indicating errors (due to ICMP messages). We do not rely on any sockopt(IP_RECVERR), we just use a connected udp socket to avoid pollution from unwanted packets.
Comment 3 Wim Taymans 2007-10-18 08:45:04 UTC
I'm concerned that when the app is not reading from the socket that udpsrc would just constantly retry the select() until the fd is without activity again. 

Maybe we want to add a signal to fdsrc to notify activity on the socket? The app could then do whatever it needs to do before we go back into select.
Comment 4 Laurent Glayal 2007-10-18 09:23:34 UTC
On an udp connected socket the ICMP messages are not frequent (not enough ICMP errors as packets sent (don't know really why...but it's quite a good thing)), so iterations around select() call are restricted.

It's quite difficult to detect errors using select, code could switch to poll().
Using poll() errors on file descriptor (shutdown/close on fd would be detected by poll), but errors from network (like ICMP messages) are only detected by next system call (such as read/recv). 

I see no easy way to handle errors as the file descriptor is full duplex and not only used for receiving.
Comment 5 Wim Taymans 2007-10-18 17:04:16 UTC
        Based on patch by: Laurent Glayal  <spglegle yahoo fr>

        * gst/udp/gstudpsrc.c: (gst_udpsrc_create):
        When the socket is used by the app for other purposes, don't generate an
        error if there is activaty on the socket that is not data related.
        Fixes #487488.