gst/: remove the hash table for miniobjects - since we can't get notified when they get destroyed, we shouldn't be ca...

Original commit message from CVS:

* gst/gst.override:
* gst/pygstminiobject.c:
remove the hash table for miniobjects - since we can't get notified
when they get destroyed, we shouldn't be caching pointer mappings
* testsuite/test_pad.py:
update refcount tests because mini objects now have a ref for
each time an object is made for it

ChangeLog

@@ -1,3 +1,13 @@
+2005-10-27  Thomas Vander Stichele  <thomas (at) apestaart (dot) org>
+
+	* gst/gst.override:
+	* gst/pygstminiobject.c:
+	  remove the hash table for miniobjects - since we can't get notified
+	  when they get destroyed, we shouldn't be caching pointer mappings
+	* testsuite/test_pad.py:
+	  update refcount tests because mini objects now have a ref for
+	  each time an object is made for it
+
 2005-10-26  Thomas Vander Stichele  <thomas at apestaart dot org>
 
 	* testsuite/test_bus.py:

gst/gst.override

@@ -274,7 +274,6 @@ init
 #endif
     if (!pygst_value_init())
         return;
-    pygst_miniobject_init();
     gst_controller_init(NULL, NULL);
 }
 %%

gst/pygstminiobject.c

@@ -35,14 +35,6 @@ static void pygstminiobject_dealloc(PyGstMiniObject *self);
 GST_DEBUG_CATEGORY_EXTERN (pygst_debug);
 #define GST_CAT_DEFAULT pygst_debug
 
-static GHashTable *_miniobjs;
-
-void
-pygst_miniobject_init ()
-{
-    _miniobjs = g_hash_table_new (NULL, NULL);
-}
-
 /**
  * pygstminiobject_lookup_class:
  * @gtype: the GType of the GstMiniObject subclass.
@@ -126,32 +118,9 @@ pygstminiobject_register_class(PyObject *dict, const gchar *type_name,
     PyDict_SetItemString(dict, (char *)class_name, (PyObject *)type);
 }
 
-/**
- * pygstminiobject_register_wrapper:
- * @self: the wrapper instance
- *
- * In the constructor of PyGTK wrappers, this function should be
- * called after setting the obj member.  It will tie the wrapper
- * instance to the Gstminiobject so that the same wrapper instance will
- * always be used for this Gstminiobject instance.  It will also sink any
- * floating references on the Gstminiobject.
- */
 void
 pygstminiobject_register_wrapper (PyObject *self)
 {
-    GstMiniObject *obj = ((PyGstMiniObject *) self)->obj;
-    PyGILState_STATE state;
-
-    g_assert (obj);
-    g_assert (GST_IS_MINI_OBJECT (obj));
-
-    state = pyg_gil_state_ensure ();
-    GST_DEBUG ("inserting self %p in the table for object %p [ref:%d]", 
-	       self, obj, GST_MINI_OBJECT_REFCOUNT_VALUE (obj));
-    g_hash_table_insert (_miniobjs, (gpointer) obj, (gpointer) self);
-    GST_DEBUG ("There are now %d elements in the hash table",
-	       g_hash_table_size (_miniobjs));
-    pyg_gil_state_release (state);
 }
 
 
@@ -160,46 +129,37 @@ pygstminiobject_register_wrapper (PyObject *self)
  * @obj: a GstMiniObject instance.
  *
  * This function gets a reference to a wrapper for the given GstMiniObject
