Clean up RTCVideoFrame

webrtc/sdk/objc/Framework/Classes/RTCVideoFrame.mm

Index: webrtc/sdk/objc/Framework/Classes/RTCVideoFrame.mm
diff --git a/webrtc/sdk/objc/Framework/Classes/RTCVideoFrame.mm b/webrtc/sdk/objc/Framework/Classes/RTCVideoFrame.mm
index 5805e30cb5fb363c8511e2084f2341b083d5144b..0699323ead366cc8f0846525e3630e62c7b069be 100644
--- a/webrtc/sdk/objc/Framework/Classes/RTCVideoFrame.mm
+++ b/webrtc/sdk/objc/Framework/Classes/RTCVideoFrame.mm
@@ -10,80 +10,46 @@
 
 #import "RTCVideoFrame+Private.h"
 
-#include <memory>
-
-#include "webrtc/api/video/video_rotation.h"
-
 @implementation RTCVideoFrame {
   rtc::scoped_refptr<webrtc::VideoFrameBuffer> _videoBuffer;
-  webrtc::VideoRotation _rotation;
+  int _rotation;
   int64_t _timeStampNs;
-  rtc::scoped_refptr<webrtc::VideoFrameBuffer> _i420Buffer;
 }
 
-- (size_t)width {
+- (int)width {
   return _videoBuffer->width();
 }
 
-- (size_t)height {
+- (int)height {
   return _videoBuffer->height();
 }
 
 - (int)rotation {
-  return static_cast<int>(_rotation);
+  return _rotation;
 }
 
-// TODO(nisse): chromaWidth and chromaHeight are used only in
-// RTCOpenGLVideoRenderer.mm. Update, and then delete these
-// properties.
-- (size_t)chromaWidth {
-  return (self.width + 1) / 2;
+- (const uint8_t *)dataY {
+  return _videoBuffer->DataY();
 }
 
-- (size_t)chromaHeight {
-  return (self.height + 1) / 2;
+- (const uint8_t *)dataU {
+  return _videoBuffer->DataU();
 }
 
-- (const uint8_t *)yPlane {
-  if (!self.i420Buffer) {
-    return nullptr;
-  }
-  return self.i420Buffer->DataY();
+- (const uint8_t *)dataV {
+  return _videoBuffer->DataV();
 }
 
-- (const uint8_t *)uPlane {
-  if (!self.i420Buffer) {
-    return nullptr;
-  }
-  return self.i420Buffer->DataU();
+- (int)strideY {
+  return _videoBuffer->StrideY();
 }
 
-- (const uint8_t *)vPlane {
-  if (!self.i420Buffer) {
-    return nullptr;
-  }
-  return self.i420Buffer->DataV();
+- (int)strideU {
+  return _videoBuffer->StrideU();
 }
 
-- (int32_t)yPitch {
-  if (!self.i420Buffer) {
-    return 0;
-  }
-  return self.i420Buffer->StrideY();
-}
-
-- (int32_t)uPitch {
-  if (!self.i420Buffer) {
-    return 0;
-  }
-  return self.i420Buffer->StrideU();
-}
-
-- (int32_t)vPitch {
-  if (!self.i420Buffer) {
-    return 0;
-  }
-  return self.i420Buffer->StrideV();
+- (int)strideV {
+  return _videoBuffer->StrideV();
 }
 
 - (int64_t)timeStampNs {
@@ -94,19 +60,20 @@
   return static_cast<CVPixelBufferRef>(_videoBuffer->native_handle());
 }
 
-- (void)convertBufferIfNeeded {
-  if (!_i420Buffer) {
-    _i420Buffer = _videoBuffer->native_handle()
-                      ? _videoBuffer->NativeToI420Buffer()
-                      : _videoBuffer;
-  }
+- (RTCVideoFrame*)convertToI420 {

[daniela-webrtc, 2017/02/17 16:26:34]

This method might not be safe from memory management perspective since it's
allocating new memory and might cause memory leaks (case in, point this patch
increases the memory usage in Mac AppRTCMobile for several GBs).
I'd suggest thinking of:
1. Renaming this method to something like newI420Frame or similar 
Source:
https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/Me...

2. Go trough this document
http://clang.llvm.org/docs/AutomaticReferenceCounting.html
and see if there is a reasonable ownership qualifier that can be applied to the
return pointer to ease memory management.

[daniela-webrtc, 2017/02/20 12:17:27]

Ok, I've done bit more thinking about this. I think the best way forward to be
to have this method always return new instance of RTCVideoFrame and name the
method as newI420VideoFrame.
We can keep the check for the native_handle() and use the NativeToI420
conversion only when needed. 
The creation of the new object *should* not take away too much from the
optimization

[magjed_webrtc, 2017/02/20 16:16:44]

Done.

+  if (!_videoBuffer->native_handle())
+    return self;
+  return [[RTCVideoFrame alloc]
+      initWithVideoBuffer:_videoBuffer->NativeToI420Buffer()
+                 rotation:_rotation
+              timeStampNs:_timeStampNs];
 }
 
 #pragma mark - Private
 
 - (instancetype)initWithVideoBuffer:
                     (rtc::scoped_refptr<webrtc::VideoFrameBuffer>)videoBuffer
-                           rotation:(webrtc::VideoRotation)rotation
+                           rotation:(int)rotation
                         timeStampNs:(int64_t)timeStampNs {
   if (self = [super init]) {
     _videoBuffer = videoBuffer;
@@ -116,9 +83,4 @@
   return self;
 }
 
-- (rtc::scoped_refptr<webrtc::VideoFrameBuffer>)i420Buffer {
-  [self convertBufferIfNeeded];
-  return _i420Buffer;
-}
-
 @end