iOS: Add interface for injecting custom shaders

ObjC: Add interface for injecting custom shaders in video views

This CL adds a way for external clients to inject their own OpenGL(ES)
shaders to RTCEAGLVideoView/RTCNSGLVideoView. The shader interface
takes textures as arguments, and not RTCVideoFrame, so that
implementations only has to deal with actual OpenGL rendering, and not
converting frames into textures.

This CL also moves the internal shader code around a bit. The current
RTCShader interface with the implementations RTCI420Shader and
RTCNativeNV12Shader are removed. RTCEAGLVideoView and RTCNSGLVideoView
will be responsible for uploading the frames to textures instead
using the helper classes RTCI420TextureCache and RTCNV12TextureCache.
They then call the shader implementation with these textures. The
rendering code that used to be in RTCI420Shader and RTCNativeNV12Shader
have been merged into one RTCDefaultShaderDelegate class.

BUG=webrtc:7473
Review-Url: https://codereview.webrtc.org/2869143002
Cr-Commit-Position: refs/heads/master@{#18326}
Committed: https://chromium.googlesource.com/external/webrtc/+/13941912b16b522d37c643e0c4413ce378c817f8

[magjed_webrtc, 2017-05-09 19:16:42]

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

[commit-bot: I haz the power, 2017-05-09 19:16:51]

Dry run: 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, 2017-05-09 19:42:11]

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

[commit-bot: I haz the power, 2017-05-09 19:42:12]

Dry run: This issue passed the CQ dry run.

[magjed_webrtc, 2017-05-10 13:46:12]

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

[commit-bot: I haz the power, 2017-05-10 13:46:23]

Dry run: CQ is trying da patch.

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

[magjed_webrtc, 2017-05-10 13:57:21]

Description was changed from

==========
iOS: Add interface for injecting custom shaders

BUG=webrtc:7473
==========

to

==========
ObjC: Add interface for injecting custom shaders in video views

This CL adds a way for external clients to inject their own OpenGL(ES)
shaders to RTCEAGLVideoView/RTCNSGLVideoView. The shader interface
takes textures as arguments, and not RTCVideoFrame, so that
implementations only has to deal with rendering, and not converting
frames into textures.

This CL also moves the internal shader code around a bit. The current
RTCShader interface with the implementations RTCI420Shader and
RTCNativeNV12Shader are removed. RTCEAGLVideoView and RTCNSGLVideoView
will be responsible for uploading the frames to textures instead
using the helper classes RTCI420TextureCache and RTCNV12TextureCache.
They then call the shader implementation with these textures. The
rendering code that used to be in RTCI420Shader and RTCNativeNV12Shader
have been merged into one RTCDefaultShaderDelegate class.

BUG=webrtc:7473
==========

[magjed_webrtc, 2017-05-10 13:57:32]

Patchset #2 (id:20001) has been deleted

[magjed_webrtc, 2017-05-10 13:57:46]

Patchset #1 (id:1) has been deleted

[magjed_webrtc, 2017-05-10 13:59:01]

Description was changed from

==========
ObjC: Add interface for injecting custom shaders in video views

This CL adds a way for external clients to inject their own OpenGL(ES)
shaders to RTCEAGLVideoView/RTCNSGLVideoView. The shader interface
takes textures as arguments, and not RTCVideoFrame, so that
implementations only has to deal with rendering, and not converting
frames into textures.

This CL also moves the internal shader code around a bit. The current
RTCShader interface with the implementations RTCI420Shader and
RTCNativeNV12Shader are removed. RTCEAGLVideoView and RTCNSGLVideoView
will be responsible for uploading the frames to textures instead
using the helper classes RTCI420TextureCache and RTCNV12TextureCache.
They then call the shader implementation with these textures. The
rendering code that used to be in RTCI420Shader and RTCNativeNV12Shader
have been merged into one RTCDefaultShaderDelegate class.

BUG=webrtc:7473
==========

to

==========
ObjC: Add interface for injecting custom shaders in video views

This CL adds a way for external clients to inject their own OpenGL(ES)
shaders to RTCEAGLVideoView/RTCNSGLVideoView. The shader interface
takes textures as arguments, and not RTCVideoFrame, so that
implementations only has to deal with actual OpenGL rendering, and not
converting frames into textures.

