further cleanups, fixes, logging and error handling

Original commit message from CVS:
further cleanups, fixes, logging and error handling

sys/ximage/ximagesink.c

@@ -46,6 +46,8 @@ MotifWmHints, MwmHints;
 #define MWM_HINTS_DECORATIONS   (1L << 1)
 
 static void gst_ximagesink_buffer_free (GstBuffer * buffer);
+static void gst_ximagesink_ximage_destroy (GstXImageSink * ximagesink,
+    GstXImage * ximage);
 
 /* ElementFactory information */
 static GstElementDetails gst_ximagesink_details =
@@ -117,34 +119,47 @@ gst_ximagesink_check_xshm_calls (GstXContext * xcontext)
   handler = XSetErrorHandler (gst_ximagesink_handle_xerror);
 
   /* Trying to create a 1x1 picture */
+  GST_DEBUG ("XShmCreateImage of 1x1");
+
   ximage->ximage = XShmCreateImage (xcontext->disp, xcontext->visual,
       xcontext->depth, ZPixmap, NULL, &ximage->SHMInfo, 1, 1);
-  if (!ximage->ximage)
+  if (!ximage->ximage) {
-    goto out;
+    GST_WARNING ("could not XShmCreateImage a 1x1 image");
+    goto beach;
+  }
+  ximage->size = ximage->ximage->height * ximage->ximage->bytes_per_line;
 
-  ximage->size = ximage->ximage->bytes_per_line;
   ximage->SHMInfo.shmid = shmget (IPC_PRIVATE, ximage->size, IPC_CREAT | 0777);
+  if (ximage->SHMInfo.shmid == -1) {
+    GST_WARNING ("could not get shared memory of %d bytes", ximage->size);
+    goto beach;
+  }
+
   ximage->SHMInfo.shmaddr = shmat (ximage->SHMInfo.shmid, 0, 0);
+  if ((int) ximage->SHMInfo.shmaddr == -1) {
+    GST_WARNING ("Failed to shmat: %s", g_strerror (errno));
+    goto beach;
+  }
+
   ximage->ximage->data = ximage->SHMInfo.shmaddr;
   ximage->SHMInfo.readOnly = FALSE;
 
-  XShmAttach (xcontext->disp, &ximage->SHMInfo);
+  if (XShmAttach (xcontext->disp, &ximage->SHMInfo) == 0) {
+    GST_WARNING ("Failed to XShmAttach");
+    goto beach;
+  }
 
   XSync (xcontext->disp, 0);
 
-  if (error_caught) {
+  XShmDetach (xcontext->disp, &ximage->SHMInfo);
-    /* Failed, detaching shared memory, destroying image and telling we can't
+  shmdt (ximage->SHMInfo.shmaddr);
-       use XShm */
+  shmctl (ximage->SHMInfo.shmid, IPC_RMID, 0);
-    error_caught = FALSE;
+
-    shmdt (ximage->SHMInfo.shmaddr);
+  /* store whether we succeeded in result and reset error_caught */
-    shmctl (ximage->SHMInfo.shmid, IPC_RMID, 0);
+  result = !error_caught;
-  } else {
+  error_caught = FALSE;
-    XShmDetach (xcontext->disp, &ximage->SHMInfo);
+
-    shmdt (ximage->SHMInfo.shmaddr);
+beach:
-    shmctl (ximage->SHMInfo.shmid, IPC_RMID, 0);
-    result = TRUE;
-  }
-out:
   XSetErrorHandler (handler);
   if (ximage->ximage)
     XFree (ximage->ximage);
@@ -159,6 +174,7 @@ static GstXImage *
 gst_ximagesink_ximage_new (GstXImageSink * ximagesink, gint width, gint height)
 {
   GstXImage *ximage = NULL;
+  gboolean succeeded = FALSE;
 
   g_return_val_if_fail (GST_IS_XIMAGESINK (ximagesink), NULL);
   GST_DEBUG_OBJECT (ximagesink, "creating %dx%d", width, height);
@@ -177,25 +193,42 @@ gst_ximagesink_ximage_new (GstXImageSink * ximagesink, gint width, gint height)
         ximagesink->xcontext->visual,
         ximagesink->xcontext->depth,
         ZPixmap, NULL, &ximage->SHMInfo, ximage->width, ximage->height);
-    /* we have to use the returned bytes_per_line, not our own calculation */
+    if (!ximage->ximage) {
+      GST_ELEMENT_ERROR (ximagesink, RESOURCE, WRITE, (NULL),
+          ("could not XShmCreateImage a %dx%d image"));
+      goto beach;
+    }
+
+    /* we have to use the returned bytes_per_line for our shm size */
     ximage->size = ximage->ximage->bytes_per_line * ximage->ximage->height;
