>
> Hi,
>
> On 9/2/19 7:04 PM, Victor Toso wrote:
> > From: Victor Toso <me@xxxxxxxxxxxxxx>
> >
> > Let's gst_deinit() if we initialize it for the same rationale pointed out
> > in 0381e62 "spicy: Add call of gst_deinit at program exit" in
> > 2017-10-20 by Christophe de Dinechin <dinechin@xxxxxxxxxx>
> >
> > Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx>
> > ---
> > src/spice-session.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/src/spice-session.c b/src/spice-session.c
> > index db40a53..2135348 100644
> > --- a/src/spice-session.c
> > +++ b/src/spice-session.c
> > @@ -123,6 +123,8 @@ struct _SpiceSessionPrivate {
> > gchar *name;
> > SpiceImageCompression preferred_compression;
> >
> > + bool gst_init_by_spice;
> > +
> > /* associated objects */
> > SpiceAudio *audio_manager;
> > SpiceUsbDeviceManager *usb_manager;
> > @@ -343,6 +345,10 @@ spice_session_dispose(GObject *gobject)
> > g_warn_if_fail(s->channels_destroying == 0);
> > g_warn_if_fail(s->channels == NULL);
> >
> > + if (session->priv->gst_init_by_spice) {
> > + gst_deinit();
>
>
> Wouldn't it deinit on migration? (IIRC a new session is created)
>
>
> Actually gstreamer documentation states:
>
> "It is normally not needed to call this function in a normal application
> as the resources will automatically be freed when the program terminates.
> This function is therefore mostly used by testsuites and other memory
> profiling tools."
>
> Before it was used only by spicy which i guess it's considered more of a
> test
> client, I'm not sure we would like to deinit by the session (on the
> other hand
> i'm also not sure how harmful it would be)
>
>
> Snir.
>
Simply don't do it. Gstreamer is not well designed to call that function.
It leads to potential memory bugs. The check should be
if (I_initialized_gstreamer && nobody_is_using_or_assuming_is_initialized)
gst_deinit()
If A initialize Gstreamer and B don't but just check is initialized then
when A call deinit the objects used by B will contain potential dangling
pointers. One right interface would be simply init/deinit and use a counter
to track the number of initialization.
Frediano
>
> > + }
> > +
> > g_clear_object(&s->audio_manager);
> > g_clear_object(&s->usb_manager);
> > g_clear_object(&s->proxy);
> > @@ -2888,5 +2894,7 @@ spice_session_enable_gstreamer(SpiceSession *session)
> > if (!gst_init_check(NULL, NULL, &err)) {
> > spice_warning("Disabling GStreamer video support: %s",
> > err->message);
> > g_clear_error(&err);
> > + } else {
> > + session->priv->gst_init_by_spice = true;
> > }
> > }
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel