I've been trying to reproduce this issue since I heard about this kind of leak in IRC and Slack. But I failed to reproduce it on both 1.12 and master.

FYI, I tried to reproduce by my sample using v4l2src, vaapih264enc and fdsink.

If you give any clue to reproduce it, it'd be nice.

Would be great if you could track the memory leaks using the GStreamer tracer[1]

GST_TRACERS="leaks" GST_DEBUG="GST_TRACER:7" 

Also, a valgrind session would be great


1. https://gstreamer.freedesktop.org/data/events/gstreamer-conference/2016/Guillaume%20Desmottes%20-%20Tracking%20Memory%20Leaks.pdf

(In reply to Hyunjun Ko from comment #1)
> I've been trying to reproduce this issue since I heard about this kind of
> leak in IRC and Slack. But I failed to reproduce it on both 1.12 and master.
> 
> FYI, I tried to reproduce by my sample using v4l2src, vaapih264enc and
> fdsink.
> 
> If you give any clue to reproduce it, it'd be nice.

1. I'm not sure if I can release our gstreamer test app publicly, but I can give access to the two of you directly.

2. Using the GST_TRACERS didn't print anything useful.

3. I have a custom suppression file where I added every generated suppression. I'll upload that along with the summary of running valgrind. Nothing is classified as a "leak".

4. Our test app continuously captures frames to a tee. One of the tee outputs is to a valve with an h264 encoder pipeline. For testing, we make 20 second recordings with 5 second delays between.

5. The leak is not a true "leak". If I stop the entire pipeline, all of the "leaked" memory is released.

(In reply to Harvey from comment #4)
> 5. The leak is not a true "leak". If I stop the entire pipeline, all of the
> "leaked" memory is released.

Thus I'm setting it as an enhancement request.

(In reply to Víctor Manuel Jáquez Leal from comment #5)
> (In reply to Harvey from comment #4)
> > 5. The leak is not a true "leak". If I stop the entire pipeline, all of the
> > "leaked" memory is released.
> 
> Thus I'm setting it as an enhancement request.

The weird thing is that after we stop encoding, we turn off the valve,
remove the encoding bin from the pipeline, and unref it. But the memory remains allocated.

(In reply to Harvey from comment #6)
> (In reply to Víctor Manuel Jáquez Leal from comment #5)
> > (In reply to Harvey from comment #4)
> > > 5. The leak is not a true "leak". If I stop the entire pipeline, all of the
> > > "leaked" memory is released.
> > 
> > Thus I'm setting it as an enhancement request.
> 
> The weird thing is that after we stop encoding, we turn off the valve,
> remove the encoding bin from the pipeline, and unref it. But the memory
> remains allocated.

Okay. 
So, "stop the entire pipeline" means call of gst_element_set_state (pipeline, GST_STATE_NULL), doesn't it? And all of the leaked memory is released after this call.

If then in "Every time we start and stop a recording (encoding of captured video), we lose 1-4 frames of memory" What does "start and stop a recording" mean?

(In reply to Hyunjun Ko from comment #7)
> (In reply to Harvey from comment #6)
> > (In reply to Víctor Manuel Jáquez Leal from comment #5)
> > > (In reply to Harvey from comment #4)
> > > > 5. The leak is not a true "leak". If I stop the entire pipeline, all of the
> > > > "leaked" memory is released.
> > > 
> > > Thus I'm setting it as an enhancement request.
> > 
> > The weird thing is that after we stop encoding, we turn off the valve,
> > remove the encoding bin from the pipeline, and unref it. But the memory
> > remains allocated.
> 
> Okay. 
> So, "stop the entire pipeline" means call of gst_element_set_state
> (pipeline, GST_STATE_NULL), doesn't it? And all of the leaked memory is
> released after this call.

Yes, setting state of the entire pipeline to NULL releases all of the leaked memory.

> If then in "Every time we start and stop a recording (encoding of captured
> video), we lose 1-4 frames of memory" What does "start and stop a recording"
> mean?

- Start of app
    - create pipeline: v4l2src -> caps -> valve (drop = true)
    - start recording:
        - create encoder bin: 
             bin[queue, videorate, videoconvert, vaapih264enc, mp4mux, fdsink ]
             - set videorate: skip-to-first=true
             - set queue:
                 max-size-buffers = 10
                 max-size-bytes = 4k*1.5*10
                 max-size-time = 0
                 leaky = GST_QUEUE_LEAK_UPSTREAM
        - add encoder bin to pipeline
        - link valve to encoder bin
        - gst_element_sync_state_to_parent(encoder bin)
        - set valve: drop = false
        - wait for fdsink to receive data
    - stop recording:
        - set valve: drop = true
        - gst_element_send_event(encoder bin, gst_event_new_eos())
        - wait for fdsink to receive EOS
        - gst_element_set_state(encoder bin, GST_STATE_NULL)
        - unlink valve from encoder bin
        - remove encoder bin from pipeline
        - unref encoder bin

(In reply to Harvey from comment #8)
> - Start of app
>     - create pipeline: v4l2src -> caps -> valve (drop = true)
>     - start recording:
>         - create encoder bin: 
>              bin[queue, videorate, videoconvert, vaapih264enc, mp4mux,
> fdsink ]
>              - set videorate: skip-to-first=true
>              - set queue:
>                  max-size-buffers = 10
>                  max-size-bytes = 4k*1.5*10
>                  max-size-time = 0
>                  leaky = GST_QUEUE_LEAK_UPSTREAM
>         - add encoder bin to pipeline
>         - link valve to encoder bin
>         - gst_element_sync_state_to_parent(encoder bin)
>         - set valve: drop = false
>         - wait for fdsink to receive data
>     - stop recording:
>         - set valve: drop = true
>         - gst_element_send_event(encoder bin, gst_event_new_eos())
>         - wait for fdsink to receive EOS
>         - gst_element_set_state(encoder bin, GST_STATE_NULL)
>         - unlink valve from encoder bin
>         - remove encoder bin from pipeline
>         - unref encoder bin

Thanks for the information.
The pipeline looks more complicated than I thought :)
Carefully speaking, I suspect other plugins but I'm going to look into it considering this kind of pipeline.

Keep in mind that appsink, queue and tee may retain some buffers, and if zero-copy is negotiated that memory would be from VA surfaces held in those elements.

(In reply to Víctor Manuel Jáquez Leal from comment #10)
> Keep in mind that appsink, queue and tee may retain some buffers, and if
> zero-copy is negotiated that memory would be from VA surfaces held in those
> elements.

I simplified our pipeline to the one I just described to eliminate that possibility. I removed the tee and the other paths.

(In reply to Harvey from comment #8)
> 
> - Start of app
>     - create pipeline: v4l2src -> caps -> valve (drop = true)
>     - start recording:
>         - create encoder bin: 
>              bin[queue, videorate, videoconvert, vaapih264enc, mp4mux,
> fdsink ]
>              - set videorate: skip-to-first=true
>              - set queue:
>                  max-size-buffers = 10
>                  max-size-bytes = 4k*1.5*10
>                  max-size-time = 0
>                  leaky = GST_QUEUE_LEAK_UPSTREAM
>         - add encoder bin to pipeline
>         - link valve to encoder bin
>         - gst_element_sync_state_to_parent(encoder bin)
>         - set valve: drop = false
>         - wait for fdsink to receive data
>     - stop recording:
>         - set valve: drop = true
>         - gst_element_send_event(encoder bin, gst_event_new_eos())
>         - wait for fdsink to receive EOS
>         - gst_element_set_state(encoder bin, GST_STATE_NULL)
>         - unlink valve from encoder bin
>         - remove encoder bin from pipeline
>         - unref encoder bin

I found some leaks but not sure it's the same leak.

Could you try the following:
1. Just once to create encode bin.
2. When start recording, do link, gst_element_sync_state_to_parent, set valve.
2. when stop recording, set valve, eos event, set state to NULL, unlink. (not remove encode bin)

I found this removed leaks that I found. But still not sure if this would affect your case.

(In reply to Hyunjun Ko from comment #12)
> 
> I found some leaks but not sure it's the same leak.
> 
> Could you try the following:
> 1. Just once to create encode bin.
> 2. When start recording, do link, gst_element_sync_state_to_parent, set
> valve.
> 2. when stop recording, set valve, eos event, set state to NULL, unlink.
> (not remove encode bin)
> 
> I found this removed leaks that I found. But still not sure if this would
> affect your case.

This doesn't seem to have had any effect (or perhaps this was another much, much smaller leak). I'm trying to test our code on Ubuntu 16.04 as well as Yocto 2.3, but I don't see any allocations at all labeled "renderD128" on Ubuntu. I'm not sure why. Can you help me to understand the difference? It's hard to step through the code on Yocto, but I can do it in Ubuntu.

We've installed a work-around:

```
static void unrefVaapiDisplayContext(PrivateData *priv) {
    GST_OBJECT_LOCK (priv->pipeline);
    GstElement* element = GST_ELEMENT_CAST(priv->pipeline);
    GList *l;
    for (l = element->contexts; l;) {
      GstContext *context = (GstContext*)l->data;

      if (!gst_context_is_persistent (context)) {
        if(g_strcmp0(gst_context_get_context_type(context), "gst.vaapi.Display") == 0) {
            GList *next;
            gst_context_unref (context);
            next = l->next;
            element->contexts = g_list_delete_link (element->contexts, l);
            l = next;
        } else {
            l = l->next;
        }
      } else {
        l = l->next;
      }
    }
    GST_OBJECT_UNLOCK (priv->pipeline);
}```

(In reply to Harvey from comment #14)
> We've installed a work-around:
> 

Could you provide a patch? Thanks.

Ah, that's in your application. No patch is required then.

When do you call that function in your app?

We call this function after 20-or-so recordings to free up the memory. Directly after recording, we release the context that vaapih264enc stores in the pipeline.

This is what GST_STATE_NULL does, and why you don't typically see the memory leak, but it's there. It just isn't exposed in most pipelines. Our pipeline is continuously running (for rendering and such), and we dynamically add/remove the encoding piece without stopping/starting the pipeline.

This is a custom workaround for a specific problem. It is not a bug per-se, but a design issue in this use-case.

If you can come with a re-design idea to improve encoders sharing context, please reopen.