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}

webrtc/api/peerconnection_unittest.cc

@@ -387,6 +387,8 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
 
   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 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
 
     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 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
   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 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
         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 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
       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 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
 
   // 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 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
       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 @@ class P2PTestConductor : public testing::Test {
   }
 
   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 @@ class P2PTestConductor : public testing::Test {
     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 @@ TEST_F(P2PTestConductor, LocalP2PTestDtlsTransferCaller) {
   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.

webrtc/api/test/fakevideotrackrenderer.h

@@ -32,6 +32,7 @@ class FakeVideoTrackRenderer
   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 {

webrtc/media/base/fakevideorenderer.h

@@ -59,7 +59,7 @@ class FakeVideoRenderer : public VideoRenderer {
     rtc::CritScope cs(&crit_);
     return height_;
   }
-  int rotation() const {
+  webrtc::VideoRotation rotation() const {
     rtc::CritScope cs(&crit_);
     return rotation_;
   }

webrtc/media/engine/webrtcvideoengine2.cc

@@ -1800,10 +1800,10 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::SetSendParameters(
     // 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)

webrtc/media/engine/webrtcvideoengine2_unittest.cc

@@ -282,8 +282,10 @@ TEST_F(WebRtcVideoEngine2Test, CVOSetHeaderExtensionAfterCapturer) {
 
   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 @@ TEST_F(WebRtcVideoEngine2Test, CVOSetHeaderExtensionAfterCapturer) {
   // 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.