Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2311833002:
Refactoring of the buffering of the nearend signal done inside the AEC (Closed)
|
Created:
4 years, 3 months ago by peah-webrtc Modified:
4 years, 3 months ago Reviewers:
hlundin-webrtc CC:
webrtc-reviews_webrtc.org, peah-webrtc, Andrew MacDonald, tterriberry_mozilla.com, audio-team_agora.io, hlundin-webrtc, kwiberg-webrtc, minyue-webrtc, the sun, aluebs-webrtc, bjornv1 Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionThis CL refactors the buffering of the incoming near-end signal inside
the AEC. This solves the following issues:
-Even though the buffering was previously done using ringbuffers, those
were inefficiently used which caused a lot of hidden memcopys.
-The ringbuffers wasted a lot of space in the AEC state as they were too
long.
-The lowest and two upper bands were decoupled in the buffering, which
required extra code to handle.
-On top of the ringbuffers there was a second linear buffer that was
stored in the state which caused even more data to be stored on the
state.
-The incoming nearend frames were passed to the functions in the form
of buffers on the state, which made the code harder to read as it was
not immediately clear where the nearend signal was used, and when it
was modified.
The CL addresses this by replacing all the buffers by two linear buffers:
-One buffer before the AEC processing for producing nearend
blocks of size 64 that can be processed by the AEC.
-One inside the AEC processing that buffers the current
nearend block until the next block is processed.
The changes have been tested to be bitexact.
This CL will be followed by several other CLs, that refactor the other
buffers in the AEC.
BUG=webrtc:5298, webrtc:6018
Committed: https://crrev.com/906f4030882ad6b7f31658e43a15e6165f3431d0
Cr-Commit-Position: refs/heads/master@{#14141}
Patch Set 1 #Patch Set 2 : Corrected initialization of the nearend buffer #
Total comments: 3
Patch Set 3 : Changes in response to reviewer comments #Patch Set 4 : Rebase #
Dependent Patchsets: Messages
Total messages: 21 (13 generated)
Description was changed from ========== Change AEC buffering BUG= ========== to ========== This CL refactors the buffering of the incoming near-end signal inside the AEC. This solves the following issues: -Even though the buffering was previously done using ringbuffers, those were inefficiently used which caused a lot of hidden memcopys. -The ringbuffers wasted a lot of space in the AEC state as they were too long. -The lowest and two upper bands were decoupled in the buffering, which required extra code to handle. -On top of the ringbuffers there was a second linear buffer that was stored in the state which caused even more data to be stored on the state. -The incoming nearend frames were passed to the functions in the form of buffers on the state, which made the code harder to read as it was not immediately clear where the nearend signal was used, and when it was modified. The CL addresses this by replacing all the buffers by two linear buffers: -One buffer before the AEC processing for producing nearend blocks of size 64 that can be processed by the AEC. -One inside the AEC processing that buffers buffers the current nearend block until the next block is processed. The changes have been tested to be bitexact. BUG=webrtc:5298, webrtc: 6018 ==========
peah@webrtc.org changed reviewers: + henrik.lundin@webrtc.org
Description was changed from ========== This CL refactors the buffering of the incoming near-end signal inside the AEC. This solves the following issues: -Even though the buffering was previously done using ringbuffers, those were inefficiently used which caused a lot of hidden memcopys. -The ringbuffers wasted a lot of space in the AEC state as they were too long. -The lowest and two upper bands were decoupled in the buffering, which required extra code to handle. -On top of the ringbuffers there was a second linear buffer that was stored in the state which caused even more data to be stored on the state. -The incoming nearend frames were passed to the functions in the form of buffers on the state, which made the code harder to read as it was not immediately clear where the nearend signal was used, and when it was modified. The CL addresses this by replacing all the buffers by two linear buffers: -One buffer before the AEC processing for producing nearend blocks of size 64 that can be processed by the AEC. -One inside the AEC processing that buffers buffers the current nearend block until the next block is processed. The changes have been tested to be bitexact. BUG=webrtc:5298, webrtc: 6018 ========== to ========== This CL refactors the buffering of the incoming near-end signal inside the AEC. This solves the following issues: -Even though the buffering was previously done using ringbuffers, those were inefficiently used which caused a lot of hidden memcopys. -The ringbuffers wasted a lot of space in the AEC state as they were too long. -The lowest and two upper bands were decoupled in the buffering, which required extra code to handle. -On top of the ringbuffers there was a second linear buffer that was stored in the state which caused even more data to be stored on the state. -The incoming nearend frames were passed to the functions in the form of buffers on the state, which made the code harder to read as it was not immediately clear where the nearend signal was used, and when it was modified. The CL addresses this by replacing all the buffers by two linear buffers: -One buffer before the AEC processing for producing nearend blocks of size 64 that can be processed by the AEC. -One inside the AEC processing that buffers buffers the current nearend block until the next block is processed. The changes have been tested to be bitexact. BUG=webrtc:5298, webrtc:6018 ==========
Description was changed from ========== This CL refactors the buffering of the incoming near-end signal inside the AEC. This solves the following issues: -Even though the buffering was previously done using ringbuffers, those were inefficiently used which caused a lot of hidden memcopys. -The ringbuffers wasted a lot of space in the AEC state as they were too long. -The lowest and two upper bands were decoupled in the buffering, which required extra code to handle. -On top of the ringbuffers there was a second linear buffer that was stored in the state which caused even more data to be stored on the state. -The incoming nearend frames were passed to the functions in the form of buffers on the state, which made the code harder to read as it was not immediately clear where the nearend signal was used, and when it was modified. The CL addresses this by replacing all the buffers by two linear buffers: -One buffer before the AEC processing for producing nearend blocks of size 64 that can be processed by the AEC. -One inside the AEC processing that buffers buffers the current nearend block until the next block is processed. The changes have been tested to be bitexact. BUG=webrtc:5298, webrtc:6018 ========== to ========== This CL refactors the buffering of the incoming near-end signal inside the AEC. This solves the following issues: -Even though the buffering was previously done using ringbuffers, those were inefficiently used which caused a lot of hidden memcopys. -The ringbuffers wasted a lot of space in the AEC state as they were too long. -The lowest and two upper bands were decoupled in the buffering, which required extra code to handle. -On top of the ringbuffers there was a second linear buffer that was stored in the state which caused even more data to be stored on the state. -The incoming nearend frames were passed to the functions in the form of buffers on the state, which made the code harder to read as it was not immediately clear where the nearend signal was used, and when it was modified. The CL addresses this by replacing all the buffers by two linear buffers: -One buffer before the AEC processing for producing nearend blocks of size 64 that can be processed by the AEC. -One inside the AEC processing that buffers buffers the current nearend block until the next block is processed. The changes have been tested to be bitexact. This CL will be followed by several other CLs, that refactor the other buffers in the AEC. BUG=webrtc:5298, webrtc:6018 ==========
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Description was changed from ========== This CL refactors the buffering of the incoming near-end signal inside the AEC. This solves the following issues: -Even though the buffering was previously done using ringbuffers, those were inefficiently used which caused a lot of hidden memcopys. -The ringbuffers wasted a lot of space in the AEC state as they were too long. -The lowest and two upper bands were decoupled in the buffering, which required extra code to handle. -On top of the ringbuffers there was a second linear buffer that was stored in the state which caused even more data to be stored on the state. -The incoming nearend frames were passed to the functions in the form of buffers on the state, which made the code harder to read as it was not immediately clear where the nearend signal was used, and when it was modified. The CL addresses this by replacing all the buffers by two linear buffers: -One buffer before the AEC processing for producing nearend blocks of size 64 that can be processed by the AEC. -One inside the AEC processing that buffers buffers the current nearend block until the next block is processed. The changes have been tested to be bitexact. This CL will be followed by several other CLs, that refactor the other buffers in the AEC. BUG=webrtc:5298, webrtc:6018 ========== to ========== This CL refactors the buffering of the incoming near-end signal inside the AEC. This solves the following issues: -Even though the buffering was previously done using ringbuffers, those were inefficiently used which caused a lot of hidden memcopys. -The ringbuffers wasted a lot of space in the AEC state as they were too long. -The lowest and two upper bands were decoupled in the buffering, which required extra code to handle. -On top of the ringbuffers there was a second linear buffer that was stored in the state which caused even more data to be stored on the state. -The incoming nearend frames were passed to the functions in the form of buffers on the state, which made the code harder to read as it was not immediately clear where the nearend signal was used, and when it was modified. The CL addresses this by replacing all the buffers by two linear buffers: -One buffer before the AEC processing for producing nearend blocks of size 64 that can be processed by the AEC. -One inside the AEC processing that buffers the current nearend block until the next block is processed. The changes have been tested to be bitexact. This CL will be followed by several other CLs, that refactor the other buffers in the AEC. BUG=webrtc:5298, webrtc:6018 ==========
LGTM with nits. https://codereview.webrtc.org/2311833002/diff/60001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/aec/aec_core.cc (right): https://codereview.webrtc.org/2311833002/diff/60001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.cc:1830: // Form an process a block of samples. Typo: an -> a https://codereview.webrtc.org/2311833002/diff/60001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/aec/aec_core.h (right): https://codereview.webrtc.org/2311833002/diff/60001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.h:132: [PART_LEN - (FRAME_LEN - PART_LEN)]; Please, explain the sizes.
https://codereview.webrtc.org/2311833002/diff/60001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/aec/aec_core.h (right): https://codereview.webrtc.org/2311833002/diff/60001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.h:132: [PART_LEN - (FRAME_LEN - PART_LEN)]; On 2016/09/07 21:28:46, hlundin-webrtc wrote: > Please, explain the sizes. Done.
The CQ bit was checked by peah@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from henrik.lundin@webrtc.org Link to the patchset: https://codereview.webrtc.org/2311833002/#ps100001 (title: "Rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_arm64_rel on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL) android_dbg on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by peah@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
Message was sent while issue was closed.
Description was changed from ========== This CL refactors the buffering of the incoming near-end signal inside the AEC. This solves the following issues: -Even though the buffering was previously done using ringbuffers, those were inefficiently used which caused a lot of hidden memcopys. -The ringbuffers wasted a lot of space in the AEC state as they were too long. -The lowest and two upper bands were decoupled in the buffering, which required extra code to handle. -On top of the ringbuffers there was a second linear buffer that was stored in the state which caused even more data to be stored on the state. -The incoming nearend frames were passed to the functions in the form of buffers on the state, which made the code harder to read as it was not immediately clear where the nearend signal was used, and when it was modified. The CL addresses this by replacing all the buffers by two linear buffers: -One buffer before the AEC processing for producing nearend blocks of size 64 that can be processed by the AEC. -One inside the AEC processing that buffers the current nearend block until the next block is processed. The changes have been tested to be bitexact. This CL will be followed by several other CLs, that refactor the other buffers in the AEC. BUG=webrtc:5298, webrtc:6018 ========== to ========== This CL refactors the buffering of the incoming near-end signal inside the AEC. This solves the following issues: -Even though the buffering was previously done using ringbuffers, those were inefficiently used which caused a lot of hidden memcopys. -The ringbuffers wasted a lot of space in the AEC state as they were too long. -The lowest and two upper bands were decoupled in the buffering, which required extra code to handle. -On top of the ringbuffers there was a second linear buffer that was stored in the state which caused even more data to be stored on the state. -The incoming nearend frames were passed to the functions in the form of buffers on the state, which made the code harder to read as it was not immediately clear where the nearend signal was used, and when it was modified. The CL addresses this by replacing all the buffers by two linear buffers: -One buffer before the AEC processing for producing nearend blocks of size 64 that can be processed by the AEC. -One inside the AEC processing that buffers the current nearend block until the next block is processed. The changes have been tested to be bitexact. This CL will be followed by several other CLs, that refactor the other buffers in the AEC. BUG=webrtc:5298, webrtc:6018 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:100001)
Message was sent while issue was closed.
Description was changed from ========== This CL refactors the buffering of the incoming near-end signal inside the AEC. This solves the following issues: -Even though the buffering was previously done using ringbuffers, those were inefficiently used which caused a lot of hidden memcopys. -The ringbuffers wasted a lot of space in the AEC state as they were too long. -The lowest and two upper bands were decoupled in the buffering, which required extra code to handle. -On top of the ringbuffers there was a second linear buffer that was stored in the state which caused even more data to be stored on the state. -The incoming nearend frames were passed to the functions in the form of buffers on the state, which made the code harder to read as it was not immediately clear where the nearend signal was used, and when it was modified. The CL addresses this by replacing all the buffers by two linear buffers: -One buffer before the AEC processing for producing nearend blocks of size 64 that can be processed by the AEC. -One inside the AEC processing that buffers the current nearend block until the next block is processed. The changes have been tested to be bitexact. This CL will be followed by several other CLs, that refactor the other buffers in the AEC. BUG=webrtc:5298, webrtc:6018 ========== to ========== This CL refactors the buffering of the incoming near-end signal inside the AEC. This solves the following issues: -Even though the buffering was previously done using ringbuffers, those were inefficiently used which caused a lot of hidden memcopys. -The ringbuffers wasted a lot of space in the AEC state as they were too long. -The lowest and two upper bands were decoupled in the buffering, which required extra code to handle. -On top of the ringbuffers there was a second linear buffer that was stored in the state which caused even more data to be stored on the state. -The incoming nearend frames were passed to the functions in the form of buffers on the state, which made the code harder to read as it was not immediately clear where the nearend signal was used, and when it was modified. The CL addresses this by replacing all the buffers by two linear buffers: -One buffer before the AEC processing for producing nearend blocks of size 64 that can be processed by the AEC. -One inside the AEC processing that buffers the current nearend block until the next block is processed. The changes have been tested to be bitexact. This CL will be followed by several other CLs, that refactor the other buffers in the AEC. BUG=webrtc:5298, webrtc:6018 Committed: https://crrev.com/906f4030882ad6b7f31658e43a15e6165f3431d0 Cr-Commit-Position: refs/heads/master@{#14141} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/906f4030882ad6b7f31658e43a15e6165f3431d0 Cr-Commit-Position: refs/heads/master@{#14141} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
