New rtc::VideoSinkInterface.

New rtc::VideoSinkInterface.

The plan is that this interface should be used by all classes which receive a stream of video frames, and replace the two generic classes webrtc::VideoRendererInterface and cricket::VideoRenderer.

And the list goes on, there's a dozen of different classes which act as video frame sinks.

At some point, we will likely add some method to handle sink properties like, e.g, maximum useful width and height. But hopefully this can be done while keeping the interface very simple.

BUG=webrtc:5426
R=perkj@webrtc.org, pthatcher@webrtc.org

Committed: https://crrev.com/a862d4563fbc26e52bed442de784094ae1dfe5ee
Cr-Commit-Position: refs/heads/master@{#11396}

Committed: https://crrev.com/e73afbaf1734043bba2b47b6afa30837dbc4a7f1
Cr-Commit-Position: refs/heads/master@{#11414}

[nisse-webrtc, 2016-01-19 13:21:59]

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

[nisse-webrtc, 2016-01-19 13:21:59]

This defines a new rtc::VideoSinkInterface. It's the first time I try to add a
new file, so please have a look also at the boilerplate. Do I need to edit any
build files when adding a new header file?

Not sure how large pieces we want to get this work landed. Just committing the
interface declaration is perhaps a little pointless. A next step could be to
completely replace webrtc::VideoRendererInterface using the new interface.

It's possible that adding this new file interferes with moving other stuff to
webrtc/media/base, we probably need to coordinate.

[pbos-webrtc, 2016-01-19 13:58:24]

https://codereview.webrtc.org/1594973006/diff/1/webrtc/media/base/videosinkin...
File webrtc/media/base/videosinkinterface.h (right):

https://codereview.webrtc.org/1594973006/diff/1/webrtc/media/base/videosinkin...
webrtc/media/base/videosinkinterface.h:16: template <typename VideoFrameT>
Do we need this to be a template? We'll just use these for cricket::VideoFrame
until we merge the two types right?

[nisse-webrtc, 2016-01-19 14:12:18]

https://codereview.webrtc.org/1594973006/diff/1/webrtc/media/base/videosinkin...
File webrtc/media/base/videosinkinterface.h (right):

https://codereview.webrtc.org/1594973006/diff/1/webrtc/media/base/videosinkin...
webrtc/media/base/videosinkinterface.h:16: template <typename VideoFrameT>
On 2016/01/19 13:58:24, pbos-webrtc wrote:
> Do we need this to be a template? We'll just use these for cricket::VideoFrame
> until we merge the two types right?

I think some of the sink-like interfaces we want to replace use
webrtc::VideoFrame. So I guess it depends in whether or not we think the frame
types can be merged soon...

[pbos-webrtc, 2016-01-19 14:14:55]

On 2016/01/19 14:12:18, nisse-webrtc wrote:
>
https://codereview.webrtc.org/1594973006/diff/1/webrtc/media/base/videosinkin...
> File webrtc/media/base/videosinkinterface.h (right):
> 
>
https://codereview.webrtc.org/1594973006/diff/1/webrtc/media/base/videosinkin...
> webrtc/media/base/videosinkinterface.h:16: template <typename VideoFrameT>
> On 2016/01/19 13:58:24, pbos-webrtc wrote:
> > Do we need this to be a template? We'll just use these for
cricket::VideoFrame
> > until we merge the two types right?
> 
> I think some of the sink-like interfaces we want to replace use
> webrtc::VideoFrame. So I guess it depends in whether or not we think the frame
> types can be merged soon...

Where? I still think we have to deliver frames to WebRtcVideoSendStream for
quite a while (since they trigger reconfigures). Are there any others you think
of?

[nisse-webrtc, 2016-01-19 14:35:27]

On 2016/01/19 14:14:55, pbos-webrtc wrote:
> On 2016/01/19 14:12:18, nisse-webrtc wrote:
> >
>
https://codereview.webrtc.org/1594973006/diff/1/webrtc/media/base/videosinkin...
> > File webrtc/media/base/videosinkinterface.h (right):
> > 
> >
>
https://codereview.webrtc.org/1594973006/diff/1/webrtc/media/base/videosinkin...
> > webrtc/media/base/videosinkinterface.h:16: template <typename VideoFrameT>
> > On 2016/01/19 13:58:24, pbos-webrtc wrote:
> > > Do we need this to be a template? We'll just use these for
> cricket::VideoFrame
> > > until we merge the two types right?
> > 
> > I think some of the sink-like interfaces we want to replace use
> > webrtc::VideoFrame. So I guess it depends in whether or not we think the
frame
> > types can be merged soon...
> 
> Where? I still think we have to deliver frames to WebRtcVideoSendStream for
> quite a while (since they trigger reconfigures). Are there any others you
think
> of?

If I do a search for interfaces that look like video frame sinks, and use
webrtc::VideoFrame, I find at least

  I420FrameCallback::FrameCallback
  webrtc::VideoRenderer::RenderFrame
  IncomingVideoStream::DeliverFrame
  VideoRenderFrames::AddFrame
  I420Encoder::Encode (some additional arguments, though)
  VideoCaptureInput::IncomingCapturedFrame

I can't really say what's best, if it's a step forward to convert some or all of
them to use rtc::VideoFrameSinkInterface<webrtc::VideoFrame>, or if we should
leave them alone until we get to merging the frame classes.

[pbos-webrtc, 2016-01-19 14:41:22]

On 2016/01/19 14:35:27, nisse-webrtc wrote:
> On 2016/01/19 14:14:55, pbos-webrtc wrote:
> > On 2016/01/19 14:12:18, nisse-webrtc wrote:
> > >
> >
>
https://codereview.webrtc.org/1594973006/diff/1/webrtc/media/base/videosinkin...
> > > File webrtc/media/base/videosinkinterface.h (right):
> > > 
> > >
> >
>
https://codereview.webrtc.org/1594973006/diff/1/webrtc/media/base/videosinkin...
> > > webrtc/media/base/videosinkinterface.h:16: template <typename VideoFrameT>
> > > On 2016/01/19 13:58:24, pbos-webrtc wrote:
> > > > Do we need this to be a template? We'll just use these for
> > cricket::VideoFrame
> > > > until we merge the two types right?
> > > 
> > > I think some of the sink-like interfaces we want to replace use
> > > webrtc::VideoFrame. So I guess it depends in whether or not we think the
> frame
> > > types can be merged soon...
> > 
> > Where? I still think we have to deliver frames to WebRtcVideoSendStream for
> > quite a while (since they trigger reconfigures). Are there any others you
> think
> > of?
> 
> If I do a search for interfaces that look like video frame sinks, and use
> webrtc::VideoFrame, I find at least
> 
>   I420FrameCallback::FrameCallback
>   webrtc::VideoRenderer::RenderFrame
>   IncomingVideoStream::DeliverFrame
>   VideoRenderFrames::AddFrame
>   I420Encoder::Encode (some additional arguments, though)
>   VideoCaptureInput::IncomingCapturedFrame
> 
> I can't really say what's best, if it's a step forward to convert some or all
of
> them to use rtc::VideoFrameSinkInterface<webrtc::VideoFrame>, or if we should
> leave them alone until we get to merging the frame classes.

I'd vote for the latter, and then see if we can get rid of cricket::VideoFrame.
That'll give some cleanup satisfaction when removing cricket::VideoFrame too.

[nisse-webrtc, 2016-01-20 13:23:04]

On 2016/01/19 14:41:22, pbos-webrtc wrote:
> On 2016/01/19 14:35:27, nisse-webrtc wrote:
> > On 2016/01/19 14:14:55, pbos-webrtc wrote:
> > > On 2016/01/19 14:12:18, nisse-webrtc wrote:
> > > >
> > >
> >
>
https://codereview.webrtc.org/1594973006/diff/1/webrtc/media/base/videosinkin...
> > > > File webrtc/media/base/videosinkinterface.h (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.webrtc.org/1594973006/diff/1/webrtc/media/base/videosinkin...
> > > > webrtc/media/base/videosinkinterface.h:16: template <typename
VideoFrameT>
> > > > On 2016/01/19 13:58:24, pbos-webrtc wrote:
> > > > > Do we need this to be a template? We'll just use these for
> > > cricket::VideoFrame
> > > > > until we merge the two types right?
> > > > 
> > > > I think some of the sink-like interfaces we want to replace use
> > > > webrtc::VideoFrame. So I guess it depends in whether or not we think the
> > frame
> > > > types can be merged soon...
> > > 
> > > Where? I still think we have to deliver frames to WebRtcVideoSendStream
for
> > > quite a while (since they trigger reconfigures). Are there any others you
> > think
> > > of?
> > 
> > If I do a search for interfaces that look like video frame sinks, and use
> > webrtc::VideoFrame, I find at least
> > 
> >   I420FrameCallback::FrameCallback
> >   webrtc::VideoRenderer::RenderFrame
> >   IncomingVideoStream::DeliverFrame
> >   VideoRenderFrames::AddFrame
> >   I420Encoder::Encode (some additional arguments, though)
> >   VideoCaptureInput::IncomingCapturedFrame
> > 
> > I can't really say what's best, if it's a step forward to convert some or
all
> of
> > them to use rtc::VideoFrameSinkInterface<webrtc::VideoFrame>, or if we
should
> > leave them alone until we get to merging the frame classes.
> 
> I'd vote for the latter, and then see if we can get rid of
cricket::VideoFrame.
> That'll give some cleanup satisfaction when removing cricket::VideoFrame too.

I'm happy with both dropping the template argument, and dropping the "Interface"
suffix from the name. I like minimalism, and try to avoid abstractions which
aren't immediately useful.

I tried to read up on optional template arguments, and it seems hard to drop the
template argument later. Even if it's possible to use default argument to make
the type optional, but one would still need to write an empty list "<>".

[perkj_webrtc, 2016-01-20 13:43:12]

lgtm

[perkj_webrtc, 2016-01-20 13:43:52]

On 2016/01/20 13:43:12, perkj_webrtc wrote:
> lgtm

I should add, can you please add a comment to the cl description why this is
inteface is added and what it it intended to replace one day.

[pbos-webrtc, 2016-01-20 13:45:32]

On 2016/01/20 13:23:04, nisse-webrtc wrote:
> On 2016/01/19 14:41:22, pbos-webrtc wrote:
> > On 2016/01/19 14:35:27, nisse-webrtc wrote:
> > > On 2016/01/19 14:14:55, pbos-webrtc wrote:
> > > > On 2016/01/19 14:12:18, nisse-webrtc wrote:
> > > > >
> > > >
> > >
> >
>
https://codereview.webrtc.org/1594973006/diff/1/webrtc/media/base/videosinkin...
> > > > > File webrtc/media/base/videosinkinterface.h (right):
> > > > > 
> > > > >
> > > >
> > >
> >
>
https://codereview.webrtc.org/1594973006/diff/1/webrtc/media/base/videosinkin...
> > > > > webrtc/media/base/videosinkinterface.h:16: template <typename
> VideoFrameT>
> > > > > On 2016/01/19 13:58:24, pbos-webrtc wrote:
> > > > > > Do we need this to be a template? We'll just use these for
> > > > cricket::VideoFrame
> > > > > > until we merge the two types right?
> > > > > 
> > > > > I think some of the sink-like interfaces we want to replace use
> > > > > webrtc::VideoFrame. So I guess it depends in whether or not we think
the
> > > frame
> > > > > types can be merged soon...
> > > > 
> > > > Where? I still think we have to deliver frames to WebRtcVideoSendStream
> for
> > > > quite a while (since they trigger reconfigures). Are there any others
you
> > > think
> > > > of?
> > > 
> > > If I do a search for interfaces that look like video frame sinks, and use
> > > webrtc::VideoFrame, I find at least
> > > 
> > >   I420FrameCallback::FrameCallback
> > >   webrtc::VideoRenderer::RenderFrame
> > >   IncomingVideoStream::DeliverFrame
> > >   VideoRenderFrames::AddFrame
> > >   I420Encoder::Encode (some additional arguments, though)
> > >   VideoCaptureInput::IncomingCapturedFrame
> > > 
> > > I can't really say what's best, if it's a step forward to convert some or
> all
> > of
> > > them to use rtc::VideoFrameSinkInterface<webrtc::VideoFrame>, or if we
> should
> > > leave them alone until we get to merging the frame classes.
> > 
> > I'd vote for the latter, and then see if we can get rid of
> cricket::VideoFrame.
> > That'll give some cleanup satisfaction when removing cricket::VideoFrame
too.
> 
> I'm happy with both dropping the template argument, and dropping the
"Interface"
> suffix from the name. I like minimalism, and try to avoid abstractions which
> aren't immediately useful.
> 
> I tried to read up on optional template arguments, and it seems hard to drop
the
> template argument later. Even if it's possible to use default argument to make
> the type optional, but one would still need to write an empty list "<>".

If you're doing this (which I suggest), update filenames and CL desc.

[nisse-webrtc, 2016-01-20 13:59:17]

Description was changed from

==========
New rtc::VideoSinkInterface.

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

to

==========
New rtc::VideoSinkInterface.

The plan is that this interface should be used by all classes which receive a
stream of video frames, and replace the two generic classes
webrtc::VideoRendererInterface and cricket::VideoRenderer.

And the list goes on, there's a dozen of different classes which act as video
frame sinks.

At some point, we will likely add some method to handle sink properties like,
e.g, maximum useful width and height. But hopefully this can be done while
keeping the interface very simple.

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

[nisse-webrtc, 2016-01-21 10:37:48]

What should we aim for to get this landable?

I'm tempted to do as pbos suggests, drop the template argument and the Interface
suffix, since these clutter up the code and are currently not needed.

In case we need a template later, we could reintroduce VideoSinkInterface, and
change the VideoSink definition to

typedef VideoSinkInterface<cricket::VideoFrame> VideoSink;

But I'd like to hear pthatcher's opinion before doing that, so I'll postpone
that until next week.

Next, I plan to try to replace cricket::VideoRenderer in CaptureRenderAdapter
and it's callers
(VideoSource->ChannelManager->CaptureManager->CaptureRenderAdapter). 
The objective of these changes is to be able define a
VideoSourceInterface::AddSink which accepts a sink rather than a
cricket::VideoRenderer, which enables the next step:

Change VideoTrack (including VideoTrackRenderers) to use rtc::VideoSink as the
interface for both input and output. As I understand it VideoTrack is
essentially a mux with an enable-state. Now, CaptureRenderAdapter is also
essentially a mux. So it would make some sense to define a mux class and let
both classes inherit that (but on the other hand, perhaps it's better to leave
CaptureRenderAdapter as is and hope it can be eliminated?).

I think it would be nice to get changes to VideoTrack in, if it's possible to
get their reasonable soon, before I go on with my other line of refactorings,
which is to hook up VideoRtpSender as sink for the VideoTrack.

Question now: What's a suitable set of changes to try to get this cl landed?

[nisse-webrtc, 2016-01-21 12:47:23]

Updated patch set, switching CaptureRenderAdapter to use
rtc::VideoSinkInterface, and all its callers up to VideoSource::AddSink and
RemoveSink.

As far as I find, these are internal interfaces (no matches in chromium code
search). But if needed, we could re-add the old AddVideoRenderer methods for
compatibility during the transition. Since cricket::VideoRenderer implements
rtc::VideoSinkInterface, that's no big deal.

[perkj_webrtc, 2016-01-25 07:45:41]

On 2016/01/21 12:47:23, nisse-webrtc wrote:
> Updated patch set, switching CaptureRenderAdapter to use
> rtc::VideoSinkInterface, and all its callers up to VideoSource::AddSink and
> RemoveSink.
> 
> As far as I find, these are internal interfaces (no matches in chromium code
> search). But if needed, we could re-add the old AddVideoRenderer methods for
> compatibility during the transition. Since cricket::VideoRenderer implements
> rtc::VideoSinkInterface, that's no big deal.

not lgtm - need to review again after latest changes

[nisse-webrtc, 2016-01-25 09:07:12]

On 2016/01/25 07:45:41, perkj_webrtc wrote:

> not lgtm - need to review again after latest changes

About the CaptureRenderAdapter changes: I have no strong feeling about them.
They look quite safe to me; if you agree I think it's nice to do right away. But
I'd also be happy to remove them from this cl and come back to them later.

The reason they're in is that I tried to also convert the VideoTrack *input*,
which is the VideoTrackRenderers class, to use the new interface. And it is
registered with the VideoSource using VideoSource::AddSink, which percolates all
the way down to CaptureRenderAdapter via ChannelManager and CaptureManager.

I don't think conversion of the VideoTrack *output* to the new interface depends
on the CaptureRenderAdapter changes.

[pthatcher1, 2016-01-27 01:11:04]

lgtm

(The comments I left are all nits)

https://codereview.webrtc.org/1594973006/diff/60001/talk/media/base/capturema...
File talk/media/base/capturemanager.h (right):

https://codereview.webrtc.org/1594973006/diff/60001/talk/media/base/capturema...
talk/media/base/capturemanager.h:85: rtc::VideoSinkInterface<VideoFrame>* sink);
Can you just make these void?  You can either return true in
ChannelManager::AddVideoRenderer (the only place that uses it) or change it also
to be void (since the only place that calls ChannelManager::AddVideoRenderer
ignores the return value).

https://codereview.webrtc.org/1594973006/diff/60001/talk/media/base/capturema...
File talk/media/base/capturemanager_unittest.cc (right):

https://codereview.webrtc.org/1594973006/diff/60001/talk/media/base/capturema...
talk/media/base/capturemanager_unittest.cc:213: &video_renderer_));
You can just remove all the expects on the return value of AddVideoSink.

