Make WebRtcVideoChannel2::WebRtcVideoSendStream implement rtc::VideoRendererInterface.

Make WebRtcVideoChannel2::WebRtcVideoSendStream implement rtc::VideoRendererInterface.

BUG=webrtc:5426

[nisse-webrtc, 2016-01-25 15:33:51]

nisse@webrtc.org changed reviewers:
+ perkj@webrtc.org, pthatcher@webrtc.org

[nisse-webrtc, 2016-01-25 15:33:52]

This is an initial step, converting VideoTrack to use the new
VideoSinkInterface.

[nisse-webrtc, 2016-01-26 10:56:47]

nisse@webrtc.org changed reviewers:
+ pbos@webrtc.org

[nisse-webrtc, 2016-01-26 10:56:48]

New version, implementing what was earlier referred to as "alternative C". I.e.,
let WebRtcVideoChannel2::WebRtcVideoSendStream implement the sink interface, and
have VideoRtpSender register it with the VideoTrack.

To make progress, I'd need a review of the basic organization. If we agree that
is sound, I'll need to examine the unit tests which are currently disabled, and
make them work with the new organization. And fix other failures, e.g,
peerconnection_unittests aren't working.

Landing https://codereview.webrtc.org/1594973006/ in some form would also help,
of course.

[perkj_webrtc, 2016-01-26 15:40:56]

I think this is a step in the right direction.

https://codereview.webrtc.org/1631703002/diff/60001/talk/app/webrtc/rtpsender.cc
File talk/app/webrtc/rtpsender.cc (right):

https://codereview.webrtc.org/1631703002/diff/60001/talk/app/webrtc/rtpsender...
talk/app/webrtc/rtpsender.cc:345: << "VideoRtpSender::Stop called with a NULL
sink. Too late???";
Is there ever a case when this is fine? If not- I think we should remove the if
(sink) to catch the error.

https://codereview.webrtc.org/1631703002/diff/60001/talk/media/base/mediachan...
File talk/media/base/mediachannel.h (right):

https://codereview.webrtc.org/1631703002/diff/60001/talk/media/base/mediachan...
talk/media/base/mediachannel.h:45: #include
"webrtc/media/base/videosinkinterface.h"
forward declare?

https://codereview.webrtc.org/1631703002/diff/60001/talk/media/webrtc/webrtcv...
File talk/media/webrtc/webrtcvideoengine2.cc (right):

https://codereview.webrtc.org/1631703002/diff/60001/talk/media/webrtc/webrtcv...
talk/media/webrtc/webrtcvideoengine2.cc:1684: // TODO(nisse): Get screencast
flag from video options.
capturer_->IsScreencast() for the time beeing. cash this in SetCapturer.

https://codereview.webrtc.org/1631703002/diff/60001/talk/media/webrtc/webrtcv...
File talk/media/webrtc/webrtcvideoengine2.h (right):

https://codereview.webrtc.org/1631703002/diff/60001/talk/media/webrtc/webrtcv...
talk/media/webrtc/webrtcvideoengine2.h:238: //    public sigslot::has_slots<>,
remove public sigslot::has_slo

https://codereview.webrtc.org/1631703002/diff/60001/talk/media/webrtc/webrtcv...
talk/media/webrtc/webrtcvideoengine2.h:251: // TODO(nisse): Make class final
instead?
remove comment...

https://codereview.webrtc.org/1631703002/diff/60001/talk/media/webrtc/webrtcv...
talk/media/webrtc/webrtcvideoengine2.h:263: void InputFrame(VideoCapturer*
capturer, const VideoFrame* frame);
remove InputFrame

[nisse-webrtc, 2016-01-28 08:24:54]

https://codereview.webrtc.org/1631703002/diff/60001/talk/app/webrtc/rtpsender.cc
File talk/app/webrtc/rtpsender.cc (right):

https://codereview.webrtc.org/1631703002/diff/60001/talk/app/webrtc/rtpsender...
talk/app/webrtc/rtpsender.cc:345: << "VideoRtpSender::Stop called with a NULL
sink. Too late???";
On 2016/01/26 15:40:56, perkj_webrtc wrote:
> Is there ever a case when this is fine? If not- I think we should remove the
if
> (sink) to catch the error.

I think track_->RemoveSink(nullptr) will succeed and do nothing (and I think
that makes some sense). So to catch the error, I think we have to check for it
here. Possibly using RTC_CHECK?

https://codereview.webrtc.org/1631703002/diff/60001/talk/media/base/mediachan...
File talk/media/base/mediachannel.h (right):

https://codereview.webrtc.org/1631703002/diff/60001/talk/media/base/mediachan...
talk/media/base/mediachannel.h:45: #include
"webrtc/media/base/videosinkinterface.h"
On 2016/01/26 15:40:56, perkj_webrtc wrote:
> forward declare?

I know how to forward declare a plain class, but how do I write it when it
involves a template?

https://codereview.webrtc.org/1631703002/diff/60001/talk/media/webrtc/webrtcv...
File talk/media/webrtc/webrtcvideoengine2.cc (right):

