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

win-wasapi: Add support for capturing a process #5218

Merged
merged 6 commits into from
Jul 24, 2022

Conversation

jpark37
Copy link
Collaborator

@jpark37 jpark37 commented Aug 29, 2021

Description

Use new Windows API for capturing audio from a process.

Motivation and Context

Users want it.

How Has This Been Tested?

Not extensively tested, but seems to work. Tested input/output/process, and fixed various issues.

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@WizardCM WizardCM added Enhancement Improvement to existing functionality Windows Affects Windows labels Aug 29, 2021
@jpark37
Copy link
Collaborator Author

jpark37 commented Aug 29, 2021

Checks are probably busted until GitHub Actions provides 20348 SDK.

@henke37
Copy link
Contributor

henke37 commented Aug 29, 2021

I wonder if this should be a mere separate mode or a separate source. Either way, should still keep it in the same dll.

As for the process selection, #2659 would greatly simplify it, right?

@WizardCM WizardCM requested a review from Warchamp7 August 29, 2021 23:40
@jpark37 jpark37 force-pushed the win-audio-per-process branch from 193aa3b to e22c49b Compare August 30, 2021 05:39
@WizardCM
Copy link
Member

So here are my thoughts on how I think this should be exposed to the user:

  1. This should not be a thing you can set to a global audio source (the default Desktop Audio source, for example). This is because the core of that is modified in the Audio section of Settings, which a user shouldn't need to modify live. It is accessible from the Audio Mixer using the gear + Properties, but in this particular situation that should remain device-only.
  2. The "Method" dropdown should have three options:
    a) Device
    b) OBS Capture Source
    c) Process / Application
  3. For the "OBS Capture Source" method, the list should be populated with a list of Game Capture sources, a ---- separator, then a list of Window Capture sources. This should behave identically to Process mode, but doesn't require the user to manually specify a process, it just mirrors the Window / Game capture source's selected window.
  4. For the Process / Application method, I agree that process ID would be bad. I'd say this method should essentially use the same window lookup/storage functions as Window Capture (same kind of dropdown). This is for when the user doesn't want to window capture the process but does want its audio (eg. a media player).

@jpark37 jpark37 force-pushed the win-audio-per-process branch from e22c49b to d6aad6c Compare August 30, 2021 09:39
@henke37
Copy link
Contributor

henke37 commented Aug 30, 2021

One thing for sure, I do not want duplicated code for the process/window selection.

@Warchamp7
Copy link
Member

Have not tested my PR myself yet, but from a UX standpoint is there any reason this can't/shouldn't just be a checkbox in game capture / window capture that says "Capture audio directly" or something?

@Gol-D-Ace
Copy link
Member

Gol-D-Ace commented Aug 30, 2021

Have not tested my PR myself yet, but from a UX standpoint is there any reason this can't/shouldn't just be a checkbox in game capture / window capture that says "Capture audio directly" or something?

What if you want to include some audio but don't want to show that window?

Sure you can hide the source but that's still taking unnecessary ressources.

Having it in the properties of window / game capture and an extra source you can add perhaps?

@henke37
Copy link
Contributor

henke37 commented Aug 30, 2021

One UX improvement would be for the Game/Window capture source to have a "add audio capture source" button somewhere.

@Warchamp7
Copy link
Member

Warchamp7 commented Aug 30, 2021

Have not tested my PR myself yet, but from a UX standpoint is there any reason this can't/shouldn't just be a checkbox in game capture / window capture that says "Capture audio directly" or something?

What if you want to include some audio but don't want to show that window?

Sure you can hide the source but that's still taking unnecessary ressources.

A dropdown then to choose between capturing video, audio, or both perhaps?

I do not think we should have two separate flows through the program for capturing a game or window depending on what the user wants to capture from it

@jpark37 jpark37 force-pushed the win-audio-per-process branch from d6aad6c to ce3edda Compare August 30, 2021 17:28
@Riotline
Copy link

Riotline commented Sep 3, 2021

Could work similarly to this plugin which does something similar but attaches before WASAPI since v2.0 I believe. It is only recent. GitHub

@jpark37 jpark37 force-pushed the win-audio-per-process branch 5 times, most recently from 9c138d5 to 8790270 Compare September 11, 2021 21:37
@Warchamp7
Copy link
Member

Have not tested my PR myself yet, but from a UX standpoint is there any reason this can't/shouldn't just be a checkbox in game capture / window capture that says "Capture audio directly" or something?

What if you want to include some audio but don't want to show that window?
Sure you can hide the source but that's still taking unnecessary ressources.

A dropdown then to choose between capturing video, audio, or both perhaps?

I do not think we should have two separate flows through the program for capturing a game or window depending on what the user wants to capture from it

I've thought on this more and changed my mind a bit. I'm mostly in agreement with what Matt suggested.

However, I don't think this should piggyback off the Audio Output Capture source type, at least UI/UX wise. Internally it can work that way if need be, but I think this should be an additional source type in the list and leave Audio Output Capture for devices only.

  1. A setting in window and game capture properties to also grab audio from their specified process
  2. An "Application Audio" source type that is purely audio capture

This way people can get video/audio from things like games all in one place, as well as intuitively grab audio only from things like Discord or a music app.

@jpark37 jpark37 force-pushed the win-audio-per-process branch 2 times, most recently from 00f3a49 to 6a7ab8b Compare September 12, 2021 07:17
@jpark37
Copy link
Collaborator Author

jpark37 commented Sep 12, 2021

I did a force-push, so you can select process by window now. I tested that Chrome and Spotify work. I'm a bit surprised that Chrome worked out of the box because I thought I was going to have to play shenanigans to find the audio process. I don't know how they map processes to windows/audio.

There's a lot of copy/paste from win-capture, but I didn't want to push that code into libobs until we get closer to deciding what to do. Window priority is not selectable and hard-coded to title to now.

Are we sure we want to add a new source type? I thought people wanted to consolidate them, e.g. merging window/game capture, but I might be remembering wrong.

@jpark37 jpark37 force-pushed the win-audio-per-process branch 4 times, most recently from 30cdda8 to 2670faf Compare September 12, 2021 21:23
Copy link
Member

@RytoEX RytoEX left a comment

Choose a reason for hiding this comment

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

Just nitpicking on a string name change.

I feel like there's a lot of code in here that is derived or copied entirely from code that we have elsewhere in the codebase for selecting application windows and determining information about a window. Would it make sense to have these in some utils or platform code that plugins could use instead of copying them around from place to place?

Edit: I should have read the most recent comment more closely, as you've already mentioned this. Addressing in another comment.

plugins/win-wasapi/data/locale/en-US.ini Outdated Show resolved Hide resolved
@jpark37 jpark37 force-pushed the win-audio-per-process branch from 2670faf to 51402d0 Compare September 13, 2021 01:07
@jpark37
Copy link
Collaborator Author

jpark37 commented Jun 26, 2022

Fwiw, I can only reproduce the scene cut issue if I use separate sources. I don't hear a cut if I add the existing source to the other scene.

@Warchamp7
Copy link
Member

Is it possible to prevent a user from picking the same process twice, or displaying a red error on the selection or anything if they do?

The audio mixer should show no audio getting through. It's similar to the video capture sources; if it doesn't work, you just get no picture in the scene preview, and we log the failure.

The mixer shows no audio, but it's not really clear to the user why unless they know they have another source trying to hook the same process.

I am aware game capture suffers the same problem, but I'm not a fan of that either :P

There is a brief cut/crackle when using a Cut transition between two scenes that both have the same audio capture.

Are you using the same source, or two separate sources pointed at the same process? Is this an issue with regular Audio Output Capture?

Same source in both scenes, as I cannot get two sources to capture the same process.

jpark37 added 6 commits July 23, 2022 17:41
Add "ms_" prefix as makeshift namespace.
Use new process output API, and retrofit existing WASAPI abstractions.

Marked as "(BETA)" until we figure out the crackling at 60 minutes.
Lets OBS find Spotify.exe even if the window title changes.
@jp9000 jp9000 force-pushed the win-audio-per-process branch from a549366 to 0d12998 Compare July 24, 2022 00:41
@jp9000 jp9000 merged commit 4e1ba08 into obsproject:master Jul 24, 2022
@jpark37
Copy link
Collaborator Author

jpark37 commented Jul 24, 2022

Probably needs a follow-up PR for Yami theme.

@jpark37 jpark37 deleted the win-audio-per-process branch July 24, 2022 01:25
@RytoEX RytoEX added this to the OBS Studio 28.0 milestone Jul 24, 2022
@micsthepick
Copy link

will this support surround sound?

@SnorreSelmer
Copy link

will this support surround sound?

If the application outputs surround sounds, then OBS should be able to pull surround sound. To the API in question, it's just data.

@pandages
Copy link

I'm having the Noisy after 30-ish minutes issue with this audio capture method. Are you still looking for logs?

@RytoEX
Copy link
Member

RytoEX commented Jan 25, 2023

I'm having the Noisy after 30-ish minutes issue with this audio capture method. Are you still looking for logs?

We are still collecting information on this, yes. I can be reached directly on Discord at RytoEX#5545. Specifically, we are still looking for the data outlined here.

@Balls0fSteel
Copy link

Could you (the devs) provide a new build with this feature? I'd love to try it for testing, etc. Thank you.

@RytoEX
Copy link
Member

RytoEX commented Mar 29, 2023

Could you (the devs) provide a new build with this feature? I'd love to try it for testing, etc. Thank you.

The feature has been included in OBS Studio since OBS Studio 28.0 which was released in August 2022.

@Balls0fSteel
Copy link

Balls0fSteel commented Mar 29, 2023

Oh terribly sorry for the necro then I'll dig deeper into the settings then. I am truly sorry and I appreciate all the work you put into the project.

Update: In case anyone is looking for this... it's a different Source. You have to add a new Source (Game Audio Capture) and that's it. Amazing feature btw.

@Destroy666x
Copy link

This feature is unfortunately still lacking, compared to an available win-capture-audio plugin, which has exclusions. Please consider adding that. So that e.g. few music apps and all non-music apps can be easily configured into 2 separate sources.

@Nipeno
Copy link

Nipeno commented Dec 27, 2023

Wish this had the exclusion feature from win-audio-capture.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Feature New feature or plugin Seeking Testers Build artifacts on CI Windows Affects Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.