robmikh / displayrecorder

A utility to record displays.
MIT License
16 stars 7 forks source link

Potential memory leak when writing to sink #7

Open gregermendle opened 1 year ago

gregermendle commented 1 year ago

Hey! First, just want to say thank you for making this repo, I've learned a lot.

I've been playing with this code for a bit and I've created a few fairly long screen recordings with it. I noticed that memory seems to increase at the same rate as the output file. I am fairly new to Rust and very new to Media Foundation and Window Graphics Capture. That said, I was able to get memory usage way down by calling RemoveAllBuffers() on the output sample.

pub fn write(&self, sample: &IMFSample) -> Result<()> {
    unsafe {
        self.sink_writer
            .WriteSample(self.sink_writer_stream_index, sample)?;
        sample.RemoveAllBuffers()
    }
}

Doing this however seems to corrupt the end of the file in some way :/

There are a few other things that seem to stick around causing memory to still increase at random times. Using VS debug profiler I can see there are two things being created repeatedly that are consistent in their size.

EDIT:

Doing this however seems to corrupt the end of the file in some way :/

The above was actually not true, I need to rethink programming at 3am.

robmikh commented 1 year ago

I'm glad you've found the project useful! And thanks for reporting this!

I'm a little concerned this is indicative of some other subtle ref-counting bug. I would think that when the IMFSample is released that it would release all of its buffers (the norm for COM). I guess there might be some pooling of samples that I'm unaware of. I'll have to do some testing with a C++ version and see if I can reproduce this leak...

mnordstr commented 4 months ago

Hi,

I also experienced the same memory increase as @gregermendle , updated the code to windows-rs 0.56 but same thing. The fix mentioned above (sample.RemoveAllBuffers()) does help a lot but there is still a slow memory increase.

Another thing I've been fighting with is giving the create_encoding_session an InMemoryRandomAccessStream instead of a file, seems to work otherwise but I can't seem to consume it, I get the same memory ballooning problem. Tried many different approaches and e.g this one will just grow indefinitely:

let reader = windows::Storage::Streams::DataReader::CreateDataReader(&stream.GetInputStreamAt(0)?)?;

loop {
    let _ = reader.LoadAsync(1024 * 1024)?;

    let bufsize = reader.UnconsumedBufferLength()?;
    if bufsize > 0 {
        println!("{}", bufsize);

        let mut content = vec![0u8; bufsize as usize];
        reader.ReadBytes(&mut content)?;
    }
}

The idea would be to then send the video stream somewhere else for further processing/delivery.

Thanks for this code sample repo, it is very informative and clear.