This CL also moves the internal shader code around a bit. The current
RTCShader interface with the implementations RTCI420Shader and
RTCNativeNV12Shader are removed. RTCEAGLVideoView and RTCNSGLVideoView
will be responsible for uploading the frames to textures instead
using the helper classes RTCI420TextureCache and RTCNV12TextureCache.
They then call the shader implementation with these textures. The
rendering code that used to be in RTCI420Shader and RTCNativeNV12Shader
have been merged into one RTCDefaultShaderDelegate class.

BUG=webrtc:7473
==========

[magjed_webrtc, 2017-05-10 14:04:55]

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

[magjed_webrtc, 2017-05-10 14:05:07]

Sami - please take a look.

[commit-bot: I haz the power, 2017-05-10 14:16:35]

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

[commit-bot: I haz the power, 2017-05-10 14:16:36]

Dry run: This issue passed the CQ dry run.

[sakal, 2017-05-11 10:45:47]

lgtm

https://codereview.webrtc.org/2869143002/diff/40001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Classes/RTCDefaultShaderDelegate.mm (right):

https://codereview.webrtc.org/2869143002/diff/40001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/RTCDefaultShaderDelegate.mm:99:
glDeleteProgram(_i420Program);
Log error?

https://codereview.webrtc.org/2869143002/diff/40001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Classes/RTCShader.mm (right):

https://codereview.webrtc.org/2869143002/diff/40001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/RTCShader.mm:137: return NO;
Delete vertex array if we created one?

https://codereview.webrtc.org/2869143002/diff/40001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCEAGLVideoView.h (right):

https://codereview.webrtc.org/2869143002/diff/40001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Headers/WebRTC/RTCEAGLVideoView.h:37: -
(instancetype)initWithFrame:(CGRect)frame
Should we mark the default initializer as unavailable?

https://codereview.webrtc.org/2869143002/diff/40001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCNSGLVideoView.h (right):

https://codereview.webrtc.org/2869143002/diff/40001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Headers/WebRTC/RTCNSGLVideoView.h:33: -
(instancetype)initWithFrame:(NSRect)frameRect
Should we mark the default initializer as unavailable?

[magjed_webrtc, 2017-05-11 12:38:07]

https://codereview.webrtc.org/2869143002/diff/40001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Classes/RTCDefaultShaderDelegate.mm (right):

https://codereview.webrtc.org/2869143002/diff/40001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/RTCDefaultShaderDelegate.mm:99:
glDeleteProgram(_i420Program);
On 2017/05/11 10:45:47, sakal wrote:
> Log error?

Done.

https://codereview.webrtc.org/2869143002/diff/40001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Classes/RTCShader.mm (right):

https://codereview.webrtc.org/2869143002/diff/40001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/RTCShader.mm:137: return NO;
On 2017/05/11 10:45:47, sakal wrote:
> Delete vertex array if we created one?

Done.

https://codereview.webrtc.org/2869143002/diff/40001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCEAGLVideoView.h (right):

https://codereview.webrtc.org/2869143002/diff/40001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Headers/WebRTC/RTCEAGLVideoView.h:37: -
(instancetype)initWithFrame:(CGRect)frame
On 2017/05/11 10:45:47, sakal wrote:
> Should we mark the default initializer as unavailable?

The default initializers are still available. That's where we create the
DefaultShader and call these initializers.

https://codereview.webrtc.org/2869143002/diff/40001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCNSGLVideoView.h (right):

https://codereview.webrtc.org/2869143002/diff/40001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Headers/WebRTC/RTCNSGLVideoView.h:33: -
(instancetype)initWithFrame:(NSRect)frameRect
On 2017/05/11 10:45:47, sakal wrote:
> Should we mark the default initializer as unavailable?

The default initializers are still available.

[magjed_webrtc, 2017-05-11 12:38:16]

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

[magjed_webrtc, 2017-05-11 12:38:29]

Daniela - please take a look.

[daniela-webrtc, 2017-05-11 17:31:21]

https://codereview.webrtc.org/2869143002/diff/60001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Classes/RTCDefaultShaderDelegate.h (right):

https://codereview.webrtc.org/2869143002/diff/60001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/RTCDefaultShaderDelegate.h:15: @interface
RTCDefaultShaderDelegate : NSObject <RTCVideoViewShaderDelegate>
Do you mind adding short doc comment about this class? The name is quite generic
so something along the lines
'Default shader for i420 and nv12 frames' should be sufficient.

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

