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

Warn about PulseAudio in the doc and rework related sections #957

Merged
merged 27 commits into from
Sep 26, 2023

Conversation

pushfoo
Copy link
Contributor

@pushfoo pushfoo commented Sep 21, 2023

Changes

  1. Document the PulseAudio bug (Pulse Audio Exception - Hard to Reproduce #952)
  2. Add "Choosing audio drivers" heading
  3. %s/Pulseaudio/PulseAudio/g and rename the Pulse heading to PulseAudio for consistency
  4. Expand the PulseAudio section with better pros / cons + bug
  5. Expand the OpenAL section with Linux install command examples and other info
  6. Add link targets for relevant headings
  7. Rearrange + add table footnotes for cleanliness
  8. Link the new doc from the quickstart
  9. Link the doc from pyglet/__init__.pys doc for the options['audio'] key

* Rephrase OpenAL heading contents

* Add table of common linux distros to their OpenAL install commands

* Move OpenAL footnote next to table

* Add link target for OpenAL heading

* Remove redundancy from OpenAL footnote by linking OpenAL heading
* Add warning directive linking OpenAL

* Add subparagraphs detailing issues

* Expand mention of problems, including PipeWire being a target for future inclusion
* Add quotes around values to improve developer UX

* Link the programming guide

* Add mention of OpenAL issue + requirements
@pushfoo pushfoo marked this pull request as ready for review September 21, 2023 08:44
@caffeinepills
Copy link
Collaborator

Personally I am not agreeing with some of the language, and there are some errors:

Calling the PulseAudio buggy implies it's unusable. I think just putting warnings of it may having issues with those 2 issues you mentioned. We've only had one report about such issues and could be system, version, or something else going on.

The 'default' section on audio is incorrect, it says it is: ('openal', 'pulse', 'xaudio2', 'directsound', 'silent') but that was just an example to show how to re-arrange the order. The default is actually ('xaudio2', 'directsound', 'openal', 'pulse', 'silent')

Pyglet does not technically consider OpenAL 'preferred', and the language implies people should install it on their platforms. That should be up to the developer if they wish their users to use OpenAL on all platforms.

The default behavior of Pyglet is just so that sound just works on all OS platforms as much as possible, it will go through all drivers until it finds one. OpenAL should be recommended over PulseAudio simply because of positional audio. However, Windows has XAudio2 (and DirectSound) backends which is more feature complete than OpenAL.

My 2c.

* Incorporate feedback from caffienepills

* Rewrite PulseAudio opening to be less negative and briefer

* Add error message example for the bug

* Add links to GitHub issue & fix comment link

* Add mentions of PipeWire compat
* Incorporate feedback from caffienepills

* Add openal string in openal description
* Add link targets for XAudio2 and DirectSound

* Link them from OpenAL section to reflect info from caffienepills

* Phrasing tweaks
@pushfoo
Copy link
Contributor Author

pushfoo commented Sep 23, 2023

@caffeinepills I think I got everything you and Ben mentioned. PulseAudio is now described as limited rather than broken.

@benmoran56 benmoran56 merged commit 0f19ee3 into pyglet:master Sep 26, 2023
11 checks passed
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

3 participants