[gst-devel] Re: [gst-cvs] ensonic gstreamer: gstreamer/ gstreamer/libs/gst/base/

[ensonic]

Hi andy,

you missed the dicussion. Please first check the mailinglist before
complaining. There is now:
http://bugzilla.gnome.org/show_bug.cgi?id=340060
and my mail about "1:n, n:1 element event handling" whew I asked for
comments of how to handle it. I volunteer to fix it, but noone yet replied
how this fix should look like.

IHMO neither behaviour is correct. If I back out the change application
that use adder don't function. If I leave it in as Tim pointed out it
causes trouble for elements like filesink.

So I am open for constructive replies. To others should I backout the
change for now?

Stefan

On 11:15:11 am 02/05/2006 Andy Wingo <wingo at pobox.com> wrote:
> Hi Stefan,
>
> >  Changes by:     ensonic
> >  Date:           Fri Apr 28 2006  18:48:50 UTC
> >
> >  Log message:
> >  * libs/gst/base/gstbasesink.c: (gst_base_sink_event):
> >  * libs/gst/base/gstcollectpads.c: (gst_collect_pads_base_init),
> >  (gst_collect_pads_is_collected), (gst_collect_pads_event):
> >  more detailed debug and formatting cleanup
> >
> >  Links:
> >  http://freedesktop.org/cgi-bin/viewcvs.cgi/gstreamer/gstreamer/Chan
> geLog.diff?r1=1.2455&r2=1.2456
> >  http://freedesktop.org/cgi-bin/viewcvs.cgi/gstreamer/gstreamer/libs
> /gst/base/gstbasesink.c.diff?r1=1.140&r2=1.141
> >  http://freedesktop.org/cgi-bin/viewcvs.cgi/gstreamer/gstreamer/libs
> /gst/base/gstcollectpads.c.diff?r1=1.26&r2=1.27
> >
> >  --- gstcollectpads.c    24 Mar 2006 17:10:46 -0000    1.26
> >  +++ gstcollectpads.c    28 Apr 2006 18:48:38 -0000    1.27
> >  @@ -739,8 +741,8 @@
> >         data->abidata.ABI.new_segment = TRUE;
> >  -      /* We eat this event */
> >  -      gst_event_unref (event);
> >  +      /* forward to src-pad, sink-elements like to get the event
> >  */ +      gst_pad_event_default (pad, event);
> >         return TRUE;
> >       }
> >       default:
>
> This commit bothers me a bit, for a couple of reasons:
>
> 1) It is not mentioned in your changelog; although you go back to
> comment it in [0] and [1], the changelog entry is still not sufficient
> -- it just says "something was changed in this function", not what was
> changed and why it was changed. This, combined with other recent
> commits, indicates a problem in your workflow. You *REALLY* need to be
> looking at your cvs diff -u, *IN FULL*, before committing. Compare the
> changelog and the changes, see if the former describes the latter.
> Please.
>
> 2) It is a fundamental change in a base library. newsegment handling
> is brittle voodoo for most of us, especially in a base library like
> collectpads -- it has the potential to affect the behavior of muxers,
> not just adder.
>
> I think it would be appropriate to back out the change until #340060
> is resolved.
>
> Regards,
> --
> Andy Wingo
> http://wingolog.org/