New static I420Buffer::Rotate method, to replace GetCopyWithRotationApplied.

webrtc/common_video/include/video_frame_buffer.h

 /*
  *  Copyright (c) 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.
  */
 
 #ifndef WEBRTC_COMMON_VIDEO_INCLUDE_VIDEO_FRAME_BUFFER_H_
 #define WEBRTC_COMMON_VIDEO_INCLUDE_VIDEO_FRAME_BUFFER_H_
 
 #include <stdint.h>
 
 #include <memory>
 
 #include "webrtc/base/callback.h"
 #include "webrtc/base/refcount.h"
 #include "webrtc/base/scoped_ref_ptr.h"
+#include "webrtc/common_video/rotation.h"
 #include "webrtc/system_wrappers/include/aligned_malloc.h"
 
 namespace webrtc {
 
 enum PlaneType {
   kYPlane = 0,
   kUPlane = 1,
   kVPlane = 2,
   kNumOfPlanes = 3,
 };
@@ skipping 25 matching lines @@
   virtual int StrideV() const = 0;
 
   // Return the handle of the underlying video frame. This is used when the
   // frame is backed by a texture.
   virtual void* native_handle() const = 0;
 
   // Returns a new memory-backed frame buffer converted from this buffer's
   // native handle.
   virtual rtc::scoped_refptr<VideoFrameBuffer> NativeToI420Buffer() = 0;
 
+  static rtc::scoped_refptr<VideoFrameBuffer> Rotate(

[perkj_webrtc, 2016/08/26 15:22:17]

I think I would prefer this to be a virtual member function just like NativeTo
if this helper method should be part of the interface. ... 
Then NativeHandles and other - none i420 frames have a chance to override it. 
The implentation in this cl can be the default implementation. 


Another totally crazy idea that would cause a lot of work would be to change
NativeToI420Buffer to return an I420Buffer.  - remove all methods that returns
pointers to i420 data and force them to call NativeToI420Buffer().... Just a
crazy idea that would allow VideoFrameBuffer to not know about that it
represents,(i420, textures, rgb etc..) 

[nisse-webrtc, 2016/08/29 09:16:50]

Makes sense to me. (magjed suggested using a static method).

If we make it non-static, is Rotate still a good enough name?
We'd need an abstract interface for a read-only I420Frame then, which could be
inherited by both I420Buffer, WrappedI420Buffer, and possibly others. We really
shouldn't allow

frame->video_frame_buffer()->NativeToI420Buffer()->MutableDataX()

[magjed_webrtc, 2016/08/30 08:11:33]

Please don't add unnecessary functions to the interface! It will cause the same
trouble in the future as GetCopyWithRotationApplied is causing now. This Rotate
helper function should not be part of the interface. It's used for legacy
clients that don't support CVO and they have to fall back to NativeToI420Buffer
and then rotate, so there is no use case for rotating non-i420 frames. We want
to keep the interface as small as possible and only add stuff that we need.
Adding rotation support outside rendering for iOS CVPixelBuffers and Chrome
GpuMemoryBuffers would require a lot of work and I don't want to do that unless
it's needed.

+      const rtc::scoped_refptr<VideoFrameBuffer>& src,
+      VideoRotation rotation);
+
  protected:
   virtual ~VideoFrameBuffer();
 };
 
 // Plain I420 buffer in standard memory.
 class I420Buffer : public VideoFrameBuffer {
  public:
   I420Buffer(int width, int height);
   I420Buffer(int width, int height, int stride_y, int stride_u, int stride_v);
 
@@ skipping 127 matching lines @@
   const uint8_t* const v_plane_;
   const int y_stride_;
   const int u_stride_;
   const int v_stride_;
   rtc::Callback0<void> no_longer_used_cb_;
 };
 
 }  // namespace webrtc
 
 #endif  // WEBRTC_COMMON_VIDEO_INCLUDE_VIDEO_FRAME_BUFFER_H_