sys/ximage/ximagesink.c: Optimize images creation for both elements. We don't create...

Original commit message from CVS:
2004-07-21  Julien MOUTTE  <julien@moutte.net>

* sys/ximage/ximagesink.c: (gst_ximagesink_ximage_new),
(gst_ximagesink_renegotiate_size), (gst_ximagesink_sink_link),
(gst_ximagesink_chain), (gst_ximagesink_set_xwindow_id): Optimize
images creation for both elements. We don't create the image on caps
nego or renego, we just destroy the internal one if present if it does
not match the needs. The chain function takes care of creating a new
image when needed.
* sys/xvimage/xvimagesink.c: (gst_xvimagesink_xvimage_new),
(gst_xvimagesink_xwindow_decorate), (gst_xvimagesink_sink_link),
(gst_xvimagesink_chain), (gst_xvimagesink_buffer_alloc),
(gst_xvimagesink_set_xwindow_id): Additionally xvimage now contains
the image format information. The buffer pool checks for the context
image format and discard images with different formats.
* sys/xvimage/xvimagesink.h: Adding im_format in the xvimage structure.

diff --git a/ChangeLog b/ChangeLog
index 05ba98d..07477b8 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,20 @@
+2004-07-21  Julien MOUTTE  <julien@moutte.net>
+
+       * sys/ximage/ximagesink.c: (gst_ximagesink_ximage_new),
+       (gst_ximagesink_renegotiate_size), (gst_ximagesink_sink_link),
+       (gst_ximagesink_chain), (gst_ximagesink_set_xwindow_id): Optimize
+       images creation for both elements. We don't create the image on caps
+       nego or renego, we just destroy the internal one if present if it does
+       not match the needs. The chain function takes care of creating a new
+       image when needed.
+       * sys/xvimage/xvimagesink.c: (gst_xvimagesink_xvimage_new),
+       (gst_xvimagesink_xwindow_decorate), (gst_xvimagesink_sink_link),
+       (gst_xvimagesink_chain), (gst_xvimagesink_buffer_alloc),
+       (gst_xvimagesink_set_xwindow_id): Additionally xvimage now contains
+       the image format information. The buffer pool checks for the context
+       image format and discard images with different formats.
+       * sys/xvimage/xvimagesink.h: Adding im_format in the xvimage structure.
+
 2004-07-21  Thomas Vander Stichele  <thomas at apestaart dot org>
 
        * gst/ffmpegcolorspace/gstffmpegcolorspace.c:

diff --git a/sys/ximage/ximagesink.c b/sys/ximage/ximagesink.c
index 3480bbe..6f9d451 100644 (file)
--- a/sys/ximage/ximagesink.c
+++ b/sys/ximage/ximagesink.c
@@ -510,10 +510,7 @@ gst_ximagesink_renegotiate_size (GstXImageSink * ximagesink)
                   ximagesink->ximage->height))) {
         /* We renew our ximage only if size changed */
         gst_ximagesink_ximage_destroy (ximagesink, ximagesink->ximage);
-
-        ximagesink->ximage = gst_ximagesink_ximage_new (ximagesink,
-            GST_VIDEOSINK_WIDTH (ximagesink),
-            GST_VIDEOSINK_HEIGHT (ximagesink));
+        ximagesink->ximage = NULL;
       }
     } else {
       ximagesink->sw_scaling_failed = TRUE;
@@ -824,6 +821,8 @@ gst_ximagesink_sink_link (GstPad * pad, const GstCaps * caps)
   gst_structure_get_int (structure, "pixel_height", &ximagesink->pixel_height);
 
   /* Creating our window and our image */
+  g_assert (GST_VIDEOSINK_WIDTH (ximagesink) > 0);
+  g_assert (GST_VIDEOSINK_HEIGHT (ximagesink) > 0);
   if (!ximagesink->xwindow) {
     ximagesink->xwindow = gst_ximagesink_xwindow_new (ximagesink,
         GST_VIDEOSINK_WIDTH (ximagesink), GST_VIDEOSINK_HEIGHT (ximagesink));
@@ -834,18 +833,13 @@ gst_ximagesink_sink_link (GstPad * pad, const GstCaps * caps)
     }
   }
 
-  if ((ximagesink->ximage)
-      && ((GST_VIDEOSINK_WIDTH (ximagesink) != ximagesink->ximage->width)
-          || (GST_VIDEOSINK_HEIGHT (ximagesink) != ximagesink->ximage->height))) {
-    /* We renew our ximage only if size changed */
+  /* If our ximage has changed we destroy it, next chain iteration will create
+     a new one */
+  if ((ximagesink->ximage) &&
+      ((GST_VIDEOSINK_WIDTH (ximagesink) != ximagesink->ximage->width) ||
+          (GST_VIDEOSINK_HEIGHT (ximagesink) != ximagesink->ximage->height))) {
     gst_ximagesink_ximage_destroy (ximagesink, ximagesink->ximage);
-
-    ximagesink->ximage = gst_ximagesink_ximage_new (ximagesink,
-        GST_VIDEOSINK_WIDTH (ximagesink), GST_VIDEOSINK_HEIGHT (ximagesink));
-  } else if (!ximagesink->ximage) {
-    /* If no ximage, creating one */
-    ximagesink->ximage = gst_ximagesink_ximage_new (ximagesink,
-        GST_VIDEOSINK_WIDTH (ximagesink), GST_VIDEOSINK_HEIGHT (ximagesink));
+    ximagesink->ximage = NULL;
   }
 
   gst_x_overlay_got_desired_size (GST_X_OVERLAY (ximagesink),
@@ -949,18 +943,22 @@ gst_ximagesink_chain (GstPad * pad, GstData * data)
   } else {
     /* Else we have to copy the data into our private image, */
     /* if we have one... */
-    if (ximagesink->ximage) {
-      memcpy (ximagesink->ximage->ximage->data,
-          GST_BUFFER_DATA (buf),
-          MIN (GST_BUFFER_SIZE (buf), ximagesink->ximage->size));
-      gst_ximagesink_ximage_put (ximagesink, ximagesink->ximage);
-    } else {
-      /* No image available. Something went wrong during capsnego ! */
-      gst_buffer_unref (buf);
-      GST_ELEMENT_ERROR (ximagesink, CORE, NEGOTIATION, (NULL),
-          ("no format defined before chain function"));
-      return;
+    if (!ximagesink->ximage) {
+      ximagesink->ximage = gst_ximagesink_ximage_new (ximagesink,
+          GST_VIDEOSINK_WIDTH (ximagesink), GST_VIDEOSINK_HEIGHT (ximagesink));
+      if (!ximagesink->ximage) {
+        /* No image available. That's very bad ! */
+        gst_buffer_unref (buf);
+        GST_ELEMENT_ERROR (ximagesink, CORE, NEGOTIATION, (NULL),
+            ("Failed creating an XImage in ximagesink chain function."));
+        return;
+      }
     }
+
+    memcpy (ximagesink->ximage->ximage->data,
+        GST_BUFFER_DATA (buf),
+        MIN (GST_BUFFER_SIZE (buf), ximagesink->ximage->size));
+    gst_ximagesink_ximage_put (ximagesink, ximagesink->ximage);
   }
 
   /* set correct time for next buffer */
@@ -1199,13 +1197,6 @@ gst_ximagesink_set_xwindow_id (GstXOverlay * overlay, XID xwindow_id)
     }
   }
 
-  /* Recreating our ximage */
-  if (!ximagesink->ximage &&
-      GST_VIDEOSINK_WIDTH (ximagesink) && GST_VIDEOSINK_HEIGHT (ximagesink)) {
-    ximagesink->ximage = gst_ximagesink_ximage_new (ximagesink,
-        GST_VIDEOSINK_WIDTH (ximagesink), GST_VIDEOSINK_HEIGHT (ximagesink));
-  }
-
   if (xwindow)
     ximagesink->xwindow = xwindow;
 }

