From 31331cfd2d3d17958942b67190c8b943c05b084f Mon Sep 17 00:00:00 2001 From: Minyue Date: Wed, 1 Apr 2015 16:19:58 +0200 Subject: [PATCH] Revert "Enable CVO by default through webrtc pipeline." This reverts commit 1b1c15cad16de57053bb6aa8a916079e0534bdae. Due to failure on http://build.chromium.org/p/client.webrtc/builders/Linux64%20Release%20%5Blarge%20tests%5D/builds/4092 and following builds (the test hangs and never finishes). R=kjellander@webrtc.org TBR=guoweis@chromium.org TESTED=Local revert + execution of libjingle_peerconnection_java_unittest show that this is the culprit. Review URL: https://webrtc-codereview.appspot.com/47909004 Cr-Commit-Position: refs/heads/master@{#8911} --- .../android/org/webrtc/VideoRendererGui.java | 95 +++++++----------- talk/media/base/videocapturer.cc | 2 - talk/media/base/videoframe.h | 2 - talk/media/webrtc/webrtcvideocapturer.cc | 8 -- talk/media/webrtc/webrtcvideoengine.cc | 41 +------- talk/media/webrtc/webrtcvideoengine2.cc | 40 +------- talk/media/webrtc/webrtcvideoengine2.h | 2 - .../webrtc/webrtcvideoengine2_unittest.cc | 96 +------------------ .../webrtc/webrtcvideoengine_unittest.cc | 9 -- talk/media/webrtc/webrtcvideoframe.cc | 21 +--- talk/media/webrtc/webrtcvideoframe.h | 10 +- .../media/webrtc/webrtcvideoframe_unittest.cc | 6 +- .../rtp_rtcp/source/rtp_header_extension.cc | 69 ++++--------- .../rtp_rtcp/source/rtp_header_extension.h | 25 +---- .../source/rtp_header_extension_unittest.cc | 23 +---- .../rtp_rtcp/source/rtp_receiver_video.cc | 15 ++- webrtc/modules/rtp_rtcp/source/rtp_sender.cc | 18 +--- webrtc/modules/rtp_rtcp/source/rtp_sender.h | 11 --- .../rtp_rtcp/source/rtp_sender_unittest.cc | 20 ++-- .../rtp_rtcp/source/rtp_sender_video.cc | 14 +-- .../video_coding/main/source/packet.cc | 3 - 21 files changed, 83 insertions(+), 447 deletions(-) diff --git a/talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java b/talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java index 0f616c9475..4ad8ccca18 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 top left coord. (2,3) is the bottom left. (4,5) is the - // top right. (6,7) is the bottom right. + // 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. private static int rotation_matrix[][] = - { {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 + { {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) private YuvImageRenderer( GLSurfaceView surface, int id, @@ -381,21 +381,12 @@ 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) { - ratio = (1.0f - videoAspectRatio / displayAspectRatio) / + texOffsetV = (1.0f - videoAspectRatio / displayAspectRatio) / 2.0f; - adjustU = (rotationDegree == 90 || rotationDegree == 270); } else { - ratio = (1.0f - displayAspectRatio / videoAspectRatio) / + texOffsetU = (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 + @@ -411,59 +402,37 @@ 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, // top left - uLeft, 1.0f - texOffsetV, // bottom left - uRight, texOffsetV, // top right - uRight, 1.0f - texOffsetV // bottom right + uLeft, texOffsetV, // left top + uLeft, 1.0f - texOffsetV, // left bottom + uRight, texOffsetV, // right top + uRight, 1.0f - texOffsetV // right bottom }; - // Rotation needs to be done before mirroring. - textureCoordinatesFloat = applyRotation(textureCoordinatesFloat, - rotationDegree); - textureCoordinatesFloat = applyMirror(textureCoordinatesFloat, - mirror); + 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]; + } + } textureCoords = - directNativeFloatBuffer(textureCoordinatesFloat); + directNativeFloatBuffer(textureCoordinatesRotatedFloat); } 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 653e7934a3..bed3b0c616 100644 --- a/talk/media/base/videocapturer.cc +++ b/talk/media/base/videocapturer.cc @@ -254,8 +254,6 @@ 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 0aac645128..5b1663c7a8 100644 --- a/talk/media/base/videoframe.h +++ b/talk/media/base/videoframe.h @@ -83,10 +83,8 @@ 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 956356ff7c..0e8f56bfdd 100644 --- a/talk/media/webrtc/webrtcvideocapturer.cc +++ b/talk/media/webrtc/webrtcvideocapturer.cc @@ -44,7 +44,6 @@ #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 { @@ -265,13 +264,6 @@ bool WebRtcVideoCapturer::SetApplyRotation(bool enable) { 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 a103caed54..287de72320 100644 --- a/talk/media/webrtc/webrtcvideoengine.cc +++ b/talk/media/webrtc/webrtcvideoengine.cc @@ -425,8 +425,7 @@ 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.rotation()); + webrtc_frame.render_time_ms() * rtc::kNumNanosecsPerMillisec); return renderer_->RenderFrame(&cricket_frame) ? 0 : -1; } @@ -1162,9 +1161,6 @@ void WebRtcVideoEngine::Construct(ViEWrapper* vie_wrapper, rtp_header_extensions_.push_back( RtpHeaderExtension(kRtpAbsoluteSenderTimeHeaderExtension, kRtpAbsoluteSenderTimeHeaderExtensionDefaultId)); - rtp_header_extensions_.push_back( - RtpHeaderExtension(kRtpVideoRotationHeaderExtension, - kRtpVideoRotationHeaderExtensionDefaultId)); } WebRtcVideoEngine::~WebRtcVideoEngine() { @@ -2836,10 +2832,6 @@ bool WebRtcVideoMediaChannel::SetCapturer(uint32 ssrc, QueueBlackFrame(ssrc, timestamp, VideoFormat::FpsToInterval(send_codec_->maxFramerate)); } - - capturer->SetApplyRotation( - !FindHeaderExtension(send_extensions_, kRtpVideoRotationHeaderExtension)); - return true; } @@ -2937,8 +2929,6 @@ 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(); @@ -2954,10 +2944,6 @@ bool WebRtcVideoMediaChannel::SetRecvRtpHeaderExtensions( send_time_extension)) { return false; } - if (!SetHeaderExtension(&webrtc::ViERTP_RTCP::SetReceiveVideoRotationStatus, - channel_id, cvo_extension)) { - return false; - } } receive_extensions_ = extensions; @@ -2974,8 +2960,6 @@ 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(); @@ -2991,10 +2975,6 @@ bool WebRtcVideoMediaChannel::SetSendRtpHeaderExtensions( send_time_extension)) { return false; } - if (!SetHeaderExtension(&webrtc::ViERTP_RTCP::SetSendVideoRotationStatus, - channel_id, cvo_extension)) { - return false; - } } if (send_time_extension) { @@ -3006,14 +2986,6 @@ 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; } @@ -3512,11 +3484,6 @@ 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( @@ -3627,12 +3594,6 @@ 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 ea1fa71b9c..c9781a7498 100644 --- a/talk/media/webrtc/webrtcvideoengine2.cc +++ b/talk/media/webrtc/webrtcvideoengine2.cc @@ -134,17 +134,6 @@ 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, @@ -379,9 +368,6 @@ WebRtcVideoEngine2::WebRtcVideoEngine2(WebRtcVoiceEngine* voice_engine) rtp_header_extensions_.push_back( RtpHeaderExtension(kRtpAbsoluteSenderTimeHeaderExtension, kRtpAbsoluteSenderTimeHeaderExtensionDefaultId)); - rtp_header_extensions_.push_back( - RtpHeaderExtension(kRtpVideoRotationHeaderExtension, - kRtpVideoRotationHeaderExtensionDefaultId)); } WebRtcVideoEngine2::~WebRtcVideoEngine2() { @@ -1149,16 +1135,7 @@ bool WebRtcVideoChannel2::SetCapturer(uint32 ssrc, VideoCapturer* capturer) { LOG(LS_ERROR) << "No sending stream on ssrc " << ssrc; return false; } - if (!send_streams_[ssrc]->SetCapturer(capturer)) { - return false; - } - - if (capturer) { - capturer->SetApplyRotation( - !FindHeaderExtension(send_rtp_extensions_, - kRtpVideoRotationHeaderExtension)); - } - return true; + return send_streams_[ssrc]->SetCapturer(capturer); } bool WebRtcVideoChannel2::SendIntraFrame() { @@ -1281,16 +1258,12 @@ 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; } @@ -1607,15 +1580,6 @@ 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_); @@ -2157,7 +2121,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.rotation()); + frame.render_time_ms() * rtc::kNumNanosecsPerMillisec); renderer_->RenderFrame(&render_frame); } diff --git a/talk/media/webrtc/webrtcvideoengine2.h b/talk/media/webrtc/webrtcvideoengine2.h index cd13a01388..a04d1e5a0a 100644 --- a/talk/media/webrtc/webrtcvideoengine2.h +++ b/talk/media/webrtc/webrtcvideoengine2.h @@ -284,8 +284,6 @@ 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 aabefd6beb..cbf516dd78 100644 --- a/talk/media/webrtc/webrtcvideoengine2_unittest.cc +++ b/talk/media/webrtc/webrtcvideoengine2_unittest.cc @@ -530,82 +530,6 @@ 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( @@ -1274,34 +1198,21 @@ 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(3u, send_stream->GetConfig().rtp.extensions.size()); + ASSERT_EQ(2u, send_stream->GetConfig().rtp.extensions.size()); // Setting the same extensions (even if in different order) shouldn't // reallocate the stream. @@ -1320,21 +1231,18 @@ 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(3u, send_stream->GetConfig().rtp.extensions.size()); + ASSERT_EQ(2u, 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 e6ac7c1f4e..edfa53a793 100644 --- a/talk/media/webrtc/webrtcvideoengine_unittest.cc +++ b/talk/media/webrtc/webrtcvideoengine_unittest.cc @@ -50,7 +50,6 @@ 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); @@ -1006,14 +1005,6 @@ 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 e308a731a3..14118e1fef 100644 --- a/talk/media/webrtc/webrtcvideoframe.cc +++ b/talk/media/webrtc/webrtcvideoframe.cc @@ -46,19 +46,6 @@ 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, @@ -75,8 +62,7 @@ WebRtcVideoFrame::WebRtcVideoFrame(webrtc::NativeHandle* handle, int width, int height, int64_t elapsed_time_ns, - int64_t time_stamp_ns, - webrtc::VideoRotation rotation) + int64_t time_stamp_ns) : video_frame_buffer_( new rtc::RefCountedObject(handle, width, @@ -85,7 +71,7 @@ WebRtcVideoFrame::WebRtcVideoFrame(webrtc::NativeHandle* handle, pixel_height_(1), elapsed_time_ns_(elapsed_time_ns), time_stamp_ns_(time_stamp_ns), - rotation_(rotation) { + rotation_(webrtc::kVideoRotation_0) { } WebRtcVideoFrame::~WebRtcVideoFrame() {} @@ -190,9 +176,10 @@ WebRtcVideoFrame::GetVideoFrameBuffer() const { VideoFrame* WebRtcVideoFrame::Copy() const { WebRtcVideoFrame* new_frame = new WebRtcVideoFrame( - video_frame_buffer_, elapsed_time_ns_, time_stamp_ns_, rotation_); + video_frame_buffer_, elapsed_time_ns_, time_stamp_ns_); 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 a1872c58be..d4380b537d 100644 --- a/talk/media/webrtc/webrtcvideoframe.h +++ b/talk/media/webrtc/webrtcvideoframe.h @@ -42,22 +42,14 @@ 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, - webrtc::VideoRotation rotation); + int64_t time_stamp_ns); ~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 8cbe479932..bbf5d4272b 100644 --- a/talk/media/webrtc/webrtcvideoframe_unittest.cc +++ b/talk/media/webrtc/webrtcvideoframe_unittest.cc @@ -340,8 +340,7 @@ TEST_F(WebRtcVideoFrameTest, InitRotated90DontApplyRotation) { TEST_F(WebRtcVideoFrameTest, TextureInitialValues) { NativeHandleImpl handle; - cricket::WebRtcVideoFrame frame(&handle, 640, 480, 100, 200, - webrtc::kVideoRotation_0); + cricket::WebRtcVideoFrame frame(&handle, 640, 480, 100, 200); EXPECT_EQ(&handle, frame.GetNativeHandle()); EXPECT_EQ(640u, frame.GetWidth()); EXPECT_EQ(480u, frame.GetHeight()); @@ -355,8 +354,7 @@ TEST_F(WebRtcVideoFrameTest, TextureInitialValues) { TEST_F(WebRtcVideoFrameTest, CopyTextureFrame) { NativeHandleImpl handle; - cricket::WebRtcVideoFrame frame1(&handle, 640, 480, 100, 200, - webrtc::kVideoRotation_0); + cricket::WebRtcVideoFrame frame1(&handle, 640, 480, 100, 200); 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 8605925785..e8109fad5f 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_header_extension.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_header_extension.cc @@ -34,17 +34,6 @@ 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; } @@ -58,24 +47,12 @@ 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, active); + extensionMap_[id] = new HeaderExtension(type); 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) { @@ -136,9 +113,7 @@ size_t RtpHeaderExtensionMap::GetTotalLengthInBytes() const { extensionMap_.begin(); while (it != extensionMap_.end()) { HeaderExtension* extension = it->second; - if (extension->active) { - length += extension->length; - } + length += extension->length; it++; } // Add RTP extension header length. @@ -165,11 +140,8 @@ int32_t RtpHeaderExtensionMap::GetLengthUntilBlockStartInBytes( while (it != extensionMap_.end()) { HeaderExtension* extension = it->second; if (extension->type == type) { - if (!extension->active) { - return -1; - } break; - } else if (extension->active) { + } else { length += extension->length; } it++; @@ -178,23 +150,17 @@ int32_t RtpHeaderExtensionMap::GetLengthUntilBlockStartInBytes( } int32_t RtpHeaderExtensionMap::Size() const { - int32_t count = 0; - for (auto& kv : extensionMap_) { - if (kv.second->active) { - count++; - } - } - return count; + return extensionMap_.size(); } RTPExtensionType RtpHeaderExtensionMap::First() const { - for (auto& kv : extensionMap_) { - if (kv.second->active) { - return kv.second->type; - } + std::map::const_iterator it = + extensionMap_.begin(); + if (it == extensionMap_.end()) { + return kRtpExtensionNone; } - - return kRtpExtensionNone; + HeaderExtension* extension = it->second; + return extension->type; } RTPExtensionType RtpHeaderExtensionMap::Next(RTPExtensionType type) const { @@ -204,16 +170,15 @@ RTPExtensionType RtpHeaderExtensionMap::Next(RTPExtensionType type) const { } std::map::const_iterator it = extensionMap_.find(id); - if (it == extensionMap_.end() || !it->second->active) { + if (it == extensionMap_.end()) { return kRtpExtensionNone; } - while ((++it) != extensionMap_.end()) { - if (it->second->active) { - return it->second->type; - } + it++; + if (it == extensionMap_.end()) { + return kRtpExtensionNone; } - - return kRtpExtensionNone; + HeaderExtension* extension = it->second; + return extension->type; } void RtpHeaderExtensionMap::GetCopy(RtpHeaderExtensionMap* map) const { @@ -222,7 +187,7 @@ void RtpHeaderExtensionMap::GetCopy(RtpHeaderExtensionMap* map) const { extensionMap_.begin(); while (it != extensionMap_.end()) { HeaderExtension* extension = it->second; - map->Register(extension->type, it->first, extension->active); + map->Register(extension->type, it->first); it++; } } diff --git a/webrtc/modules/rtp_rtcp/source/rtp_header_extension.h b/webrtc/modules/rtp_rtcp/source/rtp_header_extension.h index 7be3c2e5c4..cb49ea74cc 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_header_extension.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_header_extension.h @@ -29,16 +29,8 @@ const size_t kTransportSequenceNumberLength = 3; struct HeaderExtension { HeaderExtension(RTPExtensionType extension_type) - : type(extension_type), length(0), active(true) { - Init(); - } - - HeaderExtension(RTPExtensionType extension_type, bool active) - : type(extension_type), length(0), active(active) { - Init(); - } - - void Init() { + : type(extension_type), + length(0) { // 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. @@ -65,7 +57,6 @@ struct HeaderExtension { const RTPExtensionType type; uint8_t length; - bool active; }; class RtpHeaderExtensionMap { @@ -77,13 +68,6 @@ 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; @@ -92,10 +76,6 @@ 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; @@ -109,7 +89,6 @@ 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 520cf7a962..d8387e823c 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_header_extension_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_header_extension_unittest.cc @@ -35,16 +35,9 @@ 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) { @@ -63,14 +56,10 @@ 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()); @@ -79,11 +68,7 @@ TEST_F(RtpHeaderExtensionTest, GetTotalLength) { TEST_F(RtpHeaderExtensionTest, GetLengthUntilBlockStart) { EXPECT_EQ(-1, map_.GetLengthUntilBlockStartInBytes( kRtpExtensionTransmissionTimeOffset)); - EXPECT_EQ(0, map_.RegisterInactive(kRtpExtensionTransmissionTimeOffset, kId)); - EXPECT_EQ(-1, map_.GetLengthUntilBlockStartInBytes( - kRtpExtensionTransmissionTimeOffset)); - - EXPECT_TRUE(map_.SetActive(kRtpExtensionTransmissionTimeOffset, true)); + EXPECT_EQ(0, map_.Register(kRtpExtensionTransmissionTimeOffset, kId)); EXPECT_EQ(static_cast(kRtpOneByteHeaderLength), map_.GetLengthUntilBlockStartInBytes( kRtpExtensionTransmissionTimeOffset)); @@ -111,11 +96,7 @@ TEST_F(RtpHeaderExtensionTest, IterateTypes) { EXPECT_EQ(kRtpExtensionNone, map_.First()); EXPECT_EQ(kRtpExtensionNone, map_.Next(kRtpExtensionTransmissionTimeOffset)); - EXPECT_EQ(0, map_.RegisterInactive(kRtpExtensionTransmissionTimeOffset, kId)); - - EXPECT_EQ(kRtpExtensionNone, map_.First()); - - EXPECT_TRUE(map_.SetActive(kRtpExtensionTransmissionTimeOffset, true)); + EXPECT_EQ(0, map_.Register(kRtpExtensionTransmissionTimeOffset, kId)); 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 8d8147cd63..2823868015 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc @@ -63,6 +63,13 @@ 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; @@ -83,14 +90,6 @@ 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 cb0c6dc0c2..35f885a3e2 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc @@ -129,7 +129,6 @@ 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_(), @@ -267,10 +266,6 @@ 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); } @@ -467,16 +462,6 @@ 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, @@ -1216,7 +1201,8 @@ uint16_t RTPSender::BuildRTPHeaderExtension(uint8_t* data_buffer, block_length = BuildAbsoluteSendTimeExtension(extension_data); break; case kRtpExtensionVideoRotation: - block_length = BuildVideoRotationExtension(extension_data); + if (marker_bit) + 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 61d835d06d..baa067c5c0 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.h @@ -41,14 +41,6 @@ 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; @@ -78,7 +70,6 @@ class RTPSenderInterface { const RTPHeader& rtp_header, VideoRotation rotation) const = 0; virtual bool IsRtpHeaderExtensionRegistered(RTPExtensionType type) = 0; - virtual CVOMode ActivateCVORtpHeaderExtension() = 0; }; class RTPSender : public RTPSenderInterface { @@ -294,7 +285,6 @@ 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); @@ -388,7 +378,6 @@ 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 f6ea40b7b7..a55ca977f7 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -186,6 +186,7 @@ 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); @@ -253,7 +254,6 @@ 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,9 +286,6 @@ 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()); @@ -427,7 +424,6 @@ 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); @@ -451,11 +447,10 @@ TEST_F(RtpSenderTest, BuildRTPPacketWithVideoRotation_MarkerBit) { } // Test CVO header extension is not set when marker bit is false. -TEST_F(RtpSenderTest, DISABLED_BuildRTPPacketWithVideoRotation_NoMarkerBit) { +TEST_F(RtpSenderTest, 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); @@ -1338,15 +1333,13 @@ TEST_F(RtpSenderTest, BytesReportedCorrectly) { rtx_stats.transmitted.TotalBytes()); } -// Verify that all packets of a frame have CVO byte set. +// Verify that only the last packet of a frame has 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()); @@ -1358,12 +1351,13 @@ TEST_F(RtpSenderVideoTest, SendVideoWithCVO) { RtpHeaderExtensionMap map; map.Register(kRtpExtensionVideoRotation, kVideoRotationExtensionId); - // Verify that this packet does have CVO byte. + // Verify that this packet doesn't have CVO byte. VerifyCVOPacket( reinterpret_cast(transport_.sent_packets_[0]->data()), - transport_.sent_packets_[0]->length(), true, &map, kSeqNum, hdr.rotation); + transport_.sent_packets_[0]->size(), false, &map, kSeqNum, + kVideoRotation_0); - // Verify that this packet does have CVO byte. + // Verify that this packet doesn't 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 703ae2b9af..2b14cfefa6 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -301,13 +301,6 @@ 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; @@ -348,16 +341,13 @@ 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 every packet of every frame at this point. + // Here we are adding it to the last packet of every frame at this point. if (!rtpHdr) { assert(!_rtpSender.IsRtpHeaderExtensionRegistered( kRtpExtensionVideoRotation)); - } else if (cvo_mode == RTPSenderInterface::kCVOActivated) { + } else if (last) { // 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_coding/main/source/packet.cc b/webrtc/modules/video_coding/main/source/packet.cc index fb815c45b9..dd3743f701 100644 --- a/webrtc/modules/video_coding/main/source/packet.cc +++ b/webrtc/modules/video_coding/main/source/packet.cc @@ -99,9 +99,6 @@ 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