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.

Sign up for GitHub

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

Jump to bottom

Unit Test that warns about Stereo Audio #33094

Open
VasilisThePikachu opened this issue Oct 31, 2024 · 1 comment
Open

Unit Test that warns about Stereo Audio #33094

VasilisThePikachu opened this issue Oct 31, 2024 · 1 comment
Labels
A: Integration Tests Area: Integration tests, adding or fixing them D2: Medium Difficulty: A good amount of codebase knowledge required. P2: Raised Priority: Item has a raised priority, indicating it might get increased maintainer attention.

Comments

@VasilisThePikachu
Copy link
Member

Description

#32064 accidentally put in stereo audio.

Now, this will usually show up as a debug assert to the developer. Alerting them to down mix their audio to mono

However, if the developer simply switches to release during their development, it will be ignored. As debug asserts don't run in release environments.

Since there is no test to tell reviewers "Hey this audio is stereo when it should be mono" it was merged. And is now causing issues (look at #33092)

Solving this will require to add a unit test that will check for stereo files in places where we want to have it be mono instead.

In certain places like lobby music, stereo audio is fine. So make sure your unit tests does not falsely flag those.
@github-actions github-actions bot added the S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. label Oct 31, 2024
@VasilisThePikachu VasilisThePikachu added P2: Raised Priority: Item has a raised priority, indicating it might get increased maintainer attention. D2: Medium Difficulty: A good amount of codebase knowledge required. A: Integration Tests Area: Integration tests, adding or fixing them and removed S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. labels Oct 31, 2024
@slarticodefast
Copy link
Member

Just noticed this in another PR and it does not even cause a debug assert.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A: Integration Tests Area: Integration tests, adding or fixing them D2: Medium Difficulty: A good amount of codebase knowledge required. P2: Raised Priority: Item has a raised priority, indicating it might get increased maintainer attention.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@VasilisThePikachu @slarticodefast