diff --git a/sys/xvimage/xvimagesink.c b/sys/xvimage/xvimagesink.c
index 22e6688..d6648c6 100644 (file)
--- a/sys/xvimage/xvimagesink.c
+++ b/sys/xvimage/xvimagesink.c
@@ -178,6 +178,7 @@ gst_xvimagesink_xvimage_new (GstXvImageSink * xvimagesink,
 
   xvimage->width = width;
   xvimage->height = height;
+  xvimage->im_format = xvimagesink->xcontext->im_format;
   xvimage->xvimagesink = xvimagesink;
 
   g_mutex_lock (xvimagesink->x_lock);
@@ -189,8 +190,8 @@ gst_xvimagesink_xvimage_new (GstXvImageSink * xvimagesink,
   if (xvimagesink->xcontext->use_xshm) {
     xvimage->xvimage = XvShmCreateImage (xvimagesink->xcontext->disp,
         xvimagesink->xcontext->xv_port_id,
-        xvimagesink->xcontext->im_format,
-        NULL, xvimage->width, xvimage->height, &xvimage->SHMInfo);
+        xvimage->im_format, NULL,
+        xvimage->width, xvimage->height, &xvimage->SHMInfo);
 
     xvimage->SHMInfo.shmid = shmget (IPC_PRIVATE, xvimage->size,
         IPC_CREAT | 0777);
@@ -212,8 +213,7 @@ gst_xvimagesink_xvimage_new (GstXvImageSink * xvimagesink,
     /* We use image's internal data pointer */
     xvimage->xvimage = XvCreateImage (xvimagesink->xcontext->disp,
         xvimagesink->xcontext->xv_port_id,
-        xvimagesink->xcontext->im_format,
-        NULL, xvimage->width, xvimage->height);
+        xvimage->im_format, NULL, xvimage->width, xvimage->height);
 
     /* Allocating memory for image's data */
     xvimage->xvimage->data = g_malloc (xvimage->xvimage->data_size);
@@ -332,13 +332,12 @@ gst_xvimagesink_xwindow_decorate (GstXvImageSink * xvimagesink,
   g_mutex_lock (xvimagesink->x_lock);
 
   hints_atom = XInternAtom (xvimagesink->xcontext->disp, "_MOTIF_WM_HINTS", 1);
-
-  hints = g_malloc0 (sizeof (MotifWmHints));
-
-  if (!hints) {
+  if (hints_atom == None) {
     return FALSE;
   }
 
+  hints = g_malloc0 (sizeof (MotifWmHints));
+
   hints->flags |= MWM_HINTS_DECORATIONS;
   hints->decorations = 1 << 0;
 
@@ -1124,7 +1123,7 @@ gst_xvimagesink_sink_link (GstPad * pad, const GstCaps * caps)
 
   /* We renew our xvimage only if size or format changed */
   if ((xvimagesink->xvimage) &&
-      ((xvimagesink->xcontext->im_format != im_format) ||
+      ((im_format != xvimagesink->xvimage->im_format) ||
           (GST_VIDEOSINK_WIDTH (xvimagesink) != xvimagesink->xvimage->width) ||
           (GST_VIDEOSINK_HEIGHT (xvimagesink) !=
               xvimagesink->xvimage->height))) {
@@ -1134,17 +1133,11 @@ gst_xvimagesink_sink_link (GstPad * pad, const GstCaps * caps)
         GST_FOURCC_ARGS (im_format));
     GST_DEBUG_OBJECT (xvimagesink, "renewing xvimage");
     gst_xvimagesink_xvimage_destroy (xvimagesink, xvimagesink->xvimage);
-
-    xvimagesink->xcontext->im_format = im_format;
-    xvimagesink->xvimage = gst_xvimagesink_xvimage_new (xvimagesink,
-        GST_VIDEOSINK_WIDTH (xvimagesink), GST_VIDEOSINK_HEIGHT (xvimagesink));
-  } else if (!xvimagesink->xvimage) {
-    /* If no xvimage, creating one */
-    xvimagesink->xcontext->im_format = im_format;
-    xvimagesink->xvimage = gst_xvimagesink_xvimage_new (xvimagesink,
-        GST_VIDEOSINK_WIDTH (xvimagesink), GST_VIDEOSINK_HEIGHT (xvimagesink));
+    xvimagesink->xvimage = NULL;
   }
 
+  xvimagesink->xcontext->im_format = im_format;
+
   gst_x_overlay_got_desired_size (GST_X_OVERLAY (xvimagesink),
       GST_VIDEOSINK_WIDTH (xvimagesink), GST_VIDEOSINK_HEIGHT (xvimagesink));
 
@@ -1246,18 +1239,23 @@ gst_xvimagesink_chain (GstPad * pad, GstData * data)
   } else {
     /* Else we have to copy the data into our private image, */
     /* if we have one... */
-    if (xvimagesink->xvimage) {
-      memcpy (xvimagesink->xvimage->xvimage->data,
-          GST_BUFFER_DATA (buf),
-          MIN (GST_BUFFER_SIZE (buf), xvimagesink->xvimage->size));
-      gst_xvimagesink_xvimage_put (xvimagesink, xvimagesink->xvimage);
-    } else {
-      /* No image available. Something went wrong during capsnego ! */
-      gst_buffer_unref (buf);
-      GST_ELEMENT_ERROR (xvimagesink, CORE, NEGOTIATION, (NULL),
-          ("no format defined before chain function"));
-      return;
+    if (!xvimagesink->xvimage) {
+      xvimagesink->xvimage = gst_xvimagesink_xvimage_new (xvimagesink,
+          GST_VIDEOSINK_WIDTH (xvimagesink),
+          GST_VIDEOSINK_HEIGHT (xvimagesink));
+      if (!xvimagesink->xvimage) {
+        /* No image available. That's very bad ! */
+        gst_buffer_unref (buf);
+        GST_ELEMENT_ERROR (xvimagesink, CORE, NEGOTIATION, (NULL),
+            ("Failed creating an XvImage in xvimagesink chain function."));
+        return;
+      }
     }
+
+    memcpy (xvimagesink->xvimage->xvimage->data,
+        GST_BUFFER_DATA (buf),
+        MIN (GST_BUFFER_SIZE (buf), xvimagesink->xvimage->size));
+    gst_xvimagesink_xvimage_put (xvimagesink, xvimagesink->xvimage);
   }
 
   /* set correct time for next buffer */
@@ -1318,8 +1316,10 @@ gst_xvimagesink_buffer_alloc (GstPad * pad, guint64 offset, guint size)
       xvimagesink->image_pool = g_slist_delete_link (xvimagesink->image_pool,
           xvimagesink->image_pool);
 
+      /* We check for geometry or image format changes */
       if ((xvimage->width != GST_VIDEOSINK_WIDTH (xvimagesink)) ||
-          (xvimage->height != GST_VIDEOSINK_HEIGHT (xvimagesink))) {
+          (xvimage->height != GST_VIDEOSINK_HEIGHT (xvimagesink)) ||
+          (xvimage->im_format != xvimagesink->xcontext->im_format)) {
         /* This image is unusable. Destroying... */
         gst_xvimagesink_xvimage_destroy (xvimagesink, xvimage);
         xvimage = NULL;
@@ -1467,13 +1467,6 @@ gst_xvimagesink_set_xwindow_id (GstXOverlay * overlay, XID xwindow_id)
     g_mutex_unlock (xvimagesink->x_lock);
   }
 
-  /* Recreating our xvimage */
-  if (!xvimagesink->xvimage &&
-      GST_VIDEOSINK_WIDTH (xvimagesink) && GST_VIDEOSINK_HEIGHT (xvimagesink)) {
-    xvimagesink->xvimage = gst_xvimagesink_xvimage_new (xvimagesink,
-        GST_VIDEOSINK_WIDTH (xvimagesink), GST_VIDEOSINK_HEIGHT (xvimagesink));
-  }
-
   if (xwindow)
     xvimagesink->xwindow = xwindow;
 }

diff --git a/sys/xvimage/xvimagesink.h b/sys/xvimage/xvimagesink.h
index 0935762..b043367 100644 (file)
--- a/sys/xvimage/xvimagesink.h
+++ b/sys/xvimage/xvimagesink.h
@@ -115,7 +115,7 @@ struct _GstXvImage {
   XShmSegmentInfo SHMInfo;
 #endif /* HAVE_XSHM */
 
-  gint width, height, size;
+  gint width, height, size, im_format;
 };
 
 struct _GstXvImageSink {