Replace AudioScheduler with AudioPump.

Replace AudioScheduler with AudioPump.

This is similar to the VideoScheduler refactoring. AudioPump now is not
ref-counted, but otherwise works the same as AudioScheduler.

Also simplified AudioCapturer interface.

Committed: https://crrev.com/7bc87603f0d16d150a1759b7196c1e65f54f8fd6
Cr-Commit-Position: refs/heads/master@{#316277}

[Sergey Ulanov, 2015-02-06 00:39:30]

Patchset #1 (id:1) has been deleted

[Sergey Ulanov, 2015-02-06 00:40:01]

sergeyu@chromium.org changed reviewers:
+ wez@chromium.org

[Sergey Ulanov, 2015-02-10 19:39:18]

ping

[Wez, 2015-02-11 01:41:13]

https://codereview.chromium.org/866863004/diff/20001/remoting/host/audio_capt...
File remoting/host/audio_capturer_win.cc (right):

https://codereview.chromium.org/866863004/diff/20001/remoting/host/audio_capt...
remoting/host/audio_capturer_win.cc:47: AudioCapturerWin::~AudioCapturerWin() {
Add thread-check here, since you no longer have callers calling Stop()?

https://codereview.chromium.org/866863004/diff/20001/remoting/host/audio_pump.cc
File remoting/host/audio_pump.cc (right):

https://codereview.chromium.org/866863004/diff/20001/remoting/host/audio_pump...
remoting/host/audio_pump.cc:36: scoped_refptr<base::SingleThreadTaskRunner>
network_task_runner_;
See elsewhere re "network" naming

https://codereview.chromium.org/866863004/diff/20001/remoting/host/audio_pump...
remoting/host/audio_pump.cc:80: DCHECK(packet.get());
DCHECK(packet)

https://codereview.chromium.org/866863004/diff/20001/remoting/host/audio_pump...
remoting/host/audio_pump.cc:117: AudioPump::~AudioPump() {
Thread check network_task_runner_?

https://codereview.chromium.org/866863004/diff/20001/remoting/host/audio_pump...
remoting/host/audio_pump.cc:122: DCHECK(CalledOnValidThread());
Does this need to be both BaseNonThreadSafe and hold a pointer to its calling
TaskRunner (network_task_runner_)?

https://codereview.chromium.org/866863004/diff/20001/remoting/host/audio_pump...
remoting/host/audio_pump.cc:131: DCHECK(packet.get());
nit: DCHECK(packet)

https://codereview.chromium.org/866863004/diff/20001/remoting/host/audio_pump...
remoting/host/audio_pump.cc:133: audio_stub_->ProcessAudioPacket(packet.Pass(),
base::Closure());
nit: Do we ever use the done callback in ProcessAudioPacket, or can we get rid
of that as well...?

[Sergey Ulanov, 2015-02-11 21:18:12]

https://codereview.chromium.org/866863004/diff/20001/remoting/host/audio_capt...
File remoting/host/audio_capturer_win.cc (right):

https://codereview.chromium.org/866863004/diff/20001/remoting/host/audio_capt...
remoting/host/audio_capturer_win.cc:47: AudioCapturerWin::~AudioCapturerWin() {
On 2015/02/11 01:41:12, Wez wrote:
> Add thread-check here, since you no longer have callers calling Stop()?

Done.

https://codereview.chromium.org/866863004/diff/20001/remoting/host/audio_pump.cc
File remoting/host/audio_pump.cc (right):

https://codereview.chromium.org/866863004/diff/20001/remoting/host/audio_pump...
remoting/host/audio_pump.cc:36: scoped_refptr<base::SingleThreadTaskRunner>
network_task_runner_;
On 2015/02/11 01:41:12, Wez wrote:
> See elsewhere re "network" naming

Done.

https://codereview.chromium.org/866863004/diff/20001/remoting/host/audio_pump...
remoting/host/audio_pump.cc:80: DCHECK(packet.get());
On 2015/02/11 01:41:13, Wez wrote:
> DCHECK(packet)

Done.

https://codereview.chromium.org/866863004/diff/20001/remoting/host/audio_pump...
remoting/host/audio_pump.cc:117: AudioPump::~AudioPump() {
On 2015/02/11 01:41:13, Wez wrote:
> Thread check network_task_runner_?

Done.

https://codereview.chromium.org/866863004/diff/20001/remoting/host/audio_pump...
remoting/host/audio_pump.cc:122: DCHECK(CalledOnValidThread());
On 2015/02/11 01:41:12, Wez wrote:
> Does this need to be both BaseNonThreadSafe and hold a pointer to its calling
> TaskRunner (network_task_runner_)?

Done.

https://codereview.chromium.org/866863004/diff/20001/remoting/host/audio_pump...
remoting/host/audio_pump.cc:131: DCHECK(packet.get());
On 2015/02/11 01:41:12, Wez wrote:
> nit: DCHECK(packet)

Done.

https://codereview.chromium.org/866863004/diff/20001/remoting/host/audio_pump...
remoting/host/audio_pump.cc:133: audio_stub_->ProcessAudioPacket(packet.Pass(),
base::Closure());
On 2015/02/11 01:41:12, Wez wrote:
> nit: Do we ever use the done callback in ProcessAudioPacket, or can we get rid
> of that as well...?

Actually I think it's wrong that we are not using it here. The pump currently
pumps the data even when the network is blocked. I'll send you a separate CL to
fix it.

[Wez, 2015-02-12 02:26:18]

lgtm

https://codereview.chromium.org/866863004/diff/20001/remoting/host/audio_pump.cc
File remoting/host/audio_pump.cc (right):

https://codereview.chromium.org/866863004/diff/20001/remoting/host/audio_pump...
remoting/host/audio_pump.cc:133: audio_stub_->ProcessAudioPacket(packet.Pass(),
base::Closure());
On 2015/02/11 21:18:12, Sergey Ulanov wrote:
> On 2015/02/11 01:41:12, Wez wrote:
> > nit: Do we ever use the done callback in ProcessAudioPacket, or can we get
rid
> > of that as well...?
> 
> Actually I think it's wrong that we are not using it here. The pump currently
> pumps the data even when the network is blocked. I'll send you a separate CL
to
> fix it.

Acknowledged.

https://codereview.chromium.org/866863004/diff/40001/remoting/host/audio_pump.cc
File remoting/host/audio_pump.cc (right):

https://codereview.chromium.org/866863004/diff/40001/remoting/host/audio_pump...
remoting/host/audio_pump.cc:34: scoped_refptr<base::SingleThreadTaskRunner>
caller_task_runner_;
nit: pump_task_runner_, since it's got to be the task runner that |pump_| is
valid on?

https://codereview.chromium.org/866863004/diff/40001/remoting/host/client_ses...
File remoting/host/client_session.cc (right):

https://codereview.chromium.org/866863004/diff/40001/remoting/host/client_ses...
remoting/host/client_session.cc:486: // When using IpcDesktopCapturer the
capture thread is not useful.
nit: Got a bug filed for that change?

[Sergey Ulanov, 2015-02-13 17:40:12]

New patchsets have been uploaded after l-g-t-m from wez@chromium.org

[Sergey Ulanov, 2015-02-13 17:40:24]

https://codereview.chromium.org/866863004/diff/40001/remoting/host/audio_pump.cc
File remoting/host/audio_pump.cc (right):

https://codereview.chromium.org/866863004/diff/40001/remoting/host/audio_pump...
remoting/host/audio_pump.cc:34: scoped_refptr<base::SingleThreadTaskRunner>
caller_task_runner_;
On 2015/02/12 02:26:18, Wez wrote:
> nit: pump_task_runner_, since it's got to be the task runner that |pump_| is
> valid on?

Done.

https://codereview.chromium.org/866863004/diff/40001/remoting/host/client_ses...
File remoting/host/client_session.cc (right):

https://codereview.chromium.org/866863004/diff/40001/remoting/host/client_ses...
remoting/host/client_session.cc:486: // When using IpcDesktopCapturer the
capture thread is not useful.
On 2015/02/12 02:26:18, Wez wrote:
> nit: Got a bug filed for that change?

Yes.

[Sergey Ulanov, 2015-02-13 18:46:28]

The CQ bit was checked by sergeyu@chromium.org

[commit-bot: I haz the power, 2015-02-13 18:47:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/866863004/60001

[commit-bot: I haz the power, 2015-02-13 20:33:47]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-02-13 20:34:33]

Patchset 3 (id:??) landed as
https://crrev.com/7bc87603f0d16d150a1759b7196c1e65f54f8fd6
Cr-Commit-Position: refs/heads/master@{#316277}