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

Add channel mask support for API 32. #1548

Merged
merged 4 commits into from
Jun 14, 2022
Merged

Add channel mask support for API 32. #1548

merged 4 commits into from
Jun 14, 2022

Conversation

flamme
Copy link
Collaborator

@flamme flamme commented May 25, 2022

For AAudio, channel mask support was added in API 32. In this change,
channel mask support is added in Oboe for API 32.

Fixes #1409.

For AAudio, channel mask support was added in API 32. In this change,
channel mask support is added in Oboe for API 32.

Fixes #1409.
@flamme flamme requested review from philburk and robertwu1 May 25, 2022 17:42
Copy link
Collaborator

@philburk philburk left a comment

Choose a reason for hiding this comment

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

Fantastic. This was much bigger than I thought.

include/oboe/Definitions.h Outdated Show resolved Hide resolved
include/oboe/Definitions.h Outdated Show resolved Hide resolved
@philburk
Copy link
Collaborator

philburk commented Jun 9, 2022

I downloaded this PR and ran it on Raven.
I noticed that the downmix is clipping, which is not the fault of this PR.
I also had a problem when selecting output channelMask of 5.1.4.
Pressing OPEN and START resulted in a crash. I think this is because the sine player can only handle up to 8 channels. So an array is getting over-indexed somewhere.

@philburk
Copy link
Collaborator

philburk commented Jun 9, 2022

For TEST INPUT, if I select ChannelCount=3 then I get 3 independent mic inputs.
If I select any channelMask with >=3 bits set, e.g. "2.1", "Tri", then I get an error. Maybe this is WAI.

@flamme
Copy link
Collaborator Author

flamme commented Jun 9, 2022

For TEST INPUT, if I select ChannelCount=3 then I get 3 independent mic inputs. If I select any channelMask with >=3 bits set, e.g. "2.1", "Tri", then I get an error. Maybe this is WAI.

For input case, "2.1" and "Tri" are not valid selection. In that case, getting error is WAI. Selecting channel count as 3 will be translated to index mask, which is a different case and will work.

@philburk
Copy link
Collaborator

philburk commented Jun 9, 2022

For input case, "2.1" and "Tri" are not valid selection.

Is there a case when they would be valid? I am guessing no.

system/audio.h distinguishes between AUDIO_CHANNEL_IN_* and AUDIO_CHANNEL_OUT_*.
I don't think there is much use case for the input channel masks.
Maybe this can be handled with documentation.

@flamme
Copy link
Collaborator Author

flamme commented Jun 9, 2022

I downloaded this PR and ran it on Raven. I noticed that the downmix is clipping, which is not the fault of this PR. I also had a problem when selecting output channelMask of 5.1.4. Pressing OPEN and START resulted in a crash. I think this is because the sine player can only handle up to 8 channels. So an array is getting over-indexed somewhere.

Yes, our previous max channel count is 8. I have uploaded a new patch to address this.

@flamme
Copy link
Collaborator Author

flamme commented Jun 9, 2022

For input case, "2.1" and "Tri" are not valid selection.

Is there a case when they would be valid? I am guessing no.

system/audio.h distinguishes between AUDIO_CHANNEL_IN_* and AUDIO_CHANNEL_OUT_*. I don't think there is much use case for the input channel masks. Maybe this can be handled with documentation.

Yes, audio.h distinguishes between input and output channel. But when AIDL is introduced, we decided not to distinguish input and output channel mask AAudioStreamParameters::validateChannelMask for the valid input channel mask. For instance, 2.0.2 is a valid one. We can add that in the aaudio documentation.

Copy link
Collaborator

@philburk philburk left a comment

Choose a reason for hiding this comment

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

The scrolling channel boxes are nice.

@flamme flamme merged commit befca30 into main Jun 14, 2022
@robertwu1 robertwu1 deleted the fix-1409-add-channelmask branch August 4, 2022 01:03
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.

add setChannelMask() for API 32
3 participants