Add WebrtcVideoEncoder interface

Add WebrtcVideoEncoder interface

Previous WebrtcVideoEncoderVpx was implementing VideoEncoder interface.
That interface depends on VideoPacket, which is not applicable for
WebRTC case. Added a separate interface that will be used for WebRTC
protocol. The new interface allows to set bandwidth and frame duration
for each frame individually.
Also renamed class previously called WebrtcVideoEncoder to
WebrtcDummyVideoEncoder.

BUG=645656
Committed: https://crrev.com/2becb601192f9f6912a982e39e04e3fa0a6a776a
Cr-Commit-Position: refs/heads/master@{#418314}

[Sergey Ulanov, 2016-09-09 22:50:25]

sergeyu@chromium.org changed reviewers:
+ isheriff@chromium.org, jamiewalch@chromium.org

[Irfan, 2016-09-12 21:34:24]

Did you miss adding WebRtcVideoEncoder related files in the commit ?

https://codereview.chromium.org/2329653002/diff/1/remoting/codec/webrtc_video...
File remoting/codec/webrtc_video_encoder_vpx.cc (right):

https://codereview.chromium.org/2329653002/diff/1/remoting/codec/webrtc_video...
remoting/codec/webrtc_video_encoder_vpx.cc:464: config_.rc_target_bitrate =
new_bitrate_kbps;
I am assuming this method was just moved down, was hard to do a diff visually

https://codereview.chromium.org/2329653002/diff/1/remoting/protocol/webrtc_co...
File remoting/protocol/webrtc_connection_to_client.cc (right):

https://codereview.chromium.org/2329653002/diff/1/remoting/protocol/webrtc_co...
remoting/protocol/webrtc_connection_to_client.cc:80: // TODO(isheriff): make
this codec independent
comment goes away ?

https://codereview.chromium.org/2329653002/diff/1/remoting/protocol/webrtc_du...
File remoting/protocol/webrtc_dummy_video_encoder.h (right):

https://codereview.chromium.org/2329653002/diff/1/remoting/protocol/webrtc_du...
remoting/protocol/webrtc_dummy_video_encoder.h:25: // through the capture
pipeline.
Perhaps add comment on why these classes are "dummy" and what they do (serve
primarily as the communication interface from app to webrtc)

[Sergey Ulanov, 2016-09-12 22:50:02]

> Did you miss adding WebRtcVideoEncoder related files in the commit ?

Yes. Added it now.

https://codereview.chromium.org/2329653002/diff/1/remoting/codec/webrtc_video...
File remoting/codec/webrtc_video_encoder_vpx.cc (right):

https://codereview.chromium.org/2329653002/diff/1/remoting/codec/webrtc_video...
remoting/codec/webrtc_video_encoder_vpx.cc:464: config_.rc_target_bitrate =
new_bitrate_kbps;
On 2016/09/12 21:34:24, Irfan wrote:
> I am assuming this method was just moved down, was hard to do a diff visually

Yes. It's private now, that's why I moved it.

https://codereview.chromium.org/2329653002/diff/1/remoting/protocol/webrtc_co...
File remoting/protocol/webrtc_connection_to_client.cc (right):

https://codereview.chromium.org/2329653002/diff/1/remoting/protocol/webrtc_co...
remoting/protocol/webrtc_connection_to_client.cc:80: // TODO(isheriff): make
this codec independent
On 2016/09/12 21:34:24, Irfan wrote:
> comment goes away ?

Moved to WebrtcVideoStream::Start().

https://codereview.chromium.org/2329653002/diff/1/remoting/protocol/webrtc_du...
File remoting/protocol/webrtc_dummy_video_encoder.h (right):

https://codereview.chromium.org/2329653002/diff/1/remoting/protocol/webrtc_du...
remoting/protocol/webrtc_dummy_video_encoder.h:25: // through the capture
pipeline.
On 2016/09/12 21:34:24, Irfan wrote:
> Perhaps add comment on why these classes are "dummy" and what they do (serve
> primarily as the communication interface from app to webrtc)

Done.

[Irfan, 2016-09-12 22:55:30]

lgtm

https://codereview.chromium.org/2329653002/diff/20001/remoting/codec/webrtc_v...
File remoting/codec/webrtc_video_encoder.h (right):

https://codereview.chromium.org/2329653002/diff/20001/remoting/codec/webrtc_v...
remoting/codec/webrtc_video_encoder.h:28: bool clear_active_map = false;
It seems some of these are not used anywhere ? Can we add them when we need ?

[Sergey Ulanov, 2016-09-12 23:05:23]

https://codereview.chromium.org/2329653002/diff/20001/remoting/codec/webrtc_v...
File remoting/codec/webrtc_video_encoder.h (right):

https://codereview.chromium.org/2329653002/diff/20001/remoting/codec/webrtc_v...
remoting/codec/webrtc_video_encoder.h:28: bool clear_active_map = false;
On 2016/09/12 22:55:30, Irfan wrote:
> It seems some of these are not used anywhere ? Can we add them when we need ?

Yes. Moved these fields to the follow-up CL where I need these fields:
https://codereview.chromium.org/2335923002/

[Sergey Ulanov, 2016-09-12 23:05:47]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2016-09-12 23:05:48]

The patchset sent to the CQ was uploaded after l-g-t-m from
isheriff@chromium.org
Link to the patchset: https://codereview.chromium.org/2329653002/#ps40001
(title: ".")

[commit-bot: I haz the power, 2016-09-12 23:06:20]

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

[commit-bot: I haz the power, 2016-09-12 23:09:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-12 23:09:17]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Sergey Ulanov, 2016-09-13 04:42:03]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2016-09-13 04:42:03]

The patchset sent to the CQ was uploaded after l-g-t-m from
isheriff@chromium.org
Link to the patchset: https://codereview.chromium.org/2329653002/#ps60001
(title: "sync")

[commit-bot: I haz the power, 2016-09-13 04:42:14]

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

[commit-bot: I haz the power, 2016-09-13 05:22:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-13 05:22:30]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Sergey Ulanov, 2016-09-13 17:40:53]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2016-09-13 17:40:54]

The patchset sent to the CQ was uploaded after l-g-t-m from
isheriff@chromium.org
Link to the patchset: https://codereview.chromium.org/2329653002/#ps80001
(title: "win")

[commit-bot: I haz the power, 2016-09-13 17:41:30]

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

[commit-bot: I haz the power, 2016-09-13 18:41:52]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-09-13 18:43:16]

Description was changed from

==========
Add WebrtcVideoEncoder interface

Previous WebrtcVideoEncoderVpx was implementing VideoEncoder interface.
That interface depends on VideoPacket, which is not applicable for
WebRTC case. Added a separate interface that will be used for WebRTC
protocol. The new interface allows to set bandwidth and frame duration
for each frame individually.
Also renamed class previously called WebrtcVideoEncoder to
WebrtcDummyVideoEncoder.

BUG=645656
==========

to

==========
Add WebrtcVideoEncoder interface

Previous WebrtcVideoEncoderVpx was implementing VideoEncoder interface.
That interface depends on VideoPacket, which is not applicable for
WebRTC case. Added a separate interface that will be used for WebRTC
protocol. The new interface allows to set bandwidth and frame duration
for each frame individually.
Also renamed class previously called WebrtcVideoEncoder to
WebrtcDummyVideoEncoder.

BUG=645656

Committed: https://crrev.com/2becb601192f9f6912a982e39e04e3fa0a6a776a
Cr-Commit-Position: refs/heads/master@{#418314}
==========

[commit-bot: I haz the power, 2016-09-13 18:43:17]

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