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

[Jan Schmidt]

Stefan Kost wrote:
> Hi Mike,
>
> Michael Smith schrieb:
>   
>> On Thu, 2007-12-13 at 08:50 -0800, ensonic at kemper.freedesktop.org wrote:
>>     
>>> CVS Root:       /cvs/gstreamer
>>> Module:         gstreamer
>>> Changes by:     ensonic
>>> Date:           Thu Dec 13 2007  16:50:08 UTC
>>>
>>> Log message:
>>> 	* libs/gst/base/gstbasesink.c:
>>> 	* libs/gst/base/gstbasesrc.c:
>>> 	* libs/gst/base/gstbasetransform.c:
>>> 	  Replace gst_pad_get_parent by GST_OBJECT_PARENT inside streaming
>>> 	  thread. Correct log message in gstbasesrc.c.
>>>       
>> Why? 
>>
>> This doesn't look right, at least without a much more complete
>> explanation.
>>
>> gst_pad_get_parent() takes the object lock. You've replaced it with code
>> that does not take the object lock, and is not called with the object
>> lock taken.
>>
>> Please explain or revert. I think reversion is needed.
>>
>>     
> It was discussed on IRC. gst_pad_get_parent() does not take any lock as far as I
> see. If it does those should also be released. And reffing the pad that one own
> already is not required. You will find that e.g. elements already use the macros
> (e.g. gst_queue_loop()). Its not consistet though. Feel free to apply it elsewhere.
>   
Except for the object lock, which it takes because it's an alias for 
gst_object_get_parent.

In any case, the change looks fine to me. The pad will be a child of the 
element, and therefore the parent exists (with a ref), for at least the 
duration of the streaming thread. If the pad is unparented, that would 
imply the parent has been destroyed, which will hopefully have stopped 
the streaming thread.

J.