Skip to content

mux: fix input and output channel setting - #3674

Merged
lgirdwood merged 1 commit into
thesofproject:masterfrom
juimonen:mux_index_fix
Feb 3, 2021
Merged

mux: fix input and output channel setting#3674
lgirdwood merged 1 commit into
thesofproject:masterfrom
juimonen:mux_index_fix

Conversation

@juimonen

@juimonenjuimonen commented Dec 7, 2020

Copy link
Copy Markdown

Input and output channel settings from matrix row and column
are in reverse order, so they should be swapped.

Signed-off-by: Jaska Uimonen jaska.uimonen@intel.com

@juimonen

juimonen commented Dec 7, 2020

Copy link
Copy Markdown
Author

@bkokoszx FYI. Don't we have the input/output setting reversed currently? With this change I get the correct output with my testing at least...

lgirdwood
lgirdwood reviewed Dec 8, 2020
View reviewed changes
Comment threadsrc/audio/mux/mux_generic.c Outdated

@lgirdwoodlgirdwoodDec 8, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does it make more sense to rename j and m as row and column ?

@lgirdwoodlgirdwoodJan 7, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ping. if you can fix and re-push this will let CI do a full run too.

bkokoszx
bkokoszx reviewed Dec 8, 2020
View reviewed changes

@bkokoszxbkokoszx left a comment •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@juimonen
I have a concern, because as I understand correctly mask[j] corresponding to j channel on output, as we can see in code before your change. I suspect that if we hit some swapped channels issue, we may have problem somewhere else.

@juimonenjuimonen mentioned this pull request Dec 16, 2020
Closed
@juimonen

juimonen commented Jan 7, 2021

Copy link
Copy Markdown
Author

@akloniex your thoughts on this as original mux/demux author?

@juimonen

juimonen commented Jan 22, 2021

Copy link
Copy Markdown
Author

@bkokoszx@akloniex ping any comments about the correctness of this? I still believe in this fix :)

@lgirdwood

lgirdwood commented Jan 22, 2021

Copy link
Copy Markdown
Member

SOFCI TEST

@lgirdwood

lgirdwood commented Jan 26, 2021

Copy link
Copy Markdown
Member

@juimonen ping - can you check CI.

@bkokoszx

bkokoszx commented Jan 28, 2021•
edited
Loading

Copy link
Copy Markdown
Collaborator

@juimonen
Sorry, I was wrong here: #3674 (review)
I was talking with @akloniex and the primary assumption was that "j" in "mask[j]" corresponds to j input channel, so this PR looks correct. :)

@lgirdwood

lgirdwood commented Jan 28, 2021

Copy link
Copy Markdown
Member

@juimonen can you check CI. UT failed.

@juimonen

juimonen commented Feb 2, 2021

Copy link
Copy Markdown
Author

SOFCI TEST

Jaska Uimonen
mux: fix input and output channel setting
b1b31e7
Input and output channel settings from matrix row and column
are in reverse order, so they should be swapped.
Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>
@lgirdwood
lgirdwood merged commit 71a4e98 into thesofproject:masterFeb 3, 2021
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lgirdwoodlgirdwoodlgirdwood approved these changes
@akloniexakloniexAwaiting requested review from akloniex
@dbalutadbalutaAwaiting requested review from dbaluta
@lbetlejlbetlejAwaiting requested review from lbetlej
@mmaka1mmaka1Awaiting requested review from mmaka1
@plbossartplbossartAwaiting requested review from plbossart
+1 more reviewer
@bkokoszxbkokoszxbkokoszx left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@juimonen@lgirdwood@bkokoszx