https://codereview.webrtc.org/2869143002/diff/60001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/RTCDefaultShaderDelegate.mm:90: _i420Program =
RTCCreateProgramFromFragmentSource(kI420FragmentShaderSource);
Should we check if the program already exists? If not, can it at least be
renamed to `createAndSetupI420Program`?

https://codereview.webrtc.org/2869143002/diff/60001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/RTCDefaultShaderDelegate.mm:114: _nv12Program
= RTCCreateProgramFromFragmentSource(kNV12FragmentShaderSource);
^

https://codereview.webrtc.org/2869143002/diff/60001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Classes/RTCEAGLVideoView.m (right):

https://codereview.webrtc.org/2869143002/diff/60001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/RTCEAGLVideoView.m:102:
RTCDefaultShaderDelegate *_defaultShaderDelegate;
I find this slightly confusing.  When is the defaultShaderDelegate used?

https://codereview.webrtc.org/2869143002/diff/60001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/RTCEAGLVideoView.m:234: [_shaderDelegate
videoView:self
What if this is nil? Shouldn't the defaultDelegate be used here?

https://codereview.webrtc.org/2869143002/diff/60001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Classes/RTCNSGLVideoView.m (right):

https://codereview.webrtc.org/2869143002/diff/60001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/RTCNSGLVideoView.m:148: [_shaderDelegate
videoView:self
What if this is nil? Shouldn't the defaultDelegate be used here?

https://codereview.webrtc.org/2869143002/diff/60001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCVideoViewShaderDelegate.h
(right):

https://codereview.webrtc.org/2869143002/diff/60001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Headers/WebRTC/RTCVideoViewShaderDelegate.h:30:
@protocol RTCVideoViewShaderDelegate
I'm bit on the fence here if this should be a delegate protocol. Is it necessary
to pass the RTCVideoView in the callback? Is it relevant for the shader?

https://codereview.webrtc.org/2869143002/diff/60001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Headers/WebRTC/RTCVideoViewShaderDelegate.h:34:
didReceiveFrameWithRotation:(RTCVideoRotation)rotation
didUpload seems like better naming, but see my previous comment.

https://codereview.webrtc.org/2869143002/diff/60001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Headers/WebRTC/RTCVideoViewShaderDelegate.h:41:
didReceiveFrameWithRotation:(RTCVideoRotation)rotation
^

[magjed_webrtc, 2017-05-25 14:26:46]

https://codereview.webrtc.org/2869143002/diff/60001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Classes/RTCDefaultShaderDelegate.h (right):

https://codereview.webrtc.org/2869143002/diff/60001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/RTCDefaultShaderDelegate.h:15: @interface
RTCDefaultShaderDelegate : NSObject <RTCVideoViewShaderDelegate>
On 2017/05/11 17:31:20, daniela-webrtc wrote:
> Do you mind adding short doc comment about this class? The name is quite
generic
> so something along the lines
> 'Default shader for i420 and nv12 frames' should be sufficient.

Done.

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

https://codereview.webrtc.org/2869143002/diff/60001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/RTCDefaultShaderDelegate.mm:90: _i420Program =
RTCCreateProgramFromFragmentSource(kI420FragmentShaderSource);
On 2017/05/11 17:31:20, daniela-webrtc wrote:
> Should we check if the program already exists? If not, can it at least be
> renamed to `createAndSetupI420Program`?

Added assert and changed the name.

https://codereview.webrtc.org/2869143002/diff/60001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/RTCDefaultShaderDelegate.mm:114: _nv12Program
= RTCCreateProgramFromFragmentSource(kNV12FragmentShaderSource);
On 2017/05/11 17:31:20, daniela-webrtc wrote:
> ^

Done.

https://codereview.webrtc.org/2869143002/diff/60001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Classes/RTCEAGLVideoView.m (right):

https://codereview.webrtc.org/2869143002/diff/60001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/RTCEAGLVideoView.m:102:
RTCDefaultShaderDelegate *_defaultShaderDelegate;
On 2017/05/11 17:31:20, daniela-webrtc wrote:
> I find this slightly confusing.  When is the defaultShaderDelegate used?

I added a comment:
|_defaultShaderDelegate| is used only when no external shader is provided in the
init function, and the only purpose is to keep the shader alive by having a
strong reference. Shaders will only be accessed through |_shaderDelegate|.

https://codereview.webrtc.org/2869143002/diff/60001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCVideoViewShaderDelegate.h
(right):

