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 584389 - don't recreate plugin feature and element factories on first use
don't recreate plugin feature and element factories on first use
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal enhancement
: 0.10.24
Assigned To: Stefan Sauer (gstreamer, gtkdoc dev)
GStreamer Maintainers
: 571604 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2009-05-31 19:34 UTC by Stefan Sauer (gstreamer, gtkdoc dev)
Modified: 2009-06-07 20:55 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
reuse plugin feature and element factories on first use (5.38 KB, patch)
2009-05-31 19:41 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
none Details | Review
reuse plugin feature and element factories on first use (6.15 KB, patch)
2009-06-01 10:16 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
none Details | Review
reuse plugin feature and element factories on first use (6.56 KB, patch)
2009-06-02 14:31 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
none Details | Review

Description Stefan Sauer (gstreamer, gtkdoc dev) 2009-05-31 19:34:24 UTC
GST_DEBUG_NO_COLOR=1 GST_DEBUG="GST_REGISTRY:4" gst-launch 2>debug.log audiotestsrc num-buffers=1 ! pulsesink

grep "replacing" debug.log
gstregistry.c:447:gst_registry_add_feature:<registry0> replacing existing feature 0x80ebf50 (audiotestsrc)
gstregistry.c:447:gst_registry_add_feature:<registry0> replacing existing feature 0x81a2890 (pulsesink)
...

basically, the first time we call gst_element_factory_make(), we recreate the plugin feature and the element factory. As a side effect we ref the class to fill in detail we already have filled from the registry cache. This is bad for wrapper plugins like ladspa, as creating the first ladspa element would load *all* ladspa *so (as it invokes the class init for each type).

The attached patch changes the behaviour to not thow the features away, but update them. It won't ref the class anymore. This has the side-effect that it can't fill klass->elementfactory = factory in gst_element_register() anymore and thus relies on gst_element_factory_create() setting it (and this has a FIXME: about thread safty).

I'd like to post this here for discussion and review.
Comment 1 Stefan Sauer (gstreamer, gtkdoc dev) 2009-05-31 19:34:59 UTC
Forgot to mention that it make one test fails:
gst/gstelement.c:201:F:element tests:test_class:0: Failure 'klass->elementfactory == NULL' occured
Comment 2 Stefan Sauer (gstreamer, gtkdoc dev) 2009-05-31 19:41:18 UTC
Created attachment 135685 [details] [review]
reuse plugin feature and element factories on first use
Comment 3 Stefan Sauer (gstreamer, gtkdoc dev) 2009-06-01 10:16:52 UTC
Created attachment 135716 [details] [review]
reuse plugin feature and element factories on first use

This make setting the factory gst_element_factory_create() atomic.

Whats the thinking about the failing test in gst/gstelement.c::201 test_class()?
Comment 4 Stefan Sauer (gstreamer, gtkdoc dev) 2009-06-01 11:34:47 UTC
There are two uses of class->elementfactory and both are from an instance where this is defined (gst_element_get_factory() and gst_element_save_thyself()).
Comment 5 Stefan Sauer (gstreamer, gtkdoc dev) 2009-06-02 14:31:49 UTC
Created attachment 135812 [details] [review]
reuse plugin feature and element factories on first use

don't unref the class when finalizing, as we don't ref it anymore
Comment 6 Stefan Sauer (gstreamer, gtkdoc dev) 2009-06-07 18:46:33 UTC
*** Bug 571604 has been marked as a duplicate of this bug. ***
Comment 7 Stefan Sauer (gstreamer, gtkdoc dev) 2009-06-07 20:55:35 UTC
Almost the same as the last patch, except that now the tests pass unmodified.

commit 55577a48ea9e93ec6532b1e81684e4c0ae26157c
Author: Stefan Kost <ensonic@users.sf.net>
Date:   Sun Jun 7 22:09:14 2009 +0300

    registry: don't recreate features on first use. Fixes #584389

    The first time one calls gst_element_factory_make(), gst recreates the
plugin
    feature and the element factory. As a side effect we ref the class to fill
    in detail we already have filled from the registry cache. This patch
changes
    the behaviour to just update the existing entries. The factory is now
attached
    to the type and set in gst_element_base_class_init().