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 560601 - gstreamermm uses symbols of gstreamerbasemm
gstreamermm uses symbols of gstreamerbasemm
Status: RESOLVED FIXED
Product: gstreamermm
Classification: Bindings
Component: general
git master
Other Linux
: Normal major
: ---
Assigned To: gtkmm-forge
gtkmm-forge
Depends on:
Blocks:
 
 
Reported: 2008-11-13 08:02 UTC by Sebastian Dröge (slomo)
Modified: 2011-01-16 23:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to test using gstreamerbasemm element without calling GstBase::wrap_init() (1.94 KB, patch)
2008-11-18 04:22 UTC, José Alburquerque
none Details | Review

Description Sebastian Dröge (slomo) 2008-11-13 08:02:24 UTC
Hi,
in gstreamermm/init.cc a symbol of gstreamerbasemmm (GstBase::wrap_init) is used. This is going to cause failures in several situations.

First of all it will fail if an application just uses gstreamermm and doesn't link to gstreamerbasemm. This can't be fixed by linking gstreamermm to gstreamerbasemm as the latter also uses stuff from the former.

And then it of course fails at build time when linking with --as-needed as this symbol can't be resolved (gstreamerbasemm isn't build at that time and can't even be build at that time).

The fix for this would be, to not call GstBase::wrap_init from gstreamermm and let the user do it if he needs it.
Comment 1 José Alburquerque 2008-11-13 15:57:53 UTC
(In reply to comment #0)
> First of all it will fail if an application just uses gstreamermm and doesn't
> link to gstreamerbasemm. This can't be fixed by linking gstreamermm to
> gstreamerbasemm as the latter also uses stuff from the former.

But the pgk-config file for gstreamermm is set up in such a way as to link to gstreamerbasemm.  I would hope that the developer uses that.  In that case it would not fail.

> And then it of course fails at build time when linking with --as-needed as this
> symbol can't be resolved (gstreamerbasemm isn't build at that time and can't
> even be build at that time).
> 
> The fix for this would be, to not call GstBase::wrap_init from gstreamermm and
> let the user do it if he needs it.

Yes.  We sort of discussed this in bug #556570 and decided to not have to require users to have to call GstBase::wrap_init() separately.
Comment 2 José Alburquerque 2008-11-13 16:07:51 UTC
Murray, what do you think about having the user call GstBase::init() sepeartely?
Comment 3 Murray Cumming 2008-11-13 16:18:42 UTC
Just to allow an --as-needed build?(In reply to comment #2)
> Murray, what do you think about having the user call GstBase::init()
> sepeartely?

Just to allow an --as-needed build? What is the big advantage of that?

Comment 4 José Alburquerque 2008-11-13 17:15:36 UTC
You're right.  Also closing as WONT_FIX.  Thanks.
Comment 5 Murray Cumming 2008-11-13 17:27:42 UTC
I have no idea. I haven't given this much thought.
Comment 6 José Alburquerque 2008-11-13 17:45:18 UTC
Okay, I'll wait for Sebastian to comment first.
Comment 7 Sebastian Dröge (slomo) 2008-11-13 19:22:21 UTC
Wouldn't it be possible and enough to simply call GstBase::init() from all constructors of GstBase classes or something like that?

Whatever, if you want this tight circular dependency between gstreamermm and gstreamerbasemm, why don't you put both into a single library? This would fix this bug and would also result in less memory usage and less dynamic linker time at runtime without losing anything (as both libraries are always loaded together anyway) ;)
Comment 8 José Alburquerque 2008-11-13 19:34:05 UTC
Um...Just for the record, here's what the --as-needed flag does (from ld man page):

       --as-needed
       --no-as-needed
           This  option  affects ELF DT_NEEDED tags for dynamic libraries men‐
           tioned on the command line after the --as-needed option.  Normally,
           the  linker  will add a DT_NEEDED tag for each dynamic library men‐
           tioned on the command line, regardless of whether  the  library  is
           actually  needed.   --as-needed  causes  DT_NEEDED  tags to only be
           emitted for libraries that satisfy some symbol reference from regu‐
           lar  objects  which  is undefined at the point that the library was
           linked.  --no-as-needed restores the default behaviour.
Comment 9 Sebastian Dröge (slomo) 2008-11-13 19:42:03 UTC
Sorry, of course you're right... I meant -z,defs which will make the build fail for unresolved symbols.

The confusion came from the fact that we (Debian/Ubuntu) are using --as-needed together with -z,defs to make sure that --as-needed doesn't break anything.

Still, for cleanness and effiency it should be either a single library or some other solution should be found IMHO ;)
Comment 10 José Alburquerque 2008-11-13 20:17:10 UTC
(In reply to comment #9)
> Sorry, of course you're right... I meant -z,defs which will make the build fail
> for unresolved symbols.
> 
> The confusion came from the fact that we (Debian/Ubuntu) are using --as-needed
> together with -z,defs to make sure that --as-needed doesn't break anything.

I just wanted it to be clear the use of --as-needed. :-)

> 
> Still, for cleanness and effiency it should be either a single library or some
> other solution should be found IMHO ;)
> 

Having a single library would be one option (that's how it was originally).  The two other options are: 1) to leave the division as is and require the user to initialize the gstreamerbasemm branch or 2) to leave the division intact and not require the user to initialize the gstreamerbasemm branch (sort of what is done in the C API) but then not supporting the --as-needed linker flag or the -z,defs linker flag (which disallows undefined symbols in object files but not in shared objects when linking).

The constructor idea above is not possible because GstBase::wrap_init() (and Gst::wrap_init() for that matter) register the class wrapper GTypes and must be run before any classes are constructed, but thanks for the merge suggestion.  We'll certainly decide a little later.
Comment 11 Sebastian Dröge (slomo) 2008-11-14 06:55:49 UTC
(In reply to comment #10)
 
> Having a single library would be one option (that's how it was originally). 

Just curious but why did you decide the split? I see no advantages that this would give you
Comment 12 José Alburquerque 2008-11-14 16:58:48 UTC
I decided the split mostly for logical reasons.  I thought that developers that followed the C API docs  would notice that there is a split in GStreamer between core and gst-plugins-base.  This would help them see the same split in gstreamermm.  If you look at the docs you will see that the classes are sort of split into namespaces, mostly for this reason.  Also, I thought that there would be many classes wrapped from core and also from gst-plugins-base (I think we'll also try to include a the additional plugins by generating their source somehow) and it just felt like it would be too many classes to place in one branch, thus the split.
Comment 13 Sebastian Dröge (slomo) 2008-11-15 07:45:08 UTC
(In reply to comment #12)
> I decided the split mostly for logical reasons.  I thought that developers that
> followed the C API docs  would notice that there is a split in GStreamer
> between core and gst-plugins-base.  This would help them see the same split in
> gstreamermm.  If you look at the docs you will see that the classes are sort of
> split into namespaces, mostly for this reason.  Also, I thought that there
> would be many classes wrapped from core and also from gst-plugins-base (I think
> we'll also try to include a the additional plugins by generating their source
> somehow) and it just felt like it would be too many classes to place in one
> branch, thus the split.

IMHO it would be enough to give them a different namespace then inside the same library instead of a different library ;)

Also, GstBase contains gst-plugins-base stuff and Gst contains stuff of the libgstbase library from core? That's confusing then ;)
Comment 14 José Alburquerque 2008-11-16 02:00:07 UTC
(In reply to comment #13)
> IMHO it would be enough to give them a different namespace then inside the same
> library instead of a different library ;)

I don't think this is possible.  The C++ source files are not just handwritten, they also go through a generation process from preliminary source files (.hg and .ccg files).  The namespace needs to stay constant for generated classes throughout a branch (such as the gstreamer or the gstreamerbase branch) so that the wrap_init() method (which is generated) knows how to generate initialization code for classes.  Feel free to look at the automake structure and the generate_gst_wrap_init tool (and its usage in the build process) to confirm this yourself.

> 
> Also, GstBase contains gst-plugins-base stuff and Gst contains stuff of the
> libgstbase library from core? That's confusing then ;)
> 

GstBase is intended to wrap gst-plugin-base stuff so it should contain gst-plugins-base stuff (I think).  As far as Gst using libgstbase stuff, I think that dependency got in there accidentally (I must have misunderstood something), but it's fixed in svn, thanks:

2008-11-15  José Alburquerque  <jaalburqu@svn.gnome.org>

	* configure.ac: Remove gstreamer-base-0.10 as dependency of
	gstreamermm.

As far as this bug is concerned, I'll be investigating further to see what can be done.  I'll post back with findings soon.  This should be taken care of before next release.  Thanks.
Comment 15 José Alburquerque 2008-11-16 02:41:57 UTC
(In reply to comment #14)
> > Also, GstBase contains gst-plugins-base stuff and Gst contains stuff of the
> > libgstbase library from core? That's confusing then ;)
> > 
> 
> GstBase is intended to wrap gst-plugin-base stuff so it should contain
> gst-plugins-base stuff (I think).  As far as Gst using libgstbase stuff, I
> think that dependency got in there accidentally (I must have misunderstood
> something), but it's fixed in svn, thanks:
> 

Just so you know, this is the design I'm following for now:

The design is that gstreamermm includes (or will if the design is right) GStreamer Core[1], GStreamer Library[2] and GStreamer Elements (or plugins)[3].  gstreamerbasemm, OTHO would include GStreamer Base Plugins Library[4] and (if necessary and possible, which I think it should be) gst-plugins-base Elements (or plugins)[5].  The links are all from the GStreamer docs page[6].

[1] http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gstreamer/html/
[2] http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gstreamer-libs/html/
[3] http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gstreamer-plugins/html/
[4] http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-base-libs/html/
[5] http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-base-plugins/html/
[6] http://gstreamer.freedesktop.org/documentation/
Comment 16 Sebastian Dröge (slomo) 2008-11-16 07:21:46 UTC
Ok, that makes sense then IMHO. One question though, how do you want to wrap the plugins/elements? I mean, their instance/class struct sizes are not guaranteed to stay the same as they're not part of the ABI. You could only create hand-written wrappers that only access the elements via generic interfaces like GObject properties, gstreamer interfaces, etc. Also distributors might want to choose to not include some elements, it's not guaranteed that everybody has audioresample for example :)

Then for gstreamer-base-0.10, you need it for Gst::BaseSrc and friends so this should still be linked in, IIRC you're wrapping basesrc at least.

Whatever, for the design you're following now, it would still mean that gstreamermm and gstreamerbasemm are using symbols from each other and depend on each other, right? And that this is, because you don't want the application to call a GstBase initialization function and because you can't let the source generation put everything into one library and different namespaces, right? In that case I guess this bug can't be properly solved ;)
Comment 17 José Alburquerque 2008-11-16 20:59:24 UTC
(In reply to comment #16)
> Ok, that makes sense then IMHO. One question though, how do you want to wrap
> the plugins/elements? I mean, their instance/class struct sizes are not
> guaranteed to stay the same as they're not part of the ABI. 

Do you mean that their properties and signals are not guaranteed to remain the same (ABI stable) in major releases?  If so, then the plugin generation would have to be adjusted so that ABI is not affected when gstreamermm becomes stable (this would be something to think about).  The C++ classes will only wrap the properties and signals so if these do not change, the C++ class, I think, should not loose ABI.

> You could only
> create hand-written wrappers that only access the elements via generic
> interfaces like GObject properties, gstreamer interfaces, etc. Also
> distributors might want to choose to not include some elements, it's not
> guaranteed that everybody has audioresample for example :)

By using introspection I think it is possible to have the generating process generate C++ wrapper classes during the build process if things work well this way (depending on the ABI issues you mentioned).  If so, the build process can be used to determine which ones should be generated and which ones should not.  It's a bit tricky to explain, but if you want some idea of how this might be done, you can look at what has been done so far (the last few ChangeLog entries).  Also, keep in mind that gstreamermm is still unstable and these things can be ironed out before it becomes stable.  If plugin generation is not possible, it will probably be abandoned.

> 
> Then for gstreamer-base-0.10, you need it for Gst::BaseSrc and friends so this
> should still be linked in, IIRC you're wrapping basesrc at least.

Then it was right the first time. :-)  It's a bit difficult to keep track of which lib to include, etc. :-)

> 
> Whatever, for the design you're following now, it would still mean that
> gstreamermm and gstreamerbasemm are using symbols from each other and depend on
> each other, right? And that this is, because you don't want the application to
> call a GstBase initialization function and because you can't let the source
> generation put everything into one library and different namespaces, right? In
> that case I guess this bug can't be properly solved ;)
> 

Right.  The way I thought of fixing this is to require that GstBase::init() be called if some of the gstreamerbasemm API is used, but then not to require that Gst::init() be called because GstBase::init() would take care of this (that way only one initialization line would be required for developers as before).  Gst::init() would not call GstBase::wrap_init() because GstBase:init() would do that, so the internal interdependency can be eliminated this way.  Gst::init() would still be necessary if GstBase::init() is not used.

The added benefit of this fix is that if a program only uses core stuff (and nothing in gstreamerbasemm), they could simply use Gst::init() and their linking and execution would probably be faster.

At first, I wanted to leave things as they are (my research tends to show that there is no major difference in using the --as-needed flag), but since this is the second report of this nature (as I already mentioned, a bug[1] like this one was already reported), I think I'll fix it this way so that such confusion can be avoided in the future from distributors that by default use the "--as-needed" flag and the added benefit above can be provided to programs that only use gstreamermm API.

[1] http://bugzilla.gnome.org/show_bug.cgi?id=556570
Comment 18 Sebastian Dröge (slomo) 2008-11-17 05:00:22 UTC
(In reply to comment #17)
> (In reply to comment #16)
> > Ok, that makes sense then IMHO. One question though, how do you want to wrap
> > the plugins/elements? I mean, their instance/class struct sizes are not
> > guaranteed to stay the same as they're not part of the ABI. 
> 
> Do you mean that their properties and signals are not guaranteed to remain the
> same (ABI stable) in major releases?  If so, then the plugin generation would
> have to be adjusted so that ABI is not affected when gstreamermm becomes stable
> (this would be something to think about).  The C++ classes will only wrap the
> properties and signals so if these do not change, the C++ class, I think,
> should not loose ABI.

Properties and signals must stay the same, they're part of the guaranteed API. What can change though is the content and size of the instance and class structs of the elements. If you don't need those in your wrappers everything is fine ;)

This of course only applies to the elements in gstreamer core, base, good and ugly.

> > You could only
> > create hand-written wrappers that only access the elements via generic
> > interfaces like GObject properties, gstreamer interfaces, etc. Also
> > distributors might want to choose to not include some elements, it's not
> > guaranteed that everybody has audioresample for example :)
> 
> By using introspection I think it is possible to have the generating process
> generate C++ wrapper classes during the build process if things work well this
> way (depending on the ABI issues you mentioned).  If so, the build process can
> be used to determine which ones should be generated and which ones should not. 
> It's a bit tricky to explain, but if you want some idea of how this might be
> done, you can look at what has been done so far (the last few ChangeLog
> entries).  Also, keep in mind that gstreamermm is still unstable and these
> things can be ironed out before it becomes stable.  If plugin generation is not
> possible, it will probably be abandoned.

So essentially your wrapper for elements simply contains a pointer to an instance of the element and then set_foo and get_foo methods for all properties that internally call g_object_set? Sounds good :)

> > 
> > Whatever, for the design you're following now, it would still mean that
> > gstreamermm and gstreamerbasemm are using symbols from each other and depend on
> > each other, right? And that this is, because you don't want the application to
> > call a GstBase initialization function and because you can't let the source
> > generation put everything into one library and different namespaces, right? In
> > that case I guess this bug can't be properly solved ;)
> > 
> 
> Right.  The way I thought of fixing this is to require that GstBase::init() be
> called if some of the gstreamerbasemm API is used, but then not to require that
> Gst::init() be called because GstBase::init() would take care of this (that way
> only one initialization line would be required for developers as before). 
> Gst::init() would not call GstBase::wrap_init() because GstBase:init() would do
> that, so the internal interdependency can be eliminated this way.  Gst::init()
> would still be necessary if GstBase::init() is not used.
> 
> The added benefit of this fix is that if a program only uses core stuff (and
> nothing in gstreamerbasemm), they could simply use Gst::init() and their
> linking and execution would probably be faster.

Sounds good to me :)
Comment 19 Murray Cumming 2008-11-17 09:15:56 UTC
> So essentially your wrapper for elements simply contains a pointer to an
> instance of the element and then set_foo and get_foo methods for all properties
> that internally call g_object_set? Sounds good :)

