Expose custom configuration of internal AEC3 #44
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
skywhale
force-pushed
the
upgrade-v1-aec3-tuning
branch
from
cf4c5af to
fddf41d
Compare
strohel
reviewed
Jan 14, 2025
Comment on lines
+39
to
+43
| webrtc::EchoCanceller3Config build_aec3_config(const EchoCanceller3ConfigOverride& override) { | ||
| webrtc::EchoCanceller3Config config; | ||
| config.delay.num_filters = override.num_filters; | ||
| return config; | ||
| } |
There was a problem hiding this comment.
Supernit: override seems to be a C++ keyword (as GitHub highlights it and as I rembember). Even though it compiles, it may be better can name it differently:
Suggested change
| webrtc::EchoCanceller3Config build_aec3_config(const EchoCanceller3ConfigOverride& override) { | |
| webrtc::EchoCanceller3Config config; | |
| config.delay.num_filters = override.num_filters; | |
| return config; | |
| } | |
| webrtc::EchoCanceller3Config build_aec3_config(const EchoCanceller3ConfigOverride& config_override) { | |
| webrtc::EchoCanceller3Config config; | |
| config.delay.num_filters = config_override.num_filters; | |
| return config; | |
| } |
Comment on lines
+75
to
+79
| /// Initializes internal states, while retaining all user settings. This should be called before | ||
| /// beginning to process a new audio stream. However, it is not necessary to call before processing | ||
| /// the first stream after creation. | ||
| pub fn initialize(&mut self) { | ||
| self.inner.initialize() |
There was a problem hiding this comment.
Nice, I guess the idea is we could use this from our portal code?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR exposes some internals of the
EchoCanceller3.It's not as clean as I hope, as
EchoCanceller3isn't part of the public API of the underlying library, whereasEchoCanceller3Configis, and some C/C++ dance was necessary to make the build happy. I will later look into forking the upstream repo and more properly exposingEchoCanceller3.EchoCanceller3Confighas so many parameters, and none of them are documented unfortunately. I've no idea which parameters are useful exposing and tweaking, so I just randomly started withnum_filters, which I assumed is the number of linear regression filters.