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

Sink Audio Patch #9

Merged
merged 3 commits into from Feb 14, 2021
Merged

Sink Audio Patch #9

merged 3 commits into from Feb 14, 2021

Conversation

cdelorme
Copy link
Contributor

I'll start by saying thank you so much for sharing this awesome project.

I recently acquired a Valve Index and was thrilled to find useful tools like this.

The bluetooth base station functionality worked perfectly for me.

While enabling audio works, I had some errors running move-sink-input, and it silently fails to restore audio when running off or the daemon exits.

I'm an arch linux user with pactl version 14.2, and whatever modules it came with barring suspend-on-idle module because it regularly skips audio at the start of playback due to activation latency.

In my case I only ever have a single sink.

Changing the port via set-card-profile appears to be what works for me, which I was testing from this script before I found your tool:
https://gist.github.com/frostworx/2a1a84ea8098ddc207cc9f54793f5446

Oddly, when the profile changes the sink name also changes to match, and the default sink too (though unreliably if I have other devices connected, which I'm guessing is why empty string for the normal regex is not recommended).

Additionally, my sink-inputs appear to automatically switch to the new sink, which combined with the sink name changing makes move-sink-input unnecessary.

To avoid breaking existing functionality I added a new config that supports finding the normal profile by device name the same way it finds the Index HMD, and I added a profile change to switch_to_normal.

To avoid the errors that occur due to the sink name change I added a pre-flight-check to set_sink_for_all_sink_inputs, which makes sure the sink name still exists before attempting to run move-sink-input.

Please let me know if these changes are usable.

I may try adding a similar audio toggle for microphones tomorrow.

cdelorme and others added 2 commits February 13, 2021 22:21
add pre-flight check to avoid move-sink-input when sink name does not
exist

add port/profile step to switch_to_normal to restore audio
@DavidRisch
Copy link
Owner

Thanks for your contribution!
I am assuming your normal audio device is connected your graphics card (which causes only a single sink)? I never tested that scenario (only audio over usb).

I did a bit of refactoring and improved backwards compatibility (because your change from card_port_product_name_regex to card_port_vr_product_name_regex broke my setup).

I will test the new version with my setup soon to make sure everything still works. Please do the same on your side and report the results.
Once that is done I will merge this.

If you want to add support for microphones, please do so in a seperate branch+pr.

@cdelorme
Copy link
Contributor Author

Your changes worked for me, but I did find a typo in the config-helper.

Yeah, I have audio coming out of my GPU, and only one GPU, so it's the same card and just has to switch from HDMI to DisplayPort. I'm certainly not a pulse audio expert, so I really don't understand the different ways that can be rigged.

Sorry, I didn't think to use a branch, I'll be sure to do so next time although I may not get to the microphone today.

@DavidRisch
Copy link
Owner

Thanks for fixing the typo, I totally forgot to test the config_helper.

Sorry, I didn't think to use a branch

No problem, the master branch in your fork is effectively a separate branch. I was only making sure you use a separate branch for anything (e.g. microphones) not directly a part of this pr.

@DavidRisch DavidRisch merged commit b48cf3e into DavidRisch:master Feb 14, 2021
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.

None yet

2 participants