[gst-devel] Gst# + patch

[Owen Fraser-Green]

Hi, 

> > 1. GstCaps and GstStructure are not classes, the don't derive from 
> > gobject and therefore, shouldn't have GST_IS_xxx macros.
> >
> You changed those macros, those changes are fine.
> But GST_IS_* macros are ok for anything, not just GObjects. [1]

Yes, sorry, it's been a very long time since I did any C/glib programming.
The problem wasn't the presence of GST_IS_* macros but their definition in
gststructure.h (and the 0.8.1 gstcaps.h):

G_TYPE_CHECK_INSTANCE_TYPE ((object), GST_TYPE_STRUCTURE)

> > 2. The fixate signal in GstPad used the signal's own function 
> > declaration in the signal declarations as a shortcut. I've made the 
> > declaration explicit in keeping with all the other signal
> declarations.
> >
> If that has the same prototype, it's ok. But you renamed the structure 
> member from "appfixatefunc" to "fixate", which is a nono wrt API/ABI 
> stability.
> Feel free to file a bug for renaming when we branch 0.9. [1]

The name wasn't a problem for GAPI, that was just tidy-up for code
legibility. (It at least led me down a blind alley for five minutes while I
tried to figure out what was so special about this particular signal that
the structure member name didn't follow convention).

> > 3. GstPlugin and GstProbe had spurious getters/setters. I.e. things 
> > you could just as easily do plugin->... on.
> >
> Not caring about this part breaking ABI and API (removing functions), 
> you are not supposed to access any struct members directly but use 
> getters/setters. This makes it easier to later add code that is 
> required before getting/setting.

Yes, of course. Again, my glib memory is too hazy to really remember here
but isn't there a standard way to make these members "private" thus exposing
only the public accessors? Otherwise the wannabe-private members will be
(and I believe are) used directly.

> > 4. I added a typedef to GstQueue for the enum defined
> therein. Also a
> > #include in gst.h for gstqueue.h because of the enum gets
> generated in
> > gstenumtypes.h
> >
> GstQueue shouldn't be included in gst.h - I'm still htting my head 
> about the fact that it's installed.
> 
> I'm going to look over the patch and apply the problemless bits.

The #include in gst.h was really just to be able to typedef the
GstQueueLeaky enum. If gstqueue.h shouldn't be included there's probably a
better solution here but I still believe the enum should be typedefed.

Cheers,
Owen