- * instance.  If a wrapper has already been created, a new reference
+ * instance.  A new wrapper will always be created.
- * to that wrapper will be returned.  Otherwise, a wrapper instance
- * will be created.
  *
  * Returns: a reference to the wrapper for the GstMiniObject.
  */
 PyObject *
 pygstminiobject_new (GstMiniObject *obj)
 {
-    PyGILState_STATE state;
     PyGstMiniObject *self = NULL;
+    PyGILState_STATE state;
+    PyTypeObject *tp = NULL;
 
     if (obj == NULL) {
 	Py_INCREF (Py_None);
 	return Py_None;
     }
 
-    /* see if we already have a wrapper for this object */
+    /* since mini objects cannot notify us when they get destroyed, we
-    state = pyg_gil_state_ensure ();
+     * can't use a global hash to map GMO to PyO, and have to create a new
-    self = (PyGstMiniObject *) g_hash_table_lookup (_miniobjs, (gpointer) obj);
+     * Python object every time we see it */
-    pyg_gil_state_release (state);
+    tp = pygstminiobject_lookup_class (G_OBJECT_TYPE (obj));
-
-    if (self != NULL) {
-        GST_DEBUG ("had self %p in the table for object %p", self, obj);
-	/* make sure the lookup returned our object */
-        g_assert (self->obj);
-        g_assert (self->obj == obj);
-	GST_INFO ("Increment refcount %p", self);
-	Py_INCREF (self);
-    } else {
     GST_DEBUG ("have to create wrapper for object %p", obj);
-	/* we don't, so create one */
-	PyTypeObject *tp = pygstminiobject_lookup_class (G_OBJECT_TYPE (obj));
     if (!tp)
         g_warning ("Couldn't get class for type object : %p", obj);
     if (tp->tp_flags & Py_TPFLAGS_HEAPTYPE) {
         GST_INFO ("Increment refcount %p", tp);
         Py_INCREF (tp);
     }
+    state = pyg_gil_state_ensure();
     self = PyObject_New (PyGstMiniObject, tp);
+    pyg_gil_state_release(state);
+
     if (self == NULL)
         return NULL;
     self->obj = gst_mini_object_ref (obj);
@@ -207,16 +167,8 @@ pygstminiobject_new (GstMiniObject *obj)
     self->inst_dict = NULL;
     self->weakreflist = NULL;
 
-	/* save wrapper pointer so we can access it later */
+    GST_DEBUG ("created Python object %p for GstMiniObject %p [ref:%d]",
-        GST_DEBUG ("inserting self %p in the table for object %p [ref:%d]",
         self, obj, GST_MINI_OBJECT_REFCOUNT_VALUE (obj));
-	state = pyg_gil_state_ensure ();
-	g_hash_table_insert (_miniobjs, (gpointer) obj, (gpointer) self);
-	GST_DEBUG ("There are now %d elements in the hash table",
-		   g_hash_table_size (_miniobjs));
-	pyg_gil_state_release (state);
-
-    }
     return (PyObject *) self;
 }
 
@@ -227,15 +179,12 @@ pygstminiobject_dealloc(PyGstMiniObject *self)
 
     PyGILState_STATE state;
 
-    GST_INFO ("At the beginning %p", self);
+    GST_DEBUG ("At the beginning %p", self);
     state = pyg_gil_state_ensure();
 
     if (self->obj) {
-        GST_DEBUG ("removing self %p from the table for object %p [ref:%d]", self,
+        GST_DEBUG ("PyO %p unreffing GstMiniObject %p [ref:%d]", self,
              self->obj, GST_MINI_OBJECT_REFCOUNT_VALUE (self->obj));
-        g_assert (g_hash_table_remove (_miniobjs, (gpointer) self->obj));
-	GST_DEBUG ("There are now %d elements in the hash table",
-		   g_hash_table_size (_miniobjs));
 	gst_mini_object_unref(self->obj);
 	GST_DEBUG ("setting self %p -> obj to NULL", self);
 	self->obj = NULL;
@@ -248,7 +197,7 @@ pygstminiobject_dealloc(PyGstMiniObject *self)
 
     self->ob_type->tp_free((PyObject *) self);
     pyg_gil_state_release(state);
-    GST_INFO ("At the end %p", self);
+    GST_DEBUG ("At the end %p", self);
 }
 
 static int

testsuite/test_pad.py

@@ -21,8 +21,9 @@
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
 
 from common import gst, unittest, TestCase
+
 import sys
-import gc
+import time
 
 class PadTemplateTest(TestCase):
     def testConstructor(self):
@@ -96,20 +97,26 @@ class PadPushLinkedTest(TestCase):
         TestCase.tearDown(self)
 
     def _chain_func(self, pad, buffer):
+        gst.debug('got buffer %r, id %x, with GMO rc %d'% (
+            buffer, id(buffer), buffer.__grefcount__))
         self.buffers.append(buffer)
 
         return gst.FLOW_OK
 
     def testNoProbe(self):
         self.buffer = gst.Buffer()
+        gst.debug('created new buffer %r, id %x' % (
+            self.buffer, id(self.buffer)))
         self.assertEquals(self.buffer.__grefcount__, 1)
         gst.debug('pushing buffer on linked pad, no probe')
         self.assertEquals(self.src.push(self.buffer), gst.FLOW_OK)
         gst.debug('pushed buffer on linked pad, no probe')
-        # pushing it takes a ref in the python wrapper to keep buffer
+        # one refcount is held by our scope, another is held on
-        # alive afterwards; fakesink will get the buffer
+        # self.buffers through _chain_func
-        self.assertEquals(self.buffer.__grefcount__, 1)
+        self.assertEquals(self.buffer.__grefcount__, 2)
         self.assertEquals(len(self.buffers), 1)
+        self.buffers = None
+        self.assertEquals(self.buffer.__grefcount__, 1)
 
     def testFalseProbe(self):
         id = self.src.add_buffer_probe(self._probe_handler, False)
@@ -125,9 +132,13 @@ class PadPushLinkedTest(TestCase):
         self.buffer = gst.Buffer()
         self.assertEquals(self.buffer.__grefcount__, 1)
         self.assertEquals(self.src.push(self.buffer), gst.FLOW_OK)
-        self.assertEquals(self.buffer.__grefcount__, 1)
+        # one refcount is held by our scope, another is held on
+        # self.buffers through _chain_func
+        self.assertEquals(self.buffer.__grefcount__, 2)
         self.src.remove_buffer_probe(id)
         self.assertEquals(len(self.buffers), 1)
+        self.buffers = None
+        self.assertEquals(self.buffer.__grefcount__, 1)
 
     def _probe_handler(self, pad, buffer, ret):
         return ret
@@ -166,10 +177,13 @@ class PadPushProbeLinkTest(TestCase):
         gst.debug('pushing buffer on linked pad, no probe')
         self.assertEquals(self.src.push(self.buffer), gst.FLOW_OK)
         gst.debug('pushed buffer on linked pad, no probe')
-        # pushing it takes a ref in the python wrapper to keep buffer
+        # one refcount is held by our scope, another is held on
-        # alive afterwards; fakesink will get the buffer
+        # self.buffers through _chain_func
-        self.assertEquals(self.buffer.__grefcount__, 1)
+        self.assertEquals(self.buffer.__grefcount__, 2)
         self.assertEquals(len(self.buffers), 1)
+        self.buffers = None
+        self.assertEquals(self.buffer.__grefcount__, 1)
+
 
     def _probe_handler(self, pad, buffer):
         self.src.link(self.sink)
@@ -246,8 +260,6 @@ class PadProbePipeTest(TestCase):
         self.assertEquals(sys.getrefcount(self.pipeline), 3)
         self.assertEquals(self.fakesrc.__gstrefcount__, 2)
         self.assertEquals(sys.getrefcount(self.fakesrc), 3)
-        self.assertEquals(self.fakesink.__gstrefcount__, 2)
-        self.assertEquals(sys.getrefcount(self.fakesink), 3)
         gst.debug('deleting pipeline')
         del self.pipeline
         self.gccollect()
@@ -367,15 +379,15 @@ class PadRefCountTest(TestCase):
         self.assertEquals(pad.__gstrefcount__, 2) # added to element
 
         gst.debug('deleting element and collecting')
-        gc.collect()
+        self.gccollect()
         del e
-        self.assertEquals(gc.collect(), 1) # collected the element
+        self.assertEquals(self.gccollect(), 1) # collected the element
         self.assertEquals(sys.getrefcount(pad), 3)
         self.assertEquals(pad.__gstrefcount__, 1) # removed from element
 
         gst.debug('deleting pad and collecting')
         del pad
-        self.assertEquals(gc.collect(), 1) # collected the pad
+        self.assertEquals(self.gccollect(), 1) # collected the pad
         gst.debug('going into teardown')
 
 if __name__ == "__main__":