Fix broken CVO header extension
Adds end to end unit tests for CVO.
BUG=webrtc:5621
Review URL: https://codereview.webrtc.org/1811373002
Cr-Commit-Position: refs/heads/master@{#12063}
diff --git a/webrtc/api/peerconnection_unittest.cc b/webrtc/api/peerconnection_unittest.cc
index fbaeaff..ae37069 100644
--- a/webrtc/api/peerconnection_unittest.cc
+++ b/webrtc/api/peerconnection_unittest.cc
@@ -387,6 +387,8 @@
void RemoveBundleFromReceivedSdp(bool remove) { remove_bundle_ = remove; }
+ void RemoveCvoFromReceivedSdp(bool remove) { remove_cvo_ = remove; }
+
bool can_receive_audio() {
bool value;
if (prefer_constraint_apis_) {
@@ -452,12 +454,19 @@
cricket::FakeVideoCapturer* fake_capturer =
new webrtc::FakePeriodicVideoCapturer();
+ fake_capturer->SetRotation(capture_rotation_);
video_capturers_.push_back(fake_capturer);
rtc::scoped_refptr<webrtc::VideoTrackSourceInterface> source =
peer_connection_factory_->CreateVideoSource(fake_capturer,
&source_constraints);
std::string label = stream_label + kVideoTrackLabelBase;
- return peer_connection_factory_->CreateVideoTrack(label, source);
+
+ rtc::scoped_refptr<webrtc::VideoTrackInterface> track(
+ peer_connection_factory_->CreateVideoTrack(label, source));
+ if (!local_video_renderer_) {
+ local_video_renderer_.reset(new webrtc::FakeVideoTrackRenderer(track));
+ }
+ return track;
}
DataChannelInterface* data_channel() { return data_channel_; }
@@ -468,13 +477,16 @@
webrtc::PeerConnectionInterface* pc() const { return peer_connection_.get(); }
void StopVideoCapturers() {
- for (std::vector<cricket::VideoCapturer*>::iterator it =
- video_capturers_.begin();
- it != video_capturers_.end(); ++it) {
- (*it)->Stop();
+ for (auto* capturer : video_capturers_) {
+ capturer->Stop();
}
}
+ void SetCaptureRotation(webrtc::VideoRotation rotation) {
+ ASSERT_TRUE(video_capturers_.empty());
+ capture_rotation_ = rotation;
+ }
+
bool AudioFramesReceivedCheck(int number_of_frames) const {
return number_of_frames <= fake_audio_capture_module_->frames_received();
}
@@ -705,6 +717,21 @@
fake_video_renderers_.begin()->second->height();
}
+ webrtc::VideoRotation rendered_rotation() {
+ EXPECT_FALSE(fake_video_renderers_.empty());
+ return fake_video_renderers_.empty()
+ ? webrtc::kVideoRotation_0
+ : fake_video_renderers_.begin()->second->rotation();
+ }
+
+ int local_rendered_width() {
+ return local_video_renderer_ ? local_video_renderer_->width() : 1;
+ }
+
+ int local_rendered_height() {
+ return local_video_renderer_ ? local_video_renderer_->height() : 1;
+ }
+
size_t number_of_remote_streams() {
if (!pc())
return 0;
@@ -929,6 +956,10 @@
const char kSdpSdesCryptoAttribute[] = "a=crypto";
RemoveLinesFromSdp(kSdpSdesCryptoAttribute, sdp);
}
+ if (remove_cvo_) {
+ const char kSdpCvoExtenstion[] = "urn:3gpp:video-orientation";
+ RemoveLinesFromSdp(kSdpCvoExtenstion, sdp);
+ }
}
std::string id_;
@@ -964,7 +995,10 @@
// Store references to the video capturers we've created, so that we can stop
// them, if required.
- std::vector<cricket::VideoCapturer*> video_capturers_;
+ std::vector<cricket::FakeVideoCapturer*> video_capturers_;
+ webrtc::VideoRotation capture_rotation_ = webrtc::kVideoRotation_0;
+ // |local_video_renderer_| attached to the first created local video track.
+ rtc::scoped_ptr<webrtc::FakeVideoTrackRenderer> local_video_renderer_;
webrtc::FakeConstraints offer_answer_constraints_;
PeerConnectionInterface::RTCOfferAnswerOptions offer_answer_options_;
@@ -973,6 +1007,9 @@
false; // True if bundle should be removed in received SDP.
bool remove_sdes_ =
false; // True if a=crypto should be removed in received SDP.
+ // |remove_cvo_| is true if extension urn:3gpp:video-orientation should be
+ // removed in the received SDP.
+ bool remove_cvo_ = false;
rtc::scoped_refptr<DataChannelInterface> data_channel_;
rtc::scoped_ptr<MockDataChannelObserver> data_observer_;
@@ -1047,10 +1084,22 @@
}
void VerifyRenderedSize(int width, int height) {
+ VerifyRenderedSize(width, height, webrtc::kVideoRotation_0);
+ }
+
+ void VerifyRenderedSize(int width,
+ int height,
+ webrtc::VideoRotation rotation) {
EXPECT_EQ(width, receiving_client()->rendered_width());
EXPECT_EQ(height, receiving_client()->rendered_height());
+ EXPECT_EQ(rotation, receiving_client()->rendered_rotation());
EXPECT_EQ(width, initializing_client()->rendered_width());
EXPECT_EQ(height, initializing_client()->rendered_height());
+ EXPECT_EQ(rotation, initializing_client()->rendered_rotation());
+
+ // Verify size of the local preview.
+ EXPECT_EQ(width, initializing_client()->local_rendered_width());
+ EXPECT_EQ(height, initializing_client()->local_rendered_height());
}
void VerifySessionDescriptions() {
@@ -1119,6 +1168,11 @@
receiving_client_->SetVideoConstraints(recv_constraints);
}
+ void SetCaptureRotation(webrtc::VideoRotation rotation) {
+ initiating_client_->SetCaptureRotation(rotation);
+ receiving_client_->SetCaptureRotation(rotation);
+ }
+
void EnableVideoDecoderFactory() {
initiating_client_->EnableVideoDecoderFactory();
receiving_client_->EnableVideoDecoderFactory();
@@ -1389,6 +1443,21 @@
VerifyRenderedSize(640, 480);
}
+TEST_F(P2PTestConductor, LocalP2PTestCVO) {
+ ASSERT_TRUE(CreateTestClients());
+ SetCaptureRotation(webrtc::kVideoRotation_90);
+ LocalP2PTest();
+ VerifyRenderedSize(640, 480, webrtc::kVideoRotation_90);
+}
+
+TEST_F(P2PTestConductor, LocalP2PTestReceiverDoesntSupportCVO) {
+ ASSERT_TRUE(CreateTestClients());
+ SetCaptureRotation(webrtc::kVideoRotation_90);
+ receiving_client()->RemoveCvoFromReceivedSdp(true);
+ LocalP2PTest();
+ VerifyRenderedSize(480, 640, webrtc::kVideoRotation_0);
+}
+
// This test sets up a call between two endpoints that are configured to use
// DTLS key agreement. The offerer don't support SDES. As a result, DTLS is
// negotiated and used for transport.
diff --git a/webrtc/api/test/fakevideotrackrenderer.h b/webrtc/api/test/fakevideotrackrenderer.h
index 387ce51..312cf64 100644
--- a/webrtc/api/test/fakevideotrackrenderer.h
+++ b/webrtc/api/test/fakevideotrackrenderer.h
@@ -32,6 +32,7 @@
int errors() const { return fake_renderer_.errors(); }
int width() const { return fake_renderer_.width(); }
int height() const { return fake_renderer_.height(); }
+ webrtc::VideoRotation rotation() const { return fake_renderer_.rotation(); }
bool black_frame() const { return fake_renderer_.black_frame(); }
int num_rendered_frames() const {
diff --git a/webrtc/media/base/fakevideorenderer.h b/webrtc/media/base/fakevideorenderer.h
index 05e285b..5cc1a59 100644
--- a/webrtc/media/base/fakevideorenderer.h
+++ b/webrtc/media/base/fakevideorenderer.h
@@ -59,7 +59,7 @@
rtc::CritScope cs(&crit_);
return height_;
}
- int rotation() const {
+ webrtc::VideoRotation rotation() const {
rtc::CritScope cs(&crit_);
return rotation_;
}
diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc
index 122edfe..2d8cf00 100644
--- a/webrtc/media/engine/webrtcvideoengine2.cc
+++ b/webrtc/media/engine/webrtcvideoengine2.cc
@@ -1800,10 +1800,10 @@
// Set codecs and options.
if (params.codec) {
SetCodec(*params.codec);
- return;
+ recreate_stream = false; // SetCodec has already recreated the stream.
} else if (params.conference_mode && parameters_.codec_settings) {
SetCodec(*parameters_.codec_settings);
- return;
+ recreate_stream = false; // SetCodec has already recreated the stream.
}
if (recreate_stream) {
LOG(LS_INFO)
diff --git a/webrtc/media/engine/webrtcvideoengine2_unittest.cc b/webrtc/media/engine/webrtcvideoengine2_unittest.cc
index fc18721..21c4fd2 100644
--- a/webrtc/media/engine/webrtcvideoengine2_unittest.cc
+++ b/webrtc/media/engine/webrtcvideoengine2_unittest.cc
@@ -282,8 +282,10 @@
cricket::FakeWebRtcVideoEncoderFactory encoder_factory;
encoder_factory.AddSupportedVideoCodecType(webrtc::kVideoCodecVP8, "VP8");
+ encoder_factory.AddSupportedVideoCodecType(webrtc::kVideoCodecVP9, "VP9");
cricket::VideoSendParameters parameters;
parameters.codecs.push_back(kVp8Codec);
+ parameters.codecs.push_back(kVp9Codec);
std::unique_ptr<VideoMediaChannel> channel(
SetUpForExternalEncoderFactory(&encoder_factory, parameters.codecs));
@@ -292,10 +294,15 @@
// Set capturer.
EXPECT_TRUE(channel->SetCapturer(kSsrc, &capturer));
+ // Verify capturer has turned on applying rotation.
+ EXPECT_TRUE(capturer.GetApplyRotation());
+
// Add CVO extension.
const int id = 1;
parameters.extensions.push_back(
cricket::RtpHeaderExtension(kRtpVideoRotationHeaderExtension, id));
+ // Also remove the first codec to trigger a codec change as well.
+ parameters.codecs.erase(parameters.codecs.begin());
EXPECT_TRUE(channel->SetSendParameters(parameters));
// Verify capturer has turned off applying rotation.
