[Spice-devel] [PATCH 3/4] channel-display-gst: Don't unref appsink and pipeline objects

[Kasireddy, Vivek]

Hi Frediano,

> >
> > It looks like setting the Gst pipeline state to GST_STATE_NULL
> > would cause all the references on most of the objects associated
> > with the pipeline to be dropped; therefore, there is no need
> > to explicitly unref them while freeing the pipeline. This patch
> > prevents the following errors from showing up when remote-viewer
> > is closed:
> >
> > (remote-viewer:64344): GStreamer-CRITICAL **: 00:31:04.278:
> > Trying to dispose object "appsink0", but it still has a parent
> > "pipeline0".
> > You need to let the parent manage the object instead of unreffing the
> > object directly.
> >
> > [MOS]:  CRITICAL - mos_bo_unreference:166: Input null ptr
> >
> > [MOS]:  CRITICAL - mos_bo_unreference:166: Input null ptr
> >
> > Cc: Frediano Ziglio <freddy77 at gmail.com>
> > Cc: Dongwon Kim <dongwon.kim at intel.com>
> > Cc: Jin Chung Teng <jin.chung.teng at intel.com>
> > Cc: Hazwan Arif Mazlan <hazwan.arif.mazlan at intel.com>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy at intel.com>
> > ---
> >  src/channel-display-gst.c | 4 ----
> >  1 file changed, 4 deletions(-)
> >
> > diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> > index 3f46a65..ad0ac99 100644
> > --- a/src/channel-display-gst.c
> > +++ b/src/channel-display-gst.c
> > @@ -353,10 +353,6 @@ static void free_pipeline(SpiceGstDecoder
> *decoder)
> >
> >      gst_element_set_state(decoder->pipeline, GST_STATE_NULL);
> >      gst_object_unref(decoder->appsrc);
> > -    if (decoder->appsink) {
> > -        gst_object_unref(decoder->appsink);
> > -    }
> > -    gst_object_unref(decoder->pipeline);
> 
> That's pretty odd. It looks like we are inserting a leak. Usually if
> you call gst_object_unref once more it causes an error like either a
> double free in glibc or an assert error in gstreamer. I'm sure we used
> this path so this should have happened.
> And to be honest reading the error it seems to suggest a different
> issue, like the appsink is still attached to the pipeline.
> Maybe unrefing pipeline before appsink fix the issue?
I tried that but it did not prevent the error from showing up. TBH, I noticed
these errors while using msdkh264dec on the decode side so I am not sure
if they show up if we use avdec_h264. Regardless, I guess I need to dig deep
to figure out why they are showing up instead of silencing them with this patch.

Thanks,
Vivek

> I personally didn't see this error before.
> 
> >      gst_object_unref(decoder->clock);
> >      decoder->pipeline = NULL;
> >  }
> 
> Frediano