https://codereview.webrtc.org/1594973006/diff/60001/talk/media/base/capturere...
File talk/media/base/capturerenderadapter.cc (right):

https://codereview.webrtc.org/1594973006/diff/60001/talk/media/base/capturere...
talk/media/base/capturerenderadapter.cc:84: ++iter) {
It seems like std::remove would be easier here.

https://codereview.webrtc.org/1594973006/diff/60001/talk/media/base/capturere...
File talk/media/base/capturerenderadapter.h (right):

https://codereview.webrtc.org/1594973006/diff/60001/talk/media/base/capturere...
talk/media/base/capturerenderadapter.h:61: typedef
std::vector<rtc::VideoSinkInterface<VideoFrame>*> VideoSinks;
Can you just remove this typedef?  It looks like it's only used two places, one
of which is a for loop that can use c++11-style for loops to avoid the need.

[nisse-webrtc, 2016-01-27 09:08:51]

https://codereview.webrtc.org/1594973006/diff/60001/talk/media/base/capturema...
File talk/media/base/capturemanager.h (right):

https://codereview.webrtc.org/1594973006/diff/60001/talk/media/base/capturema...
talk/media/base/capturemanager.h:85: rtc::VideoSinkInterface<VideoFrame>* sink);
On 2016/01/27 01:11:03, pthatcher1 wrote:
> Can you just make these void?  You can either return true in
> ChannelManager::AddVideoRenderer (the only place that uses it) or change it
also
> to be void (since the only place that calls ChannelManager::AddVideoRenderer
> ignores the return value).