-    GST_DEBUG_OBJECT (ximagesink, "XShm image size is %d, stride %d",
+    GST_DEBUG_OBJECT (ximagesink, "XShm image size is %d, width %d, stride %d",
-        ximage->size, ximage->ximage->bytes_per_line);
+        ximage->size, ximage->width, ximage->ximage->bytes_per_line);
 
     ximage->SHMInfo.shmid = shmget (IPC_PRIVATE, ximage->size,
         IPC_CREAT | 0777);
+    if (ximage->SHMInfo.shmid == -1) {
+      GST_ELEMENT_ERROR (ximagesink, RESOURCE, WRITE, (NULL),
+          ("could not get shared memory of %d bytes", ximage->size));
+      goto beach;
+    }
 
     ximage->SHMInfo.shmaddr = shmat (ximage->SHMInfo.shmid, 0, 0);
-    ximage->ximage->data = ximage->SHMInfo.shmaddr;
+    if ((int) ximage->SHMInfo.shmaddr == -1) {
+      GST_ELEMENT_ERROR (ximagesink, RESOURCE, WRITE, (NULL),
+          ("Failed to shmat: %s", g_strerror (errno)));
+      goto beach;
+    }
 
+    ximage->ximage->data = ximage->SHMInfo.shmaddr;
     ximage->SHMInfo.readOnly = FALSE;
 
-    XShmAttach (ximagesink->xcontext->disp, &ximage->SHMInfo);
+    if (XShmAttach (ximagesink->xcontext->disp, &ximage->SHMInfo) == 0) {
+      GST_ELEMENT_ERROR (ximagesink, RESOURCE, WRITE, (NULL),
+          ("Failed to XShmAttach"));
+      goto beach;
+    }
 
     XSync (ximagesink->xcontext->disp, FALSE);
-
-    shmctl (ximage->SHMInfo.shmid, IPC_RMID, 0);
-    ximage->SHMInfo.shmid = -1;
   } else
 #endif /* HAVE_XSHM */
   {
@@ -204,23 +237,27 @@ gst_ximagesink_ximage_new (GstXImageSink * ximagesink, gint width, gint height)
         ximagesink->xcontext->depth,
         ZPixmap, 0, NULL,
         ximage->width, ximage->height, ximagesink->xcontext->bpp, 0);
+    if (!ximage->ximage) {
+      GST_ELEMENT_ERROR (ximagesink, RESOURCE, WRITE, (NULL),
+          ("could not XCreateImage a %dx%d image"));
+      goto beach;
+    }
 
+    /* we have to use the returned bytes_per_line for our image size */
     ximage->size = ximage->ximage->bytes_per_line * ximage->ximage->height;
     ximage->ximage->data = g_malloc (ximage->size);
 
     XSync (ximagesink->xcontext->disp, FALSE);
   }
+  succeeded = TRUE;
+  g_mutex_unlock (ximagesink->x_lock);
 
-  if (!ximage->ximage) {
+beach:
-    if (ximage->ximage->data) {
+  if (!succeeded) {
-      g_free (ximage->ximage->data);
+    gst_ximagesink_ximage_destroy (ximagesink, ximage);
-    }
-    g_free (ximage);
     ximage = NULL;
   }
 
-  g_mutex_unlock (ximagesink->x_lock);
-
   return ximage;
 }
 
@@ -239,21 +276,22 @@ gst_ximagesink_ximage_destroy (GstXImageSink * ximagesink, GstXImage * ximage)
 
 #ifdef HAVE_XSHM
   if (ximagesink->xcontext->use_xshm) {
-    if (ximage->SHMInfo.shmaddr)
+    if ((int) ximage->SHMInfo.shmaddr != -1) {
       XShmDetach (ximagesink->xcontext->disp, &ximage->SHMInfo);
-
+      shmdt (ximage->SHMInfo.shmaddr);
+    }
+    if (ximage->SHMInfo.shmid > 0)
+      shmctl (ximage->SHMInfo.shmid, IPC_RMID, 0);
     if (ximage->ximage)
       XDestroyImage (ximage->ximage);
 
-    if (ximage->SHMInfo.shmaddr)
-      shmdt (ximage->SHMInfo.shmaddr);
-
-    if (ximage->SHMInfo.shmid > 0)
-      shmctl (ximage->SHMInfo.shmid, IPC_RMID, 0);
   } else
 #endif /* HAVE_XSHM */
   {
     if (ximage->ximage) {
+      if (ximage->ximage->data) {
+        g_free (ximage->ximage->data);
+      }
       XDestroyImage (ximage->ximage);
     }
   }

sys/xvimage/xvimagesink.c

@@ -47,6 +47,9 @@ MotifWmHints, MwmHints;
 #define MWM_HINTS_DECORATIONS   (1L << 1)
 
 static void gst_xvimagesink_buffer_free (GstBuffer * buffer);
