diff --git a/ChangeLog b/ChangeLog index 69cbcc6e9a..4f168a0170 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,20 @@ +2004-07-16 Julien MOUTTE + + * sys/ximage/ximagesink.c: (gst_ximagesink_handle_xerror), + (gst_ximagesink_check_xshm_calls), (gst_ximagesink_ximage_new), + (gst_ximagesink_ximage_destroy), (gst_ximagesink_sink_link), + (gst_ximagesink_chain), (gst_ximagesink_buffer_free), + (gst_ximagesink_buffer_alloc): + * sys/ximage/ximagesink.h: + * sys/xvimage/xvimagesink.c: (gst_xvimagesink_check_xshm_calls), + (gst_xvimagesink_xvimage_new), (gst_xvimagesink_xvimage_destroy), + (gst_xvimagesink_chain), (gst_xvimagesink_buffer_free), + (gst_xvimagesink_buffer_alloc): + * sys/xvimage/xvimagesink.h: Getting the 2 video sinks synchronized + again. Using internal data pointer of the x(v)image to store image's + data to be coherent with the buffer alloc mechanism. Investigated the + image destruction code to be sure that everything gets freed correctly. + 2004-07-16 Wim Taymans * gst/playback/gstdecodebin.c: (gst_decode_bin_get_type), diff --git a/sys/ximage/ximagesink.c b/sys/ximage/ximagesink.c index c7505b167c..b5d05f7cb5 100644 --- a/sys/ximage/ximagesink.c +++ b/sys/ximage/ximagesink.c @@ -74,7 +74,7 @@ enum }; static GstVideoSinkClass *parent_class = NULL; -static gboolean error_catched = FALSE; +static gboolean error_caught = FALSE; /* ============================================================= */ /* */ @@ -91,7 +91,7 @@ gst_ximagesink_handle_xerror (Display * display, XErrorEvent * xevent) XGetErrorText (display, xevent->error_code, error_msg, 1024); GST_DEBUG ("ximagesink failed to use XShm calls. error: %s", error_msg); - error_catched = TRUE; + error_caught = TRUE; return 0; } @@ -100,15 +100,20 @@ gst_ximagesink_handle_xerror (Display * display, XErrorEvent * xevent) static gboolean gst_ximagesink_check_xshm_calls (GstXContext * xcontext) { +#ifndef HAVE_XSHM + return FALSE; +#else GstXImage *ximage = NULL; int (*handler) (Display *, XErrorEvent *); + gboolean result = FALSE; g_return_val_if_fail (xcontext != NULL, FALSE); -#ifdef HAVE_XSHM ximage = g_new0 (GstXImage, 1); + g_return_val_if_fail (ximage != NULL, FALSE); /* Setting an error handler to catch failure */ + error_caught = FALSE; handler = XSetErrorHandler (gst_ximagesink_handle_xerror); ximage->size = (xcontext->bpp / 8); @@ -116,6 +121,8 @@ gst_ximagesink_check_xshm_calls (GstXContext * xcontext) /* Trying to create a 1x1 picture */ ximage->ximage = XShmCreateImage (xcontext->disp, xcontext->visual, xcontext->depth, ZPixmap, NULL, &ximage->SHMInfo, 1, 1); + if (!ximage->ximage) + goto out; ximage->SHMInfo.shmid = shmget (IPC_PRIVATE, ximage->size, IPC_CREAT | 0777); ximage->SHMInfo.shmaddr = shmat (ximage->SHMInfo.shmid, 0, 0); @@ -124,32 +131,28 @@ gst_ximagesink_check_xshm_calls (GstXContext * xcontext) XShmAttach (xcontext->disp, &ximage->SHMInfo); - error_catched = FALSE; - XSync (xcontext->disp, 0); - XSetErrorHandler (handler); - - if (error_catched) { /* Failed, detaching shared memory, destroying image and telling we can't - use XShm */ - error_catched = FALSE; - XDestroyImage (ximage->ximage); + if (error_caught) { + /* Failed, detaching shared memory, destroying image and telling we can't + use XShm */ + error_caught = FALSE; shmdt (ximage->SHMInfo.shmaddr); shmctl (ximage->SHMInfo.shmid, IPC_RMID, 0); - g_free (ximage); - XSync (xcontext->disp, FALSE); - return FALSE; } else { XShmDetach (xcontext->disp, &ximage->SHMInfo); - XDestroyImage (ximage->ximage); shmdt (ximage->SHMInfo.shmaddr); shmctl (ximage->SHMInfo.shmid, IPC_RMID, 0); - g_free (ximage); - XSync (xcontext->disp, FALSE); + result = TRUE; } +out: + XSetErrorHandler (handler); + if (ximage->ximage) + XFree (ximage->ximage); + g_free (ximage); + XSync (xcontext->disp, FALSE); + return result; #endif /* HAVE_XSHM */ - - return TRUE; } /* This function handles GstXImage creation depending on XShm availability */ @@ -164,7 +167,6 @@ gst_ximagesink_ximage_new (GstXImageSink * ximagesink, gint width, gint height) ximage->width = width; ximage->height = height; - ximage->data = NULL; ximage->ximagesink = ximagesink; g_mutex_lock (ximagesink->x_lock); @@ -196,25 +198,24 @@ gst_ximagesink_ximage_new (GstXImageSink * ximagesink, gint width, gint height) } else #endif /* HAVE_XSHM */ { - ximage->data = g_malloc (ximage->size); - ximage->ximage = XCreateImage (ximagesink->xcontext->disp, ximagesink->xcontext->visual, ximagesink->xcontext->depth, - ZPixmap, 0, ximage->data, + ZPixmap, 0, NULL, ximage->width, ximage->height, ximagesink->xcontext->bpp, ximage->width * (ximagesink->xcontext->bpp / 8)); + ximage->ximage->data = g_malloc (ximage->size); + XSync (ximagesink->xcontext->disp, FALSE); } if (!ximage->ximage) { - if (ximage->data) - g_free (ximage->data); - + if (ximage->ximage->data) { + g_free (ximage->ximage->data); + } g_free (ximage); - ximage = NULL; } @@ -252,8 +253,9 @@ gst_ximagesink_ximage_destroy (GstXImageSink * ximagesink, GstXImage * ximage) } else #endif /* HAVE_XSHM */ { - if (ximage->ximage) + if (ximage->ximage) { XDestroyImage (ximage->ximage); + } } XSync (ximagesink->xcontext->disp, FALSE); @@ -823,23 +825,30 @@ 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 */ - if (!ximagesink->xwindow) + if (!ximagesink->xwindow) { ximagesink->xwindow = gst_ximagesink_xwindow_new (ximagesink, GST_VIDEOSINK_WIDTH (ximagesink), GST_VIDEOSINK_HEIGHT (ximagesink)); - else { - if (ximagesink->xwindow->internal) + } else { + if (ximagesink->xwindow->internal) { gst_ximagesink_xwindow_resize (ximagesink, ximagesink->xwindow, GST_VIDEOSINK_WIDTH (ximagesink), GST_VIDEOSINK_HEIGHT (ximagesink)); + } } - 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 ((ximagesink->ximage) + && ((GST_VIDEOSINK_WIDTH (ximagesink) != ximagesink->ximage->width) + || (GST_VIDEOSINK_HEIGHT (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)); - } else if (!ximagesink->ximage) /* If no ximage, creating one */ + } else if (!ximagesink->ximage) { + /* If no ximage, creating one */ ximagesink->ximage = gst_ximagesink_ximage_new (ximagesink, GST_VIDEOSINK_WIDTH (ximagesink), GST_VIDEOSINK_HEIGHT (ximagesink)); + } gst_x_overlay_got_desired_size (GST_X_OVERLAY (ximagesink), GST_VIDEOSINK_WIDTH (ximagesink), GST_VIDEOSINK_HEIGHT (ximagesink)); @@ -935,15 +944,16 @@ gst_ximagesink_chain (GstPad * pad, GstData * data) put the ximage which is in the PRIVATE pointer */ if (GST_BUFFER_FREE_DATA_FUNC (buf) == gst_ximagesink_buffer_free) { gst_ximagesink_ximage_put (ximagesink, GST_BUFFER_PRIVATE (buf)); - } else { /* Else we have to copy the data into our private image, */ + } 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 ! */ - + } 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")); @@ -978,8 +988,8 @@ gst_ximagesink_buffer_free (GstBuffer * buffer) if ((ximage->width != GST_VIDEOSINK_WIDTH (ximagesink)) || (ximage->height != GST_VIDEOSINK_HEIGHT (ximagesink))) gst_ximagesink_ximage_destroy (ximagesink, ximage); - else { /* In that case we can reuse the image and add it to our image pool. */ - + else { + /* In that case we can reuse the image and add it to our image pool. */ g_mutex_lock (ximagesink->pool_lock); ximagesink->image_pool = g_slist_prepend (ximagesink->image_pool, ximage); g_mutex_unlock (ximagesink->pool_lock); @@ -1008,11 +1018,13 @@ gst_ximagesink_buffer_alloc (GstPad * pad, guint64 offset, guint size) ximagesink->image_pool = g_slist_delete_link (ximagesink->image_pool, ximagesink->image_pool); - if ((ximage->width != GST_VIDEOSINK_WIDTH (ximagesink)) || (ximage->height != GST_VIDEOSINK_HEIGHT (ximagesink))) { /* This image is unusable. Destroying... */ + if ((ximage->width != GST_VIDEOSINK_WIDTH (ximagesink)) || + (ximage->height != GST_VIDEOSINK_HEIGHT (ximagesink))) { + /* This image is unusable. Destroying... */ gst_ximagesink_ximage_destroy (ximagesink, ximage); ximage = NULL; - } else { /* We found a suitable image */ - + } else { + /* We found a suitable image */ break; } } @@ -1020,7 +1032,8 @@ gst_ximagesink_buffer_alloc (GstPad * pad, guint64 offset, guint size) g_mutex_unlock (ximagesink->pool_lock); - if (!ximage) { /* We found no suitable image in the pool. Creating... */ + if (!ximage) { + /* We found no suitable image in the pool. Creating... */ ximage = gst_ximagesink_ximage_new (ximagesink, GST_VIDEOSINK_WIDTH (ximagesink), GST_VIDEOSINK_HEIGHT (ximagesink)); } diff --git a/sys/ximage/ximagesink.h b/sys/ximage/ximagesink.h index bfacf60ace..74e773d1fe 100644 --- a/sys/ximage/ximagesink.h +++ b/sys/ximage/ximagesink.h @@ -99,7 +99,6 @@ struct _GstXImage { XShmSegmentInfo SHMInfo; #endif /* HAVE_XSHM */ - char *data; gint width, height, size; }; diff --git a/sys/xvimage/xvimagesink.c b/sys/xvimage/xvimagesink.c index 4978701666..7b16cd5f21 100644 --- a/sys/xvimage/xvimagesink.c +++ b/sys/xvimage/xvimagesink.c @@ -119,6 +119,7 @@ gst_xvimagesink_check_xshm_calls (GstXContext * xcontext) g_return_val_if_fail (xcontext != NULL, FALSE); xvimage = g_new0 (GstXvImage, 1); + g_return_val_if_fail (xvimage != NULL, FALSE); /* Setting an error handler to catch failure */ error_caught = FALSE; @@ -148,7 +149,6 @@ gst_xvimagesink_check_xshm_calls (GstXContext * xcontext) error_caught = FALSE; shmdt (xvimage->SHMInfo.shmaddr); shmctl (xvimage->SHMInfo.shmid, IPC_RMID, 0); - XSync (xcontext->disp, FALSE); } else { XShmDetach (xcontext->disp, &xvimage->SHMInfo); shmdt (xvimage->SHMInfo.shmaddr); @@ -178,7 +178,6 @@ gst_xvimagesink_xvimage_new (GstXvImageSink * xvimagesink, xvimage->width = width; xvimage->height = height; - xvimage->data = NULL; xvimage->xvimagesink = xvimagesink; g_mutex_lock (xvimagesink->x_lock); @@ -210,22 +209,23 @@ gst_xvimagesink_xvimage_new (GstXvImageSink * xvimagesink, } else #endif /* HAVE_XSHM */ { + /* We use image's internal data pointer */ xvimage->xvimage = XvCreateImage (xvimagesink->xcontext->disp, xvimagesink->xcontext->xv_port_id, xvimagesink->xcontext->im_format, - xvimage->data, xvimage->width, xvimage->height); + NULL, xvimage->width, xvimage->height); + /* Allocating memory for image's data */ xvimage->xvimage->data = g_malloc (xvimage->xvimage->data_size); XSync (xvimagesink->xcontext->disp, FALSE); } if (!xvimage->xvimage) { - if (xvimage->data) - g_free (xvimage->data); - + if (xvimage->xvimage->data) { + g_free (xvimage->xvimage->data); + } g_free (xvimage); - xvimage = NULL; } @@ -265,7 +265,11 @@ gst_xvimagesink_xvimage_destroy (GstXvImageSink * xvimagesink, #endif /* HAVE_XSHM */ { if (xvimage->xvimage) { - g_free (xvimage->xvimage->data); + /* Freeing image data */ + if (xvimage->xvimage->data) { + g_free (xvimage->xvimage->data); + } + /* Freeing image itself */ XFree (xvimage->xvimage); } } @@ -1235,15 +1239,16 @@ gst_xvimagesink_chain (GstPad * pad, GstData * data) put the ximage which is in the PRIVATE pointer */ if (GST_BUFFER_FREE_DATA_FUNC (buf) == gst_xvimagesink_buffer_free) { gst_xvimagesink_xvimage_put (xvimagesink, GST_BUFFER_PRIVATE (buf)); - } else { /* Else we have to copy the data into our private image, */ + } 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 ! */ - + } 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")); @@ -1278,8 +1283,8 @@ gst_xvimagesink_buffer_free (GstBuffer * buffer) if ((xvimage->width != GST_VIDEOSINK_WIDTH (xvimagesink)) || (xvimage->height != GST_VIDEOSINK_HEIGHT (xvimagesink))) gst_xvimagesink_xvimage_destroy (xvimagesink, xvimage); - else { /* In that case we can reuse the image and add it to our image pool. */ - + else { + /* In that case we can reuse the image and add it to our image pool. */ g_mutex_lock (xvimagesink->pool_lock); xvimagesink->image_pool = g_slist_prepend (xvimagesink->image_pool, xvimage); @@ -1309,11 +1314,13 @@ gst_xvimagesink_buffer_alloc (GstPad * pad, guint64 offset, guint size) xvimagesink->image_pool = g_slist_delete_link (xvimagesink->image_pool, xvimagesink->image_pool); - if ((xvimage->width != GST_VIDEOSINK_WIDTH (xvimagesink)) || (xvimage->height != GST_VIDEOSINK_HEIGHT (xvimagesink))) { /* This image is unusable. Destroying... */ + if ((xvimage->width != GST_VIDEOSINK_WIDTH (xvimagesink)) || + (xvimage->height != GST_VIDEOSINK_HEIGHT (xvimagesink))) { + /* This image is unusable. Destroying... */ gst_xvimagesink_xvimage_destroy (xvimagesink, xvimage); xvimage = NULL; - } else { /* We found a suitable image */ - + } else { + /* We found a suitable image */ break; } } @@ -1321,7 +1328,8 @@ gst_xvimagesink_buffer_alloc (GstPad * pad, guint64 offset, guint size) g_mutex_unlock (xvimagesink->pool_lock); - if (!xvimage) { /* We found no suitable image in the pool. Creating... */ + if (!xvimage) { + /* We found no suitable image in the pool. Creating... */ xvimage = gst_xvimagesink_xvimage_new (xvimagesink, GST_VIDEOSINK_WIDTH (xvimagesink), GST_VIDEOSINK_HEIGHT (xvimagesink)); } diff --git a/sys/xvimage/xvimagesink.h b/sys/xvimage/xvimagesink.h index 17c9001cf9..b3f382e9ee 100644 --- a/sys/xvimage/xvimagesink.h +++ b/sys/xvimage/xvimagesink.h @@ -115,7 +115,6 @@ struct _GstXvImage { XShmSegmentInfo SHMInfo; #endif /* HAVE_XSHM */ - char *data; gint width, height, size; };