making Python GStreamer plugins work

[Benjamin Otte <in7y118 public uni-hamburg de>]

Hi,

I've been working the last few days to get a Python plugin working for
GStreamer. By Python plugin I mean code written in Python that can be used
in an app completely written in C (like Rhythmbox or Totem). See my blog
entry at http://www.advogato.org/person/company/diary.html?start=13 for an
example. The attached patch to pygtk allows me to do this.

It implements 3 things:
- Allow calling pyg_register_class_init multiple times without overwriting
  previously set handlers. This is useful to me since the autogenerated
  code calls this function and I want to do some stuff inside GStreamer,
  that's normally done during the base_init and class_init
  GObjectClass functions.
- Add pyg_closure_set_exception_handler. This makes it easy to handle
  exceptions inside the GStreamer vtable transparently by just calling
  gst_element_error, which makes the code quite a bit nicer IMO. The
  default behaviour with no exception handler still behaves like before.
- And here's the crux of the patch and where the most nastiness comes
  from: Allow classes that are derived in Python to be created with
  g_object_new from all C code. And handle the refcounting issues, too.
  It works, but it's ugly. See the patch for implementation details.

I believe the object creation and destruction process for this kind of
objects requires some support from GObject to work nicely that currently
just does not exist.
What's the problem?
Currently object creation/destruction is managed by the bindings. You
create objects via bindings code and everything takes the same path:
Python creates objects from PyTypeObjects and the bindings map this to
 g_object_new. This works fine for subclasses too, you just subclass the
