MacOS: Add metal renderer and view.

MacOS: Add metal renderer and view.

BUG=webrtc:7079
Review-Url: https://codereview.webrtc.org/2778693003
Cr-Commit-Position: refs/heads/master@{#17482}
Committed: https://chromium.googlesource.com/external/webrtc/+/124a6fcddfdf80112b7c7687d657d2fb36c9b6e7

[daniela-webrtc, 2017-03-27 12:19:19]

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

[magjed_webrtc, 2017-03-28 11:20:29]

https://codereview.webrtc.org/2778693003/diff/20001/webrtc/examples/objc/AppR...
File webrtc/examples/objc/AppRTCMobile/mac/APPRTCViewController.m (right):

https://codereview.webrtc.org/2778693003/diff/20001/webrtc/examples/objc/AppR...
webrtc/examples/objc/AppRTCMobile/mac/APPRTCViewController.m:181:
didChangeVideoSize:(NSSize)size {
Indentation must be wrong here?

https://codereview.webrtc.org/2778693003/diff/20001/webrtc/examples/objc/AppR...
webrtc/examples/objc/AppRTCMobile/mac/APPRTCViewController.m:217:
NSOpenGLPixelFormatAttribute attributes[] = {
Move this and |pixelFormat| into the else-statement below.

https://codereview.webrtc.org/2778693003/diff/20001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Classes/Metal/RTCMTLI420Renderer.mm (right):

https://codereview.webrtc.org/2778693003/diff/20001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/Metal/RTCMTLI420Renderer.mm:32: static const
float cubeVertexData[64] = {
We should try to factor out the common code in this file and
RTCMTLNV12Renderer.mm with e.g. a shared base class, but you can do that in a
follow-up CL.

https://codereview.webrtc.org/2778693003/diff/20001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/Metal/RTCMTLI420Renderer.mm:302: [_yTexture
replaceRegion:MTLRegionMake2D(0, 0, _width, _height)
You need to take into account the strides here, i.e. frame.strideY.

https://codereview.webrtc.org/2778693003/diff/20001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/Metal/RTCMTLI420Renderer.mm:308: (_chromaWidth
!= (int)frame.width / 2 && _chromaHeight != (int)frame.height / 2)) {
This should be int already, why do you need to cast it?

https://codereview.webrtc.org/2778693003/diff/20001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Classes/Metal/RTCMTLNSVideoView.m (right):

https://codereview.webrtc.org/2778693003/diff/20001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/Metal/RTCMTLNSVideoView.m:111: self.videoFrame
= [frame newI420VideoFrame];
We should look at frame.nativeHandle instead and decide what renderer to use
(NV12 vs I420).

[daniela-webrtc, 2017-03-28 14:58:05]

https://codereview.webrtc.org/2778693003/diff/20001/webrtc/examples/objc/AppR...
File webrtc/examples/objc/AppRTCMobile/mac/APPRTCViewController.m (right):

https://codereview.webrtc.org/2778693003/diff/20001/webrtc/examples/objc/AppR...
webrtc/examples/objc/AppRTCMobile/mac/APPRTCViewController.m:181:
didChangeVideoSize:(NSSize)size {
On 2017/03/28 11:20:29, magjed_webrtc wrote:
> Indentation must be wrong here?

Done.

https://codereview.webrtc.org/2778693003/diff/20001/webrtc/examples/objc/AppR...
webrtc/examples/objc/AppRTCMobile/mac/APPRTCViewController.m:217:
NSOpenGLPixelFormatAttribute attributes[] = {
On 2017/03/28 11:20:29, magjed_webrtc wrote:
> Move this and |pixelFormat| into the else-statement below.

Done.

https://codereview.webrtc.org/2778693003/diff/20001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Classes/Metal/RTCMTLI420Renderer.mm (right):

https://codereview.webrtc.org/2778693003/diff/20001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/Metal/RTCMTLI420Renderer.mm:302: [_yTexture
replaceRegion:MTLRegionMake2D(0, 0, _width, _height)
On 2017/03/28 11:20:29, magjed_webrtc wrote:
> You need to take into account the strides here, i.e. frame.strideY.

Done.

https://codereview.webrtc.org/2778693003/diff/20001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/Metal/RTCMTLI420Renderer.mm:308: (_chromaWidth
!= (int)frame.width / 2 && _chromaHeight != (int)frame.height / 2)) {
On 2017/03/28 11:20:29, magjed_webrtc wrote:
> This should be int already, why do you need to cast it?

Done.

https://codereview.webrtc.org/2778693003/diff/20001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Classes/Metal/RTCMTLNSVideoView.m (right):

https://codereview.webrtc.org/2778693003/diff/20001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/Metal/RTCMTLNSVideoView.m:111: self.videoFrame
= [frame newI420VideoFrame];
On 2017/03/28 11:20:29, magjed_webrtc wrote:
> We should look at frame.nativeHandle instead and decide what renderer to use
> (NV12 vs I420).

Is it ok to fix this in upcoming CL when I do  the extraction in super class.
NV12Renderer will cause some compiling problems at its current state

[magjed_webrtc, 2017-03-30 08:29:28]

https://codereview.webrtc.org/2778693003/diff/20001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Classes/Metal/RTCMTLNSVideoView.m (right):

https://codereview.webrtc.org/2778693003/diff/20001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/Metal/RTCMTLNSVideoView.m:111: self.videoFrame
= [frame newI420VideoFrame];
On 2017/03/28 14:58:05, daniela-webrtc wrote:
> On 2017/03/28 11:20:29, magjed_webrtc wrote:
> > We should look at frame.nativeHandle instead and decide what renderer to use
> > (NV12 vs I420).
> 
> Is it ok to fix this in upcoming CL when I do  the extraction in super class.
> NV12Renderer will cause some compiling problems at its current state

Yes, that's fine.

https://codereview.webrtc.org/2778693003/diff/40001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Classes/Metal/RTCMTLI420Renderer.h (right):

https://codereview.webrtc.org/2778693003/diff/40001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/Metal/RTCMTLI420Renderer.h:19: @protocol
RTCMTLRenderer <NSObject>
We already have very similar protocols in RTCMTLNV12Renderer.h and RTCShader.h.
Can we consolidate them?

[daniela-webrtc, 2017-03-31 08:10:49]

On 2017/03/30 08:29:28, magjed_webrtc wrote:
>
https://codereview.webrtc.org/2778693003/diff/20001/webrtc/sdk/objc/Framework...
> File webrtc/sdk/objc/Framework/Classes/Metal/RTCMTLNSVideoView.m (right):
> 
>
https://codereview.webrtc.org/2778693003/diff/20001/webrtc/sdk/objc/Framework...
> webrtc/sdk/objc/Framework/Classes/Metal/RTCMTLNSVideoView.m:111:
self.videoFrame
> = [frame newI420VideoFrame];
> On 2017/03/28 14:58:05, daniela-webrtc wrote:
> > On 2017/03/28 11:20:29, magjed_webrtc wrote:
> > > We should look at frame.nativeHandle instead and decide what renderer to
use
> > > (NV12 vs I420).
> > 
> > Is it ok to fix this in upcoming CL when I do  the extraction in super
class.
> > NV12Renderer will cause some compiling problems at its current state
> 
> Yes, that's fine.
> 
>
https://codereview.webrtc.org/2778693003/diff/40001/webrtc/sdk/objc/Framework...
> File webrtc/sdk/objc/Framework/Classes/Metal/RTCMTLI420Renderer.h (right):
> 
>
https://codereview.webrtc.org/2778693003/diff/40001/webrtc/sdk/objc/Framework...
> webrtc/sdk/objc/Framework/Classes/Metal/RTCMTLI420Renderer.h:19: @protocol
> RTCMTLRenderer <NSObject>
> We already have very similar protocols in RTCMTLNV12Renderer.h and
RTCShader.h.
> Can we consolidate them?

Done in follow up CL https://codereview.webrtc.org/2784243003/

[magjed_webrtc, 2017-03-31 08:39:10]

lgtm

[daniela-webrtc, 2017-03-31 09:24:40]

The CQ bit was checked by denicija@webrtc.org

[commit-bot: I haz the power, 2017-03-31 09:24:49]

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-03-31 09:47:30]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490952280809570,
"parent_rev": "515dff40b711c70780612f2a4850e151a28bacde", "commit_rev":
"124a6fcddfdf80112b7c7687d657d2fb36c9b6e7"}

[commit-bot: I haz the power, 2017-03-31 09:47:32]

Description was changed from

==========
MacOS: Add metal renderer and view.

BUG=webrtc:7079
==========

to

==========
MacOS: Add metal renderer and view.

BUG=webrtc:7079

Review-Url: https://codereview.webrtc.org/2778693003
Cr-Commit-Position: refs/heads/master@{#17482}
Committed:
https://chromium.googlesource.com/external/webrtc/+/124a6fcddfdf80112b7c7687d...
==========

[commit-bot: I haz the power, 2017-03-31 09:47:33]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/external/webrtc/+/124a6fcddfdf80112b7c7687d...