Done, deleting return values also for the ChannelManager methods.

https://codereview.webrtc.org/1594973006/diff/60001/talk/media/base/capturema...
File talk/media/base/capturemanager_unittest.cc (right):

https://codereview.webrtc.org/1594973006/diff/60001/talk/media/base/capturema...
talk/media/base/capturemanager_unittest.cc:213: &video_renderer_));
On 2016/01/27 01:11:03, pthatcher1 wrote:
> You can just remove all the expects on the return value of AddVideoSink.

Done. I have no choice with a void return value...

https://codereview.webrtc.org/1594973006/diff/60001/talk/media/base/capturere...
File talk/media/base/capturerenderadapter.cc (right):

https://codereview.webrtc.org/1594973006/diff/60001/talk/media/base/capturere...
talk/media/base/capturerenderadapter.cc:84: ++iter) {
On 2016/01/27 01:11:03, pthatcher1 wrote:
> It seems like std::remove would be easier here.

Done now. Tommi suggested the same for a previous cl affecting this class.

https://codereview.webrtc.org/1594973006/diff/60001/talk/media/base/capturere...
File talk/media/base/capturerenderadapter.h (right):

https://codereview.webrtc.org/1594973006/diff/60001/talk/media/base/capturere...
talk/media/base/capturerenderadapter.h:61: typedef
std::vector<rtc::VideoSinkInterface<VideoFrame>*> VideoSinks;
On 2016/01/27 01:11:03, pthatcher1 wrote:
> Can you just remove this typedef?  It looks like it's only used two places,
one
> of which is a for loop that can use c++11-style for loops to avoid the need.

Done.

[perkj_webrtc, 2016-01-27 09:14:25]

lgtm

https://codereview.webrtc.org/1594973006/diff/80001/talk/media/base/capturema...
File talk/media/base/capturemanager.cc (right):

https://codereview.webrtc.org/1594973006/diff/80001/talk/media/base/capturema...
talk/media/base/capturemanager.cc:292: // TODO(nisse): Do we really need to
tolerate NULL inputs?
remove this todo?

https://codereview.webrtc.org/1594973006/diff/80001/talk/media/base/capturema...
File talk/media/base/capturemanager_unittest.cc (right):

https://codereview.webrtc.org/1594973006/diff/80001/talk/media/base/capturema...
talk/media/base/capturemanager_unittest.cc:100:
capture_manager_.AddVideoSink(&video_capturer_, NULL);
nit: use nullptr

[nisse-webrtc, 2016-01-27 09:15:33]

The CQ bit was checked by nisse@webrtc.org

[nisse-webrtc, 2016-01-27 09:15:33]

The patchset sent to the CQ was uploaded after l-g-t-m from pthatcher@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1594973006/#ps80001 (title:
"Addressed nits.")

[commit-bot: I haz the power, 2016-01-27 09:15:39]

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

[commit-bot: I haz the power, 2016-01-27 09:17:15]

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

[commit-bot: I haz the power, 2016-01-27 09:17:16]

Try jobs failed on following builders:
  ios32_sim_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios32_sim_dbg/builds/4789)
  ios_arm64_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/7109)
  ios_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_dbg/builds/12279)
  ios_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_rel/builds/10938)
  mac_asan on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_asan/builds/12058)
  mac_compile_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_compile_dbg/builds/...)
  mac_gn_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_gn_dbg/builds/453)
  mac_gn_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_gn_rel/builds/6690)
  mac_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_rel/builds/12363)

[nisse-webrtc, 2016-01-27 13:17:05]

The CQ bit was checked by nisse@webrtc.org

[nisse-webrtc, 2016-01-27 13:17:05]

The patchset sent to the CQ was uploaded after l-g-t-m from perkj@webrtc.org,
pthatcher@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1594973006/#ps100001 (title:
"Rebased.")

[commit-bot: I haz the power, 2016-01-27 13:17:11]

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

[commit-bot: I haz the power, 2016-01-27 13:20:06]

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

[commit-bot: I haz the power, 2016-01-27 13:20:07]

Try jobs failed on following builders:
  presubmit on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/3105)

