GNOME Bugzilla – Bug 766525
framepositionner: add a weak ref on track element to know when it is finalized
Last modified: 2016-05-16 18:44:14 UTC
Currently, framepositionner store a pointer to the GstTrackElement given in ges_frame_positioner_set_source_and_filter() and install a signal handler to be notified fo GstTrackElement property change which is disconnected on finalize. But we don't own any ref on GstTrackElement so when framepositionner instance is finalized, GstTrackElement could be already freed leading in the best case, the deconnection to raise a critical message. That's happens when applying patch from bug 766449. When the timeline is finalized and ressources cleanup, the GstTrackElement is freed before the GstFramePositionner. This can be tested with the following code snippet: GESTimeline *timeline = ges_timeline_new_audio_video (); GESLayer *layer = ges_timeline_append_layer (timeline); GESClip *clip = ges_test_clip_new (); ges_layer_add_clip (layer, clip); gst_object_unref (timeline);
Until now it was correct to concider the FramePositionner would always be destroyed before its parent GESTrackElement. If that changes it is right to add a weak ref, but you should probably do so in the same patch as where you change that behaviour.
Created attachment 327994 [details] [review] framepositionner: add a weak ref on track element to know when it is finalized Add a weak reference on GstTrackElement to know when it's finalized.
(In reply to Thibault Saunier from comment #1) > Until now it was correct to concider the FramePositionner would always be > destroyed before its parent GESTrackElement. > > If that changes it is right to add a weak ref, but you should probably do so > in the same patch as where you change that behaviour. The patch in bug 766449 only reveals this issue by fixing element leaks (including framepositionner), it does not change the destroy order. So I would prefer not to merge this with the other patch. I quickly check the current life of track element and framepositionner in the example I give, here is my current understanding: We create a new GESTestClip which create a new GESVideoTestSource. GESVideoTestSource object instantiate a 'framepositionner' and set itself as source. Then clip is added to the layer. When timeline is disposed, we first remove layers which causes GESVideoTestSource to be disposed. Then we remove tracks and the disposal of the videotrack causes the last reference to the framepositionner to be dropped. If we expect FramePositionner to be destroyed before its linked GESTrackElement and so not live after it, it may be worth analyzing this issue further.
Sounds about right actually
Attachment 327994 [details] pushed as df9921f - framepositionner: add a weak ref on track element to know when it is finalized