PyTypeObjects. Destruction works the same way: Just look at the refcount
of the PyObject to deicde if the object may be destroyed.
The issues start when you want to create Python objects from your C code.
Suddenly g_object_new is called to create the object and there's no
code that creates the PyTypeObject anymore. But since we are talking about
a Python derived class, we require a PyObject to go with it, so we
absolutely must create one, before we may use the GObject inside our C
code. (This is what the constructor does in my patch, there's also a
solution in bug 161177, but that solution doesn't work for me).
The other problem is getting rid of the GObject and the PyObject once
the C code is done and calls g_object_unref. Since that will not trigger
the Python garbage collector, we might end up with lots of garbage. So the
current dispose/get_owner stuff makes sure the PyObject does not ref the
Gobject so the dispose function is called and we can properly get rid
of the PyObject. The finalizer runs the garbage collection in the end.

I'll stop explaining now, it's too complicated for me to put in plain
English. For more info, take a look at the code to see how it works
and why it's ugly in places.

Benjamin

PS: If anyone wants to try the gst-python patch, it's at
http://www.freedesktop.org/~company/stuff/gst-python.patch
But be warned, it contains lots of other unrelated fixes and newly wrapped
API required to get the elements working as nicely as they do.

? patch-04-28.patch
? pygtk.patch
Index: gobject/gobjectmodule.c
===================================================================
RCS file: /cvs/gnome/gnome-python/pygtk/gobject/gobjectmodule.c,v
retrieving revision 1.162
diff -u -3 -p -r1.162 gobjectmodule.c
--- gobject/gobjectmodule.c	22 Jan 2005 17:45:46 -0000	1.162
+++ gobject/gobjectmodule.c	30 Apr 2005 00:07:28 -0000
@@ -345,6 +345,27 @@ pyg_type_interfaces (PyObject *self, PyO
     return NULL;
 }
 
+/* FIXME: It would be nice if we could attach the owner to the PyGObject 
+ * structure. This doesn't work for a stable API though. */
+static GQuark _pyg_owner_key = 0;
+gint
+pyg_object_get_owner (GObject *object)
+{
+  if (_pyg_owner_key == 0)
+    _pyg_owner_key = g_quark_from_static_string ("PyGObject::Owner");
+
+  return GPOINTER_TO_INT (g_object_get_qdata (object, _pyg_owner_key));
+}
+
+void 
+pyg_object_set_owner (GObject *object, gint owner)
+{
+  if (_pyg_owner_key == 0)
+    _pyg_owner_key = g_quark_from_static_string ("PyGObject::Owner");
+
+  g_object_set_qdata (object, _pyg_owner_key, GINT_TO_POINTER (owner));
+}
+
 static void
 pyg_object_set_property (GObject *object, guint property_id,
 			 const GValue *value, GParamSpec *pspec)
@@ -410,11 +431,104 @@ pyg_object_get_property (GObject *object
     pyg_gil_state_release(state);
 }
 
+GStaticPrivate __pygobject_creation = G_STATIC_PRIVATE_INIT;
+static GObject *
+pyg_object_class_constructor (GType type, guint n_construct_properties, GObjectConstructParam *construct_properties){
+  int mode = pygtk_get_object_creation_mode();
+  GObject *ret;
+  
+  if (mode) {
+    /* FIXME: This will cause issues if you inherit from a Python object in C and 
+     * inherit from that object in C again. When that happens we'll need to add
+     * some type data to point to the real constructor or so.
+     */
+    GObjectClass *parent = g_type_class_peek (g_type_parent (type));
+    pygtk_set_object_creation_mode (0);
+    while (parent->constructor == pyg_object_class_constructor)
+      parent = g_type_class_peek_parent (parent);
+    return parent->constructor (type, n_construct_properties, construct_properties);
+  } else {
+    PyObject *args, *kwargs, *pyobject;
+    PyTypeObject *typeobject = pygobject_lookup_class (type);
+    guint i;
+
+    if (!typeobject) 
+      g_error ("No Python type, type %s is improperly registered\n",
+	  g_type_name (type));
+    if (n_construct_properties > 0) {
+      kwargs = PyDict_New();
+      for (i = 0; i < n_construct_properties; i++) {
+	PyObject *valobj = pyg_param_gvalue_as_pyobject (construct_properties[i].value,
+	    FALSE, construct_properties[i].pspec);
+	if (!valobj ||
+	    PyDict_SetItemString (kwargs, construct_properties[i].pspec->name, 
+		valobj) != 0)
+	  g_warning ("could not add property %s to construct properties", 
+	      construct_properties[i].pspec->name);
+      }
+    } else {
+      kwargs = NULL;
+    }
+    args = (PyObject *) PyTuple_New (0);
+    pygtk_set_object_creation_mode (1);
+    pyobject = PyObject_Call ((PyObject *) typeobject, args, NULL);
+    if (!pyobject) { 
+      PyErr_Print ();
+      g_error ("object creation for type %s failed", g_type_name (type));
+    }
+    Py_DECREF (args);
+    Py_XDECREF (kwargs);
+    Py_DECREF (pyobject); /* reference is held by the GObject's qdata */
+    ret = pygobject_get (pyobject);
+    /* weird behaviour expected from constructors */
+    g_object_freeze_notify (ret);
+    return ret;
+  }
+}
+
+static void
+pyg_object_dispose (GObject *object)
+{
+    PyGObject *self;
+    GObjectClass *parent_class;
+
+    /* run the dispose first, this might actually call vfuncs */
+    parent_class = g_type_class_peek_parent (G_OBJECT_GET_CLASS (object));
+    parent_class->dispose (object);
+
+    /* now it should be safe to get rid of the object */
+    self = (PyGObject *) pygobject_new (object);
+    if (pyg_object_get_owner (object) == PYGTK_OWNER_GOBJECT) {
+	/* One reference belongs to Python, one belongs to the GObject
+	 * and the last one is the one we just got 2 lines above. */
+      g_print ("%d\n", self->ob_refcnt);
+	if (self->ob_refcnt > 3) {
+	  g_object_ref (object);
+	  pyg_object_set_owner (object, PYGTK_OWNER_PYOBJECT);
+	} else {
+	  self->obj = NULL;
+	}
+    }
+    Py_DECREF (self);
+}
+
+static void
+pyg_object_finalize (GObject *object)
+{
+    GObjectClass *parent_class;
+    parent_class = g_type_class_peek_parent (G_OBJECT_GET_CLASS (object));
+    parent_class->finalize (object);
+    PyGC_Collect ();
+}
+
 static void
 pyg_object_class_init(GObjectClass *class, PyObject *py_class)
 {
     class->set_property = pyg_object_set_property;
     class->get_property = pyg_object_get_property;
+    class->constructor = pyg_object_class_constructor;
+    class->dispose = pyg_object_dispose;
+    class->finalize = pyg_object_finalize;
 }
 
 static gboolean
@@ -868,12 +982,15 @@ add_properties (GType instance_type, PyO
 static void
 pyg_register_class_init(GType gtype, PyGClassInitFunc class_init)
 {
-    g_type_set_qdata(gtype, pygobject_class_init_key, class_init);
+    GSList *list = g_type_get_qdata (gtype, pygobject_class_init_key);
+    list = g_slist_prepend (list, class_init);
+    g_type_set_qdata(gtype, pygobject_class_init_key, list);
 }
 
 static int
 pyg_run_class_init(GType gtype, gpointer gclass, PyTypeObject *pyclass)
 {
+    GSList *list;
     PyGClassInitFunc class_init;
     GType parent_type;
     int rv;
@@ -883,9 +1000,13 @@ pyg_run_class_init(GType gtype, gpointer
         rv = pyg_run_class_init(parent_type, gclass, pyclass);
         if (rv) return rv;
     }
-    class_init = g_type_get_qdata(gtype, pygobject_class_init_key);
-    if (class_init)
-        return class_init(gclass, pyclass);
+    list = g_type_get_qdata(gtype, pygobject_class_init_key);
+    while (list) {
+	class_init = list->data;
+        rv = class_init (gclass, pyclass);
+        if (rv) return rv;
+	list = g_slist_next (list);
+    }
     return 0;
 }
 
@@ -2386,7 +2507,9 @@ struct _PyGObject_Functions pygobject_ap
   pyg_gil_state_ensure_py23,
   pyg_gil_state_release_py23,
   pyg_register_class_init,
-  pyg_register_interface_info
+  pyg_register_interface_info,
+
+  pyg_closure_set_exception_handler
 };
 
 #define REGISTER_TYPE(d, type, name) \
Index: gobject/pygobject-private.h
===================================================================
RCS file: /cvs/gnome/gnome-python/pygtk/gobject/pygobject-private.h,v
retrieving revision 1.28
diff -u -3 -p -r1.28 pygobject-private.h
--- gobject/pygobject-private.h	9 Feb 2005 11:32:42 -0000	1.28
+++ gobject/pygobject-private.h	30 Apr 2005 00:07:29 -0000
@@ -38,6 +38,16 @@ extern struct _PyGObject_Functions pygob
             PyEval_RestoreThread(_save);                \
     } G_STMT_END
 
