diff --git a/ChangeLog b/ChangeLog index a946d03c97..f8f11120e0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,19 @@ +2007-06-22 Zaheer Abbas Merali + + * examples/switch/switcher.c (my_bus_callback, switch_timer, + last_message_received, main): + * gst/switch/gstswitch.c (gst_switch_release_pad, + gst_switch_request_new_pad, gst_switch_chain, gst_switch_event, + gst_switch_set_property, gst_switch_get_property, + gst_switch_get_linked_pad, gst_switch_getcaps, + gst_switch_bufferalloc, gst_switch_dispose, gst_switch_init): + * gst/switch/gstswitch.h (switch_mutex, GST_SWITCH_LOCK, + GST_SWITCH_UNLOCK): + Add an extra lock to protect against certain variables instead of + using the object lock. Fix case where caps are different in the + sink pads causes deadlock. Update example to use different caps + on each sink pad. + 2007-06-22 Jan Schmidt * MAINTAINERS: diff --git a/examples/switch/switcher.c b/examples/switch/switcher.c index 29f1d1a4b2..2faa26c7c1 100644 --- a/examples/switch/switcher.c +++ b/examples/switch/switcher.c @@ -68,6 +68,7 @@ switch_timer (GstElement * video_switch) gint nb_sources; gchar *active_pad; + g_message ("switching"); g_object_get (G_OBJECT (video_switch), "num-sources", &nb_sources, NULL); g_object_get (G_OBJECT (video_switch), "active-pad", &active_pad, NULL); @@ -97,7 +98,7 @@ int main (int argc, char *argv[]) { GstElement *pipeline, *src1, *src2, *video_switch, *video_sink, *segment; - GstElement *sink1_sync, *sink2_sync; + GstElement *sink1_sync, *sink2_sync, *capsfilter, *scaler; GstBus *bus; /* Initing GStreamer library */ @@ -110,11 +111,15 @@ main (int argc, char *argv[]) g_object_set (G_OBJECT (src1), "pattern", 0, NULL); src2 = gst_element_factory_make ("videotestsrc", "src2"); g_object_set (G_OBJECT (src2), "pattern", 1, NULL); + capsfilter = gst_element_factory_make ("capsfilter", "caps0"); + g_object_set (G_OBJECT (capsfilter), "caps", + gst_caps_from_string ("video/x-raw-rgb,width=640,height=480"), NULL); video_switch = gst_element_factory_make ("switch", "video_switch"); segment = gst_element_factory_make ("identity", "identity-segment"); g_signal_connect (G_OBJECT (segment), "notify::last-message", G_CALLBACK (last_message_received), segment); g_object_set (G_OBJECT (segment), "single-segment", TRUE, NULL); + scaler = gst_element_factory_make ("videoscale", "videoscale0"); video_sink = gst_element_factory_make ("ximagesink", "video_sink"); //g_object_set (G_OBJECT (video_sink), "sync", FALSE, NULL); sink1_sync = gst_element_factory_make ("identity", "sink0_sync"); @@ -122,13 +127,15 @@ main (int argc, char *argv[]) sink2_sync = gst_element_factory_make ("identity", "sink1_sync"); g_object_set (G_OBJECT (sink2_sync), "sync", TRUE, NULL); gst_bin_add_many (GST_BIN (pipeline), src1, src2, segment, video_switch, - video_sink, sink1_sync, sink2_sync, NULL); + video_sink, sink1_sync, sink2_sync, scaler, capsfilter, NULL); gst_element_link (src1, sink1_sync); gst_element_link (sink1_sync, video_switch); - gst_element_link (src2, sink2_sync); + gst_element_link (src2, capsfilter); + gst_element_link (capsfilter, sink2_sync); gst_element_link (sink2_sync, video_switch); gst_element_link (video_switch, segment); - gst_element_link (segment, video_sink); + gst_element_link (segment, scaler); + gst_element_link (scaler, video_sink); bus = gst_pipeline_get_bus (GST_PIPELINE (pipeline)); gst_bus_add_watch (bus, my_bus_callback, NULL); diff --git a/gst/switch/gstswitch.c b/gst/switch/gstswitch.c index 73639d3270..a7a20f855f 100644 --- a/gst/switch/gstswitch.c +++ b/gst/switch/gstswitch.c @@ -127,6 +127,7 @@ gst_switch_release_pad (GstElement * element, GstPad * pad) gst_element_remove_pad (element, pad); GST_OBJECT_LOCK (gstswitch); gstswitch->nb_sinkpads--; + GST_SWITCH_LOCK (gstswitch); if (gstswitch->active_sinkpad == pad) { gst_object_unref (gstswitch->active_sinkpad); gstswitch->active_sinkpad = NULL; @@ -144,6 +145,7 @@ gst_switch_release_pad (GstElement * element, GstPad * pad) gst_iterator_free (iter); } } + GST_SWITCH_UNLOCK (gstswitch); GST_OBJECT_UNLOCK (gstswitch); } @@ -173,8 +175,10 @@ gst_switch_request_new_pad (GstElement * element, if (name) g_free (name); + GST_SWITCH_LOCK (gstswitch); if (gstswitch->active_sinkpad == NULL) gstswitch->active_sinkpad = gst_object_ref (sinkpad); + GST_SWITCH_UNLOCK (gstswitch); GST_OBJECT_UNLOCK (gstswitch); gst_pad_set_getcaps_function (sinkpad, @@ -201,12 +205,13 @@ gst_switch_chain (GstPad * pad, GstBuffer * buf) GstFlowReturn res; GstPad *active_sinkpad; - GST_OBJECT_LOCK (gstswitch); + GST_SWITCH_LOCK (gstswitch); active_sinkpad = gstswitch->active_sinkpad; - GST_OBJECT_UNLOCK (gstswitch); /* Ignore buffers from pads except the selected one */ if (pad != active_sinkpad) { + GST_SWITCH_UNLOCK (gstswitch); + GST_DEBUG_OBJECT (gstswitch, "Ignoring buffer %p from pad %s:%s", buf, GST_DEBUG_PAD_NAME (pad)); @@ -216,7 +221,6 @@ gst_switch_chain (GstPad * pad, GstBuffer * buf) } /* check if we need to send a new segment event */ - GST_OBJECT_LOCK (gstswitch); if (gstswitch->need_to_send_newsegment && !gstswitch->queue_buffers) { /* check to see if we need to send a new segment update for stop */ if (gstswitch->previous_sinkpad != NULL) { @@ -237,9 +241,11 @@ gst_switch_chain (GstPad * pad, GstBuffer * buf) GST_DEBUG_OBJECT (gstswitch, "Sending new segment update with stop of %" G_GUINT64_FORMAT, gstswitch->stop_value); + GST_SWITCH_UNLOCK (gstswitch); gst_pad_push_event (gstswitch->srcpad, gst_event_new_new_segment_full (TRUE, rate, applied_rate, format, gstswitch->current_start, gstswitch->stop_value, position)); + GST_SWITCH_LOCK (gstswitch); } } gst_object_unref (GST_OBJECT (gstswitch->previous_sinkpad)); @@ -283,18 +289,22 @@ gst_switch_chain (GstPad * pad, GstBuffer * buf) g_hash_table_lookup (gstswitch->stored_buffers, active_sinkpad); while (buffers != NULL) { gst_buffer_ref (GST_BUFFER (buffers->data)); + GST_SWITCH_UNLOCK (gstswitch); gst_pad_push (gstswitch->srcpad, GST_BUFFER (buffers->data)); + GST_SWITCH_LOCK (gstswitch); buffers = buffers->next; } g_hash_table_remove (gstswitch->stored_buffers, active_sinkpad); } gstswitch->last_ts = GST_BUFFER_TIMESTAMP (buf) + GST_BUFFER_DURATION (buf); if (!gstswitch->queue_buffers) { - GST_OBJECT_UNLOCK (gstswitch); /* forward */ - GST_DEBUG_OBJECT (gstswitch, "Forwarding buffer %p from pad %s:%s", - buf, GST_DEBUG_PAD_NAME (pad)); + GST_DEBUG_OBJECT (gstswitch, "Forwarding buffer %p from pad %s:%s to %s:%s", + buf, GST_DEBUG_PAD_NAME (pad), GST_DEBUG_PAD_NAME (gstswitch->srcpad)); + GST_SWITCH_UNLOCK (gstswitch); res = gst_pad_push (gstswitch->srcpad, buf); + GST_SWITCH_LOCK (gstswitch); + GST_DEBUG_OBJECT (gstswitch, "Finished pushing buffer"); } else { GList *buffers; gboolean lookup_res = TRUE; @@ -306,10 +316,9 @@ gst_switch_chain (GstPad * pad, GstBuffer * buf) /* only need to insert it if it was NULL before because we appended */ if (!lookup_res) g_hash_table_insert (gstswitch->stored_buffers, active_sinkpad, buffers); - GST_OBJECT_UNLOCK (gstswitch); res = GST_FLOW_OK; } - + GST_SWITCH_UNLOCK (gstswitch); gst_object_unref (gstswitch); return res; @@ -323,7 +332,7 @@ gst_switch_event (GstPad * pad, GstEvent * event) switch (GST_EVENT_TYPE (event)) { case GST_EVENT_NEWSEGMENT: - GST_OBJECT_LOCK (gstswitch); + GST_SWITCH_LOCK (gstswitch); /* need to put in or replace what's in hash table */ g_hash_table_replace (gstswitch->newsegment_events, pad, event); if (pad == gstswitch->active_sinkpad) { @@ -331,7 +340,7 @@ gst_switch_event (GstPad * pad, GstEvent * event) gstswitch->need_to_send_newsegment = TRUE; } } - GST_OBJECT_UNLOCK (gstswitch); + GST_SWITCH_UNLOCK (gstswitch); break; default: ret = gst_pad_event_default (pad, event); @@ -367,9 +376,9 @@ gst_switch_set_property (GObject * object, guint prop_id, pad = gst_element_get_pad (GST_ELEMENT (object), pad_name); } - GST_OBJECT_LOCK (object); + GST_SWITCH_LOCK (gstswitch); if (pad == gstswitch->active_sinkpad) { - GST_OBJECT_UNLOCK (object); + GST_SWITCH_UNLOCK (gstswitch); if (pad) gst_object_unref (pad); break; @@ -386,22 +395,22 @@ gst_switch_set_property (GObject * object, guint prop_id, GST_DEBUG_OBJECT (gstswitch, "New active pad is %" GST_PTR_FORMAT, gstswitch->active_sinkpad); gstswitch->need_to_send_newsegment = TRUE; - GST_OBJECT_UNLOCK (object); + GST_SWITCH_UNLOCK (gstswitch); break; case ARG_START_VALUE: - GST_OBJECT_LOCK (object); + GST_SWITCH_LOCK (gstswitch); gstswitch->start_value = g_value_get_uint64 (value); - GST_OBJECT_UNLOCK (object); + GST_SWITCH_UNLOCK (gstswitch); break; case ARG_STOP_VALUE: - GST_OBJECT_LOCK (object); + GST_SWITCH_LOCK (gstswitch); gstswitch->stop_value = g_value_get_uint64 (value); - GST_OBJECT_UNLOCK (object); + GST_SWITCH_UNLOCK (gstswitch); break; case ARG_QUEUE_BUFFERS: - GST_OBJECT_LOCK (object); + GST_SWITCH_LOCK (gstswitch); gstswitch->queue_buffers = g_value_get_boolean (value); - GST_OBJECT_UNLOCK (object); + GST_SWITCH_UNLOCK (gstswitch); break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); @@ -421,14 +430,14 @@ gst_switch_get_property (GObject * object, guint prop_id, switch (prop_id) { case ARG_ACTIVE_SOURCE: - GST_OBJECT_LOCK (object); + GST_SWITCH_LOCK (gstswitch); if (gstswitch->active_sinkpad != NULL) { g_value_take_string (value, gst_pad_get_name (gstswitch->active_sinkpad)); } else { g_value_set_string (value, ""); } - GST_OBJECT_UNLOCK (object); + GST_SWITCH_UNLOCK (gstswitch); break; case ARG_NB_SOURCES: GST_OBJECT_LOCK (object); @@ -436,24 +445,24 @@ gst_switch_get_property (GObject * object, guint prop_id, GST_OBJECT_UNLOCK (object); break; case ARG_START_VALUE: - GST_OBJECT_LOCK (object); + GST_SWITCH_LOCK (gstswitch); g_value_set_uint64 (value, gstswitch->start_value); - GST_OBJECT_UNLOCK (object); + GST_SWITCH_UNLOCK (gstswitch); break; case ARG_STOP_VALUE: - GST_OBJECT_LOCK (object); + GST_SWITCH_LOCK (gstswitch); g_value_set_uint64 (value, gstswitch->stop_value); - GST_OBJECT_UNLOCK (object); + GST_SWITCH_UNLOCK (gstswitch); break; case ARG_LAST_TS: - GST_OBJECT_LOCK (object); + GST_SWITCH_LOCK (gstswitch); g_value_set_uint64 (value, gstswitch->last_ts); - GST_OBJECT_UNLOCK (object); + GST_SWITCH_UNLOCK (gstswitch); break; case ARG_QUEUE_BUFFERS: - GST_OBJECT_LOCK (object); + GST_SWITCH_LOCK (gstswitch); g_value_set_boolean (value, gstswitch->queue_buffers); - GST_OBJECT_UNLOCK (object); + GST_SWITCH_UNLOCK (gstswitch); break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); @@ -480,15 +489,18 @@ gst_switch_get_linked_pad (GstPad * pad, gboolean strict) static GstCaps * gst_switch_getcaps (GstPad * pad) { - GstPad *otherpad = gst_switch_get_linked_pad (pad, FALSE); + GstPad *otherpad; GstObject *parent; GstCaps *caps; + parent = gst_object_get_parent (GST_OBJECT (pad)); + GST_SWITCH_LOCK (GST_SWITCH (parent)); + otherpad = gst_switch_get_linked_pad (pad, FALSE); + GST_SWITCH_UNLOCK (GST_SWITCH (parent)); if (!otherpad) { GST_DEBUG_OBJECT (parent, "Pad %s:%s not linked, returning ANY", GST_DEBUG_PAD_NAME (pad)); - gst_object_unref (parent); return gst_caps_new_any (); } @@ -514,9 +526,7 @@ gst_switch_bufferalloc (GstPad * pad, guint64 offset, GstFlowReturn result; GstPad *active_sinkpad; - GST_OBJECT_LOCK (gstswitch); active_sinkpad = gstswitch->active_sinkpad; - GST_OBJECT_UNLOCK (gstswitch); /* Fallback allocation for buffers from pads except the selected one */ if (pad != active_sinkpad) { @@ -573,6 +583,9 @@ gst_switch_dispose (GObject * object) gstswitch = GST_SWITCH (object); + if (gstswitch->switch_mutex) { + g_mutex_free (gstswitch->switch_mutex); + } if (gstswitch->active_sinkpad) { gst_object_unref (gstswitch->active_sinkpad); gstswitch->active_sinkpad = NULL; @@ -626,6 +639,7 @@ gst_switch_init (GstSwitch * gstswitch) gstswitch->start_value = GST_CLOCK_TIME_NONE; gstswitch->current_start = 0; gstswitch->last_ts = GST_CLOCK_TIME_NONE; + gstswitch->switch_mutex = g_mutex_new (); } static void diff --git a/gst/switch/gstswitch.h b/gst/switch/gstswitch.h index e7a0ed867e..c9ca0ec040 100644 --- a/gst/switch/gstswitch.h +++ b/gst/switch/gstswitch.h @@ -66,8 +66,12 @@ struct _GstSwitch { * new segment has been sent */ GHashTable *stored_buffers; + GMutex *switch_mutex; }; +#define GST_SWITCH_LOCK(obj) g_mutex_lock(obj->switch_mutex) +#define GST_SWITCH_UNLOCK(obj) g_mutex_unlock(obj->switch_mutex) + struct _GstSwitchClass { GstElementClass parent_class; };