From 0ad48935fc5b92be6e10924a9ee3b0dc39c79104 Mon Sep 17 00:00:00 2001 From: "guoweis@webrtc.org" Date: Tue, 10 Mar 2015 02:42:50 +0000 Subject: [PATCH] Add concept of whether video renderer supports rotation. Rotation is best done when rendered in GPU, added the shader code which rotates the frame. For renderers which don't support rotation, the rotation will be done before sending down the frame to render. By default, assume renderer can't do rotation. BUG=4145 R=glaznev@webrtc.org, pthatcher@webrtc.org Review URL: https://webrtc-codereview.appspot.com/43569004 Cr-Commit-Position: refs/heads/master@{#8660} git-svn-id: http://webrtc.googlecode.com/svn/trunk@8660 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../org/webrtc/VideoCapturerAndroidTest.java | 3 +- .../android/org/webrtc/VideoRendererGui.java | 9 +- .../app/webrtc/java/jni/peerconnection_jni.cc | 34 +++-- .../java/src/org/webrtc/VideoRenderer.java | 3 +- .../src/org/webrtc/PeerConnectionTest.java | 25 +--- talk/app/webrtc/mediastreaminterface.h | 20 ++- talk/app/webrtc/test/fakevideotrackrenderer.h | 37 +++-- talk/app/webrtc/videotrack_unittest.cc | 129 ++++++++++++++---- talk/app/webrtc/videotrackrenderers.cc | 25 ++-- talk/app/webrtc/videotrackrenderers.h | 8 +- .../apprtc/test/PeerConnectionClientTest.java | 3 +- talk/media/base/videoframe.cc | 47 +++++++ talk/media/base/videoframe.h | 10 ++ talk/media/base/videorenderer.h | 5 +- talk/media/devices/carbonvideorenderer.cc | 10 +- talk/media/devices/gdivideorenderer.cc | 9 +- talk/media/devices/gtkvideorenderer.cc | 23 +++- talk/media/devices/gtkvideorenderer.h | 3 + talk/media/webrtc/webrtcvideoframe.h | 3 + .../media/webrtc/webrtcvideoframe_unittest.cc | 49 +++++++ 20 files changed, 345 insertions(+), 110 deletions(-) mode change 100755 => 100644 talk/media/devices/gdivideorenderer.cc diff --git a/talk/app/webrtc/androidtests/src/org/webrtc/VideoCapturerAndroidTest.java b/talk/app/webrtc/androidtests/src/org/webrtc/VideoCapturerAndroidTest.java index 1b0983a485..688ce96e84 100644 --- a/talk/app/webrtc/androidtests/src/org/webrtc/VideoCapturerAndroidTest.java +++ b/talk/app/webrtc/androidtests/src/org/webrtc/VideoCapturerAndroidTest.java @@ -41,8 +41,7 @@ public class VideoCapturerAndroidTest extends ActivityTestCase { private int framesRendered = 0; private Object frameLock = 0; - @Override - public void setSize(int width, int height) { + private 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 05def73069..ef0fbc68a5 100644 --- a/talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java +++ b/talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java @@ -528,10 +528,14 @@ public class VideoRendererGui implements GLSurfaceView.Renderer { } } - @Override - public void setSize(final int width, final int height) { + private void setSize(final int width, final int height) { + if (width == videoWidth && height == videoHeight) { + return; + } + Log.d(TAG, "ID: " + id + ". YuvImageRenderer.setSize: " + width + " x " + height); + videoWidth = width; videoHeight = height; int[] strides = { width, width / 2, width / 2 }; @@ -550,6 +554,7 @@ public class VideoRendererGui implements GLSurfaceView.Renderer { @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 ed5fbc75b7..5e26eaade5 100644 --- a/talk/app/webrtc/java/jni/peerconnection_jni.cc +++ b/talk/app/webrtc/java/jni/peerconnection_jni.cc @@ -696,21 +696,23 @@ class VideoRendererWrapper : public VideoRendererInterface { virtual ~VideoRendererWrapper() {} - void SetSize(int width, int height) override { - ScopedLocalRefFrame local_ref_frame(AttachCurrentThreadIfNeeded()); - const bool kNotReserved = false; // What does this param mean?? - renderer_->SetSize(width, height, kNotReserved); - } - - void RenderFrame(const cricket::VideoFrame* frame) override { + // This wraps VideoRenderer which still has SetSize. + void RenderFrame(const cricket::VideoFrame* video_frame) 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); + } renderer_->RenderFrame(frame); } private: explicit VideoRendererWrapper(cricket::VideoRenderer* renderer) - : renderer_(renderer) {} - + : renderer_(renderer), width_(0), height_(0) {} + int width_, height_; scoped_ptr renderer_; }; @@ -720,8 +722,6 @@ class JavaVideoRendererWrapper : public VideoRendererInterface { 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,14 +738,13 @@ class JavaVideoRendererWrapper : public VideoRendererInterface { virtual ~JavaVideoRendererWrapper() {} - void SetSize(int width, int height) override { + void RenderFrame(const cricket::VideoFrame* video_frame) override { ScopedLocalRefFrame local_ref_frame(jni()); - jni()->CallVoidMethod(*j_callbacks_, j_set_size_id_, width, height); - CHECK_EXCEPTION(jni()); - } - void RenderFrame(const cricket::VideoFrame* frame) override { - ScopedLocalRefFrame local_ref_frame(jni()); + // TODO(guoweis): Remove once the java implementation supports rotation. + const cricket::VideoFrame* frame = + video_frame->GetCopyWithRotationApplied(); + if (frame->GetNativeHandle() != NULL) { jobject j_frame = CricketToJavaTextureFrame(frame); jni()->CallVoidMethod(*j_callbacks_, j_render_frame_id_, j_frame); @@ -798,7 +797,6 @@ class JavaVideoRendererWrapper : public VideoRendererInterface { } ScopedGlobalRef j_callbacks_; - jmethodID j_set_size_id_; jmethodID j_render_frame_id_; ScopedGlobalRef 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 2dc9279ecf..45fafa51f3 100644 --- a/talk/app/webrtc/java/src/org/webrtc/VideoRenderer.java +++ b/talk/app/webrtc/java/src/org/webrtc/VideoRenderer.java @@ -144,7 +144,8 @@ public class VideoRenderer { /** The real meat of VideoRendererInterface. */ public static interface Callbacks { - public void setSize(int width, int height); + // |frame| might have pending rotation and implementation of Callbacks + // should handle that by applying rotation during rendering. 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 ab0510a8cf..4e364b5a4a 100644 --- a/talk/app/webrtc/java/testcommon/src/org/webrtc/PeerConnectionTest.java +++ b/talk/app/webrtc/java/testcommon/src/org/webrtc/PeerConnectionTest.java @@ -59,7 +59,6 @@ public class PeerConnectionTest { 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; @@ -113,19 +112,8 @@ public class PeerConnectionTest { gotIceCandidates.add(candidate); } - 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) { + private 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); @@ -146,6 +134,7 @@ public class PeerConnectionTest { @Override public synchronized void renderFrame(VideoRenderer.I420Frame frame) { + setSize(frame.width, frame.height); --expectedFramesDelivered; } @@ -315,9 +304,6 @@ public class PeerConnectionTest { stillWaitingForExpectations.add( "expectedRemoveStreamLabels: " + expectedRemoveStreamLabels.size()); } - if (expectedSetSize != 0) { - stillWaitingForExpectations.add("expectedSetSize"); - } if (expectedFramesDelivered > 0) { stillWaitingForExpectations.add( "expectedFramesDelivered: " + expectedFramesDelivered); @@ -436,8 +422,7 @@ public class PeerConnectionTest { public int height = -1; public int numFramesDelivered = 0; - @Override - public void setSize(int width, int height) { + private void setSize(int width, int height) { assertEquals(this.width, -1); assertEquals(this.height, -1); this.width = width; @@ -542,7 +527,6 @@ public class PeerConnectionTest { VideoSource videoSource = factory.createVideoSource( VideoCapturer.create(""), new MediaConstraints()); - offeringExpectations.expectSetSize(); offeringExpectations.expectRenegotiationNeeded(); WeakReference oLMS = addTracksToPC( factory, offeringPC, videoSource, "offeredMediaStream", @@ -574,7 +558,6 @@ public class PeerConnectionTest { assertTrue(sdpLatch.await()); assertNull(sdpLatch.getSdp()); - answeringExpectations.expectSetSize(); answeringExpectations.expectRenegotiationNeeded(); WeakReference aLMS = addTracksToPC( factory, answeringPC, videoSource, "answeredMediaStream", @@ -636,8 +619,6 @@ public class PeerConnectionTest { // 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 ce5e04536d..4de06488a7 100644 --- a/talk/app/webrtc/mediastreaminterface.h +++ b/talk/app/webrtc/mediastreaminterface.h @@ -115,9 +115,27 @@ class MediaStreamTrackInterface : public rtc::RefCountInterface, // Interface for rendering VideoFrames from a VideoTrack class VideoRendererInterface { public: - virtual void SetSize(int width, int height) = 0; + // 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 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 d636a56808..8bd98fa8a9 100644 --- a/talk/app/webrtc/test/fakevideotrackrenderer.h +++ b/talk/app/webrtc/test/fakevideotrackrenderer.h @@ -35,34 +35,55 @@ namespace webrtc { class FakeVideoTrackRenderer : public VideoRendererInterface { public: - explicit FakeVideoTrackRenderer(VideoTrackInterface* video_track) - : video_track_(video_track) { + 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_->AddRenderer(this); } ~FakeVideoTrackRenderer() { video_track_->RemoveRenderer(this); } - // Implements VideoRendererInterface - virtual void SetSize(int width, int height) { - fake_renderer_.SetSize(width, height, 0); - } + virtual void RenderFrame(const cricket::VideoFrame* video_frame) override { + last_frame_ = const_cast(video_frame); + + const cricket::VideoFrame* frame = + can_apply_rotation_ ? video_frame + : video_frame->GetCopyWithRotationApplied(); + + if (!fake_renderer_.SetSize(static_cast(frame->GetWidth()), + static_cast(frame->GetHeight()), 0)) { + return; + } - virtual void RenderFrame(const cricket::VideoFrame* frame) { fake_renderer_.RenderFrame(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(); } - 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 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 263fefcf6a..529866d5f8 100644 --- a/talk/app/webrtc/videotrack_unittest.cc +++ b/talk/app/webrtc/videotrack_unittest.cc @@ -43,57 +43,132 @@ using webrtc::VideoSource; 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 channel_manager_; + rtc::scoped_refptr video_track_; +}; + // Test adding renderers to a video track and render to them by providing // frames to the source. -TEST(VideoTrack, RenderVideo) { - static const char kVideoTrackId[] = "track_id"; - - rtc::scoped_ptr 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 video_track( - VideoTrack::Create(kVideoTrackId, - VideoSource::Create(channel_manager_.get(), - new webrtc::RemoteVideoCapturer(), - NULL))); - // FakeVideoTrackRenderer register itself to |video_track| +TEST_F(VideoTrackTest, RenderVideo) { + // FakeVideoTrackRenderer register itself to |video_track_| rtc::scoped_ptr renderer_1( - new FakeVideoTrackRenderer(video_track.get())); + new FakeVideoTrackRenderer(video_track_.get())); - cricket::VideoRenderer* render_input = video_track->GetSource()->FrameInput(); - ASSERT_FALSE(render_input == NULL); + cricket::VideoRenderer* renderer_input = + video_track_->GetSource()->FrameInput(); + ASSERT_FALSE(renderer_input == NULL); cricket::WebRtcVideoFrame frame; frame.InitToBlack(123, 123, 1, 1, 0, 0); - render_input->RenderFrame(&frame); + renderer_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 renderer_2( - new FakeVideoTrackRenderer(video_track.get())); + new FakeVideoTrackRenderer(video_track_.get())); - render_input->RenderFrame(&frame); + renderer_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()); - render_input->RenderFrame(&frame); + video_track_->RemoveRenderer(renderer_1.get()); + renderer_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 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 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 03f080dbf8..08bf05acb7 100644 --- a/talk/app/webrtc/videotrackrenderers.cc +++ b/talk/app/webrtc/videotrackrenderers.cc @@ -26,19 +26,21 @@ */ #include "talk/app/webrtc/videotrackrenderers.h" +#include "talk/media/base/videoframe.h" namespace webrtc { VideoTrackRenderers::VideoTrackRenderers() - : width_(0), - height_(0), - enabled_(true) { + : enabled_(true) { } VideoTrackRenderers::~VideoTrackRenderers() { } void VideoTrackRenderers::AddRenderer(VideoRendererInterface* renderer) { + if (!renderer) { + return; + } rtc::CritScope cs(&critical_section_); std::vector::iterator it = renderers_.begin(); for (; it != renderers_.end(); ++it) { @@ -65,14 +67,6 @@ void VideoTrackRenderers::SetEnabled(bool enable) { } bool VideoTrackRenderers::SetSize(int width, int height, int reserved) { - rtc::CritScope cs(&critical_section_); - width_ = width; - height_ = height; - std::vector::iterator it = renderers_.begin(); - for (; it != renderers_.end(); ++it) { - it->renderer_->SetSize(width, height); - it->size_set_ = true; - } return true; } @@ -81,13 +75,14 @@ bool VideoTrackRenderers::RenderFrame(const cricket::VideoFrame* frame) { if (!enabled_) { return true; } + std::vector::iterator it = renderers_.begin(); for (; it != renderers_.end(); ++it) { - if (!it->size_set_) { - it->renderer_->SetSize(width_, height_); - it->size_set_ = true; + if (it->can_apply_rotation_) { + it->renderer_->RenderFrame(frame); + } else { + it->renderer_->RenderFrame(frame->GetCopyWithRotationApplied()); } - it->renderer_->RenderFrame(frame); } return true; } diff --git a/talk/app/webrtc/videotrackrenderers.h b/talk/app/webrtc/videotrackrenderers.h index 5bec347cf8..02d610f706 100644 --- a/talk/app/webrtc/videotrackrenderers.h +++ b/talk/app/webrtc/videotrackrenderers.h @@ -33,6 +33,7 @@ #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 { @@ -58,14 +59,11 @@ class VideoTrackRenderers : public cricket::VideoRenderer { struct RenderObserver { explicit RenderObserver(VideoRendererInterface* renderer) : renderer_(renderer), - size_set_(false) { - } + can_apply_rotation_(renderer->CanApplyRotation()) {} VideoRendererInterface* renderer_; - bool size_set_; + bool can_apply_rotation_; }; - int width_; - int height_; bool enabled_; std::vector renderers_; diff --git a/talk/examples/androidtests/src/org/appspot/apprtc/test/PeerConnectionClientTest.java b/talk/examples/androidtests/src/org/appspot/apprtc/test/PeerConnectionClientTest.java index d10c484ab4..66483d3a56 100644 --- a/talk/examples/androidtests/src/org/appspot/apprtc/test/PeerConnectionClientTest.java +++ b/talk/examples/androidtests/src/org/appspot/apprtc/test/PeerConnectionClientTest.java @@ -98,8 +98,7 @@ public class PeerConnectionClientTest extends InstrumentationTestCase doneRendering = new CountDownLatch(expectedFrames); } - @Override - public synchronized void setSize(int width, int height) { + private synchronized void setSize(int width, int height) { Log.d(TAG, "Set size: " + width + " x " + height); this.width = width; this.height = height; diff --git a/talk/media/base/videoframe.cc b/talk/media/base/videoframe.cc index 77417a2769..990552e726 100644 --- a/talk/media/base/videoframe.cc +++ b/talk/media/base/videoframe.cc @@ -33,7 +33,9 @@ #include "libyuv/planar_functions.h" #include "libyuv/scale.h" #include "talk/media/base/videocommon.h" +#include "webrtc/base/checks.h" #include "webrtc/base/logging.h" +#include "webrtc/base/scoped_ptr.h" namespace cricket { @@ -120,6 +122,51 @@ void VideoFrame::CopyToFrame(VideoFrame* dst) const { dst->GetYPitch(), dst->GetUPitch(), dst->GetVPitch()); } +const VideoFrame* VideoFrame::GetCopyWithRotationApplied() const { + // If the frame is not rotated, the caller should reuse this frame instead of + // making a redundant copy. + if (GetVideoRotation() == webrtc::kVideoRotation_0) { + return this; + } + + // If the video frame is backed up by a native handle, it resides in the GPU + // memory which we can't rotate here. The assumption is that the renderers + // which uses GPU to render should be able to rotate themselves. + DCHECK(!GetNativeHandle()); + + if (rotated_frame_) { + return rotated_frame_.get(); + } + + int width = static_cast(GetWidth()); + int height = static_cast(GetHeight()); + + int rotated_width = width; + int rotated_height = height; + if (GetVideoRotation() == webrtc::kVideoRotation_90 || + GetVideoRotation() == webrtc::kVideoRotation_270) { + rotated_width = height; + rotated_height = width; + } + + rotated_frame_.reset(CreateEmptyFrame(rotated_width, rotated_height, + GetPixelWidth(), GetPixelHeight(), + GetElapsedTime(), GetTimeStamp())); + + // TODO(guoweis): Add a function in webrtc_libyuv.cc to convert from + // VideoRotation to libyuv::RotationMode. + int ret = libyuv::I420Rotate( + GetYPlane(), GetYPitch(), GetUPlane(), GetUPitch(), GetVPlane(), + GetVPitch(), rotated_frame_->GetYPlane(), rotated_frame_->GetYPitch(), + rotated_frame_->GetUPlane(), rotated_frame_->GetUPitch(), + rotated_frame_->GetVPlane(), rotated_frame_->GetVPitch(), width, height, + static_cast(GetVideoRotation())); + if (ret == 0) { + return rotated_frame_.get(); + } + return nullptr; +} + size_t VideoFrame::ConvertToRgbBuffer(uint32 to_fourcc, uint8* buffer, size_t size, diff --git a/talk/media/base/videoframe.h b/talk/media/base/videoframe.h index b41965df12..ce3cd284f7 100644 --- a/talk/media/base/videoframe.h +++ b/talk/media/base/videoframe.h @@ -29,6 +29,7 @@ #define TALK_MEDIA_BASE_VIDEOFRAME_H_ #include "webrtc/base/basictypes.h" +#include "webrtc/base/scoped_ptr.h" #include "webrtc/base/stream.h" #include "webrtc/common_video/rotation.h" @@ -159,6 +160,10 @@ class VideoFrame { // Writes the frame into the target VideoFrame. virtual void CopyToFrame(VideoFrame* target) const; + // Return a copy of frame which has its pending rotation applied. The + // ownership of the returned frame is held by this frame. + virtual const VideoFrame* GetCopyWithRotationApplied() const; + // Writes the frame into the given stream and returns the StreamResult. // See webrtc/base/stream.h for a description of StreamResult and error. // Error may be NULL. If a non-success value is returned from @@ -223,6 +228,11 @@ class VideoFrame { size_t pixel_height, int64_t elapsed_time, int64_t time_stamp) const = 0; + + private: + // This is mutable as the calculation is expensive but once calculated, it + // remains const. + mutable rtc::scoped_ptr rotated_frame_; }; } // namespace cricket diff --git a/talk/media/base/videorenderer.h b/talk/media/base/videorenderer.h index 73b0eab267..118942eaab 100644 --- a/talk/media/base/videorenderer.h +++ b/talk/media/base/videorenderer.h @@ -42,7 +42,10 @@ class VideoFrame; class VideoRenderer { public: virtual ~VideoRenderer() {} - // Called when the video has changed size. + // Called when the video has changed size. This is also used as an + // initialization method to set the UI size before any video frame + // rendered. webrtc::ExternalRenderer's FrameSizeChange will invoke this when + // it's called or later when a VideoRenderer is attached. virtual bool SetSize(int width, int height, int reserved) = 0; // Called when a new frame is available for display. virtual bool RenderFrame(const VideoFrame *frame) = 0; diff --git a/talk/media/devices/carbonvideorenderer.cc b/talk/media/devices/carbonvideorenderer.cc index d8c30a6d34..f96dfacad8 100644 --- a/talk/media/devices/carbonvideorenderer.cc +++ b/talk/media/devices/carbonvideorenderer.cc @@ -122,11 +122,17 @@ bool CarbonVideoRenderer::SetSize(int width, int height, int reserved) { return true; } -bool CarbonVideoRenderer::RenderFrame(const VideoFrame* frame) { - if (!frame) { +bool CarbonVideoRenderer::RenderFrame(const VideoFrame* video_frame) { + if (!video_frame) { return false; } { + const VideoFrame* frame = video_frame->GetCopyWithRotationApplied(); + + if (!SetSize(frame->GetWidth(), frame->GetHeight(), 0)) { + return false; + } + // Grab the image lock so we are not trashing up the image being drawn. rtc::CritScope cs(&image_crit_); frame->ConvertToRgbBuffer(cricket::FOURCC_ABGR, diff --git a/talk/media/devices/gdivideorenderer.cc b/talk/media/devices/gdivideorenderer.cc old mode 100755 new mode 100644 index 5ccef31fe5..f6fee1ccc8 --- a/talk/media/devices/gdivideorenderer.cc +++ b/talk/media/devices/gdivideorenderer.cc @@ -146,11 +146,18 @@ bool GdiVideoRenderer::VideoWindow::SetSize(int width, int height) { return true; } -bool GdiVideoRenderer::VideoWindow::RenderFrame(const VideoFrame* frame) { +bool GdiVideoRenderer::VideoWindow::RenderFrame(const VideoFrame* video_frame) { if (!handle()) { return false; } + const VideoFrame* frame = video_frame->GetCopyWithRotationApplied(); + + if (!SetSize(static_cast(frame->GetWidth()), + static_cast(frame->GetHeight()))) { + return false; + } + SendMessage(handle(), kRenderFrameMsg, reinterpret_cast(frame), 0); return true; } diff --git a/talk/media/devices/gtkvideorenderer.cc b/talk/media/devices/gtkvideorenderer.cc index 806762c653..b44b22cdab 100755 --- a/talk/media/devices/gtkvideorenderer.cc +++ b/talk/media/devices/gtkvideorenderer.cc @@ -53,7 +53,9 @@ GtkVideoRenderer::GtkVideoRenderer(int x, int y) : window_(NULL), draw_area_(NULL), initial_x_(x), - initial_y_(y) { + initial_y_(y), + width_(0), + height_(0) { g_type_init(); // g_thread_init API is deprecated since glib 2.31.0, see release note: // http://mail.gnome.org/archives/gnome-announce-list/2011-October/msg00041.html @@ -77,6 +79,11 @@ GtkVideoRenderer::~GtkVideoRenderer() { bool GtkVideoRenderer::SetSize(int width, int height, int reserved) { ScopedGdkLock lock; + // If the dimension is the same, no-op. + if (width_ == width && height_ == height) { + return true; + } + // For the first frame, initialize the GTK window if ((!window_ && !Initialize(width, height)) || IsClosed()) { return false; @@ -84,11 +91,21 @@ bool GtkVideoRenderer::SetSize(int width, int height, int reserved) { image_.reset(new uint8[width * height * 4]); gtk_widget_set_size_request(draw_area_, width, height); + + width_ = width; + height_ = height; return true; } -bool GtkVideoRenderer::RenderFrame(const VideoFrame* frame) { - if (!frame) { +bool GtkVideoRenderer::RenderFrame(const VideoFrame* video_frame) { + if (!video_frame) { + return false; + } + + const VideoFrame* frame = video_frame->GetCopyWithRotationApplied(); + + // Need to set size as the frame might be rotated. + if (!SetSize(frame->GetWidth(), frame->GetHeight(), 0)) { return false; } diff --git a/talk/media/devices/gtkvideorenderer.h b/talk/media/devices/gtkvideorenderer.h index 5a3e94b051..48e13640c6 100755 --- a/talk/media/devices/gtkvideorenderer.h +++ b/talk/media/devices/gtkvideorenderer.h @@ -64,6 +64,9 @@ class GtkVideoRenderer : public VideoRenderer { // The initial position of the window. int initial_x_; int initial_y_; + + int width_; + int height_; }; } // namespace cricket diff --git a/talk/media/webrtc/webrtcvideoframe.h b/talk/media/webrtc/webrtcvideoframe.h index 8aa28ae59a..29e5ec8ec7 100644 --- a/talk/media/webrtc/webrtcvideoframe.h +++ b/talk/media/webrtc/webrtcvideoframe.h @@ -124,6 +124,9 @@ class WebRtcVideoFrame : public VideoFrame { virtual size_t ConvertToRgbBuffer(uint32 to_fourcc, uint8* buffer, size_t size, int stride_rgb) const; + protected: + void SetRotation(webrtc::VideoRotation rotation) { rotation_ = rotation; } + private: virtual VideoFrame* CreateEmptyFrame(int w, int h, size_t pixel_width, size_t pixel_height, diff --git a/talk/media/webrtc/webrtcvideoframe_unittest.cc b/talk/media/webrtc/webrtcvideoframe_unittest.cc index ff9a675f87..ffce9a7bce 100644 --- a/talk/media/webrtc/webrtcvideoframe_unittest.cc +++ b/talk/media/webrtc/webrtcvideoframe_unittest.cc @@ -43,6 +43,27 @@ class NativeHandleImpl : public webrtc::NativeHandle { int32_t ref_count_; }; +namespace { + +class WebRtcVideoTestFrame : public cricket::WebRtcVideoFrame { + public: + using cricket::WebRtcVideoFrame::SetRotation; + + virtual VideoFrame* CreateEmptyFrame(int w, + int h, + size_t pixel_width, + size_t pixel_height, + int64_t elapsed_time, + int64_t time_stamp) const override { + WebRtcVideoTestFrame* frame = new WebRtcVideoTestFrame(); + frame->InitToBlack(w, h, pixel_width, pixel_height, elapsed_time, + time_stamp); + return frame; + } +}; + +} // namespace + class WebRtcVideoFrameTest : public VideoFrameTest { public: WebRtcVideoFrameTest() { @@ -343,3 +364,31 @@ TEST_F(WebRtcVideoFrameTest, CopyTextureFrame) { EXPECT_EQ(frame1.GetTimeStamp(), frame2->GetTimeStamp()); delete frame2; } + +TEST_F(WebRtcVideoFrameTest, ApplyRotationToFrame) { + WebRtcVideoTestFrame applied0; + EXPECT_TRUE(IsNull(applied0)); + rtc::scoped_ptr ms(CreateYuvSample(kWidth, kHeight, 12)); + EXPECT_TRUE( + LoadFrame(ms.get(), cricket::FOURCC_I420, kWidth, kHeight, &applied0)); + + // Claim that this frame needs to be rotated for 90 degree. + applied0.SetRotation(webrtc::kVideoRotation_90); + + // Apply rotation on frame 1. Output should be different from frame 1. + WebRtcVideoTestFrame* applied90 = const_cast( + static_cast( + applied0.GetCopyWithRotationApplied())); + EXPECT_TRUE(applied90); + EXPECT_EQ(applied90->GetVideoRotation(), webrtc::kVideoRotation_0); + EXPECT_FALSE(IsEqual(applied0, *applied90, 0)); + + // Claim the frame 2 needs to be rotated for another 270 degree. The output + // from frame 2 rotation should be the same as frame 1. + applied90->SetRotation(webrtc::kVideoRotation_270); + const cricket::VideoFrame* applied360 = + applied90->GetCopyWithRotationApplied(); + EXPECT_TRUE(applied360); + EXPECT_EQ(applied360->GetVideoRotation(), webrtc::kVideoRotation_0); + EXPECT_TRUE(IsEqual(applied0, *applied360, 0)); +}