Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support ScreenCapture loopback #894

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

Kree0
Copy link

@Kree0 Kree0 commented Jul 4, 2024

#876
I added a new host: ScreenCaptureKit in macos, which only provides input devices for capturing the sound of the Display.
When I test it in KVM virtualized mocOS-14, it captures sound intermittently, I don't know why this happens, can anyone help me check the code?

Cargo.toml Outdated Show resolved Hide resolved
@louis030195
Copy link

louis030195 commented Jul 9, 2024

any update on this? need asap for https://github.com/louis030195/screen-pipe

how can i help?

i just tried this PR to record screen to wav and it works

https://gist.github.com/louis030195/013fda2fa6b373f815bbeeb39e524090

@Kree0
Copy link
Author

Kree0 commented Jul 16, 2024

any update on this? need asap for https://github.com/louis030195/screen-pipe

how can i help?

i just tried this PR to record screen to wav and it works

https://gist.github.com/louis030195/013fda2fa6b373f815bbeeb39e524090

So it works fine in macOS? Maybe my previous testing method was incorrect.

@louis030195
Copy link

@Kree0 yes works well, I use it to record .mp3 files & transcribe with OpenAI's whisper 24/7 on my Macbook pro M3 max

@rustdesk
Copy link

Good job.

@louis030195
Copy link

update:

latest macos release broke this PR (15.0). issue does not happen on 14.5

thread '' panicked at /Users/matthewdi/.cargo/registry/src/index.crates.io-6f17d22bba15001f/objc_id-0.1.1/src/id.rs:52:9:
Attempted to construct an Id from a null pointer
note: run with RUST_BACKTRACE=1 environment variable to display a backtrace
libc++abi: terminating due to uncaught foreign exception
zsh: abort ./target/release/screenpipe --debug

@Kree0
Copy link
Author

Kree0 commented Jul 31, 2024

update:

latest macos release broke this PR (15.0). issue does not happen on 14.5

thread '' panicked at /Users/matthewdi/.cargo/registry/src/index.crates.io-6f17d22bba15001f/objc_id-0.1.1/src/id.rs:52:9:
Attempted to construct an Id from a null pointer
note: run with RUST_BACKTRACE=1 environment variable to display a backtrace
libc++abi: terminating due to uncaught foreign exception
zsh: abort ./target/release/screenpipe --debug

Can you run it with RUST_BACKTRACE=1 to show the full stack?

@louis030195
Copy link

will focus on this PR in upcoming days

@Poordeveloper
Copy link

will focus on this PR in upcoming days

Did you get a chance to check the crash?

@louis030195
Copy link

louis030195 commented Aug 16, 2024

https://github.com/SSheldon/rust-objc-id/blob/6527cdf28fef2b155c67c1681f548ecfd794003c/src/id.rs#L52

seems the crash happens here

but on the other hand i see that 15.0 macos is beta right?

so maybe this will be fixed by apple (idk)

will just push to prod my repo with this branch as deps and people that use beta macos wont have access to the feature

@louis030195
Copy link

fyi there is memory leak
image


