Memory Leak when reusing Movie instance

[oneandonlyoddo]

I build a video player which receives instructions via web sockets on which video to play. I am reusing the same movie instance. It seems like memory isn't properly cleared and therefore the memory usage is constantly growing until the app crashes.

In pseudo code this what I am doing:

import websockets.*;
import processing.video.*;

WebsocketClient wsc;
Movie myMovie;
boolean isPlaying = false;

void setup(){
    size(1280, 720, P2D);
    wsc= new WebsocketClient(this, "ws://localhost:9000");
}

void draw(){
    if(isPlaying){
        image(myMovie, 0, 0);
    }
}

void movieEvent(Movie m) {
    m.read();
}

void webSocketEvent(String msg){
    myMovie = new Movie(this, sketchpath()+"\\"+msg);
    myMovie.play();
    isPlaying = true;   
}

Is there any other way to handle this? Running Windows 10

[GoToLoop]

B/c this Java video library is just a wrapper binding for the GStreamer library written in C, Java can't automatically get rid of the hardware allocated for it by its GC alone.

We also need to Invoke method dispose() so the GStreamer's side is shut off too.

[codeanticode]

@oneandonlyoddo as @GoToLoop mentioned, Movie has the dispose() method that should be called to clear the native GStreamer resources. If you add the line if (myMovie != null) myMovie.dispose(); right before creating the new movie object, do you still run out of memory?

[oneandonlyoddo]

@codeanticode and @GoToLoop I wasn't aware of the dispose() method. It is not listed here: https://processing.org/reference/libraries/video/index.html. Thanks a lot. Memory usage looks already way better and more consistent. I'll keep it running over night to see how it behaves on the long run.

[GoToLoop]

It is not listed here:

Indeed, it's only mentioned in the source code itself here: https://GitHub.com/processing/processing-video/blob/2/src/processing/video/Movie.java#L105-L144L140

And w/ a very threatening comment: :fearful:

NOTE: This is not official API and may/will be removed at any time.

Which is completely senseless, b/c w/o it, we've got huge memory leakage! :grimacing:

[oneandonlyoddo]

So I kept it running over night, five instances of it actually, and they all kept working with a reasonable and stable amount of memory.

[neilcsmith-net]

@codeanticode it would be good if you could expose the ability to set the playbin URI / file here - there should be no reason in cases like these to have more than one Movie.

Probably also doable because playbin is public by using movie.playbin.setFile(File file); ??

B/c this Java video library is just a wrapper binding for the GStreamer library written in C, Java can't automatically get rid of the hardware allocated for it by its GC alone.

@GoToLoop this isn't actually true, although explicit disposal should still be preferred. The GStreamer 0.10 bindings (I assume this is not with v2 beta?) do have a memory leak though.

[christo]

Note that the title of this issue is misleading. In the sample code, the instance of the Movie object is not reused, a new Movie object is continually created and the expectation is that garbage collection will return the dereferenced Movie object to the heap.

As explained above dispose() is currently required to free native resources. So that sounds like a solution or at least a workaround.

Nevertheless, @codeanticode what needs to happen for this issue to be closed?

[christo]

Perhaps this is a case to implement the finalize() method on the Movie class?

[neilcsmith-net]

@christo the Movie class already implements finalize(). Finalization is deprecated, for good reason, and doesn't really help here. I'm not sure you fully understand how garbage collection works. And anyway, Garbage collection has no knowledge of memory use outside of the JVM (eg. in GStreamer) so if there's no pressure on Java memory the GC might not clear things up anyway. This is why explicit disposal should always be preferred for native resources, and Processing needs to get away from the desire to hide implementations of dispose() - can't change reality! :smile:

The old GStreamer 0.10 bindings also have a memory leak. The gst1-java-core bindings in v2 should correctly clear GStreamer resources if and when Movie is garbage collected without needing finalization here. It should also be checked that nothing being cleared in dispose() is keeping the Movie reference alive, or it won't be cleared and finalization not run anyway.

[christo]

Hi @neilcsmith-net I get that there's no GC guarantee when using finalize() but I mention it conscious that freeing up native resources is one of the last defended uses of finalize() (by Josh Bloch famously in his book Java Puzzlers). I get that finalize() is deprecated in Java 9 but Processing's build insists on Java 8. Anyway I wasn't proposing a water tight solution, only thinking it may help, which, as you seem to understand, it will in a subset of cases where there is sufficient heap pressure or some other magic to cause GC. Bear in mind also that for Processing the original JRE invocation is in many cases controlled by Processing's IDE and JVM params can be used to choose tune GC and select algorithms.

[christo]

I'm with you on attributing responsibility to fix memory leaks in gstreamer. In fact I'm with you generally even if I'm not quite ready to give up on the potential for avoiding explicit resource disposal.

[neilcsmith-net]

@christo I think you missed the bit where I mentioned that finalize() is already implemented in Movie, so obviously doesn't fix the issue! It doesn't really need to be in there, particularly with Processing Video v2 - OTOH ensuring no circular references does.

Automatic cleanup of native resources is already implemented in gst1-java-core. It doesn't use finalize, but reference queues (somewhat similar to the replacement cleaner API in Java 9, but Java 8 compatible). It is still a fallback, and I recommend using explicit disposal where possible.

Maybe also read Chapter 2, Item 8 here - https://kea.nu/files/textbooks/new/Effective%20Java%20%282017%2C%20Addison-Wesley%29.pdf

[christo]

Thanks for the tips @neilcsmith-net I should have explicitly mentioned that I see that you're right, finalize() is already implemented. Thanks also for the link to the book maybe it's time to read it again!

For what it's worth, I'm not suggesting that an automatic measure will definitely remove all benefits to explicitly disposing, nor that users shouldn't be told to explicitly dispose(); they should be told this clearly in the javadoc and user guide. I unreservedly agree that explicit disposal is advisable. I do it!

What it seems we agree on although differently emphasise is that having measures in place to attempt automatic native resource cleanup, as you say, as "a fallback" has nonzero value in a significant proportion of deployments. Therefore it belongs in the discussion of what the implementation should do.

For every rule that users are given, a percentage of them will not adhere to it and the value of Processing Video corresponds to their actual experience with it, not the theoretical world where they take 100% of the responsibility you or I believe they rightly should accept. The less pain the users feel as a result of not reading docs or being ignorant of the implementation details of a library, the less risk, time and therefore cost the library brings.

Not trying to get stuck on finalize() and it seems there is no work to do here. Thanks again for taking the time. You seem to be one of few people attentive to this library at all.