-
-
Notifications
You must be signed in to change notification settings - Fork 98
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
Feature/screenshare with audio #346
Conversation
…tionalDependencies
I resolved the conflicts, but I can't try the changes until this weekend. |
Builds are failing:
|
Fix imports
Fix issues with code based on pre-merge types.
@SpookySkeletons This appears to be an issue with the latest version of electron (not sure if this is a known/reported bug?) as downgrading from major version 22 to 21 solved the issue you mentioned for me, giving me the choice to choose either a screen or a window. Otherwise I can say that audio sharing seems to be working fine on both x11 and wayland (Arch, KDE Plasma), even streamed gameplay for a couple of hours when I had initially intended to just test to see if the feature was working. |
@PhantomShift Any word on latest electron 24? EDIT: I am unable to downgrade as electron sub 23 suffers a bug that crashes the renderer during long call sessions. |
@SpookySkeletons I just made this issue in the brave browser
|
0b0b93e
to
77c0956
Compare
Hello, may we get a rebase of this branch to help anyone wanting to still roll these patches? |
@SpacingBat3, the last pipeline says something about 'enums' in lines 391, 394, 395... I'm checking the code and I found the enums you've used for representing different favicon states could be causing the error. Could you please check this particular part of your code? Thanks! |
If that's since the new TypeScript - will do once I'll move myself to it, right now I'm nowhere near my main dev machine and I might lack of the time... |
aa35696
to
00ecf70
Compare
Is this relevant anymore, given #154 ? |
For upstream, not really, but this PR still inspires me to make things modular in the future and to allow people replacing some Electron stuff with their own native modules for better integration. I kinda feel bad about entirely killing this PR. I should also say, both this PR's and Electron's implementations have their advantages and disadvantages, I think with this one you have a greater control over the audio within the application, yet again Electron's implementation is much more portable and doesn't rely on PipeWire at all (for some this is a bad thing, but given there are still distros that are PulseAudio only, I still think it's more portable to implement things based on Pulse than PipeWire). |
My primary goal was to enable screen sharing with sound directly, without relying on external programs or scripts. If the proposed solution had been implemented, It's a win! It's a bit sad that the PR was closed but I understand the decision of choosing the native solution. Although it's a bit limited in what you can do, I think it'll be easier to maintain. Despite the outcome, I'm satisfied with the knowledge and skills I've acquired during this process. |
@kakxem I have used this pr for a very long time Hope the native solution is in the same boat |
No description provided.