Skip to content

Fixes for smart amp test module - #7101

Closed
ranj063 wants to merge 2 commits into
thesofproject:mainfrom
ranj063:fix/smart_amp_source
Closed

Fixes for smart amp test module#7101
ranj063 wants to merge 2 commits into
thesofproject:mainfrom
ranj063:fix/smart_amp_source

Conversation

@ranj063

@ranj063ranj063 commented Feb 15, 2023

Copy link
Copy Markdown
Collaborator

No description provided.

@ranj063
ranj063force-pushed the fix/smart_amp_source branch from 072f025 to 48be8c4CompareFebruary 15, 2023 17:34
@fredoh9

fredoh9 commented Feb 15, 2023

Copy link
Copy Markdown
Contributor

For alsabat test failure, captured wave file shows empty/silence. But I can generate sinewave and find frequency response right. Trying to find why we have empty wave file.

@fredoh9

fredoh9 commented Feb 15, 2023

Copy link
Copy Markdown
Contributor

Too early updated above.
Left channel is fine, 821Hz detected but only right channel is empty/silence.

Channel 1 - Checking for target frequency 821.00 Hz
Amplitude: 31127.0; Percentage: [94]
Detected peak at 821.04 Hz of 41.83 dB
Total 42.7 dB from 818.85 to 823.24 Hz
PASS: Peak detected at target frequency
Detected at least 1 signal(s) in total

Channel 2 - Checking for target frequency 821.00 Hz
Amplitude: 0.0; Percentage: [0]
Detected at least 0 signal(s) in total

@ranj063
ranj063force-pushed the fix/smart_amp_source branch 4 times, most recently from ba26f8b to 37e01d2CompareFebruary 15, 2023 20:33
@ranj063

ranj063 commented Feb 15, 2023

Copy link
Copy Markdown
CollaboratorAuthor

Left channel is fine, 821Hz detected but only right channel is empty/silence.

Thanks @fredoh9. fixed now.

@aiChaoSONG

aiChaoSONG commented Feb 16, 2023•
edited
Loading

Copy link
Copy Markdown
Collaborator

@fredoh9 Maybe there is an issue or configuration issue, I remembered that when I was in lab, I only hear front left only.

RanderWang
RanderWang reviewed Feb 16, 2023
View reviewed changes
Comment threadsrc/samples/audio/smart_amp_test.c Outdated
@ranj063
ranj063force-pushed the fix/smart_amp_source branch from 37e01d2 to d85baadCompareFebruary 16, 2023 02:32
@ranj063
smart_amp_test: remove bind/unbind ops
6588ade
Set the source_buf during prepare if the buffer source queue ID does not
match that of the feedback buffer. Set the feedback_buf during copy
to also handle the case where the playback stream is started before the
feedback stream.
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
@ranj063
smart_amp_test: clean up the copy() function
d6733b7
Clean up the variable names in the copy() function and make it easier to
follow.
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
@ranj063
ranj063force-pushed the fix/smart_amp_source branch from d85baad to d6733b7CompareFebruary 16, 2023 14:37
lyakh
lyakh reviewed Feb 20, 2023
View reviewed changes
Comment threadsrc/samples/audio/smart_amp_test.c
buffer_release(buf);
}
#if CONFIG_IPC_MAJOR_4
sad->feedback_buf = NULL;

@lyakhlyakhFeb 20, 2023

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.

so on every copy you have to re-assign the feedback buffer... TBH I'm not sure what smart-amp-test is actually for (it's "just" a test, right? So it isn't used in any real audio streaming?) but this seems rather expensive

@lgirdwoodlgirdwoodFeb 21, 2023

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.

Yeah, this will need a comment to explain the specific case if needed other wise it could be copy and pasted into other samples.

@gkbldcig

gkbldcig commented Mar 28, 2023

Copy link
Copy Markdown
Collaborator

Can one of the admins verify this patch?

lgirdwood
lgirdwood approved these changes Mar 29, 2023
View reviewed changes

@lgirdwoodlgirdwood left a comment

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.

Lots of code removal !

@kv2019i

kv2019i commented Mar 30, 2023

Copy link
Copy Markdown
Collaborator

Good now @RanderWang ?

kv2019i
kv2019i approved these changes Mar 30, 2023
View reviewed changes

@kv2019ikv2019i left a comment

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.

Code seems good, but I don't fully understand the conclusion on the long discussion in this PR on the multi-thread case. It is marked as resolved, so I guess this PR is now good to go, but would be good for @RanderWang@lyakh to ack with a +1 that this is ok to proceed.

@RanderWang

RanderWang commented Mar 31, 2023•
edited
Loading

Copy link
Copy Markdown
Collaborator

@lgirdwood@kv2019i. I also discussed my method (#7259) to remove bind & unbind with @ranj063@lyakh and got a conclusion that we will do bind & unbind job in module adapter. @andrula-song will adapt smart_amp to module interface and bind & unbind will be removed in her PR. We don't need to check feedback buffer in smart_amp_copy

@andrula-song

andrula-song commented Mar 31, 2023

Copy link
Copy Markdown
Contributor

hi @RanderWang
since the there is a limit of avail_frames = MIN(avail_passthrough_frames, avail_feedback_frames); and the avail_frames and avail_passthrough_frames could be different(#7100), I am afraid we have to check feedback buffer in smart_amp_copy function.

@RanderWang

RanderWang commented Apr 3, 2023

Copy link
Copy Markdown
Collaborator

hi @RanderWang since the there is a limit of avail_frames = MIN(avail_passthrough_frames, avail_feedback_frames); and the avail_frames and avail_passthrough_frames could be different(#7100), I am afraid we have to check feedback buffer in smart_amp_copy function.

you can discuss with Ranjani

@lgirdwood

lgirdwood commented Apr 4, 2023

Copy link
Copy Markdown
Member

@andrula-song good for you now, if so pls approve or reject.

@andrula-song

andrula-song commented Apr 6, 2023•
edited by lgirdwood
Loading

Copy link
Copy Markdown
Contributor

@andrula-song good for you now, if so pls approve or reject

apart of this PR already exist in our code, and I am using module adapter to remove bind & unbind. So I don't think merge this PR is necessary.

@lgirdwood

lgirdwood commented Apr 6, 2023

Copy link
Copy Markdown
Member

@ranj063 still needed ?

@kv2019ikv2019i mentioned this pull request Apr 17, 2023
Merged
@kv2019i

kv2019i commented Apr 28, 2023

Copy link
Copy Markdown
Collaborator

Turning into draft for now...

@kv2019i
kv2019i marked this pull request as draft April 28, 2023 11:49
@ranj063ranj063 closed this May 20, 2026
@ranj063
ranj063 deleted the fix/smart_amp_source branch May 20, 2026 22:44
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lyakhlyakhlyakh left review comments
@lgirdwoodlgirdwoodlgirdwood approved these changes
@kv2019ikv2019ikv2019i approved these changes
@plbossartplbossartAwaiting requested review from plbossartplbossart is a code owner
@mmaka1mmaka1Awaiting requested review from mmaka1mmaka1 is a code owner
@lbetlejlbetlejAwaiting requested review from lbetlejlbetlej is a code owner
@dbalutadbalutaAwaiting requested review from dbalutadbaluta is a code owner
+1 more reviewer
@RanderWangRanderWangRanderWang 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.

9 participants

@ranj063@fredoh9@aiChaoSONG@gkbldcig@kv2019i@RanderWang@andrula-song@lgirdwood@lyakh