Moved the AGC render sample queue into the audio processing module

Moved the AGC render sample queue into the audio processing module

Several subcomponents inside APM copy render audio from
the render side to the capture side using the same
pattern. Currently this is done independently for the
submodules.

This CL moves the the AGC functionality for this into
APM.

BUG=webrtc:5298, webrtc:6540
Committed: https://crrev.com/701d628f5f392a80436bd53cb118800b7845cb9d
Cr-Commit-Position: refs/heads/master@{#14770}

[peah-webrtc, 2016-10-25 08:22:13]

peah@webrtc.org changed reviewers:
+ ivoc@webrtc.org

[ivoc, 2016-10-25 11:23:54]

LGTM, see one general question below.

https://codereview.webrtc.org/2444283002/diff/1/webrtc/modules/audio_processi...
File webrtc/modules/audio_processing/audio_processing_impl.h (right):

https://codereview.webrtc.org/2444283002/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/audio_processing_impl.h:397:
agc_render_signal_queue_;
It was hoping it would be possible to do more sharing of the swapqueues between
different components. Would it be possible to put the AudioBuffer itself into
the swapqueue instead of the packed samples?

[peah-webrtc, 2016-10-25 11:34:24]

https://codereview.webrtc.org/2444283002/diff/1/webrtc/modules/audio_processi...
File webrtc/modules/audio_processing/audio_processing_impl.h (right):

https://codereview.webrtc.org/2444283002/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/audio_processing_impl.h:397:
agc_render_signal_queue_;
On 2016/10/25 11:23:54, ivoc wrote:
> It was hoping it would be possible to do more sharing of the swapqueues
between
> different components. Would it be possible to put the AudioBuffer itself into
> the swapqueue instead of the packed samples?

I was too hoping for that, but it was a bit less straightforward that I hoped.

I think we should:
1) Land this CL.
2) For any other functionality that need to pass data from the render to the
capture sides, for the time being (until bullet 3) replicate the scheme that is
used for the AEC, AECM and AGC.
3) Change the scheme to pass one copy of the render data and distribute in the
format desired for the submodules.

Step 3 has the advantage that as little data as possible is passed in the
swapqueue. Now, for instance, the AEC passes several copies of the same render
to the swapqueue.

I don't think we should pass the audiobuffer since
1) It contains several copies of the data, float, int16, bandsplit data, etc.
2) It will increase our dependency on the AudioBuffer class.

Regardless of this series of CLs not immediately achieving a unified render
queue, it allows for the submodules to be changed to be lock-free. In
particular, submodules that are not in the public APM interface do not need
locks.

WDYT?

[ivoc, 2016-10-25 11:42:23]

On 2016/10/25 11:34:24, peah-webrtc wrote:
>
https://codereview.webrtc.org/2444283002/diff/1/webrtc/modules/audio_processi...
> File webrtc/modules/audio_processing/audio_processing_impl.h (right):
> 
>
https://codereview.webrtc.org/2444283002/diff/1/webrtc/modules/audio_processi...
> webrtc/modules/audio_processing/audio_processing_impl.h:397:
> agc_render_signal_queue_;
> On 2016/10/25 11:23:54, ivoc wrote:
> > It was hoping it would be possible to do more sharing of the swapqueues
> between
> > different components. Would it be possible to put the AudioBuffer itself
into
> > the swapqueue instead of the packed samples?
> 
> I was too hoping for that, but it was a bit less straightforward that I hoped.
> 
> I think we should:
> 1) Land this CL.
> 2) For any other functionality that need to pass data from the render to the
> capture sides, for the time being (until bullet 3) replicate the scheme that
is
> used for the AEC, AECM and AGC.
> 3) Change the scheme to pass one copy of the render data and distribute in the
> format desired for the submodules.
> 
> Step 3 has the advantage that as little data as possible is passed in the
> swapqueue. Now, for instance, the AEC passes several copies of the same render
> to the swapqueue.
> 
> I don't think we should pass the audiobuffer since
> 1) It contains several copies of the data, float, int16, bandsplit data, etc.
> 2) It will increase our dependency on the AudioBuffer class.
> 
> Regardless of this series of CLs not immediately achieving a unified render
> queue, it allows for the submodules to be changed to be lock-free. In
> particular, submodules that are not in the public APM interface do not need
> locks.
> 
> WDYT?

Ok, sounds good.

[peah-webrtc, 2016-10-25 12:02:38]

The CQ bit was checked by peah@webrtc.org

[peah-webrtc, 2016-10-25 12:02:38]

The patchset sent to the CQ was uploaded after l-g-t-m from ivoc@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2444283002/#ps20001 (title:
"Rebase")

[commit-bot: I haz the power, 2016-10-25 12:02:42]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-10-25 12:42:24]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-25 12:42:31]

Description was changed from

==========
Moved the AGC render sample queue into the audio processing module

Several subcomponents inside APM copy render audio from
the render side to the capture side using the same
pattern. Currently this is done independently for the
submodules.

This CL moves the the AGC functionality for this into
APM.

BUG=webrtc:5298, webrtc:6540
==========

to

==========
Moved the AGC render sample queue into the audio processing module

Several subcomponents inside APM copy render audio from
the render side to the capture side using the same
pattern. Currently this is done independently for the
submodules.

This CL moves the the AGC functionality for this into
APM.

BUG=webrtc:5298, webrtc:6540

Committed: https://crrev.com/701d628f5f392a80436bd53cb118800b7845cb9d
Cr-Commit-Position: refs/heads/master@{#14770}
==========

[commit-bot: I haz the power, 2016-10-25 12:42:32]

Patchset 2 (id:??) landed as
https://crrev.com/701d628f5f392a80436bd53cb118800b7845cb9d
Cr-Commit-Position: refs/heads/master@{#14770}

[haibinhtn, 2017-02-13 18:54:21]

haibinhtn@gmail.com changed reviewers:
+ haibinhtn@gmail.com