Android: Remove VideoRenderer.Callbacks.canApplyRotation()

Android: Remove VideoRenderer.Callbacks.canApplyRotation()

The only real implementation of VideoRenderer.Callbacks, VideoRendererGui, can always apply rotation. We don't need this in the interface.

BUG=webrtc:4145
R=glaznev@webrtc.org, guoweis@webrtc.org

Committed: https://chromium.googlesource.com/external/webrtc/+/7ef9d9104d2143fdd775f7d77bcb698b4d919d59

[magjed_webrtc, 2015-08-23 17:06:01]

Patchset #1 (id:1) has been deleted

[magjed_webrtc, 2015-08-24 07:44:14]

magjed@webrtc.org changed reviewers:
+ glaznev@webrtc.org, guoweis@webrtc.org

[magjed_webrtc, 2015-08-24 11:29:21]

I'm pretty sure we can remove this, but I'm confused about the comment
'TODO(guoweis): Remove this once chrome code base is updated'. Chrome has never
had any implementation of VideoRenderer.Callbacks. canApplyRotation() was
introduced in the same CL that added rotation support for VideoRendererGui, so
I'm not sure which video renderer it was supposed to protect?

[guoweis_webrtc, 2015-08-24 14:07:35]

It was to protect renderer in chrome during the time when rotation was
introduced by CVO work item at sender while renderer (chrome renderer) hasn't
called GetCopyWithRotationApplied to adjust for it.

https://codereview.webrtc.org/1306073003/diff/20001/talk/app/webrtc/java/test...
File talk/app/webrtc/java/testcommon/src/org/webrtc/PeerConnectionTest.java
(left):

https://codereview.webrtc.org/1306073003/diff/20001/talk/app/webrtc/java/test...
talk/app/webrtc/java/testcommon/src/org/webrtc/PeerConnectionTest.java:146: 
will frame come to this class be handled correctly? Does it have rotation code
like VideoRendererGui?

https://codereview.webrtc.org/1306073003/diff/20001/webrtc/examples/androidte...
File
webrtc/examples/androidtests/src/org/appspot/apprtc/test/PeerConnectionClientTest.java
(left):

https://codereview.webrtc.org/1306073003/diff/20001/webrtc/examples/androidte...
webrtc/examples/androidtests/src/org/appspot/apprtc/test/PeerConnectionClientTest.java:91:

same here.

[magjed_webrtc, 2015-08-24 14:18:25]

What Chrome renderer? There is a C++ renderer in Chrome that implements
webrtc::VideoRendererInterface, but nothing related to Java
VideoRenderer.Callbacks. I'm assuming it's ok to remove this then?

https://codereview.webrtc.org/1306073003/diff/20001/talk/app/webrtc/java/test...
File talk/app/webrtc/java/testcommon/src/org/webrtc/PeerConnectionTest.java
(left):

https://codereview.webrtc.org/1306073003/diff/20001/talk/app/webrtc/java/test...
talk/app/webrtc/java/testcommon/src/org/webrtc/PeerConnectionTest.java:146: 
On 2015/08/24 14:07:35, guoweis wrote:
> will frame come to this class be handled correctly? Does it have rotation code
> like VideoRendererGui?

You can see the renderFrame() implementation right above this function. This is
just a test class that counts frames and make some asserts on frame size.

https://codereview.webrtc.org/1306073003/diff/20001/webrtc/examples/androidte...
File
webrtc/examples/androidtests/src/org/appspot/apprtc/test/PeerConnectionClientTest.java
(left):

https://codereview.webrtc.org/1306073003/diff/20001/webrtc/examples/androidte...
webrtc/examples/androidtests/src/org/appspot/apprtc/test/PeerConnectionClientTest.java:91:

On 2015/08/24 14:07:35, guoweis wrote:
> same here.

This is just a mock class. The renderFrame() below does not use |frame| for
anything except logging.

[guoweis_webrtc, 2015-08-24 14:27:12]

https://codereview.webrtc.org/1306073003/diff/20001/webrtc/examples/androidte...
File
webrtc/examples/androidtests/src/org/appspot/apprtc/test/PeerConnectionClientTest.java
(left):

https://codereview.webrtc.org/1306073003/diff/20001/webrtc/examples/androidte...
webrtc/examples/androidtests/src/org/appspot/apprtc/test/PeerConnectionClientTest.java:91:

On 2015/08/24 14:18:25, magjed_webrtc wrote:
> On 2015/08/24 14:07:35, guoweis wrote:
> > same here.
> 
> This is just a mock class. The renderFrame() below does not use |frame| for
> anything except logging.

Let's at least put a comment here that since JNI is not using
GetCopyWithRotationApplied, the incoming frame is not rotated correctly.

[magjed_webrtc, 2015-08-24 14:37:40]

Patchset #2 (id:40001) has been deleted

[magjed_webrtc, 2015-08-24 14:41:56]

https://codereview.webrtc.org/1306073003/diff/20001/talk/app/webrtc/java/test...
File talk/app/webrtc/java/testcommon/src/org/webrtc/PeerConnectionTest.java
(right):

https://codereview.webrtc.org/1306073003/diff/20001/talk/app/webrtc/java/test...
talk/app/webrtc/java/testcommon/src/org/webrtc/PeerConnectionTest.java:435:
private static class TestRenderer implements VideoRenderer.Callbacks {
This class is apparently not used at all.

https://codereview.webrtc.org/1306073003/diff/20001/webrtc/examples/androidte...
File
webrtc/examples/androidtests/src/org/appspot/apprtc/test/PeerConnectionClientTest.java
(left):

https://codereview.webrtc.org/1306073003/diff/20001/webrtc/examples/androidte...
webrtc/examples/androidtests/src/org/appspot/apprtc/test/PeerConnectionClientTest.java:91:

On 2015/08/24 14:27:12, guoweis wrote:
> On 2015/08/24 14:18:25, magjed_webrtc wrote:
> > On 2015/08/24 14:07:35, guoweis wrote:
> > > same here.
> > 
> > This is just a mock class. The renderFrame() below does not use |frame| for
> > anything except logging.
> 
> Let's at least put a comment here that since JNI is not using
> GetCopyWithRotationApplied, the incoming frame is not rotated correctly.

I replaced width/height with rotatedWidth()/rotatedHeight(). It should behave
exactly the same.

[magjed_webrtc, 2015-08-24 14:42:01]

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

[commit-bot: I haz the power, 2015-08-24 14:42:07]

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

[guoweis_webrtc, 2015-08-24 16:24:15]

On 2015/08/24 14:42:07, commit-bot: I haz the power wrote:
> Dry run: CQ is trying da patch. Follow status at
>  https://chromium-cq-status.appspot.com/patch-status/1306073003/60001
> View timeline at
>  https://chromium-cq-status.appspot.com/patch-timeline/1306073003/60001

lgtm

[commit-bot: I haz the power, 2015-08-24 16:42:10]

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

[commit-bot: I haz the power, 2015-08-24 16:42:11]

Dry run: Try jobs failed on following builders:
  android on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  mac_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[AlexG, 2015-08-24 21:42:16]

lgtm

[magjed_webrtc, 2015-08-25 07:32:23]

Committed patchset #2 (id:60001) manually as
7ef9d9104d2143fdd775f7d77bcb698b4d919d59 (presubmit successful).