After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 631901 - Proof of concept fundamental support
Proof of concept fundamental support
Status: RESOLVED OBSOLETE
Product: pygobject
Classification: Bindings
Component: introspection
unspecified
Other All
: Normal normal
: ---
Assigned To: Nobody's working on this now (help wanted and appreciated)
Python bindings maintainers
: 648531 659193 660612 664362 664513 671569 (view as bug list)
Depends on: 709700
Blocks: 685275
 
 
Reported: 2010-10-11 17:10 UTC by Tomeu Vizoso
Modified: 2018-01-10 20:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proof of concept fundamental support (17.85 KB, patch)
2010-10-11 17:11 UTC, Tomeu Vizoso
none Details | Review
Add support for fundamental types (28.79 KB, patch)
2012-04-17 15:54 UTC, Bastian Winkler
needs-work Details | Review
Some cleanups and modifications according to comment #18. (28.61 KB, patch)
2012-04-24 16:14 UTC, Bastian Winkler
needs-work Details | Review
Use accessors for getting and setting PyGBoxed pointers (5.16 KB, patch)
2014-05-16 21:28 UTC, Simon Feltman
committed Details | Review
Use accessors for getting and setting PyGPointer fields (4.35 KB, patch)
2014-05-16 21:57 UTC, Simon Feltman
committed Details | Review
Use accessors for getting and setting PyGParamSpec pointers (4.58 KB, patch)
2014-05-16 22:11 UTC, Simon Feltman
committed Details | Review

Description Tomeu Vizoso 2010-10-11 17:10:34 UTC
Not ready for review, more tests are needed, comments most welcome.
Comment 1 Tomeu Vizoso 2010-10-11 17:11:08 UTC
Created attachment 172112 [details] [review]
Proof of concept fundamental support
Comment 2 Johan (not receiving bugmail) Dahlin 2011-02-10 13:36:06 UTC
GstMiniObject is the interesting part, we should make sure that Gst.Message works fine in python before committing this.

For instance, by porting over http://cgit.freedesktop.org/gstreamer/gst-python/tree/examples/play.py
Comment 3 Tomeu Vizoso 2011-02-10 14:30:22 UTC
Actually was testing with a few examples from gst-python when developing this, but I don't remember any more which ones :/
Comment 4 Johan (not receiving bugmail) Dahlin 2011-03-18 11:44:04 UTC
GStreamer 0.11 is switching over to boxed types instead of miniobject, so this is not as necessary when GStreamer 1.0 is released. But meanwhile we might want to get it in.
Comment 5 Tim-Philipp Müller 2011-09-15 22:27:37 UTC
*** Bug 659193 has been marked as a duplicate of this bug. ***
Comment 6 Tim-Philipp Müller 2011-10-01 17:51:37 UTC
*** Bug 660612 has been marked as a duplicate of this bug. ***
Comment 7 Jason Gerard DeRose 2011-10-03 22:55:01 UTC
For what it's worth, I think this is a decent test case for Gst.Message:


#!/usr/bin/python

from gi.repository import GObject
GObject.threads_init()

from gi.repository import Gst
Gst.init(None)

mainloop = GObject.MainLoop()
pipeline = Gst.Pipeline()

def on_eos(bus, msg):
    print('eos: {!r}'.format(msg))
    pipeline.set_state(gst.STATE_NULL)
    mainloop.quit()

def on_message(bus, msg):
    print('message: {!r}'.format(msg))

bus = pipeline.get_bus()
bus.add_signal_watch()
bus.connect('message::eos', on_eos)
bus.connect('message', on_message)

bus = pipeline.get_bus()
bus.add_signal_watch()
bus.connect('message::eos', on_eos)

src = Gst.ElementFactory.make('videotestsrc', None)
src.set_property('num-buffers', 10)
sink = Gst.ElementFactory.make('fakesink', None)
pipeline.add(src)
pipeline.add(sink)
src.link(sink)

pipeline.set_state(Gst.State.PLAYING)
mainloop.run()
Comment 8 Ángel Guzmán Maeso (shakaran) 2011-11-01 10:58:08 UTC
I am having the same problem with the signal "sync-message::element". It only emit sync-message but with None message. The signal emitter seems to be broken with GTK3.

