Re: [gst-cvs] ensonic gstreamer: gstreamer/ gstreamer/gst/

[Tim-Philipp Müller-2]

On Wed, 2008-08-27 at 00:18 -0700, [hidden email] wrote:

> Log message:
> * gst/gstobject.c:
>  Due to popular request also include ObjectType in
>  gst_object_get_path_string(). Makes gst-launch -v bit more useful.

> http://freedesktop.org/cgi-bin/viewcvs.cgi/gstreamer/gstreamer/gst/gstobject.c.diff?r1=1.128&r2=1.129

Not entirely convinced about the popular request bit (or even whether
this string was supposed to be parsable in a particular way, with
path_string_separator in the class structure and all), but in any case
it would probably be good if this stayed thread-safe, as advertised in
the docs. (gst_object_get_name vs. GST_OBJECT_NAME)

 Cheers
  -Tim



-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gstreamer-devel

[Stefan Sauer]

hi,

this 'fix' causes some trouble. The issue is basically that in
gst/gstpadtemplate.c::465 we now make a copy and I unref the old caps, so that
we don't leak it. This causes a problem for e.g. ffmpeg which e.g. in
gstffmpegdemux.c:1652 keep a pointer to the caps, but never refs() it.

It think I should revert my change, but wonder why in the previous change that
tim applied in gst/gstpadtemplate.c::465 we copy? can't we change it to:
GST_PAD_TEMPLATE_CAPS (object) =
             gst_caps_ref (g_value_get_boxed (value));

but then the leaks in core are back.
4 tests had leaks or errors under valgrind:
  gst/gstpad gst/gstbin gst/gstghostpad elements/tee

If someone could have a look too. Its a bit late here :/

Stefan

[hidden email] schrieb:

> CVS Root:       /cvs/gstreamer
> Module:         gstreamer
> Changes by:     ensonic
> Date:           Thu Aug 28 2008  12:32:33 UTC
>
> Log message:
> * gst/gstpadtemplate.c:
>  The old behaviour was that gst_pad_template_new() takes ownership of
>  the caps. As we now call g_object_new() which calls g_object_set() and
>  which copies the caps, we have to unref them to not leak them. Fixes
>  make valgrid for me.
>
> Modified files:
>     .               : ChangeLog
>     gst             : gstpadtemplate.c
>
> Links:
> http://freedesktop.org/cgi-bin/viewcvs.cgi/gstreamer/gstreamer/ChangeLog.diff?r1=1.4043&r2=1.4044
> http://freedesktop.org/cgi-bin/viewcvs.cgi/gstreamer/gstreamer/gst/gstpadtemplate.c.diff?r1=1.14&r2=1.15
>
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
> Build the coolest Linux based applications with Moblin SDK & win great prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> gstreamer-cvs mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/gstreamer-cvs

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gstreamer-devel

[michael smith-6-3]

On Thu, Aug 28, 2008 at 2:02 PM, Stefan Kost <[hidden email]> wrote:
> hi,
>
> this 'fix' causes some trouble. The issue is basically that in
> gst/gstpadtemplate.c::465 we now make a copy and I unref the old caps, so that
> we don't leak it. This causes a problem for e.g. ffmpeg which e.g. in
> gstffmpegdemux.c:1652 keep a pointer to the caps, but never refs() it.

This is pretty clearly a bug in gstffmpeg (there are similar bugs in
alaw, at least, and possibly others). The documentation has always
been clear, that this function takes ownership of the caps.

However, obviously we don't want to just break everything suddenly.

I'd suggest: revert this change, fix all the plugins that rely on the
old behaviour, then after the next round of releases, reinstate the
fix, so we stop leaking caps.

Mike

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gstreamer-devel

[Stefan Sauer]

Michael Smith schrieb:

> On Thu, Aug 28, 2008 at 2:02 PM, Stefan Kost <[hidden email]> wrote:
>> hi,
>>
>> this 'fix' causes some trouble. The issue is basically that in
>> gst/gstpadtemplate.c::465 we now make a copy and I unref the old caps, so that
>> we don't leak it. This causes a problem for e.g. ffmpeg which e.g. in
>> gstffmpegdemux.c:1652 keep a pointer to the caps, but never refs() it.
>
> This is pretty clearly a bug in gstffmpeg (there are similar bugs in
> alaw, at least, and possibly others). The documentation has always
> been clear, that this function takes ownership of the caps.
>
> However, obviously we don't want to just break everything suddenly.
>
> I'd suggest: revert this change, fix all the plugins that rely on the
> old behaviour, then after the next round of releases, reinstate the
> fix, so we stop leaking caps.

david, mike thanks for taking corrective actions already. I would suggest
developers to reapply the change localy so that we can spot misbehaving plugins.

Stefan

>
> Mike
>
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
> Build the coolest Linux based applications with Moblin SDK & win great prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> gstreamer-cvs mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/gstreamer-cvs

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gstreamer-devel