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

webrtc/common_video/include/video_frame_buffer.h

Index: webrtc/common_video/include/video_frame_buffer.h
diff --git a/webrtc/common_video/include/video_frame_buffer.h b/webrtc/common_video/include/video_frame_buffer.h
index 94b3d5d8330ed320c6703bcdb890dfa40b87539c..e8586ee5b5e36e1aa660f3a200841556e99cadea 100644
--- a/webrtc/common_video/include/video_frame_buffer.h
+++ b/webrtc/common_video/include/video_frame_buffer.h
@@ -18,6 +18,7 @@
 #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 {
@@ -63,6 +64,10 @@ class VideoFrameBuffer : public rtc::RefCountInterface {
   // 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();
 };