Clean up RTCVideoFrame

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

 /*
  *  Copyright 2015 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #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 {
   return _timeStampNs;
 }
 
 - (CVPixelBufferRef)nativeHandle {
   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;
     _rotation = rotation;
     _timeStampNs = timeStampNs;
   }
   return self;
 }
 
-- (rtc::scoped_refptr<webrtc::VideoFrameBuffer>)i420Buffer {
-  [self convertBufferIfNeeded];
-  return _i420Buffer;
-}
-
 @end