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

[Andy Wingo]

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/ChangeLog.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/