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 788088 - NotInitialized should not be raised if Gst already initialized by C
NotInitialized should not be raised if Gst already initialized by C
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-python
git master
Other Linux
: Normal normal
: 1.13.90
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-09-24 09:28 UTC by Bruce Merry
Modified: 2018-02-22 14:58 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gi: Check Gst has not been initialized before loading bindings (743 bytes, patch)
2017-09-25 00:45 UTC, Thibault Saunier
committed Details | Review

Description Bruce Merry 2017-09-24 09:28:20 UTC
I develop a plugin for Rhythmbox. Rhythmbox uses Gst from C, but also embeds a Python interpreter to allow plugins to be written. The plugin does not call Gst.init, because Rhythmbox already initializes it at the C level - I can check that by calling Gst.is_initialized from the plugin with Gst 1.0.

Commit https://phabricator.freedesktop.org/rGSTPYTHON6b32ccbbb250f7516f36a1ea82c0fbba8925af09 in gst-python 1.5.2 breaks the plugin (bug report from a user: https://github.com/bmerry/rbtempo/issues/2). It's easy enough to work around by initialising Gst again (fortunately Gst seems to protect against double-init), but I still consider it to be a bug in gst-python that it raises NotInitialized when Gst has been initialised.

I suggest that at minimum, the deinit_pygst at the bottom of Gst.py should be made conditional on a check of is_initialized. A more robust approach that would allow for an out-of-band (C-level) init after gst-python is loaded would be to generate a separate fake function for each real function that would first check is_initialized, and if true would put the real functions in place and transparently forward to the real function.

I've filed this against git master because the code on master looks like it will still do the same thing, although I haven't actually tried installing it.
Comment 1 Thibault Saunier 2017-09-25 00:45:49 UTC
Created attachment 360327 [details] [review]
gi: Check Gst has not been initialized before loading bindings

It can have been initialized by some C code (in a C app with plugins
for example).

Fixes
Comment 2 Thibault Saunier 2017-09-25 00:46:27 UTC
I had not thought about that case :-)

Please check if that fixes.
Comment 3 Bruce Merry 2017-09-25 05:51:58 UTC
Thanks for the quick response! It will unfortunately take me a bit longer than that to confirm the fix, since I'll need to test up a test environment to reproduce the bug for myself.
Comment 4 Bruce Merry 2017-10-04 18:48:42 UTC
Apologies for the delay: I didn't realise that I already had >= 1.5.2 installed (Ubuntu still puts "1.0" in the package name), and hence didn't actually need to muck about with VMs to test this. On the other hand, I hadn't seen the issue myself because it only appears after installing the overrides.

I've now tested your patch against 1.6.2 and can confirm that it fixes the issue.
Comment 5 Sebastian Dröge (slomo) 2018-02-22 12:15:37 UTC
Why is it calling gst_deinit() at all? That would also break Python applications that want to initialize GStreamer again, or Python applications that initialize GStreamer via the bindings (just because they came first) but have GStreamer code still running while the bindings are gone.
Comment 6 Thibault Saunier 2018-02-22 13:24:43 UTC
(In reply to Sebastian Dröge (slomo) from comment #5)
> Why is it calling gst_deinit() at all? That would also break Python
> applications that want to initialize GStreamer again, or Python applications
> that initialize GStreamer via the bindings (just because they came first)
> but have GStreamer code still running while the bindings are gone.

It is calling `pygst_deinit` which monkey patches all Gst method to raise an exception if someone is trying to call them before initalization. Or I misunderstand your remark?
Comment 7 Sebastian Dröge (slomo) 2018-02-22 14:58:53 UTC
I was confused by the name. So this is not calling gst_deinit()? :) Then all is fine.