bytedeco / javacv

Java interface to OpenCV, FFmpeg, and more
Other
7.51k stars 1.58k forks source link

Maybe a memory leak risk in FFmpegFrameGrabber? #1943

Closed Brozen closed 1 year ago

Brozen commented 1 year ago

Hi, i'm using FFmpegFrameGrabber and FFmpegFrameRecorder to merge several videos, my code like this.

Set<String> pathes = ....;
FFmpegFrameRecorder recorder = new FFmpegFrameRecorder(savePathStr, 1920, 1080);
recorder.setVideoCodec(avcodec.AV_CODEC_ID_NONE);
recorder.setFrameRate(vp.fps);
recorder.setVideoBitrate(vp.bps);
recorder.setAudioChannels(1);
recorder.setFormat("mp4");
recorder.setCloseOutputStream(true);

for (String path: pathes) {
    FFmpegFrameGrabber grabber = new FFmpegFrameGrabber(path.toAbsolutePath().toString());
    grabber.setCloseInputStream(true);

    try {
        grabber.start();
        Duration grabberDuration = Duration.ofNanos(grabber.getLengthInTime() * 1000L);

        Frame frame;
        while ((frame = grabber.grabFrame()) != null) {
            recorder.record(frame);
        }
    } finally {
        grabber.close();
    }

    recorder.flush();
}

recorder.close();

it did't cause any OOM, but i see memory of Java process is growing, I'm not sure if i use correct method to release FFmpegFrameRecorder\FFmpegFrameGrabber\Frame.

And i find this in FFmpegFrameGrabber#releaseUnsafe:

frameGrabbed  = false;
frame         = null;

just set "null" to frame, and not close frame like Java2DFrameConverter#close, will this be a memory leak risk?

    @Override public void close() {
        if (frame != null) {
            frame.close();
            frame = null;
        }
    }
saudet commented 1 year ago

We can and should call close() on anything that implements AutoCloseable. If you still observe a "memory leak" after that, please let me know.

saudet commented 1 year ago

Also, please upgrade to JavaCV 1.5.8. There is no point in using an old version, if this is what you're trying to do.

saudet commented 1 year ago

Your code doesn't work because you're not calling recorder.start() anywhere, but I've tested with some similar code and memory usage isn't increasing, so I guess that the issue you're having has already been fixed in JavaCV 1.5.8. If you're still having issues with JavaCV 1.5.8 though, please provide a way to reproduce them! Thanks

Brozen commented 1 year ago

OK, i will try 1.5.8, thanks for answering this question!