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

Use the Dummy audio driver in the project manager #38208

Merged

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Apr 25, 2020

This prevents Godot from appearing in the list of applications outputting sound in the OS while the user is in the project manager.

This also partially addresses #38154.

@Calinou Calinou added enhancement topic:audio topic:editor cherrypick:3.x Considered for cherry-picking into a future 3.x release labels Apr 25, 2020
@Calinou Calinou added this to the 4.0 milestone Apr 25, 2020
@akien-mga
Copy link
Member

I'm not sure it's worth hacking it this way. If Godot uses too much CPU due to the coreaudio driver, working it around only in the project manager doesn't help much as 99% of user time is spent in the editor itself.

And if we ever decide to add sound features to the project manager (e.g. for accessibility), it might lead to some wasted hours trying to figure out why it's not outputting any audio.

@Calinou
Copy link
Member Author

Calinou commented Apr 27, 2020

And if we ever decide to add sound features to the project manager (e.g. for accessibility), it might lead to some wasted hours trying to figure out why it's not outputting any audio.

If we add screen reader support, I think the application emitting sound would be the screen reader, not Godot.

@ellenhp
Copy link
Contributor

ellenhp commented Feb 14, 2021

If we add screen reader support, I think the application emitting sound would be the screen reader, not Godot.

That depends on how it's implemented. I've done a bit of work to help out the developer of this screen reader editor plugin, and while I don't think it works in the project manager, using the dummy audio driver would completely prevent any future in-process accessibility tools from working

@Calinou Calinou force-pushed the project-manager-use-dummy-audio-driver branch from 5e75b83 to af3050b Compare May 6, 2021 00:33
@Calinou Calinou requested a review from a team as a code owner May 6, 2021 00:33
@Calinou
Copy link
Member Author

Calinou commented May 6, 2021

Rebased and tested again, it works successfully 🙂

That depends on how it's implemented. I've done a bit of work to help out the developer of this screen reader editor plugin, and while I don't think it works in the project manager, using the dummy audio driver would completely prevent any future in-process accessibility tools from working

The project manager doesn't support plugins yet, so I think this PR would be fine to merge for now. Also, if project manager plugins ever become supported, I guess we could have a way to initialize the audio driver only when a sound is actually played for the first time. (This would be opt-in for games because this adds significant latency to the first sound.)

@lawnjelly
Copy link
Member

This could potentially be superceded by a 4.x version of #63458 . The ability to mute until audio output in the PR means that it would also work with screen reading software.

@clayjohn
Copy link
Member

clayjohn commented Aug 9, 2022

Discussed in the PR review meeting. Reduz suggests that more research is needed to solve #38154. #63458 may be an appropriate solution, but even it may be overkill.

@YuriSizov YuriSizov modified the milestones: 4.0, 4.1 Feb 9, 2023
@YuriSizov YuriSizov modified the milestones: 4.1, 4.2 Jun 14, 2023
@AThousandShips AThousandShips modified the milestones: 4.2, 4.3 Oct 26, 2023
@akien-mga akien-mga modified the milestones: 4.3, 4.x Jun 28, 2024
This prevents Godot from appearing in the list of applications
outputting sound in the OS while the user is in the project manager.
@Calinou Calinou force-pushed the project-manager-use-dummy-audio-driver branch from eb7903a to da4683c Compare August 6, 2024 23:27
@Calinou
Copy link
Member Author

Calinou commented Aug 6, 2024

Rebased and tested again, it works as expected.

Note that TTS audio output is not emitted by Godot itself (but by the OS instead), so this doesn't affect potential screen-reader support in the future.

Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

The code is simple enough that it's fine to merge IMO. Even if it doesn't solve the mentioned issue, there is some minimal benefit.

@Repiteo Repiteo modified the milestones: 4.x, 4.4 Oct 23, 2024
@Repiteo Repiteo merged commit 9554e3c into godotengine:master Oct 25, 2024
18 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Oct 25, 2024

Thanks!

@Calinou Calinou deleted the project-manager-use-dummy-audio-driver branch November 2, 2024 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherrypick:3.x Considered for cherry-picking into a future 3.x release enhancement topic:audio topic:editor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants