Properly freeing resources

[Jim]

Hi Tim,
Thanks for your reply, please see my responses below, which are prefixed
with "Jim:"

I have also tried a much simpler pipeline that just uses playbin2. The
initialization and cleanup is the same except that I'm not registering a
callback for the pad-added signal. I still see the same results where memory
usage is slowly growing over time. I'm really hoping it's just related to me
not handling GStreamer correctly.

Jim

-----Original Message-----
From:
gstreamer-devel-bounces+restlessmindsstudio=gmail.com at lists.freedesktop.org
[mailto:gstreamer-devel-bounces+restlessmindsstudio=gmail.com at lists.freedesk
top.org] On Behalf Of Tim-Philipp Müller
Sent: Friday, February 22, 2013 12:41 PM
To: gstreamer-devel at lists.freedesktop.org
Subject: Re: Properly freeing resources

On Wed, 2013-02-20 at 07:41 -0700, Jim wrote:

Hi,

your code looks mostly ok.

>   streamData->videoDecoder = gst_element_factory_make("vpudec",
> "videoDecoder");
>   streamData->videoSink = gst_element_factory_make("mfw_v4lsink",
> "videoSink");
>   streamData->audioDecoder =
> gst_element_factory_make("mfw_mp3decoder", "audioDecoder");

I would check these elements first, if they free all resources correctly
when being shut down and/or being shut down during playback.

Jim: How do I verify that these elements are freeing their resources? I
thought by setting the pipeline state to GST_STATE_NULL, it is supposed to
free all of its resources. Are there situations where this may not be the
case?

>   g_object_set(G_OBJECT(streamData->videoQueue), "max-size-time", 0,
NULL);
>   g_object_set(G_OBJECT(streamData->audioQueue), "max-size-time", 0, 
> NULL);

You want (guint64)0 here.

Jim: Thanks, good to know

>   g_signal_connect(streamData->demux, "pad-added", 
> G_CALLBACK(pad_added_handler), streamData);

What do you do in the pad_added_handler? Are you sure you are not leaking
any references there?

Jim: Here is the code for the pad_added_handler:

  GstPad *sink_pad;
  GstPadLinkReturn ret;
  GstCaps *new_pad_caps = NULL;
  GstStructure *new_pad_struct = NULL;
  const gchar *new_pad_type = NULL;

  new_pad_caps = gst_pad_get_caps(new_pad);
  new_pad_struct = gst_caps_get_structure(new_pad_caps, 0);
  new_pad_type = gst_structure_get_name(new_pad_struct);

  g_print("Received new pad '%s' from '%s', pad type: '%s'\n",
GST_PAD_NAME(new_pad), GST_ELEMENT_NAME(src), new_pad_type);

  if (g_str_has_prefix(new_pad_type, "audio"))
  {
    sink_pad = gst_element_get_static_pad(data->audioQueue, "sink");

    // If the queue is already linked, we have nothing to do
    if (gst_pad_is_linked(sink_pad))
    {
      g_print ("We are already linked to the audio queue. Ignoring.\n");
    }
    else
    {
      // Attempt the link
      ret = gst_pad_link(new_pad, sink_pad);
      if (GST_PAD_LINK_FAILED (ret))
      {
        g_print ("Type is '%s' but link failed.\n", new_pad_type);
      }
      else
      {
        g_print ("Link succeeded (type '%s').\n", new_pad_type);
      }
    }
  }

  if (g_str_has_prefix(new_pad_type, "video"))
  {
    sink_pad = gst_element_get_static_pad(data->videoQueue, "sink");

    // If the queue is already linked, we have nothing to do
    if (gst_pad_is_linked(sink_pad))
    {
      g_print ("We are already linked to the video queue. Ignoring.\n");
    }
    else
    {
      // Attempt to link
      ret = gst_pad_link(new_pad, sink_pad);
      if (GST_PAD_LINK_FAILED(ret))
      {
        g_print ("Type is '%s' but link failed.\n", new_pad_type);
      }
      else
      {
        g_print ("Link succeeded (type '%s').\n", new_pad_type);
      }
    }
  }

  // Unreference the new pad's caps, if we got them
  if (new_pad_caps)
    gst_caps_unref(new_pad_caps);

  // Unreference the sink pad
  if (sink_pad)
    gst_object_unref(sink_pad);


>   // Retrieve pipeline signal bus
>   streamData->bus =
> gst_pipeline_get_bus(GST_PIPELINE(streamData->pipeline));
>   streamData->busWatchId = gst_bus_add_watch(streamData->bus,
> (GstBusFunc)handle_message, streamData);

FWIW, you don't have to keep a ref to the bus around if you don't need it,
you can just unref it right here and there after adding the watch if you
want to.

Jim: I went ahead and unreferenced the bus after assigning the bus watch
callback. Bernhard on the mailing list suggested that as well.

> Before switching to a another video, the following cleanup is done:
> 
>   gst_object_unref(streamData->bus);
>   gst_element_set_state(streamData->pipeline, GST_STATE_NULL);
>   gst_object_unref(streamData->pipeline);

You will also need to

  to g_source_remove (streamData->busWatchId)

Jim: Thanks, will do!

Cheers
 -Tim


_______________________________________________
gstreamer-devel mailing list
gstreamer-devel at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/gstreamer-devel