examples/switch/switcher.c (my_bus_callback, switch_timer, last_message_received, main): gst/switch/gstswitch.c (gst_...

Original commit message from CVS:
* 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.

4 files changed, 78 insertions, 37 deletions

diff --git a/ChangeLog b/ChangeLog
index a946d03c..f8f11120 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,19 @@
+2007-06-22  Zaheer Abbas Merali  <zaheerabbas at merali dot org>
+
+	* 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  <thaytan@noraisin.net>
 
 	* MAINTAINERS:
diff --git a/examples/switch/switcher.c b/examples/switch/switcher.c
index 29f1d1a4..2faa26c7 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 73639d32..a7a20f85 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 e7a0ed86..c9ca0ec0 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;
 };