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

Improve audio channel mapping #1280

Merged
merged 2 commits into from
Sep 2, 2023

Conversation

leo150
Copy link
Contributor

@leo150 leo150 commented Sep 1, 2023

Description & motivation

  • Fix audio channel mapping. My previous implementation was creating a channel map of size n, where n is number of input channels. It should use number of output output channels instead.
  • Expose channel map for external use
  • Add channel map unit tests

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Screenshots:

@leo150 leo150 changed the title WIP: Improve audio channel mapping Improve audio channel mapping Sep 1, 2023
@leo150
Copy link
Contributor Author

leo150 commented Sep 1, 2023

I added unit tests. makeChannelMap falls back to default value if the provided one is incorrect.

@shogo4405 shogo4405 merged commit bc4615a into shogo4405:main Sep 2, 2023
1 check failed
@shogo4405 shogo4405 added this to the 1.5.8 milestone Sep 2, 2023
@shogo4405
Copy link
Owner

I just noticed after the merge that the tests are failing. Is the expected outcome correct?
スクリーンショット 2023-09-02 15 55 58

@leo150
Copy link
Contributor Author

leo150 commented Sep 2, 2023

Hey, thank you for noticing this. The test is incorrect, it should expect [0, -1]. I checked again, it's the only test that is failing. Thanks.

shogo4405 added a commit that referenced this pull request Sep 3, 2023
@shogo4405
Copy link
Owner

Okay, I've fixed the tests. 2e1cb55

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