fix memory leak macos audio output #236
Labels
Comments
💎 $150 bounty • Screenpi.pe
Steps to solve:
Thank you for contributing to mediar-ai/screenpipe! Add a bounty • Share on socials
|
|
context: https://github.com/RustAudio/cpal/pull/894/files |
|
I have looked at the issue a tiny bit, and have found some leads. Sadly, I don't have a mac device, so I can't test my theory. First of all, I have found a very similar issue: which was caused by using a single-threaded API on a second thread. I see that Sync and Send are implemented for This seems a bit odd, since from what I can see, Does using this API on the main thread still cause the leak? |
|
@FractalFir i created a unit test which runs on the main thread but still detect leaks: mod tests {
use screencapturekit::sc_shareable_content::SCShareableContent;
use super::*;
use std::process::Command;
use std::time::Duration;
#[test]
fn test_stream_no_memory_leaks() {
// Create a dummy device and config
let content = SCShareableContent::current();
let displays = content.displays;
let display = displays.first().unwrap_or_else(|| {
panic!("Main display not found");
});
let display = display.to_owned();
let device = Device::new(display);
let config = StreamConfig {
channels: 2,
sample_rate: SampleRate(48000),
buffer_size: crate::BufferSize::Default,
};
// Run the function multiple times to increase chances of detecting leaks
for _ in 0..1 {
let stream = device
.build_input_stream(&config, SampleFormat::F32, |_data, _info| {}, |_err| {})
.expect("Failed to build stream");
// Play and pause the stream
stream.play().expect("Failed to play stream");
std::thread::sleep(Duration::from_millis(10000));
stream.pause().expect("Failed to pause stream");
}
// Get the current process ID
let pid = std::process::id();
// Run the 'leaks' command
let output = Command::new("leaks")
.args(&[pid.to_string()])
.output()
.expect("Failed to execute leaks command");
// Check the output for leaks
let stdout = String::from_utf8_lossy(&output.stdout);
let stderr = String::from_utf8_lossy(&output.stderr);
println!("leaks stdout: {}", stdout);
println!("leaks stderr: {}", stderr);
if stdout.is_empty() && stderr.is_empty() {
println!("Both stdout and stderr are empty. This might indicate that the 'leaks' command didn't run properly.");
} else if !stdout.contains("leaks Report Version:") {
println!("No leaks detected, but unexpected output format.");
} else {
assert!(
!stdout.contains("Process"),
"Memory leak detected: {}",
stdout
);
}
}
} |
|
Tests in Rust don't run on the main thread. The test harness spawns a test per thread, and then runs the test on that thread. Is the issue still present when running on the main thread? |
|
yes still leaking (put in an example file) |
|
|
i noticed if i comment the body of play() it will still leak but less so probably there is issue in the |
|
OK, so this is unlikely to be a thread issue. The leak seems to happen in StreamFrameInfo::status, so this string could be allocated here: It seems to be sent to the Objective-C side, so maybe something went wrong there? Could you somehow obtain a |
seems like it? many more leaks i commented inside of fn did_output_audio_sample_buffer(&self, sample_buffer: CMSampleBuffer) {
for _ in 0..100 {
sample_buffer.sys_ref.get_frame_info().status();
}
// let buffers = sample_buffer.sys_ref.get_av_audio_buffer_list();
// let channels = buffers[0].number_channels;
// let mut data = raw_buffers_to_stream_data(buffers);
// let len = data.len();
// let data = data.as_mut_ptr() as *mut ();
// let data = unsafe { Data::from_parts(data, len, self.sample_format) };
// let cm_time = sample_buffer.sys_ref.get_presentation_timestamp();
// let buffer_frames = len / channels as usize;
// let callback = host_time_to_stream_instant(cm_time);
// let delay = frames_to_duration(buffer_frames, self.stream_config.sample_rate);
// let capture = callback
// .sub(delay)
// .expect("`capture` occurs before origin of alsa `StreamInstant`");
// let timestamp = crate::InputStreamTimestamp { callback, capture };
// let info = InputCallbackInfo { timestamp };
// let mut data_callback = self.data_callback.lock().unwrap();
// data_callback(&data, &info);
} |
|
seems it creates |
|
okay i did something 
|
|
will test more |
5-20 leaks every 10 seconds (instead of 300-600), releasing new version then will iterate on this |
ok nvm this does not look good |
|
I have found something interesting, although I am not sure if this is related. The objective-C function which leaks memory( https://opensource.apple.com/source/CF/CF-855.14/CFString.c Here, it says that the memory may be not freed if the allocator is set to I also have an idea for a fix: create the string only once, and store it in static OnceLock / or a lazy_static!. This way, the leak will only happen once, and the 48 byte string will be reused. |
|
Fix idea: If I understand things correctly, this should allocate only one NString (which, AFAIK, is a reference counted type), and then return references to that type. I am not sure if the call to |
louis030195
added a commit
that referenced
this issue
Aug 28, 2024
|
I see that you closed this issue. Did my solution work? |
|
@FractalFir no your code does not compile i fixed it here https://github.com/mediar-ai/screencapturekit-rs but there is still a small leak, but won't have much more time to allocate on this today |
OK, no problem. I don't know how those bounty things work. Does what I did count as working towards the bounty, or not? |
|
@FractalFir you can claim the bounty using |
|
example how to claim |
|
/tip @FractalFir 100 |
|
Also, just to double-check: are you running with From what I can recall, this should give more detailed leak stack traces, but I am not sure if it is not already enabled, and it just does not work with Rust. |
|
https://gist.github.com/louis030195/331d1b1bea17a1bc5dba7ebe62d53843 indeed mroe verbose |
|
not sure this is what u asked but i found this: let display = SCShareableContent::current().displays.pop().unwrap();
let windows = SCShareableContent::current().windows;
let _filter = SCContentFilter::new(InitParams::DisplayExcludingWindows(display, windows)); |
|
Ok, so it seems like two of the leaks involve a function named Those two leaks happen when creating So, I would double-check if At this point, I am almost certain something goes wrong when calling |
|
hmm wrapped in autoreleasepool: mediar-ai/screencapturekit-rs@2c77304 and this does not leak anymore: let display = SCShareableContent::current().displays.pop().unwrap();
let windows = SCShareableContent::current().windows;
let _filter = SCContentFilter::new(InitParams::DisplayExcludingWindows(display, windows));cpal output: https://gist.github.com/louis030195/2140d40e0b27056b811582d1b5cb2ee1 still shows leaks, is there something else? |
|
this leaks let config = SCStreamConfiguration {
width: 1920,
height: 1080,
captures_audio: true,
pixel_format: PixelFormat::ARGB8888,
scales_to_fit: true,
shows_cursor: true,
preserves_aspect_ratio: true,
queue_depth: 5,
sample_rate: 44100,
channel_count: 2,
excludes_current_process_audio: false,
..Default::default()
};
let init_params = InitParams::Display(display);
let filter = SCContentFilter::new(init_params);
let mut sc_stream = SCStream::new(filter, config, Capturer {});
let output = Capturer {};
sc_stream.add_output(output, SCStreamOutputType::Audio); |
|
Well, the good news is that This function seems to be responsible for this conversion, and it looks a bit fishy to me. The commented-out setter is especially odd - to me, this looks like it caused some issue, and someone commented it out instead of solving the underlying problem. So, it would not be unreasonable to assume that there are more issues like this. impl From<UnsafeStreamConfiguration> for Id<UnsafeStreamConfigurationRef> {
fn from(config: UnsafeStreamConfiguration) -> Self {
unsafe {
let alloc: *mut Object = msg_send![UnsafeStreamConfigurationRef::class(), alloc];
let obj: *mut Object = objc::rc::autoreleasepool(|| {
let obj: *mut Object = msg_send![alloc, init];
// Set properties
let _: () = msg_send![obj, setWidth: config.width];
let _: () = msg_send![obj, setHeight: config.height];
let _: () = msg_send![obj, setCapturesAudio: config.captures_audio];
let _: () = msg_send![obj, setSourceRect: config.source_rect];
let _: () = msg_send![obj, setDestinationRect: config.destination_rect];
let _: () = msg_send![obj, setPixelFormat: config.pixel_format];
let _: () = msg_send![obj, setMinimumFrameInterval: config.minimum_frame_interval];
let _: () = msg_send![obj, setScalesToFit: config.scales_to_fit];
let _: () = msg_send![obj, setShowsCursor: config.shows_cursor];
let _: () = msg_send![obj, setChannelCount: config.channel_count];
let _: () = msg_send![obj, setSampleRate: config.sample_rate];
// Uncomment if this setter is available in the API
// let _: () = msg_send![obj, setPreservesAspectRatio: config.preserves_aspect_ratio];
obj
});
if obj.is_null() {
panic!("Failed to create UnsafeStreamConfigurationRef");
}
Id::from_ptr(obj as *mut UnsafeStreamConfigurationRef)
}
}
}Once again, a EDIT: I have tried to discover why the call to setPreservesAspectRatio was originally commented out, hoping someone has encountered an issue like this - however, the commit message explains almost nothing. |
i feel like |
|
Was this feature enabled before, and did enabling it fix the leaks? |
|
they have the feat on upstream https://github.com/svtlabs/screencapturekit-rs/blob/9a6b9f346bd30de0d2f9a02af36d4cf6461f4f3b/screencapturekit-sys/Cargo.toml#L27 did not change will try the |
louis030195
added a commit
that referenced
this issue
Aug 29, 2024
|
most things are fixed (i believe), thanks 🙏 ill fix this last one later (gtg) |
|
/tip $80 @FractalFir . |
|
🎉🎈 @FractalFir has been awarded $80! 🎈🎊 |
|
Thank you for the tip! |
|
adding for reference for next fixes (also think there is leak in im kind of puzzled why im the only one facing these issues despite seems to be used in many libs |
|
#199 fixed btw on the way |
|
going to fix last memory leak now |
|
solved 1 next up |
|
maybe not a leak but this code is trash that i did: async fn get_macos_version() -> Option<f32> {
let output = Command::new("sw_vers")
.arg("-productVersion")
.output()
.await
.ok()?;
let version = String::from_utf8(output.stdout).ok()?;
version.split('.').next()?.parse().ok()
}will fix this now |
|
so ... good news:
bad news:
|
|
|
solved |
|
@louis030195 what final solution have you chosen? blackhole or osx new api? |
|
^ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
/bounty 150
The text was updated successfully, but these errors were encountered: