Delete VideoRendererInterface.

Delete VideoRendererInterface.

Use in chromium was deleted a few days ago.

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

Committed: https://crrev.com/8f597628973fbc90da3041ac64a54d75ef5fb9c5
Cr-Commit-Position: refs/heads/master@{#12099}

[nisse-webrtc, 2016-03-18 11:02:29]

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

[commit-bot: I haz the power, 2016-03-18 11:02:33]

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

[commit-bot: I haz the power, 2016-03-18 11:04:26]

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

[commit-bot: I haz the power, 2016-03-18 11:04:26]

Dry run: Try jobs failed on following builders:
  ios64_sim_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_dbg/builds/6061)
  ios_arm64_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/8399)
  ios_arm64_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_rel/builds/8313)

[nisse-webrtc, 2016-03-18 11:28:17]

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

[commit-bot: I haz the power, 2016-03-18 11:28:22]

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

[commit-bot: I haz the power, 2016-03-18 11:30:14]

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

[commit-bot: I haz the power, 2016-03-18 11:30:15]

Dry run: Try jobs failed on following builders:
  ios64_sim_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_dbg/builds/6063)
  ios_arm64_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/8401)
  ios_arm64_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_rel/builds/8318)

[nisse-webrtc, 2016-03-18 12:36:23]

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

[commit-bot: I haz the power, 2016-03-18 12:36:32]

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

[nisse-webrtc, 2016-03-18 13:01:16]

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

[nisse-webrtc, 2016-03-18 13:01:17]

Hi,

this cl deletes the class webrtc::VideoRendererInterface. It is replaced by
rtc::VideoSinkInterface.

[pbos-webrtc, 2016-03-18 15:37:25]

lgtm

[nisse-webrtc, 2016-03-18 15:41:03]

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

[commit-bot: I haz the power, 2016-03-18 15:41:15]

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

[pthatcher1, 2016-03-18 17:00:58]

https://codereview.webrtc.org/1817473002/diff/180001/talk/app/webrtc/objc/RTC...
File talk/app/webrtc/objc/RTCVideoRendererAdapter.h (right):

https://codereview.webrtc.org/1817473002/diff/180001/talk/app/webrtc/objc/RTC...
talk/app/webrtc/objc/RTCVideoRendererAdapter.h:36:
rtc::VideoSinkInterface<cricket::VideoFrame> *nativeVideoRenderer;
Is that * in the right place (looks wrong for C++, but I have no idea for objc).

https://codereview.webrtc.org/1817473002/diff/180001/talk/app/webrtc/objc/RTC...
File talk/app/webrtc/objc/RTCVideoRendererAdapter.mm (right):