+#define PYGTK_OWNER_PYOBJECT 0
+#define PYGTK_OWNER_GOBJECT 1
+gint pyg_object_get_owner (GObject *object);
+void pyg_object_set_owner (GObject *object, gint owner);
+
+extern GStaticPrivate __pygobject_creation;
+#define pygtk_set_object_creation_mode(object) \
+  g_static_private_set (&__pygobject_creation, GINT_TO_POINTER (object), NULL)
+#define pygtk_get_object_creation_mode() \
+  GPOINTER_TO_INT (g_static_private_get (&__pygobject_creation))
 
 extern GType PY_TYPE_OBJECT;
 
@@ -69,6 +79,8 @@ PyObject *pyg_param_gvalue_as_pyobject(c
                                        const GParamSpec* pspec);
 
 GClosure *pyg_closure_new(PyObject *callback, PyObject *extra_args, PyObject *swap_data);
+void	  pyg_closure_set_exception_handler(GClosure *closure,
+					    PyClosureExceptionHandler handler);
 GClosure *pyg_signal_class_closure_get(void);
 
 PyObject *pyg_object_descr_doc_get(void);
Index: gobject/pygobject.c
===================================================================
RCS file: /cvs/gnome/gnome-python/pygtk/gobject/pygobject.c,v
retrieving revision 1.39
diff -u -3 -p -r1.39 pygobject.c
--- gobject/pygobject.c	27 Nov 2004 19:12:34 -0000	1.39
+++ gobject/pygobject.c	30 Apr 2005 00:07:29 -0000
@@ -506,7 +506,7 @@ pygobject_traverse(PyGObject *self, visi
 	if (ret != 0) return ret;
     }
 
