Remove VideoRendererInterface::CanApplyRotation()

All implementations handle rotation now, both internally in WebRTC and externally in Chromium.

R=glaznev@webrtc.org, guoweis@webrtc.org

Review URL: https://codereview.webrtc.org/1313753003 .

Cr-Commit-Position: refs/heads/master@{#9911}

6 files changed, 11 insertions, 129 deletions

diff --git a/talk/app/webrtc/java/jni/peerconnection_jni.cc b/talk/app/webrtc/java/jni/peerconnection_jni.cc
index e9e4c07f03..47581d9e0e 100644
--- a/talk/app/webrtc/java/jni/peerconnection_jni.cc
+++ b/talk/app/webrtc/java/jni/peerconnection_jni.cc
@@ -745,9 +745,6 @@ class VideoRendererWrapper : public VideoRendererInterface {
     renderer_->RenderFrame(frame);
   }
 
-  // TODO(guoweis): Remove this once chrome code base is updated.
-  bool CanApplyRotation() override { return true; }
-
  private:
   explicit VideoRendererWrapper(cricket::VideoRenderer* renderer)
     : width_(0), height_(0), renderer_(renderer) {}
@@ -789,10 +786,6 @@ class JavaVideoRendererWrapper : public VideoRendererInterface {
     CHECK_EXCEPTION(jni());
   }
 
-  // TODO(guoweis): Report that rotation is supported as RenderFrame calls
-  // GetCopyWithRotationApplied.
-  virtual bool CanApplyRotation() override { return true; }
-
  private:
   // Return a VideoRenderer.I420Frame referring to the data in |frame|.
   jobject CricketToJavaI420Frame(const cricket::VideoFrame* frame) {
diff --git a/talk/app/webrtc/mediastreaminterface.h b/talk/app/webrtc/mediastreaminterface.h
index ceca99b243..5911e85e8e 100644
--- a/talk/app/webrtc/mediastreaminterface.h
+++ b/talk/app/webrtc/mediastreaminterface.h
@@ -127,15 +127,6 @@ class VideoRendererInterface {
   // rotation applied.
   virtual void RenderFrame(const cricket::VideoFrame* frame) = 0;
 
-  // TODO(guoweis): Remove this function. This is added as a temporary solution
-  // until chrome renderers can apply rotation.
-  // Whether the VideoRenderer has the ability to rotate the frame before being
-  // displayed. The rotation of a frame is carried by
-  // VideoFrame.GetVideoRotation() and is the clockwise angle the frames must be
-  // rotated in order to display the frames correctly. If returning false, the
-  // frame's rotation must be applied before being delivered by RenderFrame.
-  virtual bool CanApplyRotation() { return false; }
-
  protected:
   // The destructor is protected to prevent deletion via the interface.
   // This is so that we allow reference counted classes, where the destructor
diff --git a/talk/app/webrtc/test/fakevideotrackrenderer.h b/talk/app/webrtc/test/fakevideotrackrenderer.h
index 8bd98fa8a9..38b84a6aff 100644
--- a/talk/app/webrtc/test/fakevideotrackrenderer.h
+++ b/talk/app/webrtc/test/fakevideotrackrenderer.h
@@ -36,16 +36,7 @@ namespace webrtc {
 class FakeVideoTrackRenderer : public VideoRendererInterface {
  public:
   FakeVideoTrackRenderer(VideoTrackInterface* video_track)
-      : video_track_(video_track),
-        can_apply_rotation_(true),
-        last_frame_(NULL) {
-    video_track_->AddRenderer(this);
-  }
-  FakeVideoTrackRenderer(VideoTrackInterface* video_track,
-                         bool can_apply_rotation)
-      : video_track_(video_track),
-        can_apply_rotation_(can_apply_rotation),
-        last_frame_(NULL) {
+      : video_track_(video_track), last_frame_(NULL) {
     video_track_->AddRenderer(this);
   }
   ~FakeVideoTrackRenderer() {
@@ -54,21 +45,15 @@ class FakeVideoTrackRenderer : public VideoRendererInterface {
 
   virtual void RenderFrame(const cricket::VideoFrame* video_frame) override {
     last_frame_ = const_cast<cricket::VideoFrame*>(video_frame);
-
-    const cricket::VideoFrame* frame =
-        can_apply_rotation_ ? video_frame
-                            : video_frame->GetCopyWithRotationApplied();
-
-    if (!fake_renderer_.SetSize(static_cast<int>(frame->GetWidth()),
-                                static_cast<int>(frame->GetHeight()), 0)) {
+    if (!fake_renderer_.SetSize(static_cast<int>(video_frame->GetWidth()),
+                                static_cast<int>(video_frame->GetHeight()),
+                                0)) {
       return;
     }
 
-    fake_renderer_.RenderFrame(frame);
+    fake_renderer_.RenderFrame(video_frame);
   }
 
-  virtual bool CanApplyRotation() override { return can_apply_rotation_; }
-
   int errors() const { return fake_renderer_.errors(); }
   int width() const { return fake_renderer_.width(); }
   int height() const { return fake_renderer_.height(); }
@@ -80,7 +65,6 @@ class FakeVideoTrackRenderer : public VideoRendererInterface {
  private:
   cricket::FakeVideoRenderer fake_renderer_;
   rtc::scoped_refptr<VideoTrackInterface> video_track_;
-  bool can_apply_rotation_;
 
   // Weak reference for frame pointer comparison only.
   cricket::VideoFrame* last_frame_;
diff --git a/talk/app/webrtc/videotrack_unittest.cc b/talk/app/webrtc/videotrack_unittest.cc
index 529866d5f8..324a88ba4c 100644
--- a/talk/app/webrtc/videotrack_unittest.cc
+++ b/talk/app/webrtc/videotrack_unittest.cc
@@ -111,64 +111,3 @@ TEST_F(VideoTrackTest, RenderVideo) {
   EXPECT_EQ(2, renderer_1->num_rendered_frames());
   EXPECT_EQ(2, renderer_2->num_rendered_frames());
 }
-
-// Test adding renderers which support and don't support rotation and receive
-// the right frame.
-TEST_F(VideoTrackTest, RenderVideoWithPendingRotation) {
-  const size_t kWidth = 800;
-  const size_t kHeight = 400;
-
-  // Add a renderer which supports rotation.
-  rtc::scoped_ptr<FakeVideoTrackRenderer> rotating_renderer(
-      new FakeVideoTrackRenderer(video_track_.get(), true));
-
-  cricket::VideoRenderer* renderer_input =
-      video_track_->GetSource()->FrameInput();
-  ASSERT_FALSE(renderer_input == NULL);
-
-  // Create a frame with rotation 90 degree.
-  WebRtcVideoTestFrame frame;
-  frame.InitToBlack(kWidth, kHeight, 1, 1, 0, 0);
-  frame.SetRotation(webrtc::kVideoRotation_90);
-
-  // rotating_renderer should see the frame unrotated.
-  renderer_input->RenderFrame(&frame);
-  EXPECT_EQ(1, rotating_renderer->num_rendered_frames());
-  EXPECT_EQ(kWidth, rotating_renderer->width());
-  EXPECT_EQ(kHeight, rotating_renderer->height());
-  EXPECT_EQ(&frame, rotating_renderer->last_frame());
-
-  // Add 2nd renderer which doesn't support rotation.
-  rtc::scoped_ptr<FakeVideoTrackRenderer> non_rotating_renderer(
-      new FakeVideoTrackRenderer(video_track_.get(), false));
-
-  // Render the same 90 degree frame.
-  renderer_input->RenderFrame(&frame);
-
-  // rotating_renderer should see the same frame.
-  EXPECT_EQ(kWidth, rotating_renderer->width());
-  EXPECT_EQ(kHeight, rotating_renderer->height());
-  EXPECT_EQ(&frame, rotating_renderer->last_frame());
-
-  // non_rotating_renderer should see the frame rotated.
-  EXPECT_EQ(kHeight, non_rotating_renderer->width());
-  EXPECT_EQ(kWidth, non_rotating_renderer->height());
-  EXPECT_NE(&frame, non_rotating_renderer->last_frame());
-
-  // Render the same 90 degree frame the 3rd time.
-  renderer_input->RenderFrame(&frame);
-
-  // Now render a frame without rotation.
-  frame.SetRotation(webrtc::kVideoRotation_0);
-  renderer_input->RenderFrame(&frame);
-
-  // rotating_renderer should still only have 1 setsize.
-  EXPECT_EQ(kWidth, rotating_renderer->width());
-  EXPECT_EQ(kHeight, rotating_renderer->height());
-  EXPECT_EQ(&frame, rotating_renderer->last_frame());
-
-  // render_2 should have a new size but should have the same frame.
-  EXPECT_EQ(kWidth, non_rotating_renderer->width());
-  EXPECT_EQ(kHeight, non_rotating_renderer->height());
-  EXPECT_EQ(&frame, non_rotating_renderer->last_frame());
-}
diff --git a/talk/app/webrtc/videotrackrenderers.cc b/talk/app/webrtc/videotrackrenderers.cc
index be232f3005..3c47c6edab 100644
--- a/talk/app/webrtc/videotrackrenderers.cc
+++ b/talk/app/webrtc/videotrackrenderers.cc
@@ -41,23 +41,12 @@ void VideoTrackRenderers::AddRenderer(VideoRendererInterface* renderer) {
     return;
   }
   rtc::CritScope cs(&critical_section_);
-  std::vector<RenderObserver>::iterator it =  renderers_.begin();
-  for (; it != renderers_.end(); ++it) {
-    if (it->renderer_ == renderer)
-      return;
-  }
-  renderers_.push_back(RenderObserver(renderer));
+  renderers_.insert(renderer);
 }
 
 void VideoTrackRenderers::RemoveRenderer(VideoRendererInterface* renderer) {
   rtc::CritScope cs(&critical_section_);
-  std::vector<RenderObserver>::iterator it =  renderers_.begin();
-  for (; it != renderers_.end(); ++it) {
-    if (it->renderer_ == renderer) {
-      renderers_.erase(it);
-      return;
-    }
-  }
+  renderers_.erase(renderer);
 }
 
 void VideoTrackRenderers::SetEnabled(bool enable) {
@@ -74,14 +63,8 @@ bool VideoTrackRenderers::RenderFrame(const cricket::VideoFrame* frame) {
   if (!enabled_) {
     return true;
   }
-
-  std::vector<RenderObserver>::iterator it = renderers_.begin();
-  for (; it != renderers_.end(); ++it) {
-    if (it->can_apply_rotation_) {
-      it->renderer_->RenderFrame(frame);
-    } else {
-      it->renderer_->RenderFrame(frame->GetCopyWithRotationApplied());
-    }
+  for (VideoRendererInterface* renderer : renderers_) {
+    renderer->RenderFrame(frame);
   }
   return true;
 }
diff --git a/talk/app/webrtc/videotrackrenderers.h b/talk/app/webrtc/videotrackrenderers.h
index 02d610f706..15274a1530 100644
--- a/talk/app/webrtc/videotrackrenderers.h
+++ b/talk/app/webrtc/videotrackrenderers.h
@@ -28,7 +28,7 @@
 #ifndef TALK_APP_WEBRTC_VIDEOTRACKRENDERERS_H_
 #define TALK_APP_WEBRTC_VIDEOTRACKRENDERERS_H_
 
-#include <vector>
+#include <set>
 
 #include "talk/app/webrtc/mediastreaminterface.h"
 #include "talk/media/base/videorenderer.h"
@@ -56,16 +56,8 @@ class VideoTrackRenderers : public cricket::VideoRenderer {
   void SetEnabled(bool enable);
 
  private:
-  struct RenderObserver {
-    explicit RenderObserver(VideoRendererInterface* renderer)
-        : renderer_(renderer),
-          can_apply_rotation_(renderer->CanApplyRotation()) {}
-    VideoRendererInterface* renderer_;
-    bool can_apply_rotation_;
-  };
-
   bool enabled_;
-  std::vector<RenderObserver> renderers_;
+  std::set<VideoRendererInterface*> renderers_;
 
   rtc::CriticalSection critical_section_;  // Protects the above variables
 };