[gstreamer-bugs] [Bug 415754] New GstMuxer class

[GStreamer (bugzilla.gnome.org)]

Do not reply to this via email (we are currently unable to handle email
responses and they get discarded).  You can add comments to this bug at
http://bugzilla.gnome.org/show_bug.cgi?id=415754

  GStreamer | gstreamer (core) | Ver: HEAD CVS





------- Comment #9 from Michal Benes  2007-03-22 11:22 UTC -------
Hi Mark,
I wanted to post reply together with updated patches, but I am little short of
time these weeks. Anyway that you for your reply. The patch is by no means
ready. I have curently discovered some memory corruptions, and my test muxer
hangs from time to time. Take it more as an API proposal for discussion.

> 1 and 2) gst_collect_pads_set_waiting:
> Similarly, it breaks GstCollectsData ABI; extends beyond the padding, as there
> is no more room in there (there was already none for destroy_notify).

Good point, I did not knew that the padding was so small :( I will misuse
gobject properties or find another hack.

> 3) GstBaseMux:
> Some nitpicking, it seems to use collect_data->abidata.ABI.new_segment
> directly, but this is marked as private (though a borderline)
> [will need to look some more at BaseMux, it is a quite a lot ;-) ]

I was treating these variables more like "protected", but GstMuxer handles
segments anyway so it can remember them too.

> As a design point, though, there is a lot of "pass-through" code in GstBaseMux,
> e.g. _stop, _start, _add_pad, _set_event_function, destroy_notify handling, etc
> that either all go straight to GstCollectPads or pretty much do the same = have
> similar code as GstCollectPads, causing a lot of "duplication".
_set_event_function, destroy_notify
I do not like that either, but I do not want element using GstMuxer to call
GstCollectPads functions directly. GstMuxer may need to do something more than
GstCollectPads in _stop, _start, _add_pad methods in future. But in the next
versin I will implement this as #defines not dummy wrappers.

As for _set_event_function and destroy_notify I really need to wrap them
because GstMuxer needs to handle events.

> This could be reduced by modelling (say) GstMuxPads as a subclass of
> GstCollectPads.  Such one could be pretty directly/readily used as a
> replacement/enhancement of GstCollectPads in existing muxers, if they so
> desire.

Yes, it is so. GstMuxer is subclass of GstCollectPads. I was not sure about the
name GstMuxPads came to my mind, but I have choosen GstMuxer. May be GstMuxPads
is better.

> So, in short/overall, override/extend the GstCollectPads functionality by
> inheritance rather than duplicate/wrap around 

I agree. But still I need some wrappers as I have not found better way to
implement this in gobject system.

> [and the real "core
> intelligence code" in GstBaseMux would then stand out in a good place in such a
> GstMuxPads].

See reply to David


-- 
Configure bugmail: http://bugzilla.gnome.org/userprefs.cgi?tab=email