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 610364 - udpsrc: allocates buffers with size a lot bigger than needed
udpsrc: allocates buffers with size a lot bigger than needed
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other All
: Normal normal
: 1.5.1
Assigned To: Tim-Philipp Müller
GStreamer Maintainers
Depends on: 686786
Reported: 2010-02-18 14:28 UTC by Gabriele Mondada
Modified: 2014-09-09 17:07 UTC
See Also:
GNOME target: ---
GNOME version: ---

udpsrc: sanity check size of available packet data for reading to avoid memory waste (1.68 KB, patch)
2013-01-02 00:08 UTC, Tim-Philipp Müller
committed Details | Review

Description Gabriele Mondada 2010-02-18 14:28:42 UTC
overloaded computer receiving RTP packets through udpsrc with parameter "buffer-size" set to 1*1024*1024.

GLib-ERROR **: gmem.c:136: failed to allocate 1044144 bytes

Personal analysis:
I didn't found why g_malloc() fails here but it is not so relevant for this bug report.

The bug declared here is the size of the buffer being allocated by udpsrc. In fact, each UDP packet is normally stored in one gst buffer and there is no apparent reasons to have a so big buffer.

By looking inside the source code, I saw (gstudpsrc.c:470):

  /* ask how much is available for reading on the socket, this should be exactly
   * one UDP packet. We will check the return value, though, because in some
   * case it can return 0 and we don't want a 0 sized buffer. */
  readsize = 0;
  if (G_UNLIKELY ((ret =
              IOCTL_SOCKET (udpsrc->sock.fd, FIONREAD, &readsize)) < 0))
    goto ioctl_failed;

This code, on Mac OS X, return the total amount of data stored in the reception buffer. This is not the size of the next UDP packet as expected in the code.

In my understanding, the code is allocating a buffer based on <readsize> to store a single UDP packet. So, the buffer can be a lot bigger than needed, generating waste of memory and other side effects.

On the other hand, g_malloc() have no reason to fail (my computer is not running out of memory). There is probably another bug that will be handled unlinked to this one.
Comment 1 Olivier Crête 2010-02-18 20:24:52 UTC
On Linux, ioctl(FIONREAD) on a udp socket returns the size of the next packet, so it is correct.

That said, on windows and osx, it returns the total.

On Windows, we need to call WSAIoctl(FIONREAD)l, not ioctlsocket() to get the right value.

On OSX, it should call getsockopt(SO_NREAD).

I guess this is a case for #ifdefs
Comment 2 Gabriele Mondada 2010-02-19 18:33:41 UTC
The code shown above is not the only location where readsize is computed. The same happens few line above.
Comment 3 Wim Taymans 2010-02-22 15:17:52 UTC
An #ifdef sounds acceptable. Anyone care to make a patch for this?
Comment 4 Tim-Philipp Müller 2012-10-23 18:55:35 UTC
Hrm, tricky. So we use g_socket_get_available_bytes() now in udpsrc, which however is implemented as:

  #ifndef G_OS_WIN32
    if (ioctl (socket->priv->fd, FIONREAD, &avail) < 0)
      return -1;
    if (ioctlsocket (socket->priv->fd, FIONREAD, &avail) == SOCKET_ERROR)
      return -1;

On Windows, "FIONREAD returns the amount of data that can be read in a single call to the recv function, which may not be the same as the total amount of data queued on the socket. If s is message oriented (for example, type SOCK_DGRAM), FIONREAD still returns the amount of pending data in the network buffer, however, the amount that can actually be read in a single call to the recv function is limited to the data size written in the send or sendto function call.", which means this is not really what we want.

On OSX, ioctl(FIONREAD) returns the total buffersize according to Olivier in comment #1, so also not what we want.

Question is if the GLib folks let us fix _get_available_bytes(), which arguably slightly changes the semantics, or whether we need a new function for that.
Comment 5 Tim-Philipp Müller 2013-01-01 23:25:17 UTC
Also see bug #686786.
Comment 6 Tim-Philipp Müller 2013-01-02 00:08:39 UTC
Created attachment 232483 [details] [review]
 udpsrc: sanity check size of available packet data for reading to avoid memory waste

Naive patch which might help a little in some cases. Still far from optimal of course.

    On Windows and OS/X, _get_available_bytes() may not return the size
    of the next pending packet, but the size of all pending packets in
    the kernel-side buffer, which might be rather large depending on
    configuration. Sanity-check the size returned by _get_available_bytes()
    to make sure we never allocate more memory than the max. size for
    a packet, if it's an IPv4 socket.
Comment 7 Sebastian Dröge (slomo) 2013-01-02 09:13:17 UTC
That's probably the best we can do for now, Windows does not even seem to have an API for that... and the only way seems to be to peek all the data to get the size. And then later get it. Which is quite inefficient of course
Comment 8 Tim-Philipp Müller 2013-01-04 14:24:00 UTC
Comment on attachment 232483 [details] [review]
 udpsrc: sanity check size of available packet data for reading to avoid memory waste

Committed this for now, with 65536-8 as max size instead of 65536 though.
Comment 9 Olivier Crête 2014-06-23 21:30:44 UTC
I guess we did all we can do here, so let's close this unless someone has a more clever idea.
Comment 10 Tim-Philipp Müller 2014-06-23 23:14:32 UTC
I did have a more clever idea actually, will implement that soon.
Comment 11 Tim-Philipp Müller 2014-09-09 17:07:42 UTC
This should do the trick:

commit e6f77948acb5fdd5bac673a4ad669ed4360a6952
Author: Tim-Philipp Müller <>
Date:   Tue Sep 9 13:46:56 2014 +0100

    udpsrc: more efficient memory handling
    Drop use of g_socket_get_available_bytes() which is
    not useful on all systems (where it returns the size
    of the entire buffer not that of the next pending
    packet), and is yet another syscall and apparently
    very inefficient on Windows in the UDP case.
    Instead, when reading UDP packets, use the more featureful
    g_socket_receive_message() call that allows to read into
    scattered memory, and allocate one memory chunk which is
    likely to be large enough for a packet, while also providing
    a larger allocated memory chunk just in case the packet
    is larger than expected. If the received data fits into the
    first chunk, we'll just add that to the buffer we return
    and re-use the fallback buffer for next time, otherwise we
    add both chunks to the buffer.
    This reduces memory waste more reliably on systems where
    get_available_bytes() doesn't work properly.
    In a multimedia streaming scenario, incoming UDP packets
    are almost never fragmented and thus almost always smaller
    than the MTU size, which is also why we don't try to do
    something smarter with more fallback memory chunks of
    different sizes. The fallback scenario is just for when
    someone built a broken sender pipeline (not using a
    payloader or somesuch)