Implement client-side video stream support for WebRTC-based protocol.

Implement client-side video stream support for WebRTC-based protocol.

Added WebrtcVideoRenderer adapter that implements
VideoRendererInterface and passes decoded frames to
remoting::protocol::FrameConsumer

BUG=547158
Committed: https://crrev.com/a45416062d1da32e2bb32ba6d952f36d4281594d
Cr-Commit-Position: refs/heads/master@{#370136}

[Sergey Ulanov, 2016-01-12 19:48:57]

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

[Jamie, 2016-01-12 21:00:04]

lgtm

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

https://codereview.chromium.org/1580823003/diff/1/remoting/protocol/webrtc_co...
remoting/protocol/webrtc_connection_to_host.cc:124: stream,
video_renderer_->GetFrameConsumer()));
Does this add an additional stream, or replace the existing one? Maybe add a
LOG(WARNING) if video_adapter_ is not null?

https://codereview.chromium.org/1580823003/diff/1/remoting/protocol/webrtc_vi...
File remoting/protocol/webrtc_video_renderer_adapter.cc (right):

https://codereview.chromium.org/1580823003/diff/1/remoting/protocol/webrtc_vi...
remoting/protocol/webrtc_video_renderer_adapter.cc:40:
video_tracks[0]->AddRenderer(this);
Also LOG(ERROR) if there's more than 1?

https://codereview.chromium.org/1580823003/diff/1/remoting/protocol/webrtc_vi...
remoting/protocol/webrtc_video_renderer_adapter.cc:52: // FrameConsumer
implementations. Fix this somehow.
It doesn't need to be this CL, but why not just bounce to the correct thread?

https://codereview.chromium.org/1580823003/diff/1/remoting/protocol/webrtc_vi...
File remoting/protocol/webrtc_video_stream.cc (left):

https://codereview.chromium.org/1580823003/diff/1/remoting/protocol/webrtc_vi...
remoting/protocol/webrtc_video_stream.cc:44: capturer_adapter_ =
capturer_adapter_->GetWeakPtr();
Unrelated bug-fix? If so, then please mention it in the CL description or move
it to a separate CL.

[Sergey Ulanov, 2016-01-12 22:13:45]

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

https://codereview.chromium.org/1580823003/diff/1/remoting/protocol/webrtc_co...
remoting/protocol/webrtc_connection_to_host.cc:124: stream,
video_renderer_->GetFrameConsumer()));
On 2016/01/12 21:00:04, Jamie wrote:
> Does this add an additional stream, or replace the existing one? Maybe add a
> LOG(WARNING) if video_adapter_ is not null?

Done.

https://codereview.chromium.org/1580823003/diff/1/remoting/protocol/webrtc_vi...
File remoting/protocol/webrtc_video_renderer_adapter.cc (right):

https://codereview.chromium.org/1580823003/diff/1/remoting/protocol/webrtc_vi...
remoting/protocol/webrtc_video_renderer_adapter.cc:40:
video_tracks[0]->AddRenderer(this);
On 2016/01/12 21:00:04, Jamie wrote:
> Also LOG(ERROR) if there's more than 1?

Done.

https://codereview.chromium.org/1580823003/diff/1/remoting/protocol/webrtc_vi...
remoting/protocol/webrtc_video_renderer_adapter.cc:52: // FrameConsumer
implementations. Fix this somehow.
On 2016/01/12 21:00:04, Jamie wrote:
> It doesn't need to be this CL, but why not just bounce to the correct thread?

Because we don't get ownership of the |frame| here, i.e. it would have to be
copied somewhere to bounce to a different thread, which is rather inefficient.
One potential solution is to keep a queue of pre-allocated frames somewhere, but
that would require to know screen size beforehand.

https://codereview.chromium.org/1580823003/diff/1/remoting/protocol/webrtc_vi...
File remoting/protocol/webrtc_video_stream.cc (left):

https://codereview.chromium.org/1580823003/diff/1/remoting/protocol/webrtc_vi...
remoting/protocol/webrtc_video_stream.cc:44: capturer_adapter_ =
capturer_adapter_->GetWeakPtr();
On 2016/01/12 21:00:04, Jamie wrote:
> Unrelated bug-fix? If so, then please mention it in the CL description or move
> it to a separate CL.

Moved it to https://codereview.chromium.org/1578253004/

[Sergey Ulanov, 2016-01-19 16:25:55]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2016-01-19 16:25:56]

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

[commit-bot: I haz the power, 2016-01-19 16:26:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1580823003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1580823003/40001

[commit-bot: I haz the power, 2016-01-19 17:42:14]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-01-19 17:43:15]

Description was changed from

==========
Implement client-side video stream support for WebRTC-based protocol.

Added WebrtcVideoRenderer adapter that implements
VideoRendererInterface and passes decoded frames to
remoting::protocol::FrameConsumer

BUG=547158
==========

to

==========
Implement client-side video stream support for WebRTC-based protocol.

Added WebrtcVideoRenderer adapter that implements
VideoRendererInterface and passes decoded frames to
remoting::protocol::FrameConsumer

BUG=547158

Committed: https://crrev.com/a45416062d1da32e2bb32ba6d952f36d4281594d
Cr-Commit-Position: refs/heads/master@{#370136}
==========

[commit-bot: I haz the power, 2016-01-19 17:43:16]

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