Revert "Add concept of whether video renderer supports rotation."
This reverts commit 0ad48935fc5b92be6e10924a9ee3b0dc39c79104.
TBR=guoweis@webrtc.org
BUG=
Review URL: https://webrtc-codereview.appspot.com/41199004
Cr-Commit-Position: refs/heads/master@{#8663}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8663 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/talk/app/webrtc/androidtests/src/org/webrtc/VideoCapturerAndroidTest.java b/talk/app/webrtc/androidtests/src/org/webrtc/VideoCapturerAndroidTest.java
index 688ce96..1b0983a 100644
--- a/talk/app/webrtc/androidtests/src/org/webrtc/VideoCapturerAndroidTest.java
+++ b/talk/app/webrtc/androidtests/src/org/webrtc/VideoCapturerAndroidTest.java
@@ -41,7 +41,8 @@
private int framesRendered = 0;
private Object frameLock = 0;
- private void setSize(int width, int height) {
+ @Override
+ public void setSize(int width, int height) {
}
@Override
diff --git a/talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java b/talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java
index ef0fbc6..05def73 100644
--- a/talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java
+++ b/talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java
@@ -528,14 +528,10 @@
}
}
- private void setSize(final int width, final int height) {
- if (width == videoWidth && height == videoHeight) {
- return;
- }
-
+ @Override
+ public void setSize(final int width, final int height) {
Log.d(TAG, "ID: " + id + ". YuvImageRenderer.setSize: " +
width + " x " + height);
-
videoWidth = width;
videoHeight = height;
int[] strides = { width, width / 2, width / 2 };
@@ -554,7 +550,6 @@
@Override
public synchronized void renderFrame(I420Frame frame) {
- setSize(frame.width, frame.height);
long now = System.nanoTime();
framesReceived++;
// Skip rendering of this frame if setSize() was not called.
diff --git a/talk/app/webrtc/java/jni/peerconnection_jni.cc b/talk/app/webrtc/java/jni/peerconnection_jni.cc
index 5e26eaa..ed5fbc7 100644
--- a/talk/app/webrtc/java/jni/peerconnection_jni.cc
+++ b/talk/app/webrtc/java/jni/peerconnection_jni.cc
@@ -696,23 +696,21 @@
virtual ~VideoRendererWrapper() {}
- // This wraps VideoRenderer which still has SetSize.
- void RenderFrame(const cricket::VideoFrame* video_frame) override {
+ void SetSize(int width, int height) override {
ScopedLocalRefFrame local_ref_frame(AttachCurrentThreadIfNeeded());
- const cricket::VideoFrame* frame =
- video_frame->GetCopyWithRotationApplied();
- if (width_ != frame->GetWidth() || height_ != frame->GetHeight()) {
- width_ = frame->GetWidth();
- height_ = frame->GetHeight();
- renderer_->SetSize(width_, height_, 0);
- }
+ const bool kNotReserved = false; // What does this param mean??
+ renderer_->SetSize(width, height, kNotReserved);
+ }
+
+ void RenderFrame(const cricket::VideoFrame* frame) override {
+ ScopedLocalRefFrame local_ref_frame(AttachCurrentThreadIfNeeded());
renderer_->RenderFrame(frame);
}
private:
explicit VideoRendererWrapper(cricket::VideoRenderer* renderer)
- : renderer_(renderer), width_(0), height_(0) {}
- int width_, height_;
+ : renderer_(renderer) {}
+
scoped_ptr<cricket::VideoRenderer> renderer_;
};
@@ -722,6 +720,8 @@
public:
JavaVideoRendererWrapper(JNIEnv* jni, jobject j_callbacks)
: j_callbacks_(jni, j_callbacks),
+ j_set_size_id_(GetMethodID(
+ jni, GetObjectClass(jni, j_callbacks), "setSize", "(II)V")),
j_render_frame_id_(GetMethodID(
jni, GetObjectClass(jni, j_callbacks), "renderFrame",
"(Lorg/webrtc/VideoRenderer$I420Frame;)V")),
@@ -738,13 +738,14 @@
virtual ~JavaVideoRendererWrapper() {}
- void RenderFrame(const cricket::VideoFrame* video_frame) override {
+ void SetSize(int width, int height) override {
ScopedLocalRefFrame local_ref_frame(jni());
+ jni()->CallVoidMethod(*j_callbacks_, j_set_size_id_, width, height);
+ CHECK_EXCEPTION(jni());
+ }
- // TODO(guoweis): Remove once the java implementation supports rotation.
- const cricket::VideoFrame* frame =
- video_frame->GetCopyWithRotationApplied();
-
+ void RenderFrame(const cricket::VideoFrame* frame) override {
+ ScopedLocalRefFrame local_ref_frame(jni());
if (frame->GetNativeHandle() != NULL) {
jobject j_frame = CricketToJavaTextureFrame(frame);
jni()->CallVoidMethod(*j_callbacks_, j_render_frame_id_, j_frame);
@@ -797,6 +798,7 @@
}
ScopedGlobalRef<jobject> j_callbacks_;
+ jmethodID j_set_size_id_;
jmethodID j_render_frame_id_;
ScopedGlobalRef<jclass> j_frame_class_;
jmethodID j_i420_frame_ctor_id_;
diff --git a/talk/app/webrtc/java/src/org/webrtc/VideoRenderer.java b/talk/app/webrtc/java/src/org/webrtc/VideoRenderer.java
index 45fafa5..2dc9279 100644
--- a/talk/app/webrtc/java/src/org/webrtc/VideoRenderer.java
+++ b/talk/app/webrtc/java/src/org/webrtc/VideoRenderer.java
@@ -144,8 +144,7 @@
/** The real meat of VideoRendererInterface. */
public static interface Callbacks {
- // |frame| might have pending rotation and implementation of Callbacks
- // should handle that by applying rotation during rendering.
+ public void setSize(int width, int height);
public void renderFrame(I420Frame frame);
}
diff --git a/talk/app/webrtc/java/testcommon/src/org/webrtc/PeerConnectionTest.java b/talk/app/webrtc/java/testcommon/src/org/webrtc/PeerConnectionTest.java
index 4e364b5..ab0510a 100644
--- a/talk/app/webrtc/java/testcommon/src/org/webrtc/PeerConnectionTest.java
+++ b/talk/app/webrtc/java/testcommon/src/org/webrtc/PeerConnectionTest.java
@@ -59,6 +59,7 @@
private int expectedIceCandidates = 0;
private int expectedErrors = 0;
private int expectedRenegotiations = 0;
+ private int expectedSetSize = 0;
private int previouslySeenWidth = 0;
private int previouslySeenHeight = 0;
private int expectedFramesDelivered = 0;
@@ -112,8 +113,19 @@
gotIceCandidates.add(candidate);
}
- private synchronized void setSize(int width, int height) {
+ public synchronized void expectSetSize() {
+ if (RENDER_TO_GUI) {
+ // When new frames are delivered to the GUI renderer we don't get
+ // notified of frame size info.
+ return;
+ }
+ ++expectedSetSize;
+ }
+
+ @Override
+ public synchronized void setSize(int width, int height) {
assertFalse(RENDER_TO_GUI);
+ assertTrue(--expectedSetSize >= 0);
// Because different camera devices (fake & physical) produce different
// resolutions, we only sanity-check the set sizes,
assertTrue(width > 0);
@@ -134,7 +146,6 @@
@Override
public synchronized void renderFrame(VideoRenderer.I420Frame frame) {
- setSize(frame.width, frame.height);
--expectedFramesDelivered;
}
@@ -304,6 +315,9 @@
stillWaitingForExpectations.add(
"expectedRemoveStreamLabels: " + expectedRemoveStreamLabels.size());
}
+ if (expectedSetSize != 0) {
+ stillWaitingForExpectations.add("expectedSetSize");
+ }
if (expectedFramesDelivered > 0) {
stillWaitingForExpectations.add(
"expectedFramesDelivered: " + expectedFramesDelivered);
@@ -422,7 +436,8 @@
public int height = -1;
public int numFramesDelivered = 0;
- private void setSize(int width, int height) {
+ @Override
+ public void setSize(int width, int height) {
assertEquals(this.width, -1);
assertEquals(this.height, -1);
this.width = width;
@@ -527,6 +542,7 @@
VideoSource videoSource = factory.createVideoSource(
VideoCapturer.create(""), new MediaConstraints());
+ offeringExpectations.expectSetSize();
offeringExpectations.expectRenegotiationNeeded();
WeakReference<MediaStream> oLMS = addTracksToPC(
factory, offeringPC, videoSource, "offeredMediaStream",
@@ -558,6 +574,7 @@
assertTrue(sdpLatch.await());
assertNull(sdpLatch.getSdp());
+ answeringExpectations.expectSetSize();
answeringExpectations.expectRenegotiationNeeded();
WeakReference<MediaStream> aLMS = addTracksToPC(
factory, answeringPC, videoSource, "answeredMediaStream",
@@ -619,6 +636,8 @@
// chosen arbitrarily).
offeringExpectations.expectFramesDelivered(10);
answeringExpectations.expectFramesDelivered(10);
+ offeringExpectations.expectSetSize();
+ answeringExpectations.expectSetSize();
}
offeringExpectations.expectStateChange(DataChannel.State.OPEN);
diff --git a/talk/app/webrtc/mediastreaminterface.h b/talk/app/webrtc/mediastreaminterface.h
index 4de0648..ce5e045 100644
--- a/talk/app/webrtc/mediastreaminterface.h
+++ b/talk/app/webrtc/mediastreaminterface.h
@@ -115,27 +115,9 @@
// Interface for rendering VideoFrames from a VideoTrack
class VideoRendererInterface {
public:
- // TODO(guoweis): Remove this function. Obsolete. The implementation of
- // VideoRendererInterface should be able to handle different frame size as
- // well as pending rotation. If it can't apply the frame rotation by itself,
- // it should call |frame|.GetCopyWithRotationApplied() to get a frame that has
- // the rotation applied.
- virtual void SetSize(int width, int height) {}
-
- // |frame| may have pending rotation. For clients which can't apply rotation,
- // |frame|->GetCopyWithRotationApplied() will return a frame that has the
- // rotation applied.
+ virtual void SetSize(int width, int height) = 0;
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 8bd98fa..d636a56 100644
--- a/talk/app/webrtc/test/fakevideotrackrenderer.h
+++ b/talk/app/webrtc/test/fakevideotrackrenderer.h
@@ -35,55 +35,34 @@
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) {
+ explicit FakeVideoTrackRenderer(VideoTrackInterface* video_track)
+ : video_track_(video_track) {
video_track_->AddRenderer(this);
}
~FakeVideoTrackRenderer() {
video_track_->RemoveRenderer(this);
}
- 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)) {
- return;
- }
-
- fake_renderer_.RenderFrame(frame);
+ // Implements VideoRendererInterface
+ virtual void SetSize(int width, int height) {
+ fake_renderer_.SetSize(width, height, 0);
}
- virtual bool CanApplyRotation() override { return can_apply_rotation_; }
+ virtual void RenderFrame(const cricket::VideoFrame* frame) {
+ fake_renderer_.RenderFrame(frame);
+ }
int errors() const { return fake_renderer_.errors(); }
int width() const { return fake_renderer_.width(); }
int height() const { return fake_renderer_.height(); }
+ int num_set_sizes() const { return fake_renderer_.num_set_sizes(); }
int num_rendered_frames() const {
return fake_renderer_.num_rendered_frames();
}
- const cricket::VideoFrame* last_frame() const { return last_frame_; }
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_;
};
} // namespace webrtc
diff --git a/talk/app/webrtc/videotrack_unittest.cc b/talk/app/webrtc/videotrack_unittest.cc
index 529866d..263fefc 100644
--- a/talk/app/webrtc/videotrack_unittest.cc
+++ b/talk/app/webrtc/videotrack_unittest.cc
@@ -43,132 +43,57 @@
using webrtc::VideoTrack;
using webrtc::VideoTrackInterface;
-namespace {
-
-class WebRtcVideoTestFrame : public cricket::WebRtcVideoFrame {
- public:
- using cricket::WebRtcVideoFrame::SetRotation;
-};
-
-} // namespace
-
-class VideoTrackTest : public testing::Test {
- public:
- VideoTrackTest() {
- static const char kVideoTrackId[] = "track_id";
-
- channel_manager_.reset(new cricket::ChannelManager(
- new cricket::FakeMediaEngine(), new cricket::FakeDeviceManager(),
- rtc::Thread::Current()));
- EXPECT_TRUE(channel_manager_->Init());
- video_track_ = VideoTrack::Create(
- kVideoTrackId,
- VideoSource::Create(channel_manager_.get(),
- new webrtc::RemoteVideoCapturer(), NULL));
- }
-
- protected:
- rtc::scoped_ptr<cricket::ChannelManager> channel_manager_;
- rtc::scoped_refptr<VideoTrackInterface> video_track_;
-};
-
// Test adding renderers to a video track and render to them by providing
// frames to the source.
-TEST_F(VideoTrackTest, RenderVideo) {
- // FakeVideoTrackRenderer register itself to |video_track_|
- rtc::scoped_ptr<FakeVideoTrackRenderer> renderer_1(
- new FakeVideoTrackRenderer(video_track_.get()));
+TEST(VideoTrack, RenderVideo) {
+ static const char kVideoTrackId[] = "track_id";
- cricket::VideoRenderer* renderer_input =
- video_track_->GetSource()->FrameInput();
- ASSERT_FALSE(renderer_input == NULL);
+ rtc::scoped_ptr<cricket::ChannelManager> channel_manager_;
+ channel_manager_.reset(
+ new cricket::ChannelManager(new cricket::FakeMediaEngine(),
+ new cricket::FakeDeviceManager(),
+ rtc::Thread::Current()));
+ ASSERT_TRUE(channel_manager_->Init());
+ rtc::scoped_refptr<VideoTrackInterface> video_track(
+ VideoTrack::Create(kVideoTrackId,
+ VideoSource::Create(channel_manager_.get(),
+ new webrtc::RemoteVideoCapturer(),
+ NULL)));
+ // FakeVideoTrackRenderer register itself to |video_track|
+ rtc::scoped_ptr<FakeVideoTrackRenderer> renderer_1(
+ new FakeVideoTrackRenderer(video_track.get()));
+
+ cricket::VideoRenderer* render_input = video_track->GetSource()->FrameInput();
+ ASSERT_FALSE(render_input == NULL);
cricket::WebRtcVideoFrame frame;
frame.InitToBlack(123, 123, 1, 1, 0, 0);
- renderer_input->RenderFrame(&frame);
+ render_input->RenderFrame(&frame);
EXPECT_EQ(1, renderer_1->num_rendered_frames());
+ EXPECT_EQ(1, renderer_1->num_set_sizes());
EXPECT_EQ(123, renderer_1->width());
EXPECT_EQ(123, renderer_1->height());
- // FakeVideoTrackRenderer register itself to |video_track_|
+ // FakeVideoTrackRenderer register itself to |video_track|
rtc::scoped_ptr<FakeVideoTrackRenderer> renderer_2(
- new FakeVideoTrackRenderer(video_track_.get()));
+ new FakeVideoTrackRenderer(video_track.get()));
- renderer_input->RenderFrame(&frame);
+ render_input->RenderFrame(&frame);
+ EXPECT_EQ(1, renderer_1->num_set_sizes());
EXPECT_EQ(123, renderer_1->width());
EXPECT_EQ(123, renderer_1->height());
+ EXPECT_EQ(1, renderer_2->num_set_sizes());
EXPECT_EQ(123, renderer_2->width());
EXPECT_EQ(123, renderer_2->height());
EXPECT_EQ(2, renderer_1->num_rendered_frames());
EXPECT_EQ(1, renderer_2->num_rendered_frames());
- video_track_->RemoveRenderer(renderer_1.get());
- renderer_input->RenderFrame(&frame);
+ video_track->RemoveRenderer(renderer_1.get());
+ render_input->RenderFrame(&frame);
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 08bf05a..03f080d 100644
--- a/talk/app/webrtc/videotrackrenderers.cc
+++ b/talk/app/webrtc/videotrackrenderers.cc
@@ -26,21 +26,19 @@
*/
#include "talk/app/webrtc/videotrackrenderers.h"
-#include "talk/media/base/videoframe.h"
namespace webrtc {
VideoTrackRenderers::VideoTrackRenderers()
- : enabled_(true) {
+ : width_(0),
+ height_(0),
+ enabled_(true) {
}
VideoTrackRenderers::~VideoTrackRenderers() {
}
void VideoTrackRenderers::AddRenderer(VideoRendererInterface* renderer) {
- if (!renderer) {
- return;
- }
rtc::CritScope cs(&critical_section_);
std::vector<RenderObserver>::iterator it = renderers_.begin();
for (; it != renderers_.end(); ++it) {
@@ -67,6 +65,14 @@
}
bool VideoTrackRenderers::SetSize(int width, int height, int reserved) {
+ rtc::CritScope cs(&critical_section_);
+ width_ = width;
+ height_ = height;
+ std::vector<RenderObserver>::iterator it = renderers_.begin();
+ for (; it != renderers_.end(); ++it) {
+ it->renderer_->SetSize(width, height);
+ it->size_set_ = true;
+ }
return true;
}
@@ -75,14 +81,13 @@
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());
+ if (!it->size_set_) {
+ it->renderer_->SetSize(width_, height_);
+ it->size_set_ = true;
}
+ it->renderer_->RenderFrame(frame);
}
return true;
}
diff --git a/talk/app/webrtc/videotrackrenderers.h b/talk/app/webrtc/videotrackrenderers.h
index 02d610f..5bec347 100644
--- a/talk/app/webrtc/videotrackrenderers.h
+++ b/talk/app/webrtc/videotrackrenderers.h
@@ -33,7 +33,6 @@
#include "talk/app/webrtc/mediastreaminterface.h"
#include "talk/media/base/videorenderer.h"
#include "webrtc/base/criticalsection.h"
-#include "webrtc/base/scoped_ptr.h"
namespace webrtc {
@@ -59,11 +58,14 @@
struct RenderObserver {
explicit RenderObserver(VideoRendererInterface* renderer)
: renderer_(renderer),
- can_apply_rotation_(renderer->CanApplyRotation()) {}
+ size_set_(false) {
+ }
VideoRendererInterface* renderer_;
- bool can_apply_rotation_;
+ bool size_set_;
};
+ int width_;
+ int height_;
bool enabled_;
std::vector<RenderObserver> renderers_;
