Rather: up until yesterday collectpads dropped newsegment events that they received, but this was changed yesterday to make them forward newsegment events instead.

This change doesn't seem right to me, as the newsegment events are crucial to sink behaviour (ie. things outside of the segment will be dropped, or in the case of filesinks newsegment events determine the seek position where to start writing data).

So when the segments aren't exactly the same, downstream behaviour is not really predictable and depends on which of the N upstream newsegment events happens to arrive last.

if should not forward them in any case. My reasoning is that it should update the per-sinkpad GstSegment structure so that the info is available to the plugin using collectpads. Sometimes you want to generate a new segment downstream, sometimes you just want to continue or update the current segment.

 2006-05-02  Stefan Kost  <ensonic@users.sf.net>

        * libs/gst/base/gstcollectpads.c: (gst_collect_pads_event):
          back out the newsegment handling change, see #340060 for ongoing
          discussion

The problem is not fixed, I just restored the previous behaviour. Whenever one uses adder, the sink never gets newsegment and eos events.
Thus plaing loops does not work, and playback never ends.

This needs to be handled in one way or other in adder and not in collect pads, that's why I closed it.

I right now try to come up with a proposal, that involves a GstCollectPadsEventFunction, which the element can use to get events from collected pads. Looks promissing so far and involves collect pads :)

But im fine with not making it a blocker (it certainly is for me though).

Do we want to support this behaviour in avimux?
/* FIXME: hacked way to override/extend the event function of
 * GstCollectPads; because it sets its own event function giving the
 * element no access to events */
avimux->collect_event = (GstPadEventFunction) GST_PAD_EVENTFUNC (newpad);
gst_pad_set_event_function (newpad,
    GST_DEBUG_FUNCPTR (gst_avi_mux_handle_event));

or add a GstCollectPadsEventFunction to collectpads?

Created attachment 64824 [details] [review]
add EventFunc to allow element classes to get events

2006-05-07  Stefan Kost  <ensonic@users.sf.net>

	* gst/adder/gstadder.c: (gst_adder_sink_event),
	(gst_adder_request_new_pad), (gst_adder_change_state):
	* gst/adder/gstadder.h:
	* tests/check/Makefile.am:
	* tests/check/elements/adder.c: (event_loop), (GST_START_TEST),
	(adder_suite), (main):
          Add sink-event handling to adder. It tries to merge incomming
          newsegment-events. Added test to check if segment_done is comming
          through.

still trying to figure out what would be a good thing to do for adder. It currently handles segments ok, except for the time field.

(In reply to comment #8)
> Created an attachment (id=64824) [edit]
> add EventFunc to allow element classes to get events

Hello Stefan, please what was the reason that this patch was rejected?

I am working on a subtitle support for muxers, for that I need to handle update newsegments in some way. This patch would also be useful for tag handling in matroska. I believe that it is useful for elements using CollectPads to have a way to handle events. Otherwise people will do hacks to work around this (see e.g. matroskamux).

If I recall right, the patch provided a way for the element that uses collectpads to get events. right now collectpads processes them internaly. Unfortunately I don't remember the exact reason for the patch rejection.

One problem with adder/collectpads is that they don't handle seemless looping. Flushing seeks work, but non-flushing seeks don't. When I play my pipeline, I start with a flushing segmented-seek and on segment-done I like to send a non-flushing seek. This works for pipelines without adder. For pipelines with adder it hangs. It looks like the FLUSH for SEEK events in gst_adder_src_event() was broken (not updating the segment if not flushing).

2007-04-23  Stefan Kost  <ensonic@users.sf.net>

	* gst/adder/gstadder.c: (gst_adder_setcaps), (gst_adder_src_event),
	(gst_adder_sink_event), (gst_adder_collected):
	  Fix non-flushing segmented seeks, Fixes #340060 for me

Stefan, is this bug still valid ?

There are still FIXME:s in the code. But the main issue the bug was about is working fine now.