-
Notifications
You must be signed in to change notification settings - Fork 349
comp:Add initial Google AEC (adl-003-drop-stable) #5176
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
comp:Add initial Google AEC (adl-003-drop-stable) #5176
Conversation
Introduce a new component to perform acoustic echo cancellation on capture path using a buffer from the playback as a reference. 1. Put the google_rtc_audio_processing at $SOF_REPO/third_party_libraries - libgoogle_rtc_audio_processing.a - libgoogle_rtc_audio_processing_tuning.a - libc++.a (Corresponding stdlibc++) - libc++abi.a 2. Put the header in $SOF_REPO/third_party_includes - google_rtc_audio_processing.h 3. Build firmware and tool with xcc 4. To verify it works: - aplay some speech - At the same time arecord the mic which uses AEC The mic signal should not exhibit any echo from the playback. Signed-off-by: Lionel Koenig <lionelk@google.com>
This adds a mock to be able to build with the google_rtc_audio_processing component. Signed-off-by: Lionel Koenig <lionelk@google.com>
| sink_list); | ||
| buffer_lock(source_buffer, &flags); | ||
| if (source_buffer->source->comp.type == SOF_COMP_DEMUX) | ||
| cd->aec_reference = source_buffer; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO we need a better way to look for a specific demux
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just a temporary ad-hoc solution to identify a stream from the other. When doing AEC one need to figure out which stream is the playback reference (aka echo reference) and which one is the microphone. Ideally we should have a way to tag streams in the topology so we can differentiate it not base on where it come from (DMUX/DAI) but base on what they are.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| buffer_lock(source_buffer, &flags); | ||
| if (source_buffer->source->comp.type == SOF_COMP_DEMUX) | ||
| cd->aec_reference = source_buffer; | ||
| else if (source_buffer->source->comp.type == SOF_COMP_DAI) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could be switch (source_buffer->source->comp.type)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure could do. However I upstream this change and in order to ease later backports it would be preferable to change all version in all branch at the same time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lyakh this is a stable merge, so we would have to fix that on main first.
| // | ||
| // Copyright(c) 2021 Google LLC. | ||
| // | ||
| // Author: Lionel Koenig <lionelk@google.com> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would it be possible to make this a part of the SOF cmocka test suite? Any reason not to do that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This mock is a vlaid component which just does not do the processing. It is used to verify that the firmware compiles and to be able to run code even if one has no access to the third party library.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is similar to the smart amp test, its an in place component for those who do not have the lib.
|
@mwasko fyi |
|
|
||
| aec_reference_buff_frag = 0; | ||
| for (frame = 0; frame < num_aec_reference_frames; frame++) { | ||
| src = audio_stream_read_frag_s16(&cd->aec_reference->stream, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please avoid to read/write frag functions because they are slow, see #4967
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll address that on main branch and then backport it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@singalsu are all the replacements on this branch already?
This backports Google Initial AEC to alderlake branch
The differences with the original PR are:
the buffer locking API which have changed (due to API change in SOF)
create function signature.
Original PR: #5090