Re: [Spice-devel] [PATCH 3/4] channel-display-gst: Don't unref appsink and pipeline objects
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
> > Cc: Dongwon Kim
> > Cc: Jin Chung Teng
> > Cc: Hazwan Arif Mazlan
> > Signed-off-by: Vivek Kasireddy
> > ---
> > 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
Re: [Spice-devel] [PATCH 3/4] channel-display-gst: Don't unref appsink and pipeline objects
Il giorno ven 15 set 2023 alle ore 01:33 Vivek Kasireddy
ha scritto:
>
> 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
> Cc: Dongwon Kim
> Cc: Jin Chung Teng
> Cc: Hazwan Arif Mazlan
> Signed-off-by: Vivek Kasireddy
> ---
> 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 personally didn't see this error before.
> gst_object_unref(decoder->clock);
> decoder->pipeline = NULL;
> }
Frediano
[Spice-devel] [PATCH 3/4] channel-display-gst: Don't unref appsink and pipeline objects
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
Cc: Dongwon Kim
Cc: Jin Chung Teng
Cc: Hazwan Arif Mazlan
Signed-off-by: Vivek Kasireddy
---
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);
gst_object_unref(decoder->clock);
decoder->pipeline = NULL;
}
--
2.39.2