https://codereview.webrtc.org/2869143002/diff/60001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Headers/WebRTC/RTCVideoViewShaderDelegate.h:30:
@protocol RTCVideoViewShaderDelegate
On 2017/05/11 17:31:21, daniela-webrtc wrote:
> I'm bit on the fence here if this should be a delegate protocol. Is it
necessary
> to pass the RTCVideoView in the callback? Is it relevant for the shader?

Yeah, I need your expertise here - what's the best/conventional way for doing
this in ObjC? The RTCVideoView argument will probably never be relevant, I just
added it because I thought it was customary. What options do I have besides a
delegate protocol in ObjC? I want clients to be able to customize how shading is
done in the VideoView classes.

[daniela-webrtc, 2017-05-26 08:31:18]

https://codereview.webrtc.org/2869143002/diff/60001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCVideoViewShaderDelegate.h
(right):

https://codereview.webrtc.org/2869143002/diff/60001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Headers/WebRTC/RTCVideoViewShaderDelegate.h:30:
@protocol RTCVideoViewShaderDelegate
On 2017/05/25 14:26:45, magjed_webrtc wrote:
> On 2017/05/11 17:31:21, daniela-webrtc wrote:
> > I'm bit on the fence here if this should be a delegate protocol. Is it
> necessary
> > to pass the RTCVideoView in the callback? Is it relevant for the shader?
> 
> Yeah, I need your expertise here - what's the best/conventional way for doing
> this in ObjC? The RTCVideoView argument will probably never be relevant, I
just
> added it because I thought it was customary. What options do I have besides a
> delegate protocol in ObjC? I want clients to be able to customize how shading
is
> done in the VideoView classes.

I see. If the video view is not relevant, we can ditch the delegate pattern. But
we can still use protocol i.e modify this one.
```
@protocol RTCShading : NSObject
- (void)applyShadingForFrameWithRotation:(RTCVideoRotation)rotation
yPlane:(GLuint)yPlane uPlane:(GLuint)uPlane vPlane:(GLuint)vPlane;
- (void)applyShadingForFrameWithRotation:(RTCVideoRotation)rotation
yPlane:(GLuint)yPlane uvPlane:(GLuint)uvPlane;

```

Then in RTC*VideoView we could have only one shader instance 
 id<RTCShading> *_shader;

and it could be injected via the initializer like it's currently implemented.
The difference would be that we wouldn't need the separate
_defaultShaderDelegate. But instead we could have something like this in the
initializer