https://codereview.webrtc.org/1631703002/diff/60001/talk/media/webrtc/webrtcv...
talk/media/webrtc/webrtcvideoengine2.cc:1684: // TODO(nisse): Get screencast
flag from video options.
On 2016/01/26 15:40:56, perkj_webrtc wrote:
> capturer_->IsScreencast() for the time beeing. cash this in SetCapturer.

I'm adding an is_screencast_ member, set by SetCapturer. It's a bit redundant
with last_dimensions_.is_screencast, but since this is a temporary hack I'm not
trying anything more clever.

https://codereview.webrtc.org/1631703002/diff/60001/talk/media/webrtc/webrtcv...
File talk/media/webrtc/webrtcvideoengine2.h (right):

https://codereview.webrtc.org/1631703002/diff/60001/talk/media/webrtc/webrtcv...
talk/media/webrtc/webrtcvideoengine2.h:238: //    public sigslot::has_slots<>,
On 2016/01/26 15:40:56, perkj_webrtc wrote:
> remove public sigslot::has_slo

Done.

https://codereview.webrtc.org/1631703002/diff/60001/talk/media/webrtc/webrtcv...
talk/media/webrtc/webrtcvideoengine2.h:251: // TODO(nisse): Make class final
instead?
On 2016/01/26 15:40:56, perkj_webrtc wrote:
> remove comment...

Done.

https://codereview.webrtc.org/1631703002/diff/60001/talk/media/webrtc/webrtcv...
talk/media/webrtc/webrtcvideoengine2.h:263: void InputFrame(VideoCapturer*
capturer, const VideoFrame* frame);
On 2016/01/26 15:40:56, perkj_webrtc wrote:
> remove InputFrame

Done.

[perkj_webrtc, 2016-01-28 13:45:16]

https://codereview.webrtc.org/1631703002/diff/80001/talk/app/webrtc/mediastre...
File talk/app/webrtc/mediastreaminterface.h (right):

https://codereview.webrtc.org/1631703002/diff/80001/talk/app/webrtc/mediastre...
talk/app/webrtc/mediastreaminterface.h:151: // track. Tolerates sink == nullptr,
and does nothing.
I think we should try to be strict. Prefer to not tolerate nullptr.

https://codereview.webrtc.org/1631703002/diff/80001/talk/app/webrtc/rtpsender.cc
File talk/app/webrtc/rtpsender.cc (right):

https://codereview.webrtc.org/1631703002/diff/80001/talk/app/webrtc/rtpsender...
talk/app/webrtc/rtpsender.cc:273: 
if sinkptr != null.  DCHECK(sinkptr ==provider_->GetSink(ssrc_) and deregister.

https://codereview.webrtc.org/1631703002/diff/80001/talk/app/webrtc/rtpsender...
talk/app/webrtc/rtpsender.cc:299: source ? source->GetVideoCapturer() :
nullptr);
AddSink here and store the sink in sinkptr. 
When remove dcheck that GetSink returns the same sink.

https://codereview.webrtc.org/1631703002/diff/80001/talk/app/webrtc/rtpsender...
talk/app/webrtc/rtpsender.cc:334: rtc::VideoSinkInterface<cricket::VideoFrame>*
sink =
dito

[nisse-webrtc, 2016-01-29 07:48:29]

https://codereview.webrtc.org/1631703002/diff/80001/talk/app/webrtc/mediastre...
File talk/app/webrtc/mediastreaminterface.h (right):

https://codereview.webrtc.org/1631703002/diff/80001/talk/app/webrtc/mediastre...
talk/app/webrtc/mediastreaminterface.h:151: // track. Tolerates sink == nullptr,
and does nothing.
On 2016/01/28 13:45:16, perkj_webrtc wrote:
> I think we should try to be strict. Prefer to not tolerate nullptr.

Added a RTC_DCHECK in AddSink, and hopefully made callers no pass nullptr.
RemoveSink still allows nullptr.

https://codereview.webrtc.org/1631703002/diff/80001/talk/app/webrtc/rtpsender.cc
File talk/app/webrtc/rtpsender.cc (right):

https://codereview.webrtc.org/1631703002/diff/80001/talk/app/webrtc/rtpsender...
talk/app/webrtc/rtpsender.cc:273: 
On 2016/01/28 13:45:16, perkj_webrtc wrote:
> if sinkptr != null.  DCHECK(sinkptr ==provider_->GetSink(ssrc_) and
deregister.

I'm not sure exactly what you mean, but I did something along those lines. I
also added code to SetSsrc, to unregister and reregister.

[nisse-webrtc, 2016-01-29 14:49:35]

The CQ bit was checked by nisse@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-01-29 14:49:40]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1631703002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1631703002/140001

[commit-bot: I haz the power, 2016-01-29 15:00:36]

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

[commit-bot: I haz the power, 2016-01-29 15:00:37]

Dry run: Try jobs failed on following builders:
  win_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_rel/builds/12413)

[nisse-webrtc, 2016-01-29 15:08:35]

Description was changed from

==========
Make WebRtcVideoChannel2::WebRtcVideoSendStream implement
rtc::VideoRendererInterface.

Depends on https://codereview.webrtc.org/1594973006/

BUG=webrtc:5426
==========

to

==========
Make WebRtcVideoChannel2::WebRtcVideoSendStream implement
rtc::VideoRendererInterface.

BUG=webrtc:5426
==========