Feature to unlink noise suppression for multi-channel inputs#95
Conversation
ee360b4 to
b9e7f24
Compare
b9e7f24 to
e57006b
Compare
4ea4cc5 to
2ae53cb
Compare
|
@strohel All ready for a look! And I think when this gets merged I'd be happy to cut 2.0.4 (unless you think this counts as a feature for a 2.1.0). |
strohel
left a comment
There was a problem hiding this comment.
The approach looks very good, just a bunch of small technical details.
I also see that we haven't been documenting our feature flags properly in README.md - let's do it for this one (feel free to also document other missing flag there, up to you).
da6c993 to
5c9a58b
Compare
d501ab0 to
78d5363
Compare
|
@strohel I believe all your questions have been addressed, PTAL. |
strohel
left a comment
There was a problem hiding this comment.
I've looked deeper and found some deeper problems (kinda), but should be quick to fix. 🙏
0305c7f to
600343e
Compare
|
Changes made to properly measure the RMS, patch with the dry reverse check, properly normalize |
strohel
left a comment
There was a problem hiding this comment.
LGTM! I'll go ahead and cut a path release with this.
|
@jacksongoode ah one think I've realized post-merge: if you enable the feature, build, disable the feature and build again, it stays enabled: Not a blocker for tagging, though it is suboptimal. At this point I think we should copy the source and only patch the copy. |
Yeah, that's expected. It's again a case of the caching behavior. I suppose you mean copy the source to patch outside of the standard build cache location? |
|
Right, though I wouldn't call it "caching". We modify the file in-place inside the git submodule. Another problem I see is that we leave
The build directory is under I got nerd sniped and working on a PR. |
Resolves the problems described in #95 (comment)
…patching (#96) * Copy C++ webrtc-audio-processing sources before building/patching Resolves the problems described in #95 (comment) * Portable Linux <-> Mac cp invocation
This alters a behavior that would have otherwise caused a quiet/noisy channel to apply noise suppression to a signal channel - current theory being this behavior preserves the stereo image. It adds a new feature
experimental-unlink-nsthat patches the problematic filter aggregation & cross-channel application.If the intention in a multi-channel input is to capture, as an example, a stereo image from two mics, this feature might cause distortion in the representation as each channel will have its own noise filter applied.
However for non-linked microphones, dynamic gain levels can disrupt the performance of NS on a mic, therefore we make the offender (the Wiener filter aggregation) act on each channel individually.
The fix points to https://gitlab.freedesktop.org/jacksongoode/webrtc-audio-processing/-/commit/29ac3aab4df478b3f06698578c3a66f2ed2ac41e