```
- (instancetype)initWithFrame:(CGRect)frame shader:(nullable id<RTCShading
*>)shader {
  if (self = [super initWithFrame:frame) {
     _shader = shader ? shader : [[RTCDefaultShader alloc] init];  // We could
ditch the delegate suffix everywhere.
  }
  return self;
}

Let me know if you need further clarification.

[magjed_webrtc, 2017-05-26 12:05:26]

https://codereview.webrtc.org/2869143002/diff/60001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCVideoViewShaderDelegate.h
(right):

https://codereview.webrtc.org/2869143002/diff/60001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Headers/WebRTC/RTCVideoViewShaderDelegate.h:30:
@protocol RTCVideoViewShaderDelegate
On 2017/05/26 08:31:18, daniela-webrtc wrote:
> On 2017/05/25 14:26:45, magjed_webrtc wrote:
> > On 2017/05/11 17:31:21, daniela-webrtc wrote:
> > > I'm bit on the fence here if this should be a delegate protocol. Is it
> > necessary
> > > to pass the RTCVideoView in the callback? Is it relevant for the shader?
> > 
> > Yeah, I need your expertise here - what's the best/conventional way for
doing
> > this in ObjC? The RTCVideoView argument will probably never be relevant, I
> just
> > added it because I thought it was customary. What options do I have besides
a
> > delegate protocol in ObjC? I want clients to be able to customize how
shading
> is
> > done in the VideoView classes.
> 
> I see. If the video view is not relevant, we can ditch the delegate pattern.
But
> we can still use protocol i.e modify this one.
> ```
> @protocol RTCShading : NSObject
> - (void)applyShadingForFrameWithRotation:(RTCVideoRotation)rotation
> yPlane:(GLuint)yPlane uPlane:(GLuint)uPlane vPlane:(GLuint)vPlane;
> - (void)applyShadingForFrameWithRotation:(RTCVideoRotation)rotation
> yPlane:(GLuint)yPlane uvPlane:(GLuint)uvPlane;
> 
> ```
> 
> Then in RTC*VideoView we could have only one shader instance 
>  id<RTCShading> *_shader;
> 
> and it could be injected via the initializer like it's currently implemented.
> The difference would be that we wouldn't need the separate
> _defaultShaderDelegate. But instead we could have something like this in the
> initializer
> 
> ```
> - (instancetype)initWithFrame:(CGRect)frame shader:(nullable id<RTCShading
> *>)shader {
>   if (self = [super initWithFrame:frame) {
>      _shader = shader ? shader : [[RTCDefaultShader alloc] init];  // We could
> ditch the delegate suffix everywhere.
>   }
>   return self;
> }
> 
> Let me know if you need further clarification.

Done.

[daniela-webrtc, 2017-05-26 13:48:28]

https://codereview.webrtc.org/2869143002/diff/100001/webrtc/sdk/objc/Framewor...
File webrtc/sdk/objc/Framework/Classes/RTCEAGLVideoView.m (right):

https://codereview.webrtc.org/2869143002/diff/100001/webrtc/sdk/objc/Framewor...
webrtc/sdk/objc/Framework/Classes/RTCEAGLVideoView.m:120: -
(instancetype)initWithFrame:(CGRect)frame
shaderDelegate:(id<RTCVideoViewShader>)shaderDelegate {
The Delegate pre/suffix is not relevant any more.

https://codereview.webrtc.org/2869143002/diff/100001/webrtc/sdk/objc/Framewor...
File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCVideoViewShader.h (right):

https://codereview.webrtc.org/2869143002/diff/100001/webrtc/sdk/objc/Framewor...
webrtc/sdk/objc/Framework/Headers/WebRTC/RTCVideoViewShader.h:22: @protocol
RTCVideoViewShader
It's common for protocols that encapsulate certain logical functionality and
that are not necessarily tied to certain class to use gerund form (...ing).
For instance NSCopying, NSLocking etc.
Also this should extend the NSObject protocol. (Named without ing because it
closely encapsulates functionality tied to the NSObject class) :)
So something like
@protocol RTCVideoViewShading : NSObject
@end

[magjed_webrtc, 2017-05-26 14:35:38]

Patchset #5 (id:120001) has been deleted

[magjed_webrtc, 2017-05-26 14:36:10]

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

[commit-bot: I haz the power, 2017-05-26 14:36:19]

Dry run: CQ is trying da patch.

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

[magjed_webrtc, 2017-05-26 14:41:35]

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

[commit-bot: I haz the power, 2017-05-26 14:41:39]

Dry run: 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, 2017-05-26 15:03:42]

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

[commit-bot: I haz the power, 2017-05-26 15:03:43]

Dry run: This issue passed the CQ dry run.

[magjed_webrtc, 2017-05-28 10:20:36]

Patchset #5 (id:140001) has been deleted

[magjed_webrtc, 2017-05-28 10:23:46]

Patchset #5 (id:160001) has been deleted

[magjed_webrtc, 2017-05-28 10:26:07]

https://codereview.webrtc.org/2869143002/diff/100001/webrtc/sdk/objc/Framewor...
File webrtc/sdk/objc/Framework/Classes/RTCEAGLVideoView.m (right):

https://codereview.webrtc.org/2869143002/diff/100001/webrtc/sdk/objc/Framewor...
webrtc/sdk/objc/Framework/Classes/RTCEAGLVideoView.m:120: -
(instancetype)initWithFrame:(CGRect)frame
shaderDelegate:(id<RTCVideoViewShader>)shaderDelegate {
On 2017/05/26 13:48:28, daniela-webrtc wrote:
> The Delegate pre/suffix is not relevant any more.

Done.

https://codereview.webrtc.org/2869143002/diff/100001/webrtc/sdk/objc/Framewor...
File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCVideoViewShader.h (right):

https://codereview.webrtc.org/2869143002/diff/100001/webrtc/sdk/objc/Framewor...
webrtc/sdk/objc/Framework/Headers/WebRTC/RTCVideoViewShader.h:22: @protocol
RTCVideoViewShader
On 2017/05/26 13:48:28, daniela-webrtc wrote:
> It's common for protocols that encapsulate certain logical functionality and
> that are not necessarily tied to certain class to use gerund form (...ing).
> For instance NSCopying, NSLocking etc.
> Also this should extend the NSObject protocol. (Named without ing because it
> closely encapsulates functionality tied to the NSObject class) :)
> So something like
> @protocol RTCVideoViewShading : NSObject
> @end