https://codereview.webrtc.org/1817473002/diff/180001/talk/app/webrtc/objc/RTC...
talk/app/webrtc/objc/RTCVideoRendererAdapter.mm:78: -
(rtc::VideoSinkInterface<cricket::VideoFrame> *)nativeVideoRenderer {
Same here

https://codereview.webrtc.org/1817473002/diff/180001/webrtc/api/java/android/...
File webrtc/api/java/android/org/webrtc/SurfaceViewRenderer.java (right):

https://codereview.webrtc.org/1817473002/diff/180001/webrtc/api/java/android/...
webrtc/api/java/android/org/webrtc/SurfaceViewRenderer.java:34: * Interaction
from C++ rtc::VideoSinkInterface in renderFrame and canApplyRotation.
Isn't the comment about canApplyRotation out of date now?

https://codereview.webrtc.org/1817473002/diff/180001/webrtc/api/objc/RTCVideo...
File webrtc/api/objc/RTCVideoRendererAdapter.mm (right):

https://codereview.webrtc.org/1817473002/diff/180001/webrtc/api/objc/RTCVideo...
webrtc/api/objc/RTCVideoRendererAdapter.mm:60: -
(rtc::VideoSinkInterface<cricket::VideoFrame>*)nativeVideoRenderer {
And here the * is the exact opposite.  Shouldn't it at least be consistent?

https://codereview.webrtc.org/1817473002/diff/180001/webrtc/examples/peerconn...
File webrtc/examples/peerconnection/client/linux/main_wnd.h (right):

https://codereview.webrtc.org/1817473002/diff/180001/webrtc/examples/peerconn...
webrtc/examples/peerconnection/client/linux/main_wnd.h:83: void SetSize(int
width, int height);
Is anything ever going to call this?

[commit-bot: I haz the power, 2016-03-18 17:41:52]

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

[commit-bot: I haz the power, 2016-03-18 17:41:53]

Dry run: Try jobs failed on following builders:
  android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  linux_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  win_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[tkchin_webrtc, 2016-03-18 19:14:17]

lgtm for objc parts % nits + file a bug to clean up RTCVideoRenderer or
volunteer to do it :)

https://codereview.webrtc.org/1817473002/diff/180001/talk/app/webrtc/objc/RTC...
File talk/app/webrtc/objc/RTCVideoRendererAdapter.h (right):

https://codereview.webrtc.org/1817473002/diff/180001/talk/app/webrtc/objc/RTC...
talk/app/webrtc/objc/RTCVideoRendererAdapter.h:36:
rtc::VideoSinkInterface<cricket::VideoFrame> *nativeVideoRenderer;
On 2016/03/18 17:00:58, pthatcher1 wrote:
> Is that * in the right place (looks wrong for C++, but I have no idea for
objc).

It is correct for ObjC

https://codereview.webrtc.org/1817473002/diff/180001/talk/app/webrtc/objc/RTC...
File talk/app/webrtc/objc/RTCVideoRendererAdapter.mm (right):

https://codereview.webrtc.org/1817473002/diff/180001/talk/app/webrtc/objc/RTC...
talk/app/webrtc/objc/RTCVideoRendererAdapter.mm:39: class
RTCVideoRendererNativeAdapter
we should eventually update these class names since videorenderer is a thing of
the past. If you don't mind doing it, then this would become

namespace webrtc {
class VideoSinkAdapter {
};
}

@interface RTCVideoRendererAdapter -> @interface RTCVideoSinkAdapter

Otherwise, please file a bug.

@protocol RTCVideoRenderer -> @protocol RTCVideoSink

https://codereview.webrtc.org/1817473002/diff/180001/talk/app/webrtc/objc/RTC...
talk/app/webrtc/objc/RTCVideoRendererAdapter.mm:52: [_adapter.videoRenderer
setSize:_size];
hm. This should be updated, the renderer shouldn't be using an old API surface
if setSize is gone now.
I think just file a bug on me and I'll get it updated.

https://codereview.webrtc.org/1817473002/diff/180001/talk/app/webrtc/objc/RTC...
talk/app/webrtc/objc/RTCVideoRendererAdapter.mm:78: -
(rtc::VideoSinkInterface<cricket::VideoFrame> *)nativeVideoRenderer {
On 2016/03/18 17:00:58, pthatcher1 wrote:
> Same here

All good. The rules are weird in ObjC++ -> If it's a C/C++ function, C++ style
guide applies. If it is an ObjC method, ObjC rules apply.

So VideoFrame& in the C++ class above, but (rtc::..<.> *) here.

https://codereview.webrtc.org/1817473002/diff/180001/webrtc/api/objc/RTCVideo...
File webrtc/api/objc/RTCVideoRendererAdapter+Private.h (right):

https://codereview.webrtc.org/1817473002/diff/180001/webrtc/api/objc/RTCVideo...
webrtc/api/objc/RTCVideoRendererAdapter+Private.h:34:
rtc::VideoSinkInterface<cricket::VideoFrame>* nativeVideoRenderer;
me> *native

https://codereview.webrtc.org/1817473002/diff/180001/webrtc/api/objc/RTCVideo...
File webrtc/api/objc/RTCVideoRendererAdapter.mm (right):

https://codereview.webrtc.org/1817473002/diff/180001/webrtc/api/objc/RTCVideo...
webrtc/api/objc/RTCVideoRendererAdapter.mm:60: -
(rtc::VideoSinkInterface<cricket::VideoFrame>*)nativeVideoRenderer {
On 2016/03/18 17:00:58, pthatcher1 wrote:
> And here the * is the exact opposite.  Shouldn't it at least be consistent?

Follow the other one, that was correct.

https://codereview.webrtc.org/1817473002/diff/180001/webrtc/api/objc/RTCVideo...
File webrtc/api/objc/RTCVideoTrack.mm (right):

https://codereview.webrtc.org/1817473002/diff/180001/webrtc/api/objc/RTCVideo...
webrtc/api/objc/RTCVideoTrack.mm:83: rtc::VideoSinkWants());
for my benefit, what is a videosinkwants?

[nisse-webrtc, 2016-03-21 08:37:56]

https://codereview.webrtc.org/1817473002/diff/180001/talk/app/webrtc/objc/RTC...
File talk/app/webrtc/objc/RTCVideoRendererAdapter.mm (right):

https://codereview.webrtc.org/1817473002/diff/180001/talk/app/webrtc/objc/RTC...
talk/app/webrtc/objc/RTCVideoRendererAdapter.mm:39: class
RTCVideoRendererNativeAdapter
On 2016/03/18 19:14:16, tkchin_webrtc wrote:
> we should eventually update these class names since videorenderer is a thing
of
> the past. If you don't mind doing it, then this would become
> 
> namespace webrtc {
> class VideoSinkAdapter {
> };
> }
> 
> @interface RTCVideoRendererAdapter -> @interface RTCVideoSinkAdapter
> 
> Otherwise, please file a bug.
> 
> @protocol RTCVideoRenderer -> @protocol RTCVideoSink
> 

I think I'd prefer to delay as much work as possibly until the transition out of
talk/ is complete.

I've filed bug https://bugs.chromium.org/p/webrtc/issues/detail?id=5673

https://codereview.webrtc.org/1817473002/diff/180001/webrtc/api/java/android/...
File webrtc/api/java/android/org/webrtc/SurfaceViewRenderer.java (right):

https://codereview.webrtc.org/1817473002/diff/180001/webrtc/api/java/android/...
webrtc/api/java/android/org/webrtc/SurfaceViewRenderer.java:34: * Interaction
from C++ rtc::VideoSinkInterface in renderFrame and canApplyRotation.
On 2016/03/18 17:00:58, pthatcher1 wrote:
> Isn't the comment about canApplyRotation out of date now?

Right, it seems the canApplyRotation method disappeared a while ago.

https://codereview.webrtc.org/1817473002/diff/180001/webrtc/api/objc/RTCVideo...
File webrtc/api/objc/RTCVideoRendererAdapter+Private.h (right):

https://codereview.webrtc.org/1817473002/diff/180001/webrtc/api/objc/RTCVideo...
webrtc/api/objc/RTCVideoRendererAdapter+Private.h:34:
rtc::VideoSinkInterface<cricket::VideoFrame>* nativeVideoRenderer;
On 2016/03/18 19:14:16, tkchin_webrtc wrote:
> me> *native

Done.

https://codereview.webrtc.org/1817473002/diff/180001/webrtc/api/objc/RTCVideo...
File webrtc/api/objc/RTCVideoRendererAdapter.mm (right):

https://codereview.webrtc.org/1817473002/diff/180001/webrtc/api/objc/RTCVideo...
webrtc/api/objc/RTCVideoRendererAdapter.mm:60: -
(rtc::VideoSinkInterface<cricket::VideoFrame>*)nativeVideoRenderer {
On 2016/03/18 19:14:16, tkchin_webrtc wrote:
> On 2016/03/18 17:00:58, pthatcher1 wrote:
> > And here the * is the exact opposite.  Shouldn't it at least be consistent?
> 
> Follow the other one, that was correct.

Done.

https://codereview.webrtc.org/1817473002/diff/180001/webrtc/api/objc/RTCVideo...
File webrtc/api/objc/RTCVideoTrack.mm (right):

https://codereview.webrtc.org/1817473002/diff/180001/webrtc/api/objc/RTCVideo...
webrtc/api/objc/RTCVideoTrack.mm:83: rtc::VideoSinkWants());
On 2016/03/18 19:14:16, tkchin_webrtc wrote:
> for my benefit, what is a videosinkwants?

It's a couple of settings to specify the sink's preferences, e.g, maximum
resolution, and whether or not it is willing to rotate frames. Declared in
webrtc/media/base/videosourceinterface.h.

https://codereview.webrtc.org/1817473002/diff/180001/webrtc/examples/peerconn...
File webrtc/examples/peerconnection/client/linux/main_wnd.h (right):

https://codereview.webrtc.org/1817473002/diff/180001/webrtc/examples/peerconn...
webrtc/examples/peerconnection/client/linux/main_wnd.h:83: void SetSize(int
width, int height);
On 2016/03/18 17:00:58, pthatcher1 wrote:
> Is anything ever going to call this?

It's called by OnFrame. But there's no need for it to be a public method. I'll
move it to protected.

[nisse-webrtc, 2016-03-21 09:19:17]

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

[commit-bot: I haz the power, 2016-03-21 09:19:21]

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

[perkj_webrtc, 2016-03-21 09:19:41]

lgtm with a nit

https://codereview.webrtc.org/1817473002/diff/200001/webrtc/api/java/src/org/...
File webrtc/api/java/src/org/webrtc/VideoRenderer.java (right):

https://codereview.webrtc.org/1817473002/diff/200001/webrtc/api/java/src/org/...
webrtc/api/java/src/org/webrtc/VideoRenderer.java:110: * TODO(nisse): Can we
rename to OnFrame? */
I think you should just remove this todo. Otherwise you will have to update
Google3 clients. don't bother.

[nisse-webrtc, 2016-03-21 09:34:07]

https://codereview.webrtc.org/1817473002/diff/200001/webrtc/api/java/src/org/...
File webrtc/api/java/src/org/webrtc/VideoRenderer.java (right):

https://codereview.webrtc.org/1817473002/diff/200001/webrtc/api/java/src/org/...
webrtc/api/java/src/org/webrtc/VideoRenderer.java:110: * TODO(nisse): Can we
rename to OnFrame? */
On 2016/03/21 09:19:41, perkj_webrtc wrote:
> I think you should just remove this todo. Otherwise you will have to update
> Google3 clients. don't bother.

Done. I guess it's for someone else to decide if and when to update the java
APIs.

[magjed_webrtc, 2016-03-22 15:18:31]

lgtm

[pthatcher1, 2016-03-22 21:15:21]

lgtm

[nisse-webrtc, 2016-03-23 07:24:23]

The CQ bit was checked by nisse@webrtc.org

[nisse-webrtc, 2016-03-23 07:24:24]

The patchset sent to the CQ was uploaded after l-g-t-m from pbos@webrtc.org,
tkchin@webrtc.org, perkj@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1817473002/#ps220001 (title:
"Deleted a TODO comment.")

[commit-bot: I haz the power, 2016-03-23 07:24:26]

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

[commit-bot: I haz the power, 2016-03-23 08:27:59]

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

[commit-bot: I haz the power, 2016-03-23 08:28:01]

Failed to apply patch for webrtc/api/videotrack_unittest.cc:
While running git apply --index -3 -p1;
  error: patch failed: webrtc/api/videotrack_unittest.cc:20
  Falling back to three-way merge...
  Applied patch to 'webrtc/api/videotrack_unittest.cc' with conflicts.
  U webrtc/api/videotrack_unittest.cc

Patch:       webrtc/api/videotrack_unittest.cc
Index: webrtc/api/videotrack_unittest.cc
diff --git a/webrtc/api/videotrack_unittest.cc
b/webrtc/api/videotrack_unittest.cc
index
6dac507020321b1abd6f0f9866791247fa577a0e..4204cbb8f6082985650151a3e4bc373d5cecc410
100644
--- a/webrtc/api/videotrack_unittest.cc
+++ b/webrtc/api/videotrack_unittest.cc
@@ -20,7 +20,6 @@
 #include "webrtc/media/engine/webrtcvideoframe.h"
 
 using webrtc::FakeVideoTrackRenderer;
-using webrtc::FakeVideoTrackRendererOld;
 using webrtc::VideoTrackSource;
 using webrtc::VideoTrack;
 using webrtc::VideoTrackInterface;
@@ -66,30 +65,6 @@ TEST_F(VideoTrackTest, RenderVideo) {
   EXPECT_EQ(2, renderer_2->num_rendered_frames());
 }
 
-// Test adding renderers to a video track and render to them by
-// providing frames to the source. Uses the old VideoTrack interface
-// with AddRenderer and RemoveRenderer.
-TEST_F(VideoTrackTest, RenderVideoOld) {
-  // FakeVideoTrackRenderer register itself to |video_track_|
-  rtc::scoped_ptr<FakeVideoTrackRendererOld> renderer_1(
-      new FakeVideoTrackRendererOld(video_track_.get()));
-
-  capturer_.CaptureFrame();
-  EXPECT_EQ(1, renderer_1->num_rendered_frames());
-
-  // FakeVideoTrackRenderer register itself to |video_track_|
-  rtc::scoped_ptr<FakeVideoTrackRenderer> renderer_2(
-      new FakeVideoTrackRenderer(video_track_.get()));
-
-  capturer_.CaptureFrame();
-  EXPECT_EQ(2, renderer_1->num_rendered_frames());
-  EXPECT_EQ(1, renderer_2->num_rendered_frames());
-
-  renderer_1.reset(nullptr);
-  capturer_.CaptureFrame();
-  EXPECT_EQ(2, renderer_2->num_rendered_frames());
-}
-
 // Test that disabling the track results in blacked out frames.
 TEST_F(VideoTrackTest, DisableTrackBlackout) {
   rtc::scoped_ptr<FakeVideoTrackRenderer> renderer(

[nisse-webrtc, 2016-03-23 08:42:07]

The CQ bit was checked by nisse@webrtc.org

[nisse-webrtc, 2016-03-23 08:42:08]

The patchset sent to the CQ was uploaded after l-g-t-m from magjed@webrtc.org,
pbos@webrtc.org, perkj@webrtc.org, pthatcher@webrtc.org, tkchin@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1817473002/#ps240001 (title:
"Rebase.")

[commit-bot: I haz the power, 2016-03-23 08:42:15]

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

[nisse-webrtc, 2016-03-23 09:33:25]

Description was changed from

==========
Delete VideoRendererInterface.

Use in chromium was deleted a few days ago.

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

to

==========
Delete VideoRendererInterface.

Use in chromium was deleted a few days ago.

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

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

[nisse-webrtc, 2016-03-23 09:33:27]

Committed patchset #13 (id:240001) manually as
8f597628973fbc90da3041ac64a54d75ef5fb9c5 (presubmit successful).

[commit-bot: I haz the power, 2016-03-23 09:33:30]

Description was changed from

==========
Delete VideoRendererInterface.

Use in chromium was deleted a few days ago.

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

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

to

==========
Delete VideoRendererInterface.

Use in chromium was deleted a few days ago.

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

Committed: https://crrev.com/8f597628973fbc90da3041ac64a54d75ef5fb9c5
Cr-Commit-Position: refs/heads/master@{#12099}
==========

[commit-bot: I haz the power, 2016-03-23 09:33:31]

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