-    if (self->obj && self->obj->ref_count == 1)
+    if (self->obj && pyg_object_get_owner (self->obj) == PYGTK_OWNER_PYOBJECT && self->obj->ref_count == 1)
 	ret = visit((PyObject *)self, arg);
     if (ret != 0) return ret;
 
@@ -557,6 +557,7 @@ pygobject_free(PyObject *op)
 
 /* ---------------- PyGObject methods ----------------- */
 
+extern GStaticPrivate __pygobject_creation;
 static int
 pygobject_init(PyGObject *self, PyObject *args, PyObject *kwargs)
 {
@@ -564,6 +565,7 @@ pygobject_init(PyGObject *self, PyObject
     guint n_params = 0, i;
     GParameter *params = NULL;
     GObjectClass *class;
+    int owner;
 
     if (!PyArg_ParseTuple(args, ":GObject.__init__", &object_type))
 	return -1;
@@ -584,6 +586,12 @@ pygobject_init(PyGObject *self, PyObject
 	return -1;
     }
 
+    if (pygtk_get_object_creation_mode ())
+	owner = PYGTK_OWNER_GOBJECT;
+    else
+	owner = PYGTK_OWNER_PYOBJECT;
+    pygtk_set_object_creation_mode (0);
+    
     if (kwargs) {
 	int pos = 0;
 	PyObject *key;
@@ -614,10 +622,26 @@ pygobject_init(PyGObject *self, PyObject
 	}
     }
 
+    /* FIXME: It would be nicer if we only set this when we know that we have
+     * a Python subclasses object to avoid screwups.
+     * That requires a gboolean pygtk_gtype_created_in_python (GType) though
+     */
+    pygtk_set_object_creation_mode (1);
     self->obj = g_object_newv(object_type, n_params, params);
-    if (self->obj)
-	pygobject_register_wrapper((PyObject *)self);
-    else
+    pygtk_set_object_creation_mode (0);
+    if (self->obj) {
+	pyg_object_set_owner (self->obj, owner);
+	if (owner == PYGTK_OWNER_GOBJECT) {
+	    if (!pygobject_wrapper_key)
+		pygobject_wrapper_key=g_quark_from_static_string(pygobject_wrapper_id);
+
+	    Py_INCREF(self);
+	    g_object_set_qdata_full(self->obj, pygobject_wrapper_key, self,
+				    pyg_destroy_notify);
+	} else {
+	    pygobject_register_wrapper((PyObject *)self);
+	}
+    } else
 	PyErr_SetString (PyExc_RuntimeError, "could not create object");
 	   
  cleanup:
Index: gobject/pygobject.h
===================================================================
RCS file: /cvs/gnome/gnome-python/pygtk/gobject/pygobject.h,v
retrieving revision 1.50
diff -u -3 -p -r1.50 pygobject.h
--- gobject/pygobject.h	10 Jan 2005 23:39:04 -0000	1.50
+++ gobject/pygobject.h	30 Apr 2005 00:07:29 -0000
@@ -17,12 +17,14 @@ G_BEGIN_DECLS
 #endif
 
   /* PyGClosure is a _private_ structure */
+typedef void (* PyClosureExceptionHandler) (GValue *ret, guint n_param_values, const GValue *params);
 typedef struct _PyGClosure PyGClosure;
 struct _PyGClosure {
     GClosure closure;
     PyObject *callback;
     PyObject *extra_args; /* tuple of extra args to pass to callback */
     PyObject *swap_data; /* other object for gtk_signal_connect_object */
+    PyClosureExceptionHandler exception_handler;
 };
 
 typedef struct {
@@ -160,6 +162,7 @@ struct _PyGObject_Functions {
     void      (*gil_state_release) (int flag);
     void      (*register_class_init) (GType gtype, PyGClassInitFunc class_init);
     void      (*register_interface_info) (GType gtype, const GInterfaceInfo *info);
+    void      (*closure_set_exception_handler) (GClosure *closure, PyClosureExceptionHandler handler);
 };
 
 #ifndef _INSIDE_PYGOBJECT_
@@ -177,6 +180,8 @@ struct _PyGObject_Functions *_PyGObject_
 #define pygobject_new               (_PyGObject_API->newgobj)
 #define pyg_closure_new             (_PyGObject_API->closure_new)
 #define pygobject_watch_closure     (_PyGObject_API->object_watch_closure)
+#define pyg_closure_set_exception_handler \
+				    (_PyGObject_API->closure_set_exception_handler)
 #define pyg_destroy_notify          (_PyGObject_API->destroy_notify)
 #define pyg_type_from_object        (_PyGObject_API->type_from_object)
 #define pyg_type_wrapper_new        (_PyGObject_API->type_wrapper_new)
Index: gobject/pygtype.c
===================================================================
RCS file: /cvs/gnome/gnome-python/pygtk/gobject/pygtype.c,v
retrieving revision 1.38
diff -u -3 -p -r1.38 pygtype.c
--- gobject/pygtype.c	23 Feb 2005 20:53:07 -0000	1.38
+++ gobject/pygtype.c	30 Apr 2005 00:07:30 -0000
@@ -846,16 +846,24 @@ pyg_closure_marshal(GClosure *closure,
     }
     ret = PyObject_CallObject(pc->callback, params);
     if (ret == NULL) {
-	PyErr_Print();
+	if (pc->exception_handler)
+	    pc->exception_handler (return_value, n_param_values, param_values);
+	else
+	    PyErr_Print();
 	goto out;
     }
-    if (return_value)
-	pyg_value_from_pyobject(return_value, ret);
+    if (return_value &&
+	pyg_value_from_pyobject(return_value, ret) != 0) {
+	PyErr_SetString(PyExc_TypeError, "can't convert return value to desired type");
+	if (pc->exception_handler)
+	    pc->exception_handler (return_value, n_param_values, param_values);
+	else
+	    PyErr_Print();
+    }
     Py_DECREF(ret);
     
  out:
     Py_DECREF(params);
-    
     pyg_gil_state_release(state);
 }
 
@@ -899,6 +907,28 @@ pyg_closure_new(PyObject *callback, PyOb
     return closure;
 }
 
+/**
+ * pyg_closure_set_exception_handler:
+ * @closure: a closure created with pyg_closure_new()
+ * @handler: the handler to call when an exception occurs or NULL for none
+ *
+ * Sets the handler to call when an exception occurs during closure invocation.
+ * The handler is responsible for providing a proper return value to the 
+ * closure invocation. If @handler is %NULL, the default handler will be used.
+ * The default handler prints the exception to stderr and doesn't touch the
+ * closure's return value.
+ */
+void
+pyg_closure_set_exception_handler(GClosure *closure,
+					PyClosureExceptionHandler handler)
+{
+    PyGClosure *pygclosure;
+    
+    g_return_if_fail(closure != NULL);
+
+    pygclosure = (PyGClosure *) closure;
+    pygclosure->exception_handler = handler;
+}
 /* -------------- PySignalClassClosure ----------------- */
 /* a closure used for the `class closure' of a signal.  As this gets
  * all the info from the first argument to the closure and the