It could be a problem with GstMessageType (in this case: GST_MESSAGE_EOS. GST_MESSAGE_ERROR and GST_MESSAGE_ELEMENT      
http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gstreamer/html/gstreamer-GstMessage.html

Perhaps GstMessage doesn't send the type structure GstMessageType:

struct GstMessage {
  GstMiniObject mini_object;

  GstMessageType type;
  guint64 timestamp;
  GstObject *src;

  GstStructure *structure;
};

The problem maybe is near of _wrap_gst_message_new_element

http://code.google.com/p/ossbuild/source/browse/trunk/Main/GStreamer/Generated/gst-python/gst.c?r=114&spec=svn114#23185

There are some progress with this? It is important support this for new migration to GTK3.
Comment 9 Tim-Philipp Müller 2011-11-22 00:39:19 UTC
*** Bug 664513 has been marked as a duplicate of this bug. ***
Comment 10 Tim-Philipp Müller 2012-02-19 12:51:34 UTC
*** Bug 664362 has been marked as a duplicate of this bug. ***
Comment 11 Tim-Philipp Müller 2012-03-07 16:15:18 UTC
*** Bug 671569 has been marked as a duplicate of this bug. ***
Comment 12 Gonzalo Odiard 2012-03-07 16:37:18 UTC
Any advance in this issue?
Comment 13 Tomeu Vizoso 2012-03-07 16:49:38 UTC
(In reply to comment #12)
> Any advance in this issue?

As far as I know, nobody is working on it.
Comment 14 Tim-Philipp Müller 2012-03-25 22:31:23 UTC
Regarding GstMiniObject, apparently that's supposed to work already ?!

<ebassi> tpm: http://git.gnome.org/browse/clutter/tree/clutter/clutter-paint-node.c#n44
<ebassi> tpm: support for fundamental types has been in g-i for at least a year
<ebassi> tpm: it was added especially for GstMiniObject :-)
Comment 15 Tomeu Vizoso 2012-03-26 06:50:46 UTC
(In reply to comment #14)
> Regarding GstMiniObject, apparently that's supposed to work already ?!
> 
> <ebassi> tpm:
> http://git.gnome.org/browse/clutter/tree/clutter/clutter-paint-node.c#n44
> <ebassi> tpm: support for fundamental types has been in g-i for at least a year
> <ebassi> tpm: it was added especially for GstMiniObject :-)

Yes, there's support in g-i, which is what made this patch possible, but we still need someone who will take the patch through review into master.
Comment 16 Bastian Winkler 2012-04-17 15:54:06 UTC
Created attachment 212221 [details] [review]
Add support for fundamental types

Implement support for fundamental types like Gst.MiniObject and
Clutter.PaintNode
Comment 17 Martin Pitt 2012-04-23 17:53:07 UTC
*** Bug 648531 has been marked as a duplicate of this bug. ***
Comment 18 Martin Pitt 2012-04-24 05:53:55 UTC
Thanks for working on this!

I took a closer look to the patch now, and I understand most of it. But I'm afraid you need to help me with some things, as I'm not familiar with some of the things that it changes, so I have some questions:

In the changes to _pygi_argument_to_object(), this does not take the transfer mode into account, like in the GObject case. I take it that's because the gi.Fundamentals object are not wrapped like GObjects?

In _fundamental_dealloc(), why does this not call info's g_object_info_get_unref_function()? Is the actual Fundamental object already freed at this point, and this is only for freeing the Python wrapper around it?

gi/pygi-fundamental.h defines

   +#define _pygi_check_fundamental ((info_type, info)) (

which looks like the phone rang when you were writing this, and it's not being used anywhere. It was probably meant to be defined to something like

  (info_type == GI_INFO_TYPE_OBJECT && g_object_info_get_fundamental ( (GIObjectInfo *)info))

Can you please add some test cases which exercise the new code paths? For example:

 - setting properties (obj.data = ...), to test the change in pygi_set_property_value_real(), using compatible and incompatible types

 - getting values of different data types, including None and objects

 - Calling methods on the Fundamental, testing none and full transfer modes for an argument and return value

 - constructor and method calls with keyword arguments

 - Passing a Fundamental object in an array to a method; array handling keeps causing crashes and malfunctions, so I think it's important to make sure that this keeps working.

With this patch I get some ref count failures in make check. Are these actual leaks, or do the tests need to be adapted?

======================================================================
FAIL: test_object_full_inout (test_gi.TestGObject)
----------------------------------------------------------------------
Traceback (most recent call last):
  • File "/home/martin/upstream/pygobject/tests/test_gi.py", line 1580 in test_object_full_inout
    self.assertEqual(new_object.__grefcount__, 1)
AssertionError: 2 != 1

======================================================================
FAIL: test_object_full_out (test_gi.TestGObject)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/martin/upstream/pygobject/tests/test_gi.py", line 1553, in test_object_full_out
    self.assertEqual(object_.__grefcount__, 1)
AssertionError: 2 != 1

======================================================================
FAIL: test_object_full_return (test_gi.TestGObject)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/martin/upstream/pygobject/tests/test_gi.py", line 1527, in test_object_full_return
    self.assertEqual(object_.__grefcount__, 1)
AssertionError: 2 != 1

======================================================================
FAIL: test_object_new (test_gi.TestGObject)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/martin/upstream/pygobject/tests/test_gi.py", line 1468, in test_object_new
    self.assertEqual(object_.__grefcount__, 1)
AssertionError: 2 != 1


Thanks again!
Comment 19 Bastian Winkler 2012-04-24 16:10:34 UTC
(In reply to comment #18)

> gi/pygi-fundamental.h defines
> 
>    +#define _pygi_check_fundamental ((info_type, info)) (
> 
> which looks like the phone rang when you were writing this, and it's not being
> used anywhere. It was probably meant to be defined to something like
> 
>   (info_type == GI_INFO_TYPE_OBJECT && g_object_info_get_fundamental (
> (GIObjectInfo *)info))

Ouch! Sorry for that, I was on holiday when I wrote the patch... I really
should have noticed that

> With this patch I get some ref count failures in make check. Are these actual
> leaks, or do the tests need to be adapted?

No, it's a typo in my code
Comment 20 Bastian Winkler 2012-04-24 16:14:49 UTC
Created attachment 212717 [details] [review]
Some cleanups and modifications according to comment #18.

I'll try to come up with some more test cases the next days.
Comment 21 Martin Pitt 2012-05-04 05:09:11 UTC
Comment on attachment 212717 [details] [review]
Some cleanups and modifications according to comment #18.

Setting to "needs-work", as this needs a lot more test cases (see the previous comments).

Thanks muchly for working on this!
Comment 22 Ángel Guzmán Maeso (shakaran) 2012-06-09 11:14:25 UTC
There are some progress with this? Could someone increase the priority?

This is showstopping bug for play videos with PyGObject and Gst 0.11. I am waiting a fix since 6 months ago. I will try to fix this if I have knowledge about GTK and PyGObject or enough money for pay somebody, but unfortunaly I can't.

Upgrade APIs like PyGObject it is ok, but introduce regressions breaking functionality should be taken more seriously, at least if open source world wants more market share.
Comment 23 Tim-Philipp Müller 2012-06-09 11:26:35 UTC
> This is showstopping bug for play videos with PyGObject and Gst 0.11. I am

Gst 0.11/1.0 doesn't use fundamental types any longer, it's all boxed types now, which should work fine already (with pygi), as far as I know anyway.
Comment 24 Ángel Guzmán Maeso (shakaran) 2012-06-09 11:36:38 UTC
Tim, could you post a simple test as working example playing a video with playbin and getting message::eos?

The example of Jason Gerard DeRose crash:

pipeline.add(src)
  • File "/usr/lib/python2.7/dist-packages/gi/types.py", line 43 in function
    return info.invoke(*args, **kwargs)
TypeError: Argument 1 does not allow None as a value

(python2.7:17553): GLib-GObject-CRITICAL **: g_object_get_qdata: assertion `G_IS_OBJECT (object)' failed

(python2.7:17553): GLib-GObject-CRITICAL **: g_object_set_qdata_full: assertion `G_IS_OBJECT (object)' failed

(python2.7:17553): GLib-GObject-CRITICAL **: g_object_get_qdata: assertion `G_IS_OBJECT (object)' failed

(python2.7:17553): GLib-GObject-CRITICAL **: g_object_set_qdata_full: assertion `G_IS_OBJECT (object)' failed

(python2.7:17553): GLib-GObject-CRITICAL **: g_object_get_qdata: assertion `G_IS_OBJECT (object)' failed

(python2.7:17553): GLib-GObject-CRITICAL **: g_object_set_qdata_full: assertion `G_IS_OBJECT (object)' failed

(python2.7:17553): GLib-GObject-CRITICAL **: g_object_set_qdata_full: assertion `G_IS_OBJECT (object)' failed

(python2.7:17553): GLib-GObject-CRITICAL **: g_object_unref: assertion `G_IS_OBJECT (object)' failed

I am testing too with another simple example

print Gst.ElementFactory.find('playbin') // This prints <GType invalid (0)>
        
print Gst.ElementFactory.make('playbin', 'MultimediaPlayer') // This prints None
Comment 25 Tim-Philipp Müller 2012-06-09 12:58:27 UTC
> Tim, could you post a simple test as working example playing a video with
> playbin and getting message::eos? The example of Jason Gerard DeRose crash

Here you go: http://people.freedesktop.org/~tpm/code/playbin-gst-1.0-pygi.py

I believe Jason was playing with pygi and GStreamer 0.10. In any case, let's move this discussion somewhere else (e.g. #gstreamer IRC channel or gstreamer-devel mailing list).
Comment 26 Simon Feltman 2014-01-10 00:18:51 UTC
I've been working a different approach to the proposal here. The idea is to make PyGPointer the base Python type for almost everything in the bindings. Python based fundamentals are then dynamically generated using PyGPointer as a base class with their memory management functions as specializations. However, we will still most likely need a GBoxed based version of GPointer because these require the GType as an argument for memory management functions. The layout would look something like this:

PyGPointer
  PyGObject
  PyGBoxed
  PyGStruct (unclear if this is still be needed)
  <dynamically derived fundamentals>


In this setup, PyGPointer has ref/unref slots (probably stashed in a custom meta-class) which derived types can specialize. So for fundamentals, they are filled in with the results of g_object_info_get_ref_function_pointer and g_object_info_get_unref_function_pointer. What is interesting is this could actually work for GObject itself (but PyGObject also requires custom behavior beyond memory management).

I'm adding bug 712197 and bug 709700 as a dependencies because the refactoring work in those tickets is needed to accomplish this cleanly.
Comment 27 Simon Feltman 2014-05-16 21:28:10 UTC
The following fix has been pushed:
92fe522 Use accessors for getting and setting PyGBoxed pointers
Comment 28 Simon Feltman 2014-05-16 21:28:16 UTC
Created attachment 276688 [details] [review]
Use accessors for getting and setting PyGBoxed pointers

Add pyg_boxed_get_ptr and pyg_boxed_set_ptr macros for getting and setting
the boxed pointer field. This is preliminary cleanup work for supporting
fundamental types.
Comment 29 Simon Feltman 2014-05-16 21:56:56 UTC
The following fix has been pushed:
b49179b Use accessors for getting and setting PyGPointer fields
Comment 30 Simon Feltman 2014-05-16 21:57:00 UTC
Created attachment 276691 [details] [review]
Use accessors for getting and setting PyGPointer fields

Add pyg_pointer_get_ptr and pyg_pointer_set_ptr macros for getting and
setting the pointer field. This is preliminary cleanup work for supporting
fundamental types.
Comment 31 Simon Feltman 2014-05-16 22:11:51 UTC
The following fix has been pushed:
bbbfa96 Use accessors for getting and setting PyGParamSpec pointers
Comment 32 Simon Feltman 2014-05-16 22:11:56 UTC
Created attachment 276693 [details] [review]
Use accessors for getting and setting PyGParamSpec pointers

Add pyg_param_spec_get and pyg_param_spec_set macros for getting and
setting the GParamSpec pointer field held by the Python wrapper. This
is preliminary cleanup work for supporting fundamental types.
Comment 33 GNOME Infrastructure Team 2018-01-10 20:02:07 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/pygobject/issues/8.