Webrtcbin: Huge increasing memory leak while using appsink instead of autovideosink

[Neil Young]

Hi,

I was reporting this as an issue here https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/issues/1010, but was redirected to this list. Not sure if it is the right place, but I will try.

I'm referring to the Java sample app: https://github.com/centricular/gstwebrtc-demos/tree/master/sendrecv/gst-java

With respect to increasing memory consumption everything is fine and balanced if we are using this approach to display the received video in a pop-up window:

if (name.startsWith("video")) {
            Element queue = ElementFactory.make("queue", "my-videoqueue");
            Element videoconvert = ElementFactory.make("videoconvert", "my-videoconvert");
            Element autovideosink = ElementFactory.make("osxvideosink", "my-autovideosink");
//            Element autovideosink = ElementFactory.make("autovideosink", "my-autovideosink");
            pipe.addMany(queue, videoconvert, autovideosink);
            queue.syncStateWithParent();
            videoconvert.syncStateWithParent();
            autovideosink.syncStateWithParent();
            pad.link(queue.getStaticPad("sink"));
            queue.link(videoconvert);
            videoconvert.link(autovideosink);
}

(autovideosink is replaced by osxvideosink for macOS)

If this sequence above is replaced by the following, the memory consumption of the Java app grows by 200 MB/s, which after a while leads to a complete crash of the app if not of the entire system:

if (name.startsWith("video")) {
            Element queue = ElementFactory.make("queue", "my-videoqueue");
            Element videoconvert = ElementFactory.make("videoconvert", "my-videoconvert");
            AppSink sink = (AppSink) ElementFactory.make("appsink", "my-appsink");
            sink.set("emit-signals", true);
            sink.connect(new AppSink.NEW_SAMPLE() {
                @Override
                public FlowReturn newSample(AppSink elem) {
                    Sample sample = elem.pullSample();
                    Structure capsStruct = sample.getCaps().getStructure(0);
                    String format = capsStruct.getString("format");
                    int width = capsStruct.getInteger("width");
                    int height = capsStruct.getInteger("height");

                    ByteBuffer bytes = sample.getBuffer().map(false);
                    byte[] buffer = new byte[bytes.remaining()];
                    bytes.get(buffer);
                    dragonfly.onNextFrame(format, buffer, width, height);

                    sample.dispose();
                    return FlowReturn.OK;
                }
            });

            sink.connect(new AppSink.NEW_PREROLL() {
                @Override
                public FlowReturn newPreroll(AppSink elem) {
                    Sample sample = elem.pullPreroll();
                    Structure capsStruct = sample.getCaps().getStructure(0);
                    String format = capsStruct.getString("format");

                    int width = capsStruct.getInteger("width");
                    int height = capsStruct.getInteger("height");

                    ByteBuffer bytes = sample.getBuffer().map(false);
                    byte[] buffer = new byte[bytes.remaining()];
                    bytes.get(buffer);
                    dragonfly.onNextFrame(format, buffer, width, height);

                    sample.dispose();
                    return FlowReturn.OK;
                }
            });

            pipe.addMany(queue, videoconvert, sink);
            queue.syncStateWithParent();

            videoconvert.syncStateWithParent();
            sink.syncStateWithParent();
            pad.link(queue.getStaticPad("sink"));
            queue.link(videoconvert);
            videoconvert.link(sink);
}

Is there anything wrong with the sequence above, which could cause the leak? The copy of the buffers (instead of forwarding) is necessary, since the final handling is done in native code, not under my control. And even if we don't do anything in the callbacks - the simple replace of autovideosink vs appsink causes the leak.

TIA

_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/gstreamer-devel

[Neil C Smith]

On Mon, 8 Jul 2019 at 10:57, Neil Young <[hidden email]> wrote:
> Is there anything wrong with the sequence above, which could cause the leak?

You appear to be missing a call to Buffer::unmap before Sample::dispose?

You might want to use our mailing list at
https://groups.google.com/forum/#!forum/gstreamer-java for Java
specific questions.  We're still somewhat semi-official! :-)

Best wishes,

Neil

--
Neil C Smith
Artist & Technologist
www.neilcsmith.net

PraxisLIVE - hybrid visual live programming
for creatives, for programmers, for students, for tinkerers
www.praxislive.org
_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/gstreamer-devel

[Neil Young]

Thanks for the quick response.

I was shrinking the sequence. This is what is sufficient to cause the leak:

if (name.startsWith("video")) {
            Element queue = ElementFactory.make("queue", "my-videoqueue");
            Element videoconvert = ElementFactory.make("videoconvert", "my-videoconvert");
            AppSink sink = (AppSink) ElementFactory.make("appsink", "my-appsink");

            sink.set("emit-signals", true);

            pipe.addMany(queue, videoconvert, sink);
            queue.syncStateWithParent();

            videoconvert.syncStateWithParent();
            sink.syncStateWithParent();
            pad.link(queue.getStaticPad("sink"));
            queue.link(videoconvert);
            videoconvert.link(sink);
}

Am 08.07.2019 um 12:04 schrieb Neil C Smith <[hidden email]>:

You appear to be missing a call to Buffer::unmap before Sample::dispose?

_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/gstreamer-devel

[Neil Young]

@Neil: Sorry, I got the information from another developer, that your hint fixes the leak. So it seems, that the other information I got ("the leak is also present, if there is no handling of the samples at all") was wrong.

Sorry for the hassle, and once again thanks for your quick and solid help. You are my hero again :)

Regards

> Am 08.07.2019 um 12:04 schrieb Neil C Smith <[hidden email]>:
>
> On Mon, 8 Jul 2019 at 10:57, Neil Young <[hidden email]> wrote:
>> Is there anything wrong with the sequence above, which could cause the leak?
>
> You appear to be missing a call to Buffer::unmap before Sample::dispose?
>
> You might want to use our mailing list at
> https://groups.google.com/forum/#!forum/gstreamer-java for Java
> specific questions.  We're still somewhat semi-official! :-)
>
> Best wishes,
>
> Neil
>
> --
> Neil C Smith
> Artist & Technologist
> www.neilcsmith.net
>
> PraxisLIVE - hybrid visual live programming
> for creatives, for programmers, for students, for tinkerers
> www.praxislive.org
> _______________________________________________
> gstreamer-devel mailing list
> [hidden email]
> https://lists.freedesktop.org/mailman/listinfo/gstreamer-devel

_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/gstreamer-devel

[Sebastian Dröge-3]

On Mon, 2019-07-08 at 12:19 +0200, Neil Young wrote:
> @Neil: Sorry, I got the information from another developer, that your
> hint fixes the leak. So it seems, that the other information I got
> ("the leak is also present, if there is no handling of the samples at
> all") was wrong.

Where did you get that information? Not in the GitLab issue in any
case.


If you use an appsink and never pull any samples from it then all the
samples will accumulate in the appsink until you run out of memory. See
the documentation of appsink.

--
Sebastian Dröge, Centricular Ltd · https://www.centricular.com


_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/gstreamer-devel

[Sebastian Dröge-3]

On Mon, 2019-07-08 at 11:04 +0100, Neil C Smith wrote:
> On Mon, 8 Jul 2019 at 10:57, Neil Young <
> [hidden email]> wrote:
> > Is there anything wrong with the sequence above, which could cause
> > the leak?
>
> You appear to be missing a call to Buffer::unmap before
> Sample::dispose?

With Java doing memory management automatically, wouldn't it be
possible to do the unmapping automatically too once the GstMapInfo goes
out of scope?

That's at least how it's handled in the Rust bindings to prevent
situations like the above, and generally make everything more memory-
safe.

--
Sebastian Dröge, Centricular Ltd · https://www.centricular.com


_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/gstreamer-devel

[Neil Young]

Both information came from another developer in my team sitting 1000 km apart from me, dealing with this and investigation on the leak. Apologies.

You are innocent :)

> Am 08.07.2019 um 12:41 schrieb Sebastian Dröge <[hidden email]>:
>
> On Mon, 2019-07-08 at 12:19 +0200, Neil Young wrote:
>> @Neil: Sorry, I got the information from another developer, that your
>> hint fixes the leak. So it seems, that the other information I got
>> ("the leak is also present, if there is no handling of the samples at
>> all") was wrong.
>
> Where did you get that information? Not in the GitLab issue in any
> case.
>
>
> If you use an appsink and never pull any samples from it then all the
> samples will accumulate in the appsink until you run out of memory. See
> the documentation of appsink.
>
> --
> Sebastian Dröge, Centricular Ltd · https://www.centricular.com
>
> _______________________________________________
> gstreamer-devel mailing list
> [hidden email]
> https://lists.freedesktop.org/mailman/listinfo/gstreamer-devel

_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/gstreamer-devel

[Neil C Smith]

On Mon, 8 Jul 2019 at 11:43, Sebastian Dröge <[hidden email]> wrote:
> With Java doing memory management automatically, wouldn't it be
> possible to do the unmapping automatically too once the GstMapInfo goes
> out of scope?
>
> That's at least how it's handled in the Rust bindings to prevent
> situations like the above, and generally make everything more memory-
> safe.

Java is very different to Rust here though.  Java is very good at
handling its own memory automatically, but relying on a GC to clear
memory or resources allocated outside of the heap that its managing is
"problematic"!  We do, from a historical perspective, support implicit
memory management of GStreamer objects, but it's not always the ideal
way to handle this (and sometimes the bane of my life! :-) )

We also support explicit disposal (free / unref), and as of v1.0 of
the bindings we also support Java's AutoCloseable for most things
(unfortunately not Buffer::map at present) which allows explicit
scoping of native resources and disposal when they go out of scope.

Deprecating our existing Buffer map support and switching to something
which can be AutoCloseable, perhaps our GstMapInfo wrapper, sounds a
good plan!

Thanks and best wishes,

Neil

--
Neil C Smith
Artist & Technologist
www.neilcsmith.net

PraxisLIVE - hybrid visual live programming
for creatives, for programmers, for students, for tinkerers
www.praxislive.org
_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/gstreamer-devel