[nisse-webrtc, 2016-01-27 13:31:52]

The CQ bit was checked by nisse@webrtc.org

[nisse-webrtc, 2016-01-27 13:31:52]

The patchset sent to the CQ was uploaded after l-g-t-m from perkj@webrtc.org,
pthatcher@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1594973006/#ps120001 (title:
"New OWNERS file for webrtc/media.")

[commit-bot: I haz the power, 2016-01-27 13:32:14]

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

[nisse-webrtc, 2016-01-27 13:41:07]

Description was changed from

==========
New rtc::VideoSinkInterface.

The plan is that this interface should be used by all classes which receive a
stream of video frames, and replace the two generic classes
webrtc::VideoRendererInterface and cricket::VideoRenderer.

And the list goes on, there's a dozen of different classes which act as video
frame sinks.

At some point, we will likely add some method to handle sink properties like,
e.g, maximum useful width and height. But hopefully this can be done while
keeping the interface very simple.

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

to

==========
New rtc::VideoSinkInterface.

The plan is that this interface should be used by all classes which receive a
stream of video frames, and replace the two generic classes
webrtc::VideoRendererInterface and cricket::VideoRenderer.

And the list goes on, there's a dozen of different classes which act as video
frame sinks.

At some point, we will likely add some method to handle sink properties like,
e.g, maximum useful width and height. But hopefully this can be done while
keeping the interface very simple.