+static void gst_xvimagesink_xvimage_destroy (GstXvImageSink * xvimagesink,
+    GstXvImage * xvimage);
+
 
 /* ElementFactory information */
 static GstElementDetails gst_xvimagesink_details =
@@ -126,35 +129,47 @@ gst_xvimagesink_check_xshm_calls (GstXContext * xcontext)
   handler = XSetErrorHandler (gst_xvimagesink_handle_xerror);
 
   /* Trying to create a 1x1 picture */
+  GST_DEBUG ("XvShmCreateImage of 1x1");
   xvimage->xvimage = XvShmCreateImage (xcontext->disp, xcontext->xv_port_id,
       xcontext->im_format, NULL, 1, 1, &xvimage->SHMInfo);
-  if (!xvimage->xvimage)
+  if (!xvimage->xvimage) {
-    goto out;
+    GST_WARNING ("could not XvShmCreateImage a 1x1 image");
+    goto beach;
+  }
+  xvimage->size = xvimage->xvimage->data_size;
 
-  xvimage->size = xvimage->xvimage->bytes_per_line;
   xvimage->SHMInfo.shmid = shmget (IPC_PRIVATE, xvimage->size,
       IPC_CREAT | 0777);
+  if (xvimage->SHMInfo.shmid == -1) {
+    GST_WARNING ("could not get shared memory of %d bytes", xvimage->size);
+    goto beach;
+  }
+
   xvimage->SHMInfo.shmaddr = shmat (xvimage->SHMInfo.shmid, 0, 0);
+  if ((int) xvimage->SHMInfo.shmaddr == -1) {
+    GST_WARNING ("Failed to shmat: %s", g_strerror (errno));
+    goto beach;
+  }
+
   xvimage->xvimage->data = xvimage->SHMInfo.shmaddr;
   xvimage->SHMInfo.readOnly = FALSE;
 
-  XShmAttach (xcontext->disp, &xvimage->SHMInfo);
+  if (XShmAttach (xcontext->disp, &xvimage->SHMInfo) == 0) {
+    GST_WARNING ("Failed to XShmAttach");
+    goto beach;
+  }
 
   XSync (xcontext->disp, 0);
 
-  if (error_caught) {
+  XShmDetach (xcontext->disp, &xvimage->SHMInfo);
-    /* Failed, detaching shared memory, destroying image and telling we can't
+  shmdt (xvimage->SHMInfo.shmaddr);
-       use XShm */
+  shmctl (xvimage->SHMInfo.shmid, IPC_RMID, 0);
-    error_caught = FALSE;
+
-    shmdt (xvimage->SHMInfo.shmaddr);
+  /* store whether we succeeded in result and reset error_caught */
-    shmctl (xvimage->SHMInfo.shmid, IPC_RMID, 0);
+  result = !error_caught;
-  } else {
+  error_caught = FALSE;
-    XShmDetach (xcontext->disp, &xvimage->SHMInfo);
+
-    shmdt (xvimage->SHMInfo.shmaddr);
+beach:
-    shmctl (xvimage->SHMInfo.shmid, IPC_RMID, 0);
-    result = TRUE;
-  }
-out:
   XSetErrorHandler (handler);
   if (xvimage->xvimage)
     XFree (xvimage->xvimage);