Done.

[daniela-webrtc, 2017-05-29 08:13:46]

lgtm

[magjed_webrtc, 2017-05-29 13:07:16]

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

[commit-bot: I haz the power, 2017-05-29 13:07:28]

Dry run: 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, 2017-05-29 13:09:16]

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

[commit-bot: I haz the power, 2017-05-29 13:09:18]

Dry run: Try jobs failed on following builders:
  linux_more_configs on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_more_configs/buil...)
  mac_baremetal on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_baremetal/builds/21822)
  mac_compile_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_compile_dbg/builds/...)
  presubmit on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/17493)

[magjed_webrtc, 2017-05-29 13:44:09]

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

[commit-bot: I haz the power, 2017-05-29 13:44:16]

Dry run: 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, 2017-05-29 14:01:07]

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

[commit-bot: I haz the power, 2017-05-29 14:01:08]

Dry run: This issue passed the CQ dry run.

[magjed_webrtc, 2017-05-30 11:39:49]

The CQ bit was checked by magjed@webrtc.org

[magjed_webrtc, 2017-05-30 11:39:49]

The patchset sent to the CQ was uploaded after l-g-t-m from sakal@webrtc.org,
denicija@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2869143002/#ps200001 (title:
"Rebase")

[commit-bot: I haz the power, 2017-05-30 11:39:53]

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, 2017-05-30 11:43:22]

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

[commit-bot: I haz the power, 2017-05-30 11:43:23]

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

[magjed_webrtc, 2017-05-30 13:10:02]

The CQ bit was checked by magjed@webrtc.org

[commit-bot: I haz the power, 2017-05-30 13:10:05]

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, 2017-05-30 13:11:58]

CQ is committing da patch.

Bot data: {"patchset_id": 200001, "attempt_start_ts": 1496149802380820,
"parent_rev": "2cc70facaf97c56be39a00901a1c3c9a50d5b146", "commit_rev":
"13941912b16b522d37c643e0c4413ce378c817f8"}

[commit-bot: I haz the power, 2017-05-30 13:12:02]

Description was changed from

==========
ObjC: Add interface for injecting custom shaders in video views

This CL adds a way for external clients to inject their own OpenGL(ES)
shaders to RTCEAGLVideoView/RTCNSGLVideoView. The shader interface
takes textures as arguments, and not RTCVideoFrame, so that
implementations only has to deal with actual OpenGL rendering, and not
converting frames into textures.

This CL also moves the internal shader code around a bit. The current
RTCShader interface with the implementations RTCI420Shader and
RTCNativeNV12Shader are removed. RTCEAGLVideoView and RTCNSGLVideoView
will be responsible for uploading the frames to textures instead
using the helper classes RTCI420TextureCache and RTCNV12TextureCache.
They then call the shader implementation with these textures. The
rendering code that used to be in RTCI420Shader and RTCNativeNV12Shader
have been merged into one RTCDefaultShaderDelegate class.

BUG=webrtc:7473
==========

to

==========
ObjC: Add interface for injecting custom shaders in video views

This CL adds a way for external clients to inject their own OpenGL(ES)
shaders to RTCEAGLVideoView/RTCNSGLVideoView. The shader interface
takes textures as arguments, and not RTCVideoFrame, so that
implementations only has to deal with actual OpenGL rendering, and not
converting frames into textures.

This CL also moves the internal shader code around a bit. The current
RTCShader interface with the implementations RTCI420Shader and
RTCNativeNV12Shader are removed. RTCEAGLVideoView and RTCNSGLVideoView
will be responsible for uploading the frames to textures instead
using the helper classes RTCI420TextureCache and RTCNV12TextureCache.
They then call the shader implementation with these textures. The
rendering code that used to be in RTCI420Shader and RTCNativeNV12Shader
have been merged into one RTCDefaultShaderDelegate class.

BUG=webrtc:7473

Review-Url: https://codereview.webrtc.org/2869143002
Cr-Commit-Position: refs/heads/master@{#18326}
Committed:
https://chromium.googlesource.com/external/webrtc/+/13941912b16b522d37c643e0c...
==========

[commit-bot: I haz the power, 2017-05-30 13:12:04]

Committed patchset #6 (id:200001) as
https://chromium.googlesource.com/external/webrtc/+/13941912b16b522d37c643e0c...