iOS render: Handle frame rotation in OpenGL

iOS render: Handle frame rotation in OpenGL

This CL handles frame rotation by updating the OpenGL vertex data in
RTCOpenGLVideoRenderer, instead of calling the expensive
libyuv::I420Rotate that will rotate the actual memory. Also, we can
handle rotated native frames instead of falling back to
NativeToI420Buffer.

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

[magjed_webrtc, 2016-07-22 12:26:35]

Patchset #1 (id:1) has been deleted

[magjed_webrtc, 2016-07-22 13:25:01]

tkchin - please take a look.

[magjed_webrtc, 2016-08-08 09:20:42]

Description was changed from

==========
iOS render: Handle frame rotation in OpenGL

This CL handles frame rotation by updating the OpenGL vertex data in
RTCOpenGLVideoRenderer, instead of calling the expensive
libyuv::I420Rotate that will rotate the actual memory. Also, we can
handle rotated native frames instead of falling back to
NativeToI420Buffer.
==========

to

==========
iOS render: Handle frame rotation in OpenGL

This CL handles frame rotation by updating the OpenGL vertex data in
RTCOpenGLVideoRenderer, instead of calling the expensive
libyuv::I420Rotate that will rotate the actual memory. Also, we can
handle rotated native frames instead of falling back to
NativeToI420Buffer.
==========

[magjed_webrtc, 2016-08-08 09:20:42]

magjed@webrtc.org changed reviewers:
+ tkchin@webrtc.org

[magjed_webrtc, 2016-08-08 13:53:31]

PTAL

[tkchin_webrtc, 2016-08-08 22:58:46]

lgtm

I think we may need this elsewhere in the future. Right now when orientation
changes the capturer has a flash of black because we apply rotation on the
capturer. We probably should just set rotation bit to avoid the flash.

Can look into using CIImage / CIContext using either an EAGLContext or a
MTLContext for converting CVPixelBufferRef. That may do the same thing as this
GL code on the GPU, not sure about relative perf though.

https://codereview.webrtc.org/2176623002/diff/60001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Classes/RTCI420Shader.mm (right):

https://codereview.webrtc.org/2176623002/diff/60001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/RTCI420Shader.mm:64: rtc::Optional<int>
_current_rotation;
nit: camelcase for objc files
_currentRotation

https://codereview.webrtc.org/2176623002/diff/60001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Classes/RTCNativeNV12Shader.mm (right):

https://codereview.webrtc.org/2176623002/diff/60001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/RTCNativeNV12Shader.mm:52: rtc::Optional<int>
_current_rotation;
ditto

https://codereview.webrtc.org/2176623002/diff/60001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Classes/RTCVideoFrame+Private.h (right):

https://codereview.webrtc.org/2176623002/diff/60001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/RTCVideoFrame+Private.h:24:
rotation:(int)rotation
would be nice to use the enum type, or have the enum explicitly be of int type

[magjed_webrtc, 2016-08-09 13:54:28]

PTAL

https://codereview.webrtc.org/2176623002/diff/60001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Classes/RTCI420Shader.mm (right):

https://codereview.webrtc.org/2176623002/diff/60001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/RTCI420Shader.mm:64: rtc::Optional<int>
_current_rotation;
On 2016/08/08 22:58:45, tkchin_webrtc wrote:
> nit: camelcase for objc files
> _currentRotation

Done.

https://codereview.webrtc.org/2176623002/diff/60001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Classes/RTCNativeNV12Shader.mm (right):

https://codereview.webrtc.org/2176623002/diff/60001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/RTCNativeNV12Shader.mm:52: rtc::Optional<int>
_current_rotation;
On 2016/08/08 22:58:45, tkchin_webrtc wrote:
> ditto

Done.

https://codereview.webrtc.org/2176623002/diff/60001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Classes/RTCVideoFrame+Private.h (right):

https://codereview.webrtc.org/2176623002/diff/60001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/RTCVideoFrame+Private.h:24:
rotation:(int)rotation
On 2016/08/08 22:58:45, tkchin_webrtc wrote:
> would be nice to use the enum type, or have the enum explicitly be of int type