We might be deriving new GTypes from them. I guess that would be a problem. We can maybe avoid that, though it would make it difficult to deal with signals and vfuncs.
Comment 20 José Alburquerque 2008-11-18 04:22:42 UTC
Created attachment 122911 [details] [review]
Patch to test using gstreamerbasemm element without calling GstBase::wrap_init()

Murray, a quick question on this:  Am I right in assuming that a call to GstBase::wrap_init() is needed before using gstreamerbasemm classes?  I just tested this patch (which doesn't use GstBase::wrap_init() and uses GstBase::AudioClock) and the test runs without GstBase::wrap_init() being called.  Where am I wrong?  Thanks.
Comment 21 José Alburquerque 2008-11-18 16:56:54 UTC
Nevermind.  I was working a little late and didn't notice that if GstBase::wrap_init() is not used, the gstreamerbasemm classes' wrap_new() functions are not registered by Glib::wrap_register().  Sorry about that.
Comment 22 José Alburquerque 2008-11-18 22:55:43 UTC
The following changes, I think, fixes this bug.  Please re-open otherwise.  Thanks.

2008-11-18  José Alburquerque  <jaalburqu@svn.gnome.org>

	* gstreamer/gstreamermm-0.10.pc.in:
	* gstreamer/gstreamermm/init.cc:
	* gstreamer/gstreamermm/init.h: Removed gstreamermm dependency on
	gstreamerbasemm by not using GstBase::wrap_init() in gstreamermm
	initialization.  Modified init() and init_check() methods so that they
	only call gst_init() or gst_init_check() only once regardless of how
	many times the methods are used.
	* gstreamerbase/gstreamerbasemm/init.cc:
	* gstreamerbase/gstreamerbasemm/init.h: Added init() and init_check()
	methods.  Modifed docs to explain that GstBase::init() and
	GstBase::init_check() methods can be used to initialize gsteamermm.
	* gstreamerbase/gstreamerbasemm/wrap_init.h:
	* gstreamerbase/gstreamerbasemm/gst_wrap_init.h: Modified docs to
	suggest to use GstBase::init() or GstBase::init_check() methods
	instead of GstBase::wrap_init() or GstBase::gst_wrap_init().
	* tests/test-init-check.cc:
	* tests/test-init.cc: Modified two tests to ensure that
	GstBase::init() and GstBase::init_check() initialize both gstreamermm
	and gstreamerbasemm.