iOS: Add support for rendering native CVPixelBuffers directly

iOS: Add support for rendering native CVPixelBuffers directly

This CL adds support in RTCEAGLVideoView for rendering CVPixelBuffers as
OpenGL ES textures directly, compared to the current code that first
converts the CVPixelBuffers to I420, and then reuploads them as
textures. This is only supported on iOS with the use of a
CVOpenGLESTextureCache.

The I420 rendering and native rendering are separated in two different
implementations of a simple shader interface:
@protocol Shader
- (BOOL)drawFrame:(RTCVideoFrame*)frame;
@end
GL resources are allocated when the shader is instantiated and released
when the shader is destroyed. RTCEAGLVideoView will lazily instantiate
the necessary shader when it receives the first frame of that kind. This
is primarily done to avoid allocating GL resources for both I420 and
native rendering.

Some other changes are:
 - Print GL shader compilation errors.
 - Remove updateTextureSizesForFrame() function. The textures will
   resize automatically anyway when the texture data is uploaded with
   glTexImage2D().

patch from issue 2154243002 at patchset 140001 (http://crrev.com/2154243002#ps140001)
Continuing magjed@'s work since he is OOO this week.

BUG=

Committed: https://crrev.com/04dbb34d51aa0dfc458f29c220bf770504cd52d3
Cr-Commit-Position: refs/heads/master@{#13668}

[tkchin_webrtc, 2016-08-02 22:15:02]

Description was changed from

==========
iOS: Add support for rendering native CVPixelBuffers directly

This CL adds support in RTCEAGLVideoView for rendering CVPixelBuffers as
OpenGL ES textures directly, compared to the current code that first
converts the CVPixelBuffers to I420, and then reuploads them as
textures. This is only supported on iOS with the use of a
CVOpenGLESTextureCache.

The I420 rendering and native rendering are separated in two different
implementations of a simple shader interface:
@protocol Shader
- (BOOL)drawFrame:(RTCVideoFrame*)frame;
@end
GL resources are allocated when the shader is instantiated and released
when the shader is destroyed. RTCEAGLVideoView will lazily instantiate
the necessary shader when it receives the first frame of that kind. This
is primarily done to avoid allocating GL resources for both I420 and
native rendering.

Some other changes are:
 - Print GL shader compilation errors.
 - Remove updateTextureSizesForFrame() function. The textures will
   resize automatically anyway when the texture data is uploaded with
   glTexImage2D().

patch from issue 2154243002 at patchset 140001
(http://crrev.com/2154243002#ps140001)
Continuing magjed@'s work since he is OOO this week.

BUG=
==========

to

==========
iOS: Add support for rendering native CVPixelBuffers directly

This CL adds support in RTCEAGLVideoView for rendering CVPixelBuffers as
OpenGL ES textures directly, compared to the current code that first
converts the CVPixelBuffers to I420, and then reuploads them as
textures. This is only supported on iOS with the use of a
CVOpenGLESTextureCache.

The I420 rendering and native rendering are separated in two different
implementations of a simple shader interface:
@protocol Shader
- (BOOL)drawFrame:(RTCVideoFrame*)frame;
@end
GL resources are allocated when the shader is instantiated and released
when the shader is destroyed. RTCEAGLVideoView will lazily instantiate
the necessary shader when it receives the first frame of that kind. This
is primarily done to avoid allocating GL resources for both I420 and
native rendering.

Some other changes are:
 - Print GL shader compilation errors.
 - Remove updateTextureSizesForFrame() function. The textures will
   resize automatically anyway when the texture data is uploaded with
   glTexImage2D().

patch from issue 2154243002 at patchset 140001
(http://crrev.com/2154243002#ps140001)
Continuing magjed@'s work since he is OOO this week.

BUG=
==========

[tkchin_webrtc, 2016-08-02 22:15:02]

tkchin@webrtc.org changed reviewers:
+ haysc@webrtc.org, magjed@webrtc.org, mflodman@webrtc.org

[tkchin_webrtc, 2016-08-02 22:18:32]

All I did was move stuff around and rename a few things to address comments in
source CL.

[mflodman, 2016-08-03 07:07:26]

mflodman@webrtc.org changed reviewers:
+ sakal@webrtc.org

[mflodman, 2016-08-03 07:07:26]

Sami,
I'll review this today as well, but can you take an extra look at this patch
during the day as well? Since magjed is ooo.

[sakal, 2016-08-03 10:53:00]

I don't really know ObjC but here are my comments.

https://codereview.webrtc.org/2202823004/diff/1/webrtc/sdk/objc/Framework/Cla...
File webrtc/sdk/objc/Framework/Classes/RTCI420Shader.mm (right):

https://codereview.webrtc.org/2202823004/diff/1/webrtc/sdk/objc/Framework/Cla...
webrtc/sdk/objc/Framework/Classes/RTCI420Shader.mm:23: static const GLsizei
kNumTextures = 3 * kNumTextureSets;
I would prefer 3 be a constant like kNumTexturesPerSet.

https://codereview.webrtc.org/2202823004/diff/1/webrtc/sdk/objc/Framework/Cla...
webrtc/sdk/objc/Framework/Classes/RTCI420Shader.mm:81: if (_i420Program != 0) {
These if statements are unnecessary. OpenGL silently ignores trying to delete
something with index 0.

https://codereview.webrtc.org/2202823004/diff/1/webrtc/sdk/objc/Framework/Cla...
webrtc/sdk/objc/Framework/Classes/RTCI420Shader.mm:111:
glActiveTexture(GL_TEXTURE0 + i);
I don't think this is needed. We can just set up the textures using any active
texture.

https://codereview.webrtc.org/2202823004/diff/1/webrtc/sdk/objc/Framework/Cla...
webrtc/sdk/objc/Framework/Classes/RTCI420Shader.mm:141:
glActiveTexture(static_cast<GLenum>(GL_TEXTURE0 + offset));
Is it not possible to just always use GL_TEXTURE0-2 or does that cause CPU
blockage? If we only used GL_TEXTURE0-2 that would allow kNumTextures be over 8,
right?

https://codereview.webrtc.org/2202823004/diff/1/webrtc/sdk/objc/Framework/Cla...
File webrtc/sdk/objc/Framework/Classes/RTCNativeNV12Shader.mm (right):

https://codereview.webrtc.org/2202823004/diff/1/webrtc/sdk/objc/Framework/Cla...
webrtc/sdk/objc/Framework/Classes/RTCNativeNV12Shader.mm:61: if (_nv12Program !=
0) {
These if statements are unnecessary. OpenGL silently ignores trying to delete
something with index 0.

[tkchin_webrtc, 2016-08-03 17:53:19]

Would prefer to preserve as much as old behavior as possible at this point to
reduce risk. Old behavior for RTCI420Shader is known to be stable.

https://codereview.webrtc.org/2202823004/diff/1/webrtc/sdk/objc/Framework/Cla...
File webrtc/sdk/objc/Framework/Classes/RTCI420Shader.mm (right):

https://codereview.webrtc.org/2202823004/diff/1/webrtc/sdk/objc/Framework/Cla...
webrtc/sdk/objc/Framework/Classes/RTCI420Shader.mm:23: static const GLsizei
kNumTextures = 3 * kNumTextureSets;
On 2016/08/03 10:52:59, sakal wrote:
> I would prefer 3 be a constant like kNumTexturesPerSet.

Done.

https://codereview.webrtc.org/2202823004/diff/1/webrtc/sdk/objc/Framework/Cla...
webrtc/sdk/objc/Framework/Classes/RTCI420Shader.mm:81: if (_i420Program != 0) {
On 2016/08/03 10:52:59, sakal wrote:
> These if statements are unnecessary. OpenGL silently ignores trying to delete
> something with index 0.

Done.

https://codereview.webrtc.org/2202823004/diff/1/webrtc/sdk/objc/Framework/Cla...
webrtc/sdk/objc/Framework/Classes/RTCI420Shader.mm:111:
glActiveTexture(GL_TEXTURE0 + i);
On 2016/08/03 10:52:59, sakal wrote:
> I don't think this is needed. We can just set up the textures using any active
> texture.

Done.

https://codereview.webrtc.org/2202823004/diff/1/webrtc/sdk/objc/Framework/Cla...
webrtc/sdk/objc/Framework/Classes/RTCI420Shader.mm:141:
glActiveTexture(static_cast<GLenum>(GL_TEXTURE0 + offset));
On 2016/08/03 10:52:59, sakal wrote:
> Is it not possible to just always use GL_TEXTURE0-2 or does that cause CPU
> blockage? If we only used GL_TEXTURE0-2 that would allow kNumTextures be over
8,
> right?

See comment at top of file.

https://codereview.webrtc.org/2202823004/diff/1/webrtc/sdk/objc/Framework/Cla...
File webrtc/sdk/objc/Framework/Classes/RTCNativeNV12Shader.mm (right):

https://codereview.webrtc.org/2202823004/diff/1/webrtc/sdk/objc/Framework/Cla...
webrtc/sdk/objc/Framework/Classes/RTCNativeNV12Shader.mm:61: if (_nv12Program !=
0) {
On 2016/08/03 10:52:59, sakal wrote:
> These if statements are unnecessary. OpenGL silently ignores trying to delete
> something with index 0.

Done.

[tkchin_webrtc, 2016-08-04 00:11:06]

On 2016/08/03 17:53:19, tkchin_webrtc wrote:
> Would prefer to preserve as much as old behavior as possible at this point to
> reduce risk. Old behavior for RTCI420Shader is known to be stable.
> 
>
https://codereview.webrtc.org/2202823004/diff/1/webrtc/sdk/objc/Framework/Cla...
> File webrtc/sdk/objc/Framework/Classes/RTCI420Shader.mm (right):
> 
>
https://codereview.webrtc.org/2202823004/diff/1/webrtc/sdk/objc/Framework/Cla...
> webrtc/sdk/objc/Framework/Classes/RTCI420Shader.mm:23: static const GLsizei
> kNumTextures = 3 * kNumTextureSets;
> On 2016/08/03 10:52:59, sakal wrote:
> > I would prefer 3 be a constant like kNumTexturesPerSet.
> 
> Done.
> 
>
https://codereview.webrtc.org/2202823004/diff/1/webrtc/sdk/objc/Framework/Cla...
> webrtc/sdk/objc/Framework/Classes/RTCI420Shader.mm:81: if (_i420Program != 0)
{
> On 2016/08/03 10:52:59, sakal wrote:
> > These if statements are unnecessary. OpenGL silently ignores trying to
delete
> > something with index 0.
> 
> Done.
> 
>
https://codereview.webrtc.org/2202823004/diff/1/webrtc/sdk/objc/Framework/Cla...
> webrtc/sdk/objc/Framework/Classes/RTCI420Shader.mm:111:
> glActiveTexture(GL_TEXTURE0 + i);
> On 2016/08/03 10:52:59, sakal wrote:
> > I don't think this is needed. We can just set up the textures using any
active
> > texture.
> 
> Done.
> 
>
https://codereview.webrtc.org/2202823004/diff/1/webrtc/sdk/objc/Framework/Cla...
> webrtc/sdk/objc/Framework/Classes/RTCI420Shader.mm:141:
> glActiveTexture(static_cast<GLenum>(GL_TEXTURE0 + offset));
> On 2016/08/03 10:52:59, sakal wrote:
> > Is it not possible to just always use GL_TEXTURE0-2 or does that cause CPU
> > blockage? If we only used GL_TEXTURE0-2 that would allow kNumTextures be
over
> 8,
> > right?
> 
> See comment at top of file.
> 
>
https://codereview.webrtc.org/2202823004/diff/1/webrtc/sdk/objc/Framework/Cla...
> File webrtc/sdk/objc/Framework/Classes/RTCNativeNV12Shader.mm (right):
> 
>
https://codereview.webrtc.org/2202823004/diff/1/webrtc/sdk/objc/Framework/Cla...
> webrtc/sdk/objc/Framework/Classes/RTCNativeNV12Shader.mm:61: if (_nv12Program
!=
> 0) {
> On 2016/08/03 10:52:59, sakal wrote:
> > These if statements are unnecessary. OpenGL silently ignores trying to
delete
> > something with index 0.
> 
> Done.

Tested on:
iPhone4S iOS 9.3
iPhone5S iOS10 beta
iPhone6S iOS 9.3
iPhone6 iOS8.3
iPad Mini non-Retina iOS9.3

using AppRTCDemo
for both VPX / h264.

[sakal, 2016-08-04 07:01:26]

lgtm

[mflodman, 2016-08-04 09:40:02]

LGTM

Note though it has been a long time since I wrote/reviewed OpenGL and
Objective-C code.

[tkchin_webrtc, 2016-08-04 17:33:53]

On 2016/08/04 09:40:02, mflodman wrote:
> LGTM
> 
> Note though it has been a long time since I wrote/reviewed OpenGL and
> Objective-C code.

Since this is no longer immediately urgent, I'll wait for magjed@ to also take a
look.

[magjed_webrtc, 2016-08-08 09:29:22]

Thanks for splitting out the shaders in their own files and addressing your own
comments :) I originally had the shaders in the same file to make the diff as
small as possible to make it easier to review, but this is better.

Please note that this CL will only accelerate rendering when frame rotation is
0, i.e. one out of four possible device orientations, because the shaders don't
support rotation yet, and RTCVideoRendererAdapter.mm will convert the frames to
I420 if rotation != 0. I have a follow-up CL to add rotation support to the
shaders here: https://codereview.webrtc.org/2176623002/

lgtm. I'm sending this to the CQ.

[magjed_webrtc, 2016-08-08 09:29:30]

The CQ bit was checked by magjed@webrtc.org

[commit-bot: I haz the power, 2016-08-08 09:29:36]

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-08 10:10:10]

Description was changed from

==========
iOS: Add support for rendering native CVPixelBuffers directly

This CL adds support in RTCEAGLVideoView for rendering CVPixelBuffers as
OpenGL ES textures directly, compared to the current code that first
converts the CVPixelBuffers to I420, and then reuploads them as
textures. This is only supported on iOS with the use of a
CVOpenGLESTextureCache.

The I420 rendering and native rendering are separated in two different
implementations of a simple shader interface:
@protocol Shader
- (BOOL)drawFrame:(RTCVideoFrame*)frame;
@end
GL resources are allocated when the shader is instantiated and released
when the shader is destroyed. RTCEAGLVideoView will lazily instantiate
the necessary shader when it receives the first frame of that kind. This
is primarily done to avoid allocating GL resources for both I420 and
native rendering.

Some other changes are:
 - Print GL shader compilation errors.
 - Remove updateTextureSizesForFrame() function. The textures will
   resize automatically anyway when the texture data is uploaded with
   glTexImage2D().

patch from issue 2154243002 at patchset 140001
(http://crrev.com/2154243002#ps140001)
Continuing magjed@'s work since he is OOO this week.

BUG=
==========

to

==========
iOS: Add support for rendering native CVPixelBuffers directly

This CL adds support in RTCEAGLVideoView for rendering CVPixelBuffers as
OpenGL ES textures directly, compared to the current code that first
converts the CVPixelBuffers to I420, and then reuploads them as
textures. This is only supported on iOS with the use of a
CVOpenGLESTextureCache.

The I420 rendering and native rendering are separated in two different
implementations of a simple shader interface:
@protocol Shader
- (BOOL)drawFrame:(RTCVideoFrame*)frame;
@end
GL resources are allocated when the shader is instantiated and released
when the shader is destroyed. RTCEAGLVideoView will lazily instantiate
the necessary shader when it receives the first frame of that kind. This
is primarily done to avoid allocating GL resources for both I420 and
native rendering.

Some other changes are:
 - Print GL shader compilation errors.
 - Remove updateTextureSizesForFrame() function. The textures will
   resize automatically anyway when the texture data is uploaded with
   glTexImage2D().

patch from issue 2154243002 at patchset 140001
(http://crrev.com/2154243002#ps140001)
Continuing magjed@'s work since he is OOO this week.

BUG=
==========

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

Committed patchset #3 (id:40001)

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

Description was changed from

==========
iOS: Add support for rendering native CVPixelBuffers directly

This CL adds support in RTCEAGLVideoView for rendering CVPixelBuffers as
OpenGL ES textures directly, compared to the current code that first
converts the CVPixelBuffers to I420, and then reuploads them as
textures. This is only supported on iOS with the use of a
CVOpenGLESTextureCache.

The I420 rendering and native rendering are separated in two different
implementations of a simple shader interface:
@protocol Shader
- (BOOL)drawFrame:(RTCVideoFrame*)frame;
@end
GL resources are allocated when the shader is instantiated and released
when the shader is destroyed. RTCEAGLVideoView will lazily instantiate
the necessary shader when it receives the first frame of that kind. This
is primarily done to avoid allocating GL resources for both I420 and
native rendering.

Some other changes are:
 - Print GL shader compilation errors.
 - Remove updateTextureSizesForFrame() function. The textures will
   resize automatically anyway when the texture data is uploaded with
   glTexImage2D().

patch from issue 2154243002 at patchset 140001
(http://crrev.com/2154243002#ps140001)
Continuing magjed@'s work since he is OOO this week.

BUG=
==========

to

==========
iOS: Add support for rendering native CVPixelBuffers directly

This CL adds support in RTCEAGLVideoView for rendering CVPixelBuffers as
OpenGL ES textures directly, compared to the current code that first
converts the CVPixelBuffers to I420, and then reuploads them as
textures. This is only supported on iOS with the use of a
CVOpenGLESTextureCache.

The I420 rendering and native rendering are separated in two different
implementations of a simple shader interface:
@protocol Shader
- (BOOL)drawFrame:(RTCVideoFrame*)frame;
@end
GL resources are allocated when the shader is instantiated and released
when the shader is destroyed. RTCEAGLVideoView will lazily instantiate
the necessary shader when it receives the first frame of that kind. This
is primarily done to avoid allocating GL resources for both I420 and
native rendering.

Some other changes are:
 - Print GL shader compilation errors.
 - Remove updateTextureSizesForFrame() function. The textures will
   resize automatically anyway when the texture data is uploaded with
   glTexImage2D().

patch from issue 2154243002 at patchset 140001
(http://crrev.com/2154243002#ps140001)
Continuing magjed@'s work since he is OOO this week.

BUG=

Committed: https://crrev.com/04dbb34d51aa0dfc458f29c220bf770504cd52d3
Cr-Commit-Position: refs/heads/master@{#13668}
==========

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

Patchset 3 (id:??) landed as
https://crrev.com/04dbb34d51aa0dfc458f29c220bf770504cd52d3
Cr-Commit-Position: refs/heads/master@{#13668}