BUG=webrtc:5426
R=perkj@webrtc.org, pthatcher@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/a862d4563fbc26e52bed442de...
==========

[nisse-webrtc, 2016-01-27 13:41:08]

Committed patchset #7 (id:120001) manually as
a862d4563fbc26e52bed442de784094ae1dfe5ee (presubmit successful).

[commit-bot: I haz the power, 2016-01-27 13:41:11]

Description was changed from

==========
New rtc::VideoSinkInterface.

The plan is that this interface should be used by all classes which receive a
stream of video frames, and replace the two generic classes
webrtc::VideoRendererInterface and cricket::VideoRenderer.

And the list goes on, there's a dozen of different classes which act as video
frame sinks.

At some point, we will likely add some method to handle sink properties like,
e.g, maximum useful width and height. But hopefully this can be done while
keeping the interface very simple.

BUG=webrtc:5426
R=perkj@webrtc.org, pthatcher@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/a862d4563fbc26e52bed442de...
==========

to

==========
New rtc::VideoSinkInterface.

The plan is that this interface should be used by all classes which receive a
stream of video frames, and replace the two generic classes
webrtc::VideoRendererInterface and cricket::VideoRenderer.

And the list goes on, there's a dozen of different classes which act as video
frame sinks.

At some point, we will likely add some method to handle sink properties like,
e.g, maximum useful width and height. But hopefully this can be done while
keeping the interface very simple.

