GNOME Bugzilla – Bug 768009
webrtcdsp: Stops cancelling after several spaced packet lost
Last modified: 2016-10-31 14:26:24 UTC
I just started testing the canceller in real life situation and found that the resync code only work for large lost, but for small and sporadic lost, it get confused (or don't detect the discont). This is a combination of bugs here, the discont detection works with 10ms buffer, but fail otherwise. This is specially visible when we move from mulaw to opus (opus by default will aggregate to 20ms). And it's even worst if you use an odd buffer size, like 11ms. Another issue, is that each time we resync, it changes the delay slightly, and it take a small about of time for the canceller to lock again. This leaks a bit of echo and is annoying. For the reverse data, I think the best approach is to fill the losts with silence. After all, silence is exactly what we are going to play on the speaker. Now, this is a bit tricky to implement. As of now, we moved to using the delay setting, that means we consume the reverse data immediately at start. On life pipeline, that keeps the probe adapter level near to empty. When a lost packet occure, the adapter would contain less then 10ms. When the probe data is not aligned to 10ms, that means we may consume let's say 2ms and fill with silence. if the lost was 5ms, it means we have 2ms of valid echo that we are going to drop, which will leak a bit of echo. As the echo probe is always ahead of the recording (there is always latency between playing to speaker and recording this echo in life cases), I would suggest to delay by 10ms the moment we start consuming from the probe adapter. This will still produce good buffering. Another possibility is to use the documented filter length. It's 128ms, As we found, matching with no delay didn't work so well, cause it may be slightly miss-aligned in the opposite direction, though we could find a compromise. A quick way to test: pulsesrc latency-time=xx ! webrtcdsp ! opusenc ! identity drop-probability=.. ! opusdec ! webrtcechoprobe ! pulsesink For real life, use an udpsink, over the wifi, and a microwave (don't turn it on) to help getting drops.
Created attachment 330621 [details] [review] webrtcdsp: Rewrite echo data synchronization The previous code would run out of sync if there was packet lost or clock skews. When that happened, the echo cancellation feature would completely stop working. As this is crucial for audio calls, this patch re-implement synchronization completely. Instead of letting it drift until next discont, we now synchronize against the record data at every iteration. This way we simply never let the stream drift for longer then 10ms period. We also shorter the delay by using the latency up the probe (basically excluding the sink latency. This is a decent delay to avoid starving in the probe queue.
Review of attachment 330621 [details] [review]: ::: ext/webrtcdsp/gstwebrtcdsp.cpp @@ -316,5 +285,2 @@ GST_OBJECT_UNLOCK (self); - if (!probe) - return; - I think I need to keep this one and return FLOW_OK.
Created attachment 330622 [details] [review] An updated version with the small error I found. It broke the case where you disable the echo canceller.
Committed as 71c9cdeff4b2994616dc6c54f5f5cf3233c8e166