I initially tried to use webrtc::VideoRotation, but gave up when I hit this
compile error:
../../webrtc/sdk/objc/Framework/Classes/RTCEAGLVideoView.m -o
obj/webrtc/sdk/objc/Framework/Classes/rtc_sdk_peerconnection_objc.RTCEAGLVideoView.o
In file included from
../../webrtc/sdk/objc/Framework/Classes/RTCEAGLVideoView.m:16:
In file included from
../../webrtc/sdk/objc/Framework/Headers/WebRTC//RTCVideoFrame.h:16:
../../webrtc/common_video/rotation.h:14:1: error: unknown type name 'namespace'
namespace webrtc {

I tried a little harder this time, and it seems it's enough to rename
RTCEAGLVideoView.m to RTCEAGLVideoView.mm. Is that ok to do?

[tkchin_webrtc, 2016-08-09 17:42:20]

https://codereview.webrtc.org/2176623002/diff/80001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Classes/RTCEAGLVideoView.mm (right):

https://codereview.webrtc.org/2176623002/diff/80001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/RTCEAGLVideoView.mm:16: #import
"WebRTC//RTCVideoFrame.h"
Mind fixing this typo? //

https://codereview.webrtc.org/2176623002/diff/80001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCVideoFrame.h (right):

https://codereview.webrtc.org/2176623002/diff/80001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Headers/WebRTC/RTCVideoFrame.h:16: #include
"webrtc/common_video/rotation.h"
C++ includes cannot be present in pure ObjC headers as this one, because it will
force every dependency to compile their files as ObjC++. This is why the
compiler made you rename eaglvideoview to .mm

In the public header we can just keep this as an int and do the casting when we
return it in the impl

[magjed_webrtc, 2016-08-10 10:22:08]

Patchset #5 (id:100001) has been deleted

[magjed_webrtc, 2016-08-10 10:26:07]

https://codereview.webrtc.org/2176623002/diff/80001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Classes/RTCEAGLVideoView.mm (right):

https://codereview.webrtc.org/2176623002/diff/80001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/RTCEAGLVideoView.mm:16: #import
"WebRTC//RTCVideoFrame.h"
On 2016/08/09 17:42:20, tkchin_webrtc wrote:
> Mind fixing this typo? //

Done.

https://codereview.webrtc.org/2176623002/diff/80001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCVideoFrame.h (right):

https://codereview.webrtc.org/2176623002/diff/80001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Headers/WebRTC/RTCVideoFrame.h:16: #include
"webrtc/common_video/rotation.h"
On 2016/08/09 17:42:20, tkchin_webrtc wrote:
> C++ includes cannot be present in pure ObjC headers as this one, because it
will
> force every dependency to compile their files as ObjC++. This is why the
> compiler made you rename eaglvideoview to .mm
> 
> In the public header we can just keep this as an int and do the casting when
we
> return it in the impl

Done. I removed the C++ include from this file and use an int as return type
instead of the enum, but still use the C++ enum as much as possible in the .mm
files.

[magjed_webrtc, 2016-08-10 10:26:14]

The CQ bit was checked by magjed@webrtc.org

[magjed_webrtc, 2016-08-10 10:26:14]

The patchset sent to the CQ was uploaded after l-g-t-m from tkchin@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2176623002/#ps120001 (title:
"Keep RTCVideoFrame.h pure ObjC")

[commit-bot: I haz the power, 2016-08-10 10:26:20]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-08-10 10:33:29]

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

[commit-bot: I haz the power, 2016-08-10 10:33:29]

Try jobs failed on following builders:
  linux_libfuzzer_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_libfuzzer_rel/bui...)

[magjed_webrtc, 2016-08-10 14:52:32]

The CQ bit was checked by magjed@webrtc.org

[commit-bot: I haz the power, 2016-08-10 14:52:34]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-08-10 14:58:33]

Description was changed from

==========
iOS render: Handle frame rotation in OpenGL

This CL handles frame rotation by updating the OpenGL vertex data in
RTCOpenGLVideoRenderer, instead of calling the expensive
libyuv::I420Rotate that will rotate the actual memory. Also, we can
handle rotated native frames instead of falling back to
NativeToI420Buffer.
==========

to

==========
iOS render: Handle frame rotation in OpenGL

This CL handles frame rotation by updating the OpenGL vertex data in
RTCOpenGLVideoRenderer, instead of calling the expensive
libyuv::I420Rotate that will rotate the actual memory. Also, we can
handle rotated native frames instead of falling back to
NativeToI420Buffer.
==========

[commit-bot: I haz the power, 2016-08-10 14:58:34]

Committed patchset #5 (id:120001)

[commit-bot: I haz the power, 2016-08-10 14:58:38]

Description was changed from

==========
iOS render: Handle frame rotation in OpenGL

This CL handles frame rotation by updating the OpenGL vertex data in
RTCOpenGLVideoRenderer, instead of calling the expensive
libyuv::I420Rotate that will rotate the actual memory. Also, we can
handle rotated native frames instead of falling back to
NativeToI420Buffer.
==========

to

==========
iOS render: Handle frame rotation in OpenGL

This CL handles frame rotation by updating the OpenGL vertex data in
RTCOpenGLVideoRenderer, instead of calling the expensive
libyuv::I420Rotate that will rotate the actual memory. Also, we can
handle rotated native frames instead of falling back to
NativeToI420Buffer.

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

[commit-bot: I haz the power, 2016-08-10 14:58:39]

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