let mut res = Vec::new();
for display in sc_shareable_content.displays.into_iter() {
res.push(Device::new(display));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here memory leak

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or actually this one too?
image

@Kree0 do you have any idea how to fix this leak? i use this branch in production and it grows memory forever, trying to find a fix

@louis030195
Copy link

fyi $150 bounty on fixing this: mediar-ai/screenpipe#183 (comment)

@louis030195
Copy link

fyi working on major leaks (there are multiple) related to screencapturekit

mediar-ai/screenpipe#236

@Kree0
Copy link
Author

Kree0 commented Oct 14, 2024

Sorry for the late reply. I have been busy in the past few months. The memory leak seems to have been resolved in mediar-ai/screencapturekit-rs, mediar-ai/screencapturekit-rs. Should I replace the dependencies with them?

@louis030195
Copy link

@Kree0 considering we should implement this PR using another lib than screencapturekit-rs

mediar-ai/screenpipe#297

currently i experienced 5-8 users having issues with screencapture kit when macos version below 13, usually seg fault, not sure it's an issue in my implementation or something with macos api

https://github.com/yury/cidre

@kaiwen-wang
Copy link

kaiwen-wang commented Oct 14, 2024

Did any of you try https://crates.io/crates/screen-capture-kit yet?

I also tried ruhear on https://crates.io/crates/ruhear, which seems to cause null pointer problems aizcutei/ruhear#6

@louis030195
Copy link

what is the best way to impl this?

@Kree0 @rustdesk @Poordeveloper @kaiwen-wang

$300 bounty to make this work without seg fault or memory leaks

mediar-ai/screenpipe#297 (comment)

@Kree0
Copy link
Author

Kree0 commented Oct 23, 2024

This looks a little difficult. And I don't have a mac device or time to do this in the near future, can see if others have time.

@thewh1teagle
Copy link

See yury/cidre#23
This crate seems more stable and makes it easier to add screen capture support to cpal.

@Kree0
Copy link
Author

Kree0 commented Nov 13, 2024

See yury/cidre#23 This crate seems more stable and makes it easier to add screen capture support to cpal.

I‘ll take a look in upcoming days.

@Kree0
Copy link
Author

Kree0 commented Nov 17, 2024

See yury/cidre#23 This crate seems more stable and makes it easier to add screen capture support to cpal.

@louis030195
I replaced it with this, you can see if there is still any problem.

@thewh1teagle
Copy link

thewh1teagle commented Nov 22, 2024

@Kree0

Tested on macos 14.5 with an M1 chip, and it works! the host/device is visible, and i was able to record wav files using the examples. note that the system prompts for screen recording permissions. however, I think that it's possible to request audio-only recording using screencapturekit.

yury/cidre#23 (comment)

.

@thewh1teagle
Copy link

The crate cidre just added option to capture system audio only
cidre/examples/core-audio-record/main.rs
yury/cidre#23 (comment)

@louis030195
Copy link

See yury/cidre#23 This crate seems more stable and makes it easier to add screen capture support to cpal.

@louis030195 I replaced it with this, you can see if there is still any problem.

any chance you can send a PR here with the switch in dependencies in screenpipe-audio/Cargo.toml

mediar-ai/screenpipe#297

so i can test & give you the bounty

@louis030195
Copy link

@louis030195
Copy link

how do i know tehre is no memory leak this time though?

@thewh1teagle
Copy link

any chance you can send a PR here with the switch in dependencies in screenpipe-audio/Cargo.toml

I think that It's not ready yet and need a bit more testing
how can we declare a vector that holds both regular cpal devices and ScreenCaptureDevice?
I tried use:

let devices: Vec<Box<DeviceTrait>>;

It should hold both Device and ScreenCaptureDevice, but I can't call the methods. I must be missing something.

@Kree0
Copy link
Author

Kree0 commented Nov 30, 2024

The crate cidre just added option to capture system audio only cidre/examples/core-audio-record/main.rs yury/cidre#23 (comment)

I think it is not using ScreenCaptureKit but Core Audio. Core Audio seems to have been encapsulated into cpal before, but I don't know if it supports loopback. Currently, a Device of ScreenCaptureKit host captures the output of a display, and it seems that screen recording permission is required.

@thewh1teagle
Copy link

thewh1teagle commented Nov 30, 2024

but I don't know if it supports loopback.

It support loopback
See
https://github.com/yury/cidre/blob/main/cidre/examples/core-audio-record/main.rs

yury/cidre#23 (comment)

Maybe with that we won't need another host

Note: I suggest keep this PR changes and maybe create another one with Core Audio or at least keep the commits

@Kree0
Copy link
Author

Kree0 commented Dec 2, 2024

Note: I suggest keep this PR changes and maybe create another one with Core Audio or at least keep the commits

Ok, I'll keep this commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants