Update I420Buffer to new VideoFrameBuffer interface

webrtc/media/base/adaptedvideotracksource.cc

Index: webrtc/media/base/adaptedvideotracksource.cc
diff --git a/webrtc/media/base/adaptedvideotracksource.cc b/webrtc/media/base/adaptedvideotracksource.cc
index 7a3e9f2f2da363ea8088ed71d78e0bc92bb0dd4d..9513be8230ed7e5a3d6f8acdc474f8a66424c52b 100644
--- a/webrtc/media/base/adaptedvideotracksource.cc
+++ b/webrtc/media/base/adaptedvideotracksource.cc
@@ -45,12 +45,11 @@ void AdaptedVideoTrackSource::OnFrame(const webrtc::VideoFrame& frame) {
      true was just added. The VideoBroadcaster enforces
      synchronization for us in this case, by not passing the frame on
      to sinks which don't want it. */
-  if (apply_rotation() &&
-      frame.rotation() != webrtc::kVideoRotation_0 &&
-      !buffer->native_handle()) {
+  if (apply_rotation() && frame.rotation() != webrtc::kVideoRotation_0 &&
+      buffer->type() == webrtc::VideoFrameBuffer::Type::kI420) {
     /* Apply pending rotation. */
     broadcaster_.OnFrame(webrtc::VideoFrame(
-        webrtc::I420Buffer::Rotate(*buffer, frame.rotation()),
+        webrtc::I420Buffer::Rotate(*buffer->GetI420(), frame.rotation()),

[nisse-webrtc, 2017/05/29 08:22:22]

What's the right way to add support for rotating non-I420 frames here? It would
be nice to not have a |switch(buffer->type())|.

[magjed_webrtc, 2017/05/29 12:12:06]

Hopefully we don't need to handle rotating non-I420 frames. Rotating buffers is
something we want to avoid, and it's fine for me to always use ToI420() when
apply_rotation() is true. In general, I prefer to switch on the type instead of
adding more virtual functions to VideoFrameBuffer.

[nisse-webrtc, 2017/05/29 12:37:17]

Hmm, in the case of texture frames rotation mismatch here is always temporary
and we prefer dropping a frame or two over suddenly inserting a non-texture
frame into the pipeline. I guess we need the same to hold for I444 frames, or
else we'll pass I444 or I420 downstream depending on the current orientation.

Not sure what code depends on apply_rotation being handled here.

[magjed_webrtc, 2017/05/29 13:57:02]

Only the Android capture implementation depends on it. We could move this logic
there if you prefer.

[nisse-webrtc, 2017/05/29 14:35:55]

I think logic which is generally useful to any video source belongs in this
class. If almost all sources do their own rotation, then it makes sense to
delete the code here (but keep the apply_rotation() method).

Failure mode is like this, I think: If a source doesn't implement apply rotation
properly, and some sink sets wants.rotation_applied, then that sink will not get
any frames, by the logic in VideoBroadcaster::OnFrame.

         webrtc::kVideoRotation_0, frame.timestamp_us()));
   } else {
     broadcaster_.OnFrame(frame);