diff --git a/talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java b/talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java index 4ad8ccca18..0f616c9475 100644 --- a/talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java +++ b/talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java @@ -271,15 +271,15 @@ public class VideoRendererGui implements GLSurfaceView.Renderer { // Mapping array from original UV mapping to the rotated mapping. The number // is the position where the original UV coordination should be mapped - // to. (0,1) is the left top coord. (2,3) is the top bottom. (4,5) is the - // right top. (6,7) is the right bottom. Note that this is the coordination - // that got rotated. For example, using the original left bottom (2,3) as - // the top left (0,1) means 90 degree clockwise rotation. + // to. (0,1) is the top left coord. (2,3) is the bottom left. (4,5) is the + // top right. (6,7) is the bottom right. private static int rotation_matrix[][] = - { {0, 1, 2, 3, 4, 5, 6, 7}, // 0 degree - {2, 3, 6, 7, 0, 1, 4, 5}, // 90 degree (clockwise) - {6, 7, 4, 5, 2, 3, 0, 1}, // 180 degree (clockwise) - {4, 5, 0, 1, 6, 7, 2, 3} };// 270 degree (clockwise) + { {4, 5, 0, 1, 6, 7, 2, 3}, // 90 degree (clockwise) + {6, 7, 4, 5, 2, 3, 0, 1}, // 180 degree (clockwise) + {2, 3, 6, 7, 0, 1, 4, 5} }; // 270 degree (clockwise) + + private static int mirror_matrix[] = + {4, 1, 6, 3, 0, 5, 2, 7}; // mirrored private YuvImageRenderer( GLSurfaceView surface, int id, @@ -381,12 +381,21 @@ public class VideoRendererGui implements GLSurfaceView.Renderer { } if (scalingType == ScalingType.SCALE_ASPECT_FILL) { // Need to re-adjust UV coordinates to match display AR. + boolean adjustU = true; + float ratio = 0; if (displayAspectRatio > videoAspectRatio) { - texOffsetV = (1.0f - videoAspectRatio / displayAspectRatio) / + ratio = (1.0f - videoAspectRatio / displayAspectRatio) / 2.0f; + adjustU = (rotationDegree == 90 || rotationDegree == 270); } else { - texOffsetU = (1.0f - displayAspectRatio / videoAspectRatio) / + ratio = (1.0f - displayAspectRatio / videoAspectRatio) / 2.0f; + adjustU = (rotationDegree == 0 || rotationDegree == 180); + } + if (adjustU) { + texOffsetU = ratio; + } else { + texOffsetV = ratio; } } Log.d(TAG, " Texture vertices: (" + texLeft + "," + texBottom + @@ -402,37 +411,59 @@ public class VideoRendererGui implements GLSurfaceView.Renderer { Log.d(TAG, " Texture UV offsets: " + texOffsetU + ", " + texOffsetV); float uLeft = texOffsetU; float uRight = 1.0f - texOffsetU; - if (mirror) { - // Swap U coordinates for mirror image. - uLeft = 1.0f - texOffsetU; - uRight = texOffsetU; - } float textureCoordinatesFloat[] = new float[] { - uLeft, texOffsetV, // left top - uLeft, 1.0f - texOffsetV, // left bottom - uRight, texOffsetV, // right top - uRight, 1.0f - texOffsetV // right bottom + uLeft, texOffsetV, // top left + uLeft, 1.0f - texOffsetV, // bottom left + uRight, texOffsetV, // top right + uRight, 1.0f - texOffsetV // bottom right }; - float textureCoordinatesRotatedFloat[]; - if (rotationDegree == 0) { - textureCoordinatesRotatedFloat = textureCoordinatesFloat; - } else { - textureCoordinatesRotatedFloat = - new float[textureCoordinatesFloat.length]; - int index = rotationDegree / 90; - for(int i = 0; i < textureCoordinatesFloat.length; i++) { - textureCoordinatesRotatedFloat[rotation_matrix[index][i]] = - textureCoordinatesFloat[i]; - } - } + // Rotation needs to be done before mirroring. + textureCoordinatesFloat = applyRotation(textureCoordinatesFloat, + rotationDegree); + textureCoordinatesFloat = applyMirror(textureCoordinatesFloat, + mirror); textureCoords = - directNativeFloatBuffer(textureCoordinatesRotatedFloat); + directNativeFloatBuffer(textureCoordinatesFloat); } updateTextureProperties = false; } } + + private float[] applyMirror(float textureCoordinatesFloat[], + boolean mirror) { + if (!mirror) { + return textureCoordinatesFloat; + } + + return applyMatrixOperation(textureCoordinatesFloat, + mirror_matrix); + } + + private float[] applyRotation(float textureCoordinatesFloat[], + int rotationDegree) { + if (rotationDegree == 0) { + return textureCoordinatesFloat; + } + + int index = rotationDegree / 90 - 1; + return applyMatrixOperation(textureCoordinatesFloat, + rotation_matrix[index]); + } + + private float[] applyMatrixOperation(float textureCoordinatesFloat[], + int matrix_operation[]) { + float textureCoordinatesModifiedFloat[] = + new float[textureCoordinatesFloat.length]; + + for(int i = 0; i < textureCoordinatesFloat.length; i++) { + textureCoordinatesModifiedFloat[matrix_operation[i]] = + textureCoordinatesFloat[i]; + } + return textureCoordinatesModifiedFloat; + } + private void draw() { if (!seenFrame) { // No frame received yet - nothing to render. diff --git a/talk/media/base/videocapturer.cc b/talk/media/base/videocapturer.cc index bed3b0c616..653e7934a3 100644 --- a/talk/media/base/videocapturer.cc +++ b/talk/media/base/videocapturer.cc @@ -254,6 +254,8 @@ bool VideoCapturer::MuteToBlackThenPause(bool muted) { return Pause(false); } +// Note that the last caller decides whether rotation should be applied if there +// are multiple send streams using the same camera. bool VideoCapturer::SetApplyRotation(bool enable) { apply_rotation_ = enable; if (frame_factory_) { diff --git a/talk/media/base/videoframe.h b/talk/media/base/videoframe.h index 5b1663c7a8..0aac645128 100644 --- a/talk/media/base/videoframe.h +++ b/talk/media/base/videoframe.h @@ -83,8 +83,10 @@ class VideoFrame { } // Basic accessors. + // Note this is the width and height without rotation applied. virtual size_t GetWidth() const = 0; virtual size_t GetHeight() const = 0; + size_t GetChromaWidth() const { return (GetWidth() + 1) / 2; } size_t GetChromaHeight() const { return (GetHeight() + 1) / 2; } size_t GetChromaSize() const { return GetUPitch() * GetChromaHeight(); } diff --git a/talk/media/webrtc/webrtcvideocapturer.cc b/talk/media/webrtc/webrtcvideocapturer.cc index 0e8f56bfdd..bcaeb89ea4 100644 --- a/talk/media/webrtc/webrtcvideocapturer.cc +++ b/talk/media/webrtc/webrtcvideocapturer.cc @@ -44,6 +44,7 @@ #include "webrtc/base/win32.h" // Need this to #include the impl files. #include "webrtc/modules/video_capture/include/video_capture_factory.h" +#include "webrtc/system_wrappers/interface/field_trial.h" namespace cricket { @@ -260,10 +261,18 @@ bool WebRtcVideoCapturer::GetBestCaptureFormat(const VideoFormat& desired, return true; } bool WebRtcVideoCapturer::SetApplyRotation(bool enable) { - rtc::CritScope cs(&critical_section_stopping_); - + // Can't take lock here as this will cause deadlock with + // OnIncomingCapturedFrame. In fact, the whole method, including methods it + // calls, can't take lock. assert(module_); + const std::string group_name = + webrtc::field_trial::FindFullName("WebRTC-CVO"); + + if (group_name == "Disabled") { + return true; + } + if (!VideoCapturer::SetApplyRotation(enable)) { return false; } diff --git a/talk/media/webrtc/webrtcvideoengine.cc b/talk/media/webrtc/webrtcvideoengine.cc index 287de72320..a103caed54 100644 --- a/talk/media/webrtc/webrtcvideoengine.cc +++ b/talk/media/webrtc/webrtcvideoengine.cc @@ -425,7 +425,8 @@ class WebRtcRenderAdapter : public webrtc::ExternalRenderer { WebRtcVideoFrame cricket_frame( webrtc_frame.video_frame_buffer(), elapsed_time_ms * rtc::kNumNanosecsPerMillisec, - webrtc_frame.render_time_ms() * rtc::kNumNanosecsPerMillisec); + webrtc_frame.render_time_ms() * rtc::kNumNanosecsPerMillisec, + webrtc_frame.rotation()); return renderer_->RenderFrame(&cricket_frame) ? 0 : -1; } @@ -1161,6 +1162,9 @@ void WebRtcVideoEngine::Construct(ViEWrapper* vie_wrapper, rtp_header_extensions_.push_back( RtpHeaderExtension(kRtpAbsoluteSenderTimeHeaderExtension, kRtpAbsoluteSenderTimeHeaderExtensionDefaultId)); + rtp_header_extensions_.push_back( + RtpHeaderExtension(kRtpVideoRotationHeaderExtension, + kRtpVideoRotationHeaderExtensionDefaultId)); } WebRtcVideoEngine::~WebRtcVideoEngine() { @@ -2832,6 +2836,10 @@ bool WebRtcVideoMediaChannel::SetCapturer(uint32 ssrc, QueueBlackFrame(ssrc, timestamp, VideoFormat::FpsToInterval(send_codec_->maxFramerate)); } + + capturer->SetApplyRotation( + !FindHeaderExtension(send_extensions_, kRtpVideoRotationHeaderExtension)); + return true; } @@ -2929,6 +2937,8 @@ bool WebRtcVideoMediaChannel::SetRecvRtpHeaderExtensions( FindHeaderExtension(extensions, kRtpTimestampOffsetHeaderExtension); const RtpHeaderExtension* send_time_extension = FindHeaderExtension(extensions, kRtpAbsoluteSenderTimeHeaderExtension); + const RtpHeaderExtension* cvo_extension = + FindHeaderExtension(extensions, kRtpVideoRotationHeaderExtension); // Loop through all receive channels and enable/disable the extensions. for (RecvChannelMap::iterator channel_it = recv_channels_.begin(); @@ -2944,6 +2954,10 @@ bool WebRtcVideoMediaChannel::SetRecvRtpHeaderExtensions( send_time_extension)) { return false; } + if (!SetHeaderExtension(&webrtc::ViERTP_RTCP::SetReceiveVideoRotationStatus, + channel_id, cvo_extension)) { + return false; + } } receive_extensions_ = extensions; @@ -2960,6 +2974,8 @@ bool WebRtcVideoMediaChannel::SetSendRtpHeaderExtensions( FindHeaderExtension(extensions, kRtpTimestampOffsetHeaderExtension); const RtpHeaderExtension* send_time_extension = FindHeaderExtension(extensions, kRtpAbsoluteSenderTimeHeaderExtension); + const RtpHeaderExtension* cvo_extension = + FindHeaderExtension(extensions, kRtpVideoRotationHeaderExtension); // Loop through all send channels and enable/disable the extensions. for (SendChannelMap::iterator channel_it = send_channels_.begin(); @@ -2975,6 +2991,10 @@ bool WebRtcVideoMediaChannel::SetSendRtpHeaderExtensions( send_time_extension)) { return false; } + if (!SetHeaderExtension(&webrtc::ViERTP_RTCP::SetSendVideoRotationStatus, + channel_id, cvo_extension)) { + return false; + } } if (send_time_extension) { @@ -2986,6 +3006,14 @@ bool WebRtcVideoMediaChannel::SetSendRtpHeaderExtensions( send_time_extension->id); } + // For now assume that all streams want the same CVO setting. + // TODO(guoweis): Remove the need for this assumption. + for (const auto& kv : send_channels_) { + if (kv.second->video_capturer()) { + kv.second->video_capturer()->SetApplyRotation(!cvo_extension); + } + } + send_extensions_ = extensions; return true; } @@ -3484,6 +3512,11 @@ bool WebRtcVideoMediaChannel::ConfigureReceiving(int channel_id, receive_extensions_, kRtpAbsoluteSenderTimeHeaderExtension)) { return false; } + if (!SetHeaderExtension(&webrtc::ViERTP_RTCP::SetReceiveVideoRotationStatus, + channel_id, receive_extensions_, + kRtpVideoRotationHeaderExtension)) { + return false; + } if (receiver_report_ssrc_ != kSsrcUnset) { if (engine()->vie()->rtp()->SetLocalSSRC( @@ -3594,6 +3627,12 @@ bool WebRtcVideoMediaChannel::ConfigureSending(int channel_id, return false; } + if (!SetHeaderExtension(&webrtc::ViERTP_RTCP::SetSendVideoRotationStatus, + channel_id, send_extensions_, + kRtpVideoRotationHeaderExtension)) { + return false; + } + if (engine()->vie()->rtp()->SetTransmissionSmoothingStatus(channel_id, true) != 0) { LOG_RTCERR2(SetTransmissionSmoothingStatus, channel_id, true); diff --git a/talk/media/webrtc/webrtcvideoengine2.cc b/talk/media/webrtc/webrtcvideoengine2.cc index c9781a7498..ea1fa71b9c 100644 --- a/talk/media/webrtc/webrtcvideoengine2.cc +++ b/talk/media/webrtc/webrtcvideoengine2.cc @@ -134,6 +134,17 @@ static std::string RtpExtensionsToString( return out.str(); } +inline const webrtc::RtpExtension* FindHeaderExtension( + const std::vector& extensions, + const std::string& name) { + for (const auto& kv : extensions) { + if (kv.name == name) { + return &kv; + } + } + return NULL; +} + // Merges two fec configs and logs an error if a conflict arises // such that merging in diferent order would trigger a diferent output. static void MergeFecConfig(const webrtc::FecConfig& other, @@ -368,6 +379,9 @@ WebRtcVideoEngine2::WebRtcVideoEngine2(WebRtcVoiceEngine* voice_engine) rtp_header_extensions_.push_back( RtpHeaderExtension(kRtpAbsoluteSenderTimeHeaderExtension, kRtpAbsoluteSenderTimeHeaderExtensionDefaultId)); + rtp_header_extensions_.push_back( + RtpHeaderExtension(kRtpVideoRotationHeaderExtension, + kRtpVideoRotationHeaderExtensionDefaultId)); } WebRtcVideoEngine2::~WebRtcVideoEngine2() { @@ -1135,7 +1149,16 @@ bool WebRtcVideoChannel2::SetCapturer(uint32 ssrc, VideoCapturer* capturer) { LOG(LS_ERROR) << "No sending stream on ssrc " << ssrc; return false; } - return send_streams_[ssrc]->SetCapturer(capturer); + if (!send_streams_[ssrc]->SetCapturer(capturer)) { + return false; + } + + if (capturer) { + capturer->SetApplyRotation( + !FindHeaderExtension(send_rtp_extensions_, + kRtpVideoRotationHeaderExtension)); + } + return true; } bool WebRtcVideoChannel2::SendIntraFrame() { @@ -1258,12 +1281,16 @@ bool WebRtcVideoChannel2::SetSendRtpHeaderExtensions( send_rtp_extensions_ = filtered_extensions; + const webrtc::RtpExtension* cvo_extension = FindHeaderExtension( + send_rtp_extensions_, kRtpVideoRotationHeaderExtension); + rtc::CritScope stream_lock(&stream_crit_); for (std::map::iterator it = send_streams_.begin(); it != send_streams_.end(); ++it) { it->second->SetRtpExtensions(send_rtp_extensions_); + it->second->SetApplyRotation(!cvo_extension); } return true; } @@ -1580,6 +1607,15 @@ WebRtcVideoChannel2::WebRtcVideoSendStream::GetSsrcs() const { return ssrcs_; } +void WebRtcVideoChannel2::WebRtcVideoSendStream::SetApplyRotation( + bool apply_rotation) { + rtc::CritScope cs(&lock_); + if (capturer_ == NULL) + return; + + capturer_->SetApplyRotation(apply_rotation); +} + void WebRtcVideoChannel2::WebRtcVideoSendStream::SetOptions( const VideoOptions& options) { rtc::CritScope cs(&lock_); @@ -2121,7 +2157,7 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::RenderFrame( const WebRtcVideoFrame render_frame( frame.video_frame_buffer(), elapsed_time_ms * rtc::kNumNanosecsPerMillisec, - frame.render_time_ms() * rtc::kNumNanosecsPerMillisec); + frame.render_time_ms() * rtc::kNumNanosecsPerMillisec, frame.rotation()); renderer_->RenderFrame(&render_frame); } diff --git a/talk/media/webrtc/webrtcvideoengine2.h b/talk/media/webrtc/webrtcvideoengine2.h index a04d1e5a0a..cd13a01388 100644 --- a/talk/media/webrtc/webrtcvideoengine2.h +++ b/talk/media/webrtc/webrtcvideoengine2.h @@ -284,6 +284,8 @@ class WebRtcVideoChannel2 : public rtc::MessageHandler, void MuteStream(bool mute); bool DisconnectCapturer(); + void SetApplyRotation(bool apply_rotation); + void Start(); void Stop(); diff --git a/talk/media/webrtc/webrtcvideoengine2_unittest.cc b/talk/media/webrtc/webrtcvideoengine2_unittest.cc index cbf516dd78..aabefd6beb 100644 --- a/talk/media/webrtc/webrtcvideoengine2_unittest.cc +++ b/talk/media/webrtc/webrtcvideoengine2_unittest.cc @@ -530,6 +530,82 @@ TEST_F(WebRtcVideoEngine2Test, SupportsAbsoluteSenderTimeHeaderExtension) { FAIL() << "Absolute Sender Time extension not in header-extension list."; } +TEST_F(WebRtcVideoEngine2Test, SupportsVideoRotationHeaderExtension) { + std::vector extensions = engine_.rtp_header_extensions(); + ASSERT_FALSE(extensions.empty()); + for (size_t i = 0; i < extensions.size(); ++i) { + if (extensions[i].uri == kRtpVideoRotationHeaderExtension) { + EXPECT_EQ(kRtpVideoRotationHeaderExtensionDefaultId, extensions[i].id); + return; + } + } + FAIL() << "Video Rotation extension not in header-extension list."; +} + +TEST_F(WebRtcVideoEngine2Test, CVOSetHeaderExtensionBeforeCapturer) { + // Allocate the capturer first to prevent early destruction before channel's + // dtor is called. + cricket::FakeVideoCapturer capturer; + + cricket::FakeWebRtcVideoEncoderFactory encoder_factory; + encoder_factory.AddSupportedVideoCodecType(webrtc::kVideoCodecVP8, "VP8"); + std::vector codecs; + codecs.push_back(kVp8Codec); + + rtc::scoped_ptr channel( + SetUpForExternalEncoderFactory(&encoder_factory, codecs)); + EXPECT_TRUE(channel->AddSendStream(StreamParams::CreateLegacy(kSsrc))); + + // Add CVO extension. + const int id = 1; + std::vector extensions; + extensions.push_back( + cricket::RtpHeaderExtension(kRtpVideoRotationHeaderExtension, id)); + EXPECT_TRUE(channel->SetSendRtpHeaderExtensions(extensions)); + + // Set capturer. + EXPECT_TRUE(channel->SetCapturer(kSsrc, &capturer)); + + // Verify capturer has turned off applying rotation. + EXPECT_FALSE(capturer.GetApplyRotation()); + + // Verify removing header extension turns on applying rotation. + extensions.clear(); + EXPECT_TRUE(channel->SetSendRtpHeaderExtensions(extensions)); + EXPECT_TRUE(capturer.GetApplyRotation()); +} + +TEST_F(WebRtcVideoEngine2Test, CVOSetHeaderExtensionAfterCapturer) { + cricket::FakeVideoCapturer capturer; + + cricket::FakeWebRtcVideoEncoderFactory encoder_factory; + encoder_factory.AddSupportedVideoCodecType(webrtc::kVideoCodecVP8, "VP8"); + std::vector codecs; + codecs.push_back(kVp8Codec); + + rtc::scoped_ptr channel( + SetUpForExternalEncoderFactory(&encoder_factory, codecs)); + EXPECT_TRUE(channel->AddSendStream(StreamParams::CreateLegacy(kSsrc))); + + // Set capturer. + EXPECT_TRUE(channel->SetCapturer(kSsrc, &capturer)); + + // Add CVO extension. + const int id = 1; + std::vector extensions; + extensions.push_back( + cricket::RtpHeaderExtension(kRtpVideoRotationHeaderExtension, id)); + EXPECT_TRUE(channel->SetSendRtpHeaderExtensions(extensions)); + + // Verify capturer has turned off applying rotation. + EXPECT_FALSE(capturer.GetApplyRotation()); + + // Verify removing header extension turns on applying rotation. + extensions.clear(); + EXPECT_TRUE(channel->SetSendRtpHeaderExtensions(extensions)); + EXPECT_TRUE(capturer.GetApplyRotation()); +} + TEST_F(WebRtcVideoEngine2Test, SetSendFailsBeforeSettingCodecs) { engine_.Init(rtc::Thread::Current()); rtc::scoped_ptr channel( @@ -1198,21 +1274,34 @@ TEST_F(WebRtcVideoChannel2Test, RecvAbsoluteSendTimeHeaderExtensions) { webrtc::RtpExtension::kAbsSendTime); } +// Test support for video rotation header extension. +TEST_F(WebRtcVideoChannel2Test, SendVideoRotationHeaderExtensions) { + TestSetSendRtpHeaderExtensions(kRtpVideoRotationHeaderExtension, + webrtc::RtpExtension::kVideoRotation); +} +TEST_F(WebRtcVideoChannel2Test, RecvVideoRotationHeaderExtensions) { + TestSetRecvRtpHeaderExtensions(kRtpVideoRotationHeaderExtension, + webrtc::RtpExtension::kVideoRotation); +} + TEST_F(WebRtcVideoChannel2Test, IdenticalSendExtensionsDoesntRecreateStream) { const int kTOffsetId = 1; const int kAbsSendTimeId = 2; + const int kVideoRotationId = 3; std::vector extensions; extensions.push_back(cricket::RtpHeaderExtension( kRtpAbsoluteSenderTimeHeaderExtension, kAbsSendTimeId)); extensions.push_back(cricket::RtpHeaderExtension( kRtpTimestampOffsetHeaderExtension, kTOffsetId)); + extensions.push_back(cricket::RtpHeaderExtension( + kRtpVideoRotationHeaderExtension, kVideoRotationId)); EXPECT_TRUE(channel_->SetSendRtpHeaderExtensions(extensions)); FakeVideoSendStream* send_stream = AddSendStream(cricket::StreamParams::CreateLegacy(123)); EXPECT_EQ(1, fake_call_->GetNumCreatedSendStreams()); - ASSERT_EQ(2u, send_stream->GetConfig().rtp.extensions.size()); + ASSERT_EQ(3u, send_stream->GetConfig().rtp.extensions.size()); // Setting the same extensions (even if in different order) shouldn't // reallocate the stream. @@ -1231,18 +1320,21 @@ TEST_F(WebRtcVideoChannel2Test, IdenticalSendExtensionsDoesntRecreateStream) { TEST_F(WebRtcVideoChannel2Test, IdenticalRecvExtensionsDoesntRecreateStream) { const int kTOffsetId = 1; const int kAbsSendTimeId = 2; + const int kVideoRotationId = 3; std::vector extensions; extensions.push_back(cricket::RtpHeaderExtension( kRtpAbsoluteSenderTimeHeaderExtension, kAbsSendTimeId)); extensions.push_back(cricket::RtpHeaderExtension( kRtpTimestampOffsetHeaderExtension, kTOffsetId)); + extensions.push_back(cricket::RtpHeaderExtension( + kRtpVideoRotationHeaderExtension, kVideoRotationId)); EXPECT_TRUE(channel_->SetRecvRtpHeaderExtensions(extensions)); FakeVideoReceiveStream* send_stream = AddRecvStream(cricket::StreamParams::CreateLegacy(123)); EXPECT_EQ(1, fake_call_->GetNumCreatedReceiveStreams()); - ASSERT_EQ(2u, send_stream->GetConfig().rtp.extensions.size()); + ASSERT_EQ(3u, send_stream->GetConfig().rtp.extensions.size()); // Setting the same extensions (even if in different order) shouldn't // reallocate the stream. diff --git a/talk/media/webrtc/webrtcvideoengine_unittest.cc b/talk/media/webrtc/webrtcvideoengine_unittest.cc index edfa53a793..e6ac7c1f4e 100644 --- a/talk/media/webrtc/webrtcvideoengine_unittest.cc +++ b/talk/media/webrtc/webrtcvideoengine_unittest.cc @@ -50,6 +50,7 @@ using cricket::kRtpTimestampOffsetHeaderExtension; using cricket::kRtpAbsoluteSenderTimeHeaderExtension; +using cricket::kRtpVideoRotationHeaderExtension; static const cricket::VideoCodec kVP8Codec720p(100, "VP8", 1280, 720, 30, 0); static const cricket::VideoCodec kVP8Codec360p(100, "VP8", 640, 360, 30, 0); @@ -1005,6 +1006,14 @@ TEST_F(WebRtcVideoEngineTestFake, RecvAbsoluteSendTimeHeaderExtensions) { TestSetRecvRtpHeaderExtensions(kRtpAbsoluteSenderTimeHeaderExtension); } +// Test support for Coordination of Video Orientation (CVO) header extension. +TEST_F(WebRtcVideoEngineTestFake, SendVideoRotationHeaderExtensions) { + TestSetSendRtpHeaderExtensions(kRtpVideoRotationHeaderExtension); +} +TEST_F(WebRtcVideoEngineTestFake, RecvVideoRotationHeaderExtensions) { + TestSetRecvRtpHeaderExtensions(kRtpVideoRotationHeaderExtension); +} + TEST_F(WebRtcVideoEngineTestFake, LeakyBucketTest) { EXPECT_TRUE(SetupEngine()); diff --git a/talk/media/webrtc/webrtcvideoframe.cc b/talk/media/webrtc/webrtcvideoframe.cc index 14118e1fef..e308a731a3 100644 --- a/talk/media/webrtc/webrtcvideoframe.cc +++ b/talk/media/webrtc/webrtcvideoframe.cc @@ -46,6 +46,19 @@ WebRtcVideoFrame::WebRtcVideoFrame(): time_stamp_ns_(0), rotation_(webrtc::kVideoRotation_0) {} +WebRtcVideoFrame::WebRtcVideoFrame( + const rtc::scoped_refptr& buffer, + int64_t elapsed_time_ns, + int64_t time_stamp_ns, + webrtc::VideoRotation rotation) + : video_frame_buffer_(buffer), + pixel_width_(1), + pixel_height_(1), + elapsed_time_ns_(elapsed_time_ns), + time_stamp_ns_(time_stamp_ns), + rotation_(rotation) { +} + WebRtcVideoFrame::WebRtcVideoFrame( const rtc::scoped_refptr& buffer, int64_t elapsed_time_ns, @@ -62,7 +75,8 @@ WebRtcVideoFrame::WebRtcVideoFrame(webrtc::NativeHandle* handle, int width, int height, int64_t elapsed_time_ns, - int64_t time_stamp_ns) + int64_t time_stamp_ns, + webrtc::VideoRotation rotation) : video_frame_buffer_( new rtc::RefCountedObject(handle, width, @@ -71,7 +85,7 @@ WebRtcVideoFrame::WebRtcVideoFrame(webrtc::NativeHandle* handle, pixel_height_(1), elapsed_time_ns_(elapsed_time_ns), time_stamp_ns_(time_stamp_ns), - rotation_(webrtc::kVideoRotation_0) { + rotation_(rotation) { } WebRtcVideoFrame::~WebRtcVideoFrame() {} @@ -176,10 +190,9 @@ WebRtcVideoFrame::GetVideoFrameBuffer() const { VideoFrame* WebRtcVideoFrame::Copy() const { WebRtcVideoFrame* new_frame = new WebRtcVideoFrame( - video_frame_buffer_, elapsed_time_ns_, time_stamp_ns_); + video_frame_buffer_, elapsed_time_ns_, time_stamp_ns_, rotation_); new_frame->pixel_width_ = pixel_width_; new_frame->pixel_height_ = pixel_height_; - new_frame->rotation_ = rotation_; return new_frame; } diff --git a/talk/media/webrtc/webrtcvideoframe.h b/talk/media/webrtc/webrtcvideoframe.h index d4380b537d..a1872c58be 100644 --- a/talk/media/webrtc/webrtcvideoframe.h +++ b/talk/media/webrtc/webrtcvideoframe.h @@ -42,14 +42,22 @@ struct CapturedFrame; class WebRtcVideoFrame : public VideoFrame { public: WebRtcVideoFrame(); + WebRtcVideoFrame(const rtc::scoped_refptr& buffer, + int64_t elapsed_time_ns, + int64_t time_stamp_ns, + webrtc::VideoRotation rotation); + + // TODO(guoweis): Remove this when chrome code base is updated. WebRtcVideoFrame(const rtc::scoped_refptr& buffer, int64_t elapsed_time_ns, int64_t time_stamp_ns); + WebRtcVideoFrame(webrtc::NativeHandle* handle, int width, int height, int64_t elapsed_time_ns, - int64_t time_stamp_ns); + int64_t time_stamp_ns, + webrtc::VideoRotation rotation); ~WebRtcVideoFrame(); // Creates a frame from a raw sample with FourCC "format" and size "w" x "h". diff --git a/talk/media/webrtc/webrtcvideoframe_unittest.cc b/talk/media/webrtc/webrtcvideoframe_unittest.cc index bbf5d4272b..8cbe479932 100644 --- a/talk/media/webrtc/webrtcvideoframe_unittest.cc +++ b/talk/media/webrtc/webrtcvideoframe_unittest.cc @@ -340,7 +340,8 @@ TEST_F(WebRtcVideoFrameTest, InitRotated90DontApplyRotation) { TEST_F(WebRtcVideoFrameTest, TextureInitialValues) { NativeHandleImpl handle; - cricket::WebRtcVideoFrame frame(&handle, 640, 480, 100, 200); + cricket::WebRtcVideoFrame frame(&handle, 640, 480, 100, 200, + webrtc::kVideoRotation_0); EXPECT_EQ(&handle, frame.GetNativeHandle()); EXPECT_EQ(640u, frame.GetWidth()); EXPECT_EQ(480u, frame.GetHeight()); @@ -354,7 +355,8 @@ TEST_F(WebRtcVideoFrameTest, TextureInitialValues) { TEST_F(WebRtcVideoFrameTest, CopyTextureFrame) { NativeHandleImpl handle; - cricket::WebRtcVideoFrame frame1(&handle, 640, 480, 100, 200); + cricket::WebRtcVideoFrame frame1(&handle, 640, 480, 100, 200, + webrtc::kVideoRotation_0); cricket::VideoFrame* frame2 = frame1.Copy(); EXPECT_EQ(frame1.GetNativeHandle(), frame2->GetNativeHandle()); EXPECT_EQ(frame1.GetWidth(), frame2->GetWidth()); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_header_extension.cc b/webrtc/modules/rtp_rtcp/source/rtp_header_extension.cc index e8109fad5f..8605925785 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_header_extension.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_header_extension.cc @@ -34,6 +34,17 @@ void RtpHeaderExtensionMap::Erase() { int32_t RtpHeaderExtensionMap::Register(const RTPExtensionType type, const uint8_t id) { + return Register(type, id, true); +} + +int32_t RtpHeaderExtensionMap::RegisterInactive(const RTPExtensionType type, + const uint8_t id) { + return Register(type, id, false); +} + +int32_t RtpHeaderExtensionMap::Register(const RTPExtensionType type, + const uint8_t id, + bool active) { if (id < 1 || id > 14) { return -1; } @@ -47,12 +58,24 @@ int32_t RtpHeaderExtensionMap::Register(const RTPExtensionType type, } // This extension type is already registered with this id, // so return success. + it->second->active = active; return 0; } - extensionMap_[id] = new HeaderExtension(type); + extensionMap_[id] = new HeaderExtension(type, active); return 0; } +bool RtpHeaderExtensionMap::SetActive(const RTPExtensionType type, + bool active) { + for (auto& kv : extensionMap_) { + if (kv.second->type == type) { + kv.second->active = active; + return true; + } + } + return false; +} + int32_t RtpHeaderExtensionMap::Deregister(const RTPExtensionType type) { uint8_t id; if (GetId(type, &id) != 0) { @@ -113,7 +136,9 @@ size_t RtpHeaderExtensionMap::GetTotalLengthInBytes() const { extensionMap_.begin(); while (it != extensionMap_.end()) { HeaderExtension* extension = it->second; - length += extension->length; + if (extension->active) { + length += extension->length; + } it++; } // Add RTP extension header length. @@ -140,8 +165,11 @@ int32_t RtpHeaderExtensionMap::GetLengthUntilBlockStartInBytes( while (it != extensionMap_.end()) { HeaderExtension* extension = it->second; if (extension->type == type) { + if (!extension->active) { + return -1; + } break; - } else { + } else if (extension->active) { length += extension->length; } it++; @@ -150,17 +178,23 @@ int32_t RtpHeaderExtensionMap::GetLengthUntilBlockStartInBytes( } int32_t RtpHeaderExtensionMap::Size() const { - return extensionMap_.size(); + int32_t count = 0; + for (auto& kv : extensionMap_) { + if (kv.second->active) { + count++; + } + } + return count; } RTPExtensionType RtpHeaderExtensionMap::First() const { - std::map::const_iterator it = - extensionMap_.begin(); - if (it == extensionMap_.end()) { - return kRtpExtensionNone; + for (auto& kv : extensionMap_) { + if (kv.second->active) { + return kv.second->type; + } } - HeaderExtension* extension = it->second; - return extension->type; + + return kRtpExtensionNone; } RTPExtensionType RtpHeaderExtensionMap::Next(RTPExtensionType type) const { @@ -170,15 +204,16 @@ RTPExtensionType RtpHeaderExtensionMap::Next(RTPExtensionType type) const { } std::map::const_iterator it = extensionMap_.find(id); - if (it == extensionMap_.end()) { + if (it == extensionMap_.end() || !it->second->active) { return kRtpExtensionNone; } - it++; - if (it == extensionMap_.end()) { - return kRtpExtensionNone; + while ((++it) != extensionMap_.end()) { + if (it->second->active) { + return it->second->type; + } } - HeaderExtension* extension = it->second; - return extension->type; + + return kRtpExtensionNone; } void RtpHeaderExtensionMap::GetCopy(RtpHeaderExtensionMap* map) const { @@ -187,7 +222,7 @@ void RtpHeaderExtensionMap::GetCopy(RtpHeaderExtensionMap* map) const { extensionMap_.begin(); while (it != extensionMap_.end()) { HeaderExtension* extension = it->second; - map->Register(extension->type, it->first); + map->Register(extension->type, it->first, extension->active); it++; } } diff --git a/webrtc/modules/rtp_rtcp/source/rtp_header_extension.h b/webrtc/modules/rtp_rtcp/source/rtp_header_extension.h index cb49ea74cc..7be3c2e5c4 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_header_extension.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_header_extension.h @@ -29,8 +29,16 @@ const size_t kTransportSequenceNumberLength = 3; struct HeaderExtension { HeaderExtension(RTPExtensionType extension_type) - : type(extension_type), - length(0) { + : type(extension_type), length(0), active(true) { + Init(); + } + + HeaderExtension(RTPExtensionType extension_type, bool active) + : type(extension_type), length(0), active(active) { + Init(); + } + + void Init() { // TODO(solenberg): Create handler classes for header extensions so we can // get rid of switches like these as well as handling code spread out all // over. @@ -57,6 +65,7 @@ struct HeaderExtension { const RTPExtensionType type; uint8_t length; + bool active; }; class RtpHeaderExtensionMap { @@ -68,6 +77,13 @@ class RtpHeaderExtensionMap { int32_t Register(const RTPExtensionType type, const uint8_t id); + // Active is a concept for a registered rtp header extension which doesn't + // take effect yet until being activated. Inactive RTP header extensions do + // not take effect and should not be included in size calculations until they + // are activated. + int32_t RegisterInactive(const RTPExtensionType type, const uint8_t id); + bool SetActive(const RTPExtensionType type, bool active); + int32_t Deregister(const RTPExtensionType type); bool IsRegistered(RTPExtensionType type) const; @@ -76,6 +92,10 @@ class RtpHeaderExtensionMap { int32_t GetId(const RTPExtensionType type, uint8_t* id) const; + // + // Methods below ignore any inactive rtp header extensions. + // + size_t GetTotalLengthInBytes() const; int32_t GetLengthUntilBlockStartInBytes(const RTPExtensionType type) const; @@ -89,6 +109,7 @@ class RtpHeaderExtensionMap { RTPExtensionType Next(RTPExtensionType type) const; private: + int32_t Register(const RTPExtensionType type, const uint8_t id, bool active); std::map extensionMap_; }; } diff --git a/webrtc/modules/rtp_rtcp/source/rtp_header_extension_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_header_extension_unittest.cc index d8387e823c..520cf7a962 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_header_extension_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_header_extension_unittest.cc @@ -35,9 +35,16 @@ const uint8_t RtpHeaderExtensionTest::kId = 3; TEST_F(RtpHeaderExtensionTest, Register) { EXPECT_EQ(0, map_.Size()); EXPECT_EQ(0, map_.Register(kRtpExtensionTransmissionTimeOffset, kId)); + EXPECT_TRUE(map_.IsRegistered(kRtpExtensionTransmissionTimeOffset)); EXPECT_EQ(1, map_.Size()); EXPECT_EQ(0, map_.Deregister(kRtpExtensionTransmissionTimeOffset)); EXPECT_EQ(0, map_.Size()); + + EXPECT_EQ(0, map_.RegisterInactive(kRtpExtensionTransmissionTimeOffset, kId)); + EXPECT_EQ(0, map_.Size()); + EXPECT_TRUE(map_.IsRegistered(kRtpExtensionTransmissionTimeOffset)); + EXPECT_TRUE(map_.SetActive(kRtpExtensionTransmissionTimeOffset, true)); + EXPECT_EQ(1, map_.Size()); } TEST_F(RtpHeaderExtensionTest, RegisterIllegalArg) { @@ -56,10 +63,14 @@ TEST_F(RtpHeaderExtensionTest, Idempotent) { TEST_F(RtpHeaderExtensionTest, NonUniqueId) { EXPECT_EQ(0, map_.Register(kRtpExtensionTransmissionTimeOffset, kId)); EXPECT_EQ(-1, map_.Register(kRtpExtensionAudioLevel, kId)); + EXPECT_EQ(-1, map_.RegisterInactive(kRtpExtensionAudioLevel, kId)); } TEST_F(RtpHeaderExtensionTest, GetTotalLength) { EXPECT_EQ(0u, map_.GetTotalLengthInBytes()); + EXPECT_EQ(0, map_.RegisterInactive(kRtpExtensionTransmissionTimeOffset, kId)); + EXPECT_EQ(0u, map_.GetTotalLengthInBytes()); + EXPECT_EQ(0, map_.Register(kRtpExtensionTransmissionTimeOffset, kId)); EXPECT_EQ(kRtpOneByteHeaderLength + kTransmissionTimeOffsetLength, map_.GetTotalLengthInBytes()); @@ -68,7 +79,11 @@ TEST_F(RtpHeaderExtensionTest, GetTotalLength) { TEST_F(RtpHeaderExtensionTest, GetLengthUntilBlockStart) { EXPECT_EQ(-1, map_.GetLengthUntilBlockStartInBytes( kRtpExtensionTransmissionTimeOffset)); - EXPECT_EQ(0, map_.Register(kRtpExtensionTransmissionTimeOffset, kId)); + EXPECT_EQ(0, map_.RegisterInactive(kRtpExtensionTransmissionTimeOffset, kId)); + EXPECT_EQ(-1, map_.GetLengthUntilBlockStartInBytes( + kRtpExtensionTransmissionTimeOffset)); + + EXPECT_TRUE(map_.SetActive(kRtpExtensionTransmissionTimeOffset, true)); EXPECT_EQ(static_cast(kRtpOneByteHeaderLength), map_.GetLengthUntilBlockStartInBytes( kRtpExtensionTransmissionTimeOffset)); @@ -96,7 +111,11 @@ TEST_F(RtpHeaderExtensionTest, IterateTypes) { EXPECT_EQ(kRtpExtensionNone, map_.First()); EXPECT_EQ(kRtpExtensionNone, map_.Next(kRtpExtensionTransmissionTimeOffset)); - EXPECT_EQ(0, map_.Register(kRtpExtensionTransmissionTimeOffset, kId)); + EXPECT_EQ(0, map_.RegisterInactive(kRtpExtensionTransmissionTimeOffset, kId)); + + EXPECT_EQ(kRtpExtensionNone, map_.First()); + + EXPECT_TRUE(map_.SetActive(kRtpExtensionTransmissionTimeOffset, true)); EXPECT_EQ(kRtpExtensionTransmissionTimeOffset, map_.First()); EXPECT_EQ(kRtpExtensionNone, map_.Next(kRtpExtensionTransmissionTimeOffset)); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc b/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc index 2823868015..8d8147cd63 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc @@ -63,13 +63,6 @@ int32_t RTPReceiverVideo::ParseRtpPacket(WebRtcRTPHeader* rtp_header, const size_t payload_data_length = payload_length - rtp_header->header.paddingLength; - // Retrieve the video rotation information. - rtp_header->type.Video.rotation = kVideoRotation_0; - if (rtp_header->header.extension.hasVideoRotation) { - rtp_header->type.Video.rotation = ConvertCVOByteToVideoRotation( - rtp_header->header.extension.videoRotation); - } - if (payload == NULL || payload_data_length == 0) { return data_callback_->OnReceivedPayloadData(NULL, 0, rtp_header) == 0 ? 0 : -1; @@ -90,6 +83,14 @@ int32_t RTPReceiverVideo::ParseRtpPacket(WebRtcRTPHeader* rtp_header, rtp_header->frameType = parsed_payload.frame_type; rtp_header->type = parsed_payload.type; + rtp_header->type.Video.rotation = kVideoRotation_0; + + // Retrieve the video rotation information. + if (rtp_header->header.extension.hasVideoRotation) { + rtp_header->type.Video.rotation = ConvertCVOByteToVideoRotation( + rtp_header->header.extension.videoRotation); + } + return data_callback_->OnReceivedPayloadData(parsed_payload.payload, parsed_payload.payload_length, rtp_header) == 0 diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc index 35f885a3e2..cb0c6dc0c2 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc @@ -129,6 +129,7 @@ RTPSender::RTPSender(int32_t id, transmission_time_offset_(0), absolute_send_time_(0), rotation_(kVideoRotation_0), + cvo_mode_(kCVONone), transport_sequence_number_(0), // NACK. nack_byte_count_times_(), @@ -266,6 +267,10 @@ int32_t RTPSender::SetTransportSequenceNumber(uint16_t sequence_number) { int32_t RTPSender::RegisterRtpHeaderExtension(RTPExtensionType type, uint8_t id) { CriticalSectionScoped cs(send_critsect_.get()); + if (type == kRtpExtensionVideoRotation) { + cvo_mode_ = kCVOInactive; + return rtp_header_extension_map_.RegisterInactive(type, id); + } return rtp_header_extension_map_.Register(type, id); } @@ -462,6 +467,16 @@ int32_t RTPSender::CheckPayloadType(int8_t payload_type, return 0; } +RTPSenderInterface::CVOMode RTPSender::ActivateCVORtpHeaderExtension() { + if (cvo_mode_ == kCVOInactive) { + CriticalSectionScoped cs(send_critsect_.get()); + if (rtp_header_extension_map_.SetActive(kRtpExtensionVideoRotation, true)) { + cvo_mode_ = kCVOActivated; + } + } + return cvo_mode_; +} + int32_t RTPSender::SendOutgoingData(FrameType frame_type, int8_t payload_type, uint32_t capture_timestamp, @@ -1201,8 +1216,7 @@ uint16_t RTPSender::BuildRTPHeaderExtension(uint8_t* data_buffer, block_length = BuildAbsoluteSendTimeExtension(extension_data); break; case kRtpExtensionVideoRotation: - if (marker_bit) - block_length = BuildVideoRotationExtension(extension_data); + block_length = BuildVideoRotationExtension(extension_data); break; case kRtpExtensionTransportSequenceNumber: block_length = BuildTransportSequenceNumberExtension(extension_data); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.h b/webrtc/modules/rtp_rtcp/source/rtp_sender.h index baa067c5c0..61d835d06d 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.h @@ -41,6 +41,14 @@ class RTPSenderInterface { RTPSenderInterface() {} virtual ~RTPSenderInterface() {} + enum CVOMode { + kCVONone, + kCVOInactive, // CVO rtp header extension is registered but haven't + // received any frame with rotation pending. + kCVOActivated, // CVO rtp header extension will be present in the rtp + // packets. + }; + virtual uint32_t SSRC() const = 0; virtual uint32_t Timestamp() const = 0; @@ -70,6 +78,7 @@ class RTPSenderInterface { const RTPHeader& rtp_header, VideoRotation rotation) const = 0; virtual bool IsRtpHeaderExtensionRegistered(RTPExtensionType type) = 0; + virtual CVOMode ActivateCVORtpHeaderExtension() = 0; }; class RTPSender : public RTPSenderInterface { @@ -285,6 +294,7 @@ class RTPSender : public RTPSenderInterface { RtpState GetRtpState() const; void SetRtxRtpState(const RtpState& rtp_state); RtpState GetRtxRtpState() const; + CVOMode ActivateCVORtpHeaderExtension() override; protected: int32_t CheckPayloadType(int8_t payload_type, RtpVideoCodecTypes* video_type); @@ -378,6 +388,7 @@ class RTPSender : public RTPSenderInterface { int32_t transmission_time_offset_; uint32_t absolute_send_time_; VideoRotation rotation_; + CVOMode cvo_mode_; uint16_t transport_sequence_number_; // NACK diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc index a55ca977f7..f6ea40b7b7 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -186,7 +186,6 @@ class RtpSenderVideoTest : public RtpSenderTest { } ASSERT_TRUE(rtp_parser.Parse(rtp_header, map)); ASSERT_FALSE(rtp_parser.RTCP()); - EXPECT_EQ(expect_cvo, rtp_header.markerBit); EXPECT_EQ(payload_, rtp_header.payloadType); EXPECT_EQ(seq_num, rtp_header.sequenceNumber); EXPECT_EQ(kTimestamp, rtp_header.timestamp); @@ -254,6 +253,7 @@ TEST_F(RtpSenderTest, RegisterRtpHeaderExtensions) { rtp_sender_->RtpHeaderExtensionTotalLength()); EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension( kRtpExtensionVideoRotation, kVideoRotationExtensionId)); + EXPECT_TRUE(rtp_sender_->ActivateCVORtpHeaderExtension()); EXPECT_EQ(RtpUtility::Word32Align(kRtpOneByteHeaderLength + kTransmissionTimeOffsetLength + kAbsoluteSendTimeLength + @@ -286,6 +286,9 @@ TEST_F(RtpSenderTest, RegisterRtpVideoRotationHeaderExtension) { EXPECT_EQ(0u, rtp_sender_->RtpHeaderExtensionTotalLength()); EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension( kRtpExtensionVideoRotation, kVideoRotationExtensionId)); + EXPECT_EQ(0u, rtp_sender_->RtpHeaderExtensionTotalLength()); + + EXPECT_TRUE(rtp_sender_->ActivateCVORtpHeaderExtension()); EXPECT_EQ( RtpUtility::Word32Align(kRtpOneByteHeaderLength + kVideoRotationLength), rtp_sender_->RtpHeaderExtensionTotalLength()); @@ -424,6 +427,7 @@ TEST_F(RtpSenderTest, BuildRTPPacketWithVideoRotation_MarkerBit) { rtp_sender_->SetVideoRotation(kRotation); EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension( kRtpExtensionVideoRotation, kVideoRotationExtensionId)); + EXPECT_TRUE(rtp_sender_->ActivateCVORtpHeaderExtension()); RtpHeaderExtensionMap map; map.Register(kRtpExtensionVideoRotation, kVideoRotationExtensionId); @@ -447,10 +451,11 @@ TEST_F(RtpSenderTest, BuildRTPPacketWithVideoRotation_MarkerBit) { } // Test CVO header extension is not set when marker bit is false. -TEST_F(RtpSenderTest, BuildRTPPacketWithVideoRotation_NoMarkerBit) { +TEST_F(RtpSenderTest, DISABLED_BuildRTPPacketWithVideoRotation_NoMarkerBit) { rtp_sender_->SetVideoRotation(kRotation); EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension( kRtpExtensionVideoRotation, kVideoRotationExtensionId)); + EXPECT_TRUE(rtp_sender_->ActivateCVORtpHeaderExtension()); RtpHeaderExtensionMap map; map.Register(kRtpExtensionVideoRotation, kVideoRotationExtensionId); @@ -1333,13 +1338,15 @@ TEST_F(RtpSenderTest, BytesReportedCorrectly) { rtx_stats.transmitted.TotalBytes()); } -// Verify that only the last packet of a frame has CVO byte set. +// Verify that all packets of a frame have CVO byte set. TEST_F(RtpSenderVideoTest, SendVideoWithCVO) { RTPVideoHeader hdr = {0}; hdr.rotation = kVideoRotation_90; EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension( kRtpExtensionVideoRotation, kVideoRotationExtensionId)); + EXPECT_TRUE(rtp_sender_->ActivateCVORtpHeaderExtension()); + EXPECT_EQ( RtpUtility::Word32Align(kRtpOneByteHeaderLength + kVideoRotationLength), rtp_sender_->RtpHeaderExtensionTotalLength()); @@ -1351,13 +1358,12 @@ TEST_F(RtpSenderVideoTest, SendVideoWithCVO) { RtpHeaderExtensionMap map; map.Register(kRtpExtensionVideoRotation, kVideoRotationExtensionId); - // Verify that this packet doesn't have CVO byte. + // Verify that this packet does have CVO byte. VerifyCVOPacket( reinterpret_cast(transport_.sent_packets_[0]->data()), - transport_.sent_packets_[0]->size(), false, &map, kSeqNum, - kVideoRotation_0); + transport_.sent_packets_[0]->length(), true, &map, kSeqNum, hdr.rotation); - // Verify that this packet doesn't have CVO byte. + // Verify that this packet does have CVO byte. VerifyCVOPacket( reinterpret_cast(transport_.sent_packets_[1]->data()), transport_.sent_packets_[1]->size(), true, &map, kSeqNum + 1, diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc index 2b14cfefa6..703ae2b9af 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -301,6 +301,13 @@ bool RTPSenderVideo::Send(const RtpVideoCodecTypes videoType, const size_t payloadSize, const RTPFragmentationHeader* fragmentation, const RTPVideoHeader* rtpHdr) { + // Register CVO rtp header extension at the first time when we receive a frame + // with pending rotation. + RTPSenderInterface::CVOMode cvo_mode = RTPSenderInterface::kCVONone; + if (rtpHdr && rtpHdr->rotation != kVideoRotation_0) { + cvo_mode = _rtpSender.ActivateCVORtpHeaderExtension(); + } + uint16_t rtp_header_length = _rtpSender.RTPHeaderLength(); size_t payload_bytes_to_send = payloadSize; const uint8_t* data = payloadData; @@ -341,13 +348,16 @@ bool RTPSenderVideo::Send(const RtpVideoCodecTypes videoType, // packet in each group of packets which make up another type of frame // (e.g. a P-Frame) only if the current value is different from the previous // value sent. - // Here we are adding it to the last packet of every frame at this point. + // Here we are adding it to every packet of every frame at this point. if (!rtpHdr) { assert(!_rtpSender.IsRtpHeaderExtensionRegistered( kRtpExtensionVideoRotation)); - } else if (last) { + } else if (cvo_mode == RTPSenderInterface::kCVOActivated) { // Checking whether CVO header extension is registered will require taking // a lock. It'll be a no-op if it's not registered. + // TODO(guoweis): For now, all packets sent will carry the CVO such that + // the RTP header length is consistent, although the receiver side will + // only exam the packets with market bit set. size_t packetSize = payloadSize + rtp_header_length; RtpUtility::RtpHeaderParser rtp_parser(dataBuffer, packetSize); RTPHeader rtp_header; diff --git a/webrtc/modules/video_capture/video_capture_impl.cc b/webrtc/modules/video_capture/video_capture_impl.cc index 3bf5e4bb8a..e9838a6ecf 100644 --- a/webrtc/modules/video_capture/video_capture_impl.cc +++ b/webrtc/modules/video_capture/video_capture_impl.cc @@ -263,7 +263,10 @@ int32_t VideoCaptureImpl::IncomingFrame( int target_width = width; int target_height = height; - if (apply_rotation_) { + // SetApplyRotation doesn't take any lock. Make a local copy here. + bool apply_rotation = apply_rotation_; + + if (apply_rotation) { // Rotating resolution when for 90/270 degree rotations. if (_rotateFrame == kVideoRotation_90 || _rotateFrame == kVideoRotation_270) { @@ -290,7 +293,7 @@ int32_t VideoCaptureImpl::IncomingFrame( const int conversionResult = ConvertToI420( commonVideoType, videoFrame, 0, 0, // No cropping width, height, videoFrameLength, - apply_rotation_ ? _rotateFrame : kVideoRotation_0, &_captureFrame); + apply_rotation ? _rotateFrame : kVideoRotation_0, &_captureFrame); if (conversionResult < 0) { LOG(LS_ERROR) << "Failed to convert capture frame from type " @@ -298,7 +301,7 @@ int32_t VideoCaptureImpl::IncomingFrame( return -1; } - if (!apply_rotation_) { + if (!apply_rotation) { _captureFrame.set_rotation(_rotateFrame); } else { _captureFrame.set_rotation(kVideoRotation_0); @@ -335,8 +338,8 @@ void VideoCaptureImpl::EnableFrameRateCallback(const bool enable) { } bool VideoCaptureImpl::SetApplyRotation(bool enable) { - CriticalSectionScoped cs(&_apiCs); - CriticalSectionScoped cs2(&_callBackCs); + // We can't take any lock here as it'll cause deadlock with IncomingFrame. + // The effect of this is the last caller wins. apply_rotation_ = enable; return true; diff --git a/webrtc/modules/video_coding/main/source/packet.cc b/webrtc/modules/video_coding/main/source/packet.cc index dd3743f701..fb815c45b9 100644 --- a/webrtc/modules/video_coding/main/source/packet.cc +++ b/webrtc/modules/video_coding/main/source/packet.cc @@ -99,6 +99,9 @@ void VCMPacket::Reset() { } void VCMPacket::CopyCodecSpecifics(const RTPVideoHeader& videoHeader) { + if (markerBit) { + codecSpecificHeader.rotation = videoHeader.rotation; + } switch (videoHeader.codec) { case kRtpVideoVp8: // Handle all packets within a frame as depending on the previous packet