From b10414dfb44ff3830d829108bf46b1bd06d9ae0a Mon Sep 17 00:00:00 2001 From: Julien Moutte Date: Fri, 16 Jul 2004 10:48:52 +0000 Subject: [PATCH] sys/: Getting the 2 video sinks synchronized again. Using internal data pointer of the x(v)image to store image's dat... Original commit message from CVS: 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. --- ChangeLog | 17 +++++++ sys/ximage/ximagesink.c | 97 ++++++++++++++++++++++----------------- sys/ximage/ximagesink.h | 1 - sys/xvimage/xvimagesink.c | 42 ++++++++++------- sys/xvimage/xvimagesink.h | 1 - 5 files changed, 97 insertions(+), 61 deletions(-) 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; };