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 05ba98d08906d6b64d6def86896430943737dc25..07477b84becd726781855447a39abbb0ced7246c 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 3480bbe5e4aaac5bfa8ddcf64482aa86b6f34d05..6f9d451772eb173f1fe3b09c19437099e82708f3 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 22e6688c03871f8052ca567c8c185820fa3b44d3..d6648c6a2569bd45e517492b3ffe2ce3a45a02c1 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 0935762ffaf515ca645f78f5b48eb979361aedac..b043367fd58eb79ef5aa557c66c659366079cb42 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 {