From e2f1b66fb2f73b8f5cdbc79127e986ad5ef4c889 Mon Sep 17 00:00:00 2001 From: Sjoerd Simons Date: Sat, 16 Dec 2006 16:21:26 +0000 Subject: gst/videomixer/videomixer.c: Introduce some locking around the videomixer state so that it does not crash when adding... Original commit message from CVS: Patch by: Sjoerd Simons * gst/videomixer/videomixer.c: (gst_videomixer_pad_set_property), (gst_videomixer_set_master_geometry), (gst_videomixer_pad_sink_setcaps), (gst_videomixer_collect_free), (gst_videomixer_reset), (gst_videomixer_init), (gst_videomixer_finalize), (gst_videomixer_request_new_pad), (gst_videomixer_release_pad), (gst_videomixer_collected), (gst_videomixer_change_state): Introduce some locking around the videomixer state so that it does not crash when adding/removing pads. Fixes #383043. --- gst/videomixer/videomixer.c | 261 ++++++++++++++++++-------------------------- 1 file changed, 109 insertions(+), 152 deletions(-) (limited to 'gst/videomixer') diff --git a/gst/videomixer/videomixer.c b/gst/videomixer/videomixer.c index a84225b3..fb9b8218 100644 --- a/gst/videomixer/videomixer.c +++ b/gst/videomixer/videomixer.c @@ -82,11 +82,73 @@ typedef struct _GstVideoMixerCollect GstVideoMixerCollect; #define GST_IS_VIDEO_MIXER_CLASS(klass) \ (G_TYPE_CHECK_CLASS_TYPE((klass),GST_TYPE_VIDEO_MIXER)) +#define GST_VIDEO_MIXER_GET_STATE_LOCK(mix) \ + (GST_VIDEO_MIXER(mix)->state_lock) +#define GST_VIDEO_MIXER_STATE_LOCK(mix) \ + (g_mutex_lock(GST_VIDEO_MIXER_GET_STATE_LOCK (mix))) +#define GST_VIDEO_MIXER_STATE_UNLOCK(mix) \ + (g_mutex_unlock(GST_VIDEO_MIXER_GET_STATE_LOCK (mix))) + static GType gst_videomixer_get_type (void); typedef struct _GstVideoMixer GstVideoMixer; typedef struct _GstVideoMixerClass GstVideoMixerClass; +/** + * GstVideoMixerBackground: + * @VIDEO_MIXER_BACKGROUND_CHECKER: checker pattern background + * @VIDEO_MIXER_BACKGROUND_BLACK: solid color black background + * @VIDEO_MIXER_BACKGROUND_WHITE: solid color white background + * + * The different backgrounds videomixer can blend over. + */ +typedef enum +{ + VIDEO_MIXER_BACKGROUND_CHECKER, + VIDEO_MIXER_BACKGROUND_BLACK, + VIDEO_MIXER_BACKGROUND_WHITE +} +GstVideoMixerBackground; + +/** + * GstVideoMixer: + * + * The opaque #GstVideoMixer structure. + */ +struct _GstVideoMixer +{ + GstElement element; + + /* pad */ + GstPad *srcpad; + + /* Lock to prevent the state to change while blending */ + GMutex *state_lock; + /* Sink pads using Collect Pads from core's base library */ + GstCollectPads *collect; + /* sinkpads, a GSList of GstVideoMixerPads */ + GSList *sinkpads; + + gint numpads; + + /* the master pad */ + GstVideoMixerPad *master; + + gint in_width, in_height; + gint out_width, out_height; + gboolean setcaps; + + GstVideoMixerBackground background; + + gint fps_n; + gint fps_d; +}; + +struct _GstVideoMixerClass +{ + GstElementClass parent_class; +}; + static void gst_videomixer_pad_base_init (gpointer g_class); static void gst_videomixer_pad_class_init (GstVideoMixerPadClass * klass); static void gst_videomixer_pad_init (GstVideoMixerPad * mixerpad); @@ -236,8 +298,10 @@ gst_videomixer_pad_set_property (GObject * object, guint prop_id, switch (prop_id) { case ARG_PAD_ZORDER: + GST_VIDEO_MIXER_STATE_LOCK (mix); pad->zorder = g_value_get_uint (value); gst_videomixer_sort_pads (mix); + GST_VIDEO_MIXER_STATE_UNLOCK (mix); break; case ARG_PAD_XPOS: pad->xpos = g_value_get_int (value); @@ -256,59 +320,6 @@ gst_videomixer_pad_set_property (GObject * object, guint prop_id, gst_object_unref (mix); } -/** - * GstVideoMixerBackground: - * @VIDEO_MIXER_BACKGROUND_CHECKER: checker pattern background - * @VIDEO_MIXER_BACKGROUND_BLACK: solid color black background - * @VIDEO_MIXER_BACKGROUND_WHITE: solid color white background - * - * The different backgrounds videomixer can blend over. - */ -typedef enum -{ - VIDEO_MIXER_BACKGROUND_CHECKER, - VIDEO_MIXER_BACKGROUND_BLACK, - VIDEO_MIXER_BACKGROUND_WHITE -} -GstVideoMixerBackground; - -/** - * GstVideoMixer: - * - * The opaque #GstVideoMixer structure. - */ -struct _GstVideoMixer -{ - GstElement element; - - /* pad */ - GstPad *srcpad; - - /* Sink pads using Collect Pads from core's base library */ - GstCollectPads *collect; - /* sinkpads, a GSList of GstVideoMixerPads */ - GSList *sinkpads; - - gint numpads; - - /* the master pad */ - GstVideoMixerPad *master; - - gint in_width, in_height; - gint out_width, out_height; - gboolean setcaps; - - GstVideoMixerBackground background; - - gint fps_n; - gint fps_d; -}; - -struct _GstVideoMixerClass -{ - GstElementClass parent_class; -}; - static void gst_videomixer_set_master_geometry (GstVideoMixer * mix) { @@ -376,6 +387,7 @@ gst_videomixer_pad_sink_setcaps (GstPad * pad, GstCaps * vscaps) || (framerate = gst_structure_get_value (structure, "framerate")) == NULL) goto beach; + GST_VIDEO_MIXER_STATE_LOCK (mix); mixpad->fps_n = gst_value_get_fraction_numerator (framerate); mixpad->fps_d = gst_value_get_fraction_denominator (framerate); @@ -383,6 +395,7 @@ gst_videomixer_pad_sink_setcaps (GstPad * pad, GstCaps * vscaps) mixpad->in_height = in_height; gst_videomixer_set_master_geometry (mix); + GST_VIDEO_MIXER_STATE_UNLOCK (mix); ret = TRUE; @@ -573,12 +586,8 @@ gst_videomixer_class_init (GstVideoMixerClass * klass) } static void -gst_videomixer_collect_free (GstCollectData * collect) +gst_videomixer_collect_free (GstVideoMixerCollect * mixcol) { - GstVideoMixerCollect *mixcol; - - mixcol = (GstVideoMixerCollect *) collect; - if (mixcol->buffer) { gst_buffer_unref (mixcol->buffer); mixcol->buffer = NULL; @@ -600,7 +609,7 @@ gst_videomixer_reset (GstVideoMixer * mix) /* clean up collect data */ walk = mix->collect->data; while (walk) { - GstCollectData *data = (GstCollectData *) walk->data; + GstVideoMixerCollect *data = (GstVideoMixerCollect *) walk->data; gst_videomixer_collect_free (data); walk = g_slist_next (walk); @@ -626,6 +635,7 @@ gst_videomixer_init (GstVideoMixer * mix) (GstCollectPadsFunction) GST_DEBUG_FUNCPTR (gst_videomixer_collected), mix); + mix->state_lock = g_mutex_new (); /* initialize variables */ gst_videomixer_reset (mix); @@ -637,6 +647,7 @@ gst_videomixer_finalize (GObject * object) GstVideoMixer *mix = GST_VIDEO_MIXER (object); gst_object_unref (mix->collect); + g_mutex_free (mix->state_lock); G_OBJECT_CLASS (parent_class)->finalize (object); } @@ -706,6 +717,7 @@ gst_videomixer_request_new_pad (GstElement * element, templ->direction, "template", templ, NULL); g_free (name); + GST_VIDEO_MIXER_STATE_LOCK (mix); mixpad->zorder = mix->numpads; mixpad->xpos = DEFAULT_PAD_XPOS; mixpad->ypos = DEFAULT_PAD_YPOS; @@ -722,6 +734,7 @@ gst_videomixer_request_new_pad (GstElement * element, /* Keep an internal list of mixpads for zordering */ mix->sinkpads = g_slist_append (mix->sinkpads, mixpad); mix->numpads++; + GST_VIDEO_MIXER_STATE_UNLOCK (mix); } else { g_warning ("videomixer: this is not our template!\n"); return NULL; @@ -738,31 +751,29 @@ static void gst_videomixer_release_pad (GstElement * element, GstPad * pad) { GstVideoMixer *mix = NULL; - GSList *walk = NULL; + GstVideoMixerPad *mixpad; mix = GST_VIDEO_MIXER (element); + GST_VIDEO_MIXER_STATE_LOCK (mix); + if (G_UNLIKELY (g_slist_find (mix->sinkpads, pad) == NULL)) { + g_warning ("Unknown pad %s", GST_PAD_NAME (pad)); + goto error; + } - walk = mix->collect->data; - while (walk) { - GstCollectData *data = (GstCollectData *) walk->data; - GstVideoMixerCollect *mixcol = (GstVideoMixerCollect *) data; - GstVideoMixerPad *mixpad = mixcol->mixpad; - - walk = g_slist_next (walk); + mixpad = GST_VIDEO_MIXER_PAD (pad); - if (mixpad == GST_VIDEO_MIXER_PAD (pad)) { - /* found it, remove from all sorts of lists */ - mix->sinkpads = g_slist_remove (mix->sinkpads, pad); - gst_videomixer_collect_free (data); - gst_collect_pads_remove_pad (mix->collect, pad); - gst_element_remove_pad (element, pad); - /* determine possibly new geometry and master */ - gst_videomixer_set_master_geometry (mix); - return; - } - } + mix->sinkpads = g_slist_remove (mix->sinkpads, pad); + gst_videomixer_collect_free (mixpad->mixcol); + gst_collect_pads_remove_pad (mix->collect, pad); + /* determine possibly new geometry and master */ + gst_videomixer_set_master_geometry (mix); + mix->numpads--; + GST_VIDEO_MIXER_STATE_UNLOCK (mix); - g_warning ("Unknown pad %s", GST_PAD_NAME (pad)); + gst_element_remove_pad (element, pad); + return; +error: + GST_VIDEO_MIXER_STATE_UNLOCK (mix); } #define BLEND_NORMAL(Y1,U1,V1,Y2,U2,V2,alpha,Y,U,V) \ @@ -1155,76 +1166,6 @@ gst_videomixer_update_queues (GstVideoMixer * mix) } } -/* - * The basic idea is to get a buffer on all pads and mix them together. - * Based on the framerate, buffers are removed from the queues to make room - * for a new buffer. - -static void -gst_videomixer_loop (GstElement * element) -{ - GstVideoMixer *mix; - GstBuffer *outbuf; - gint outsize; - gint new_width, new_height; - gboolean eos; - - mix = GST_VIDEO_MIXER (element); - - eos = gst_videomixer_fill_queues (mix); - if (eos) { - gst_pad_push (mix->srcpad, GST_DATA (gst_event_new (GST_EVENT_EOS))); - gst_element_set_eos (GST_ELEMENT (mix)); - return; - } - - new_width = mix->in_width; - new_height = mix->in_height; - - if (new_width != mix->out_width || - new_height != mix->out_height || !GST_PAD_CAPS (mix->srcpad)) { - GstCaps *newcaps; - - newcaps = gst_caps_make_writable ( - gst_pad_get_negotiated_caps (GST_PAD (mix->master))); - gst_caps_set_simple (newcaps, "format", GST_TYPE_FOURCC, - GST_STR_FOURCC ("AYUV"), "width", G_TYPE_INT, new_width, "height", - G_TYPE_INT, new_height, NULL); - - if (GST_PAD_LINK_FAILED (gst_pad_try_set_caps (mix->srcpad, newcaps))) { - GST_ELEMENT_ERROR (mix, CORE, NEGOTIATION, (NULL), (NULL)); - return; - } - - mix->out_width = new_width; - mix->out_height = new_height; - } - - outsize = ROUND_UP_2 (mix->out_width) * ROUND_UP_2 (mix->out_height) * 4; - - outbuf = gst_pad_alloc_buffer_and_set_caps (mix->srcpad, GST_BUFFER_OFFSET_NONE, outsize); - switch (mix->background) { - case VIDEO_MIXER_BACKGROUND_CHECKER: - gst_videomixer_fill_checker (GST_BUFFER_DATA (outbuf), - new_width, new_height); - break; - case VIDEO_MIXER_BACKGROUND_BLACK: - gst_videomixer_fill_color (GST_BUFFER_DATA (outbuf), - new_width, new_height, 16, 128, 128); - break; - case VIDEO_MIXER_BACKGROUND_WHITE: - gst_videomixer_fill_color (GST_BUFFER_DATA (outbuf), - new_width, new_height, 240, 128, 128); - break; - } - - gst_videomixer_blend_buffers (mix, outbuf); - - gst_videomixer_update_queues (mix); - - gst_pad_push (mix->srcpad, GST_DATA (outbuf)); -}*/ - static GstFlowReturn gst_videomixer_collected (GstCollectPads * pads, GstVideoMixer * mix) { @@ -1236,6 +1177,7 @@ gst_videomixer_collected (GstCollectPads * pads, GstVideoMixer * mix) g_return_val_if_fail (GST_IS_VIDEO_MIXER (mix), GST_FLOW_ERROR); GST_LOG ("all pads are collected"); + GST_VIDEO_MIXER_STATE_LOCK (mix); eos = gst_videomixer_fill_queues (mix); @@ -1244,7 +1186,7 @@ gst_videomixer_collected (GstCollectPads * pads, GstVideoMixer * mix) GST_LOG ("all our sinkpads are EOS, pushing downstream"); gst_pad_push_event (mix->srcpad, gst_event_new_eos ()); ret = GST_FLOW_WRONG_STATE; - goto beach; + goto error; } /* Calculating out buffer size from input size */ @@ -1277,7 +1219,7 @@ gst_videomixer_collected (GstCollectPads * pads, GstVideoMixer * mix) } if (ret != GST_FLOW_OK) { - goto beach; + goto error; } switch (mix->background) { @@ -1298,11 +1240,19 @@ gst_videomixer_collected (GstCollectPads * pads, GstVideoMixer * mix) gst_videomixer_blend_buffers (mix, outbuf); gst_videomixer_update_queues (mix); + GST_VIDEO_MIXER_STATE_UNLOCK (mix); ret = gst_pad_push (mix->srcpad, outbuf); beach: return ret; + + /* ERRORS */ +error: + { + GST_VIDEO_MIXER_STATE_UNLOCK (mix); + goto beach; + } } static void @@ -1349,7 +1299,6 @@ gst_videomixer_change_state (GstElement * element, GstStateChange transition) switch (transition) { case GST_STATE_CHANGE_READY_TO_PAUSED: - gst_videomixer_reset (mix); GST_LOG ("starting collectpads"); gst_collect_pads_start (mix->collect); break; @@ -1363,6 +1312,14 @@ gst_videomixer_change_state (GstElement * element, GstStateChange transition) ret = GST_ELEMENT_CLASS (parent_class)->change_state (element, transition); + switch (transition) { + case GST_STATE_CHANGE_PAUSED_TO_READY: + gst_videomixer_reset (mix); + break; + default: + break; + } + return ret; } -- cgit