@@ -170,6 +185,7 @@ gst_xvimagesink_xvimage_new (GstXvImageSink * xvimagesink,
     gint width, gint height)
 {
   GstXvImage *xvimage = NULL;
+  gboolean succeeded = FALSE;
 
   g_return_val_if_fail (GST_IS_XVIMAGESINK (xvimagesink), NULL);
   GST_DEBUG_OBJECT (xvimagesink, "creating %dx%d", width, height);
@@ -189,49 +205,68 @@ gst_xvimagesink_xvimage_new (GstXvImageSink * xvimagesink,
         xvimagesink->xcontext->xv_port_id,
         xvimage->im_format, NULL,
         xvimage->width, xvimage->height, &xvimage->SHMInfo);
-    /* we have to use the returned data_size, not our own calculation */
+    if (!xvimage->xvimage) {
+      GST_ELEMENT_ERROR (xvimagesink, RESOURCE, WRITE, (NULL),
+          ("could not XvShmCreateImage a %dx%d image"));
+      goto beach;
+    }
+
+    /* we have to use the returned data_size for our shm size */
     xvimage->size = xvimage->xvimage->data_size;
     GST_DEBUG_OBJECT (xvimagesink, "XShm image size is %d", xvimage->size);
 
     xvimage->SHMInfo.shmid = shmget (IPC_PRIVATE, xvimage->size,
         IPC_CREAT | 0777);
+    if (xvimage->SHMInfo.shmid == -1) {
+      GST_ELEMENT_ERROR (xvimagesink, RESOURCE, WRITE, (NULL),
+          ("could not get shared memory of %d bytes", xvimage->size));
+      goto beach;
+    }
 
     xvimage->SHMInfo.shmaddr = shmat (xvimage->SHMInfo.shmid, 0, 0);
-    xvimage->xvimage->data = xvimage->SHMInfo.shmaddr;
+    if ((int) xvimage->SHMInfo.shmaddr == -1) {
+      GST_ELEMENT_ERROR (xvimagesink, RESOURCE, WRITE, (NULL),
+          ("Failed to shmat: %s", g_strerror (errno)));
+      goto beach;
+    }
 
+    xvimage->xvimage->data = xvimage->SHMInfo.shmaddr;
     xvimage->SHMInfo.readOnly = FALSE;
 
-    XShmAttach (xvimagesink->xcontext->disp, &xvimage->SHMInfo);
+    if (XShmAttach (xvimagesink->xcontext->disp, &xvimage->SHMInfo) == 0) {
+      GST_ELEMENT_ERROR (xvimagesink, RESOURCE, WRITE, (NULL),
+          ("Failed to XShmAttach"));
+      goto beach;
+    }
 
     XSync (xvimagesink->xcontext->disp, FALSE);
-
-    shmctl (xvimage->SHMInfo.shmid, IPC_RMID, 0);
-    xvimage->SHMInfo.shmid = -1;
   } else
 #endif /* HAVE_XSHM */
   {
-    /* We use image's internal data pointer */
     xvimage->xvimage = XvCreateImage (xvimagesink->xcontext->disp,
         xvimagesink->xcontext->xv_port_id,
         xvimage->im_format, NULL, xvimage->width, xvimage->height);
+    if (!xvimage->xvimage) {
+      GST_ELEMENT_ERROR (xvimagesink, RESOURCE, WRITE, (NULL),
+          ("could not XvCreateImage a %dx%d image"));
+      goto beach;
+    }
 
-    /* Allocating memory for image's data */
+    /* we have to use the returned data_size for our image size */
     xvimage->size = xvimage->xvimage->data_size;
     xvimage->xvimage->data = g_malloc (xvimage->size);
 
     XSync (xvimagesink->xcontext->disp, FALSE);
   }
+  succeeded = TRUE;
+  g_mutex_unlock (xvimagesink->x_lock);
 
-  if (!xvimage->xvimage) {
+beach:
-    if (xvimage->xvimage->data) {
+  if (!succeeded) {
-      g_free (xvimage->xvimage->data);
+    gst_xvimagesink_xvimage_destroy (xvimagesink, xvimage);
-    }
-    g_free (xvimage);
     xvimage = NULL;
   }
 
-  g_mutex_unlock (xvimagesink->x_lock);
-
   return xvimage;
 }
 
@@ -251,26 +286,21 @@ gst_xvimagesink_xvimage_destroy (GstXvImageSink * xvimagesink,
 
 #ifdef HAVE_XSHM
   if (xvimagesink->xcontext->use_xshm) {
-    if (xvimage->SHMInfo.shmaddr)
+    if ((int) xvimage->SHMInfo.shmaddr != -1) {
       XShmDetach (xvimagesink->xcontext->disp, &xvimage->SHMInfo);
-
-    if (xvimage->xvimage)
-      XFree (xvimage->xvimage);
-
-    if (xvimage->SHMInfo.shmaddr)
       shmdt (xvimage->SHMInfo.shmaddr);
-
+    }
     if (xvimage->SHMInfo.shmid > 0)
       shmctl (xvimage->SHMInfo.shmid, IPC_RMID, 0);
+    if (xvimage->xvimage)
+      XFree (xvimage->xvimage);
   } else
 #endif /* HAVE_XSHM */
   {
     if (xvimage->xvimage) {
-      /* Freeing image data */
       if (xvimage->xvimage->data) {
         g_free (xvimage->xvimage->data);
       }
-      /* Freeing image itself */
       XFree (xvimage->xvimage);
     }
   }
@@ -298,6 +328,10 @@ gst_xvimagesink_xvimage_put (GstXvImageSink * xvimagesink, GstXvImage * xvimage)
   /* We scale to the window's geometry */
 #ifdef HAVE_XSHM
   if (xvimagesink->xcontext->use_xshm) {
+    GST_LOG_OBJECT (xvimagesink,
+        "XvShmPutImage with image %dx%d and window %dx%d",
+        xvimage->width, xvimage->height,
+        xvimagesink->xwindow->width, xvimagesink->xwindow->height);
     XvShmPutImage (xvimagesink->xcontext->disp,
         xvimagesink->xcontext->xv_port_id,
         xvimagesink->xwindow->win,