From 09eeb714c26ae7112a8e06aee6ec82f4f5468785 Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Fri, 28 Apr 2006 13:16:03 +0000 Subject: [PATCH] plugins/elements/gstfdsink.c: handle EAGAIN, EINTR and short writes correctly. Also clean up some error cases, avoid ... Original commit message from CVS: * plugins/elements/gstfdsink.c: (gst_fd_sink_render), (gst_fd_sink_check_fd), (gst_fd_sink_update_fd): handle EAGAIN, EINTR and short writes correctly. Also clean up some error cases, avoid a deadlock on bad file descriptors and use GST_DEBUG_OBJECT. Fixes #339843 --- ChangeLog | 9 ++++ plugins/elements/gstfdsink.c | 86 +++++++++++++++++++++++++----------- 2 files changed, 69 insertions(+), 26 deletions(-) diff --git a/ChangeLog b/ChangeLog index f041955297..201261b771 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,12 @@ +2006-04-28 Wim Taymans + + * plugins/elements/gstfdsink.c: (gst_fd_sink_render), + (gst_fd_sink_check_fd), (gst_fd_sink_update_fd): + handle EAGAIN, EINTR and short writes correctly. Also clean + up some error cases, avoid a deadlock on bad file descriptors and + use GST_DEBUG_OBJECT. + Fixes #339843 + 2006-04-28 Wim Taymans * gst/gstvalue.c: (gst_value_serialize_buffer), diff --git a/plugins/elements/gstfdsink.c b/plugins/elements/gstfdsink.c index f04490c77f..373f7fbc0b 100644 --- a/plugins/elements/gstfdsink.c +++ b/plugins/elements/gstfdsink.c @@ -26,6 +26,12 @@ * @see_also: #GstFdSrc * * Write data to a unix file descriptor. + * + * This element will sycnhronize on the clock before writing the data on the + * socket. For file descriptors where this does not make sense (files, ...) the + * ::sync property can be used to disable synchronisation. + * + * Last reviewed on 2006-04-28 (0.10.6) */ #ifdef HAVE_CONFIG_H @@ -237,12 +243,20 @@ gst_fd_sink_render (GstBaseSink * sink, GstBuffer * buffer) fd_set writefds; gint retval; #endif + guint8 *data; + guint size; + gint written; fdsink = GST_FD_SINK (sink); g_return_val_if_fail (fdsink->fd >= 0, GST_FLOW_ERROR); + data = GST_BUFFER_DATA (buffer); + size = GST_BUFFER_SIZE (buffer); + +again: #ifndef HAVE_WIN32 + FD_ZERO (&readfds); FD_SET (READ_SOCKET (fdsink), &readfds); @@ -250,6 +264,8 @@ gst_fd_sink_render (GstBaseSink * sink, GstBuffer * buffer) FD_SET (fdsink->fd, &writefds); do { + GST_DEBUG_OBJECT (fdsink, "going into select, have %d bytes to write", + size); retval = select (FD_SETSIZE, &readfds, &writefds, NULL, NULL); } while ((retval == -1 && errno == EINTR)); @@ -273,20 +289,32 @@ gst_fd_sink_render (GstBaseSink * sink, GstBuffer * buffer) } #endif - if (GST_BUFFER_DATA (buffer)) { - guint bytes_written; + GST_DEBUG_OBJECT (fdsink, "writing %d bytes to file descriptor %d", size, + fdsink->fd); - GST_DEBUG ("writing %d bytes to file descriptor %d", - GST_BUFFER_SIZE (buffer), fdsink->fd); - /* FIXME: short writes are perfectly valid and may happen; also, - * we should probably handle EINTR and EAGAIN in a non-fatal way */ - bytes_written = - write (fdsink->fd, GST_BUFFER_DATA (buffer), GST_BUFFER_SIZE (buffer)); - fdsink->bytes_written += bytes_written; - if (bytes_written != GST_BUFFER_SIZE (buffer)) - goto write_error; + written = write (fdsink->fd, data, size); + + /* check for errors */ + if (G_UNLIKELY (written < 0)) { + /* try to write again on non-fatal errors */ + if (errno == EAGAIN || errno == EINTR) + goto again; + + /* else go to our error handler */ + goto write_error; } + /* all is fine when we get here */ + size -= written; + data += written; + fdsink->bytes_written += written; + + GST_DEBUG_OBJECT (fdsink, "wrote %d bytes, %d left", written, size); + + /* short write, select and try to write the remainder */ + if (G_UNLIKELY (size > 0)) + goto again; + return GST_FLOW_OK; #ifndef HAVE_WIN32 @@ -345,19 +373,22 @@ gst_fd_sink_check_fd (GstFdSink * fdsink, int fd) goto not_seekable; } } else - GST_DEBUG ("File descriptor \"%d\" is seekable", fd); + GST_DEBUG_OBJECT (fdsink, "File descriptor \"%d\" is seekable", fd); return TRUE; invalid: - GST_ELEMENT_ERROR (fdsink, RESOURCE, WRITE, - (_("File descriptor \"%d\" is not valid."), fd), - ("%s", g_strerror (errno))); - return FALSE; - + { + GST_ELEMENT_ERROR (fdsink, RESOURCE, WRITE, + (_("File descriptor \"%d\" is not valid."), fd), + ("%s", g_strerror (errno))); + return FALSE; + } not_seekable: - GST_DEBUG ("File descriptor \"%d\" is a pipe", fd); - return TRUE; + { + GST_DEBUG_OBJECT (fdsink, "File descriptor \"%d\" is a pipe", fd); + return TRUE; + } } static gboolean @@ -417,19 +448,22 @@ gst_fd_sink_update_fd (GstFdSink * fdsink, int new_fd) if (new_fd < 0) return FALSE; + if (!gst_fd_sink_check_fd (fdsink, new_fd)) + goto invalid; + + /* assign the fd */ GST_OBJECT_LOCK (fdsink); - - if (!gst_fd_sink_check_fd (fdsink, new_fd)) { - GST_OBJECT_UNLOCK (fdsink); - return FALSE; - } - fdsink->fd = new_fd; g_free (fdsink->uri); fdsink->uri = g_strdup_printf ("fd://%d", fdsink->fd); - GST_OBJECT_UNLOCK (fdsink); + return TRUE; + +invalid: + { + return FALSE; + } } static void