BUG=webrtc:5426
R=perkj@webrtc.org, pthatcher@webrtc.org

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

[commit-bot: I haz the power, 2016-01-27 13:41:13]

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

[nisse-webrtc, 2016-01-27 14:12:29]

A revert of this CL (patchset #7 id:120001) has been created in
https://codereview.webrtc.org/1646463002/ by nisse@webrtc.org.

The reason for reverting is: Broke chrome build. Investigating.

First error relating to AddSink method in
mock_peer_connection_dependency_factory.h.

[nisse-webrtc, 2016-01-28 10:05:16]

Description was changed from

==========
New rtc::VideoSinkInterface.

The plan is that this interface should be used by all classes which receive a
stream of video frames, and replace the two generic classes
webrtc::VideoRendererInterface and cricket::VideoRenderer.

And the list goes on, there's a dozen of different classes which act as video
frame sinks.

At some point, we will likely add some method to handle sink properties like,
e.g, maximum useful width and height. But hopefully this can be done while
keeping the interface very simple.

BUG=webrtc:5426
R=perkj@webrtc.org, pthatcher@webrtc.org

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

to

==========
New rtc::VideoSinkInterface.

The plan is that this interface should be used by all classes which receive a
stream of video frames, and replace the two generic classes
webrtc::VideoRendererInterface and cricket::VideoRenderer.

And the list goes on, there's a dozen of different classes which act as video
frame sinks.

At some point, we will likely add some method to handle sink properties like,
e.g, maximum useful width and height. But hopefully this can be done while
keeping the interface very simple.

BUG=webrtc:5426
R=perkj@webrtc.org, pthatcher@webrtc.org

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

[nisse-webrtc, 2016-01-28 10:11:48]

Added a workaround for chrome. Works in local testing, after
https://codereview.chromium.org/1642873002/ is applied.

The reason for the problem is the method 

void MockVideoSource::AddSink(cricket::VideoRenderer* output) {
  NOTIMPLEMENTED();
}

(and RemoveSink). So it's all a little silly. The workaround is not pretty and
isn't a solution for real backwards compatibility. But I think it solves the
problem, and it should be removed as soon as chrome is updated.

[nisse-webrtc, 2016-01-28 11:43:02]

The CQ bit was checked by nisse@webrtc.org

[nisse-webrtc, 2016-01-28 11:43:02]

The patchset sent to the CQ was uploaded after l-g-t-m from perkj@webrtc.org,
pthatcher@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1594973006/#ps140001 (title:
"Workaround to not break chrome.")

[commit-bot: I haz the power, 2016-01-28 11:43:04]

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

[commit-bot: I haz the power, 2016-01-28 12:47:10]

Description was changed from

==========
New rtc::VideoSinkInterface.

The plan is that this interface should be used by all classes which receive a
stream of video frames, and replace the two generic classes
webrtc::VideoRendererInterface and cricket::VideoRenderer.

And the list goes on, there's a dozen of different classes which act as video
frame sinks.

At some point, we will likely add some method to handle sink properties like,
e.g, maximum useful width and height. But hopefully this can be done while
keeping the interface very simple.

BUG=webrtc:5426
R=perkj@webrtc.org, pthatcher@webrtc.org

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

to

==========
New rtc::VideoSinkInterface.

The plan is that this interface should be used by all classes which receive a
stream of video frames, and replace the two generic classes
webrtc::VideoRendererInterface and cricket::VideoRenderer.

And the list goes on, there's a dozen of different classes which act as video
frame sinks.

At some point, we will likely add some method to handle sink properties like,
e.g, maximum useful width and height. But hopefully this can be done while
keeping the interface very simple.

BUG=webrtc:5426
R=perkj@webrtc.org, pthatcher@webrtc.org

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

[commit-bot: I haz the power, 2016-01-28 12:47:11]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-01-28 12:47:21]

Description was changed from

==========
New rtc::VideoSinkInterface.

The plan is that this interface should be used by all classes which receive a
stream of video frames, and replace the two generic classes
webrtc::VideoRendererInterface and cricket::VideoRenderer.

And the list goes on, there's a dozen of different classes which act as video
frame sinks.

At some point, we will likely add some method to handle sink properties like,
e.g, maximum useful width and height. But hopefully this can be done while
keeping the interface very simple.

BUG=webrtc:5426
R=perkj@webrtc.org, pthatcher@webrtc.org

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

to

==========
New rtc::VideoSinkInterface.

The plan is that this interface should be used by all classes which receive a
stream of video frames, and replace the two generic classes
webrtc::VideoRendererInterface and cricket::VideoRenderer.

And the list goes on, there's a dozen of different classes which act as video
frame sinks.

At some point, we will likely add some method to handle sink properties like,
e.g, maximum useful width and height. But hopefully this can be done while
keeping the interface very simple.

BUG=webrtc:5426
R=perkj@webrtc.org, pthatcher@webrtc.org

Committed: https://crrev.com/a862d4563fbc26e52bed442de784094ae1dfe5ee
Cr-Commit-Position: refs/heads/master@{#11396}

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

[commit-bot: I haz the power, 2016-01-28 12:47:22]

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