Resync libwebrtc with latest M72 branch
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 23 Jan 2019 04:42:54 +0000 (04:42 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 23 Jan 2019 04:42:54 +0000 (04:42 +0000)
https://bugs.webkit.org/show_bug.cgi?id=193693
LayoutTests/imported/w3c:

<rdar://problem/47463803>

Reviewed by Eric Carlson.

* web-platform-tests/webrtc/RTCRtpTransceiver.https-expected.txt:

Source/ThirdParty/libwebrtc:

Reviewed by Eric Carlson.

Update libwebrtc up to latest M72 branch to fix some identified issues:
- Bad bandwidth estimation in case of multiple transceivers
- mid handling for legacy endpoints
- msid handling for updating mediastreams accordingly.

* Source/webrtc/modules/congestion_controller/goog_cc/delay_based_bwe.cc:
* Source/webrtc/modules/congestion_controller/goog_cc/delay_based_bwe.h:
* Source/webrtc/modules/congestion_controller/goog_cc/goog_cc_network_control.cc:
* Source/webrtc/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc:
* Source/webrtc/modules/congestion_controller/send_side_congestion_controller_unittest.cc:
* Source/webrtc/pc/jsepsessiondescription_unittest.cc:
* Source/webrtc/pc/mediasession.cc:
* Source/webrtc/pc/mediasession_unittest.cc:
* Source/webrtc/pc/peerconnection.cc:
* Source/webrtc/pc/peerconnection.h:
* Source/webrtc/pc/peerconnection_jsep_unittest.cc:
* Source/webrtc/pc/peerconnection_media_unittest.cc:
* Source/webrtc/pc/peerconnection_rtp_unittest.cc:
* Source/webrtc/pc/sessiondescription.cc:
* Source/webrtc/pc/sessiondescription.h:
* Source/webrtc/pc/webrtcsdp.cc:
* Source/webrtc/pc/webrtcsdp_unittest.cc:
* Source/webrtc/system_wrappers/include/metrics.h:
* Source/webrtc/video/BUILD.gn:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@240320 268f45cc-cd09-0410-ab3c-d52691b4dbfc

22 files changed:
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCRtpTransceiver.https-expected.txt
Source/ThirdParty/libwebrtc/ChangeLog
Source/ThirdParty/libwebrtc/Source/webrtc/modules/congestion_controller/goog_cc/delay_based_bwe.cc
Source/ThirdParty/libwebrtc/Source/webrtc/modules/congestion_controller/goog_cc/delay_based_bwe.h
Source/ThirdParty/libwebrtc/Source/webrtc/modules/congestion_controller/goog_cc/goog_cc_network_control.cc
Source/ThirdParty/libwebrtc/Source/webrtc/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc
Source/ThirdParty/libwebrtc/Source/webrtc/modules/congestion_controller/send_side_congestion_controller_unittest.cc
Source/ThirdParty/libwebrtc/Source/webrtc/pc/jsepsessiondescription_unittest.cc
Source/ThirdParty/libwebrtc/Source/webrtc/pc/mediasession.cc
Source/ThirdParty/libwebrtc/Source/webrtc/pc/mediasession_unittest.cc
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.h
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection_jsep_unittest.cc
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection_media_unittest.cc
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection_rtp_unittest.cc
Source/ThirdParty/libwebrtc/Source/webrtc/pc/sessiondescription.cc
Source/ThirdParty/libwebrtc/Source/webrtc/pc/sessiondescription.h
Source/ThirdParty/libwebrtc/Source/webrtc/pc/webrtcsdp.cc
Source/ThirdParty/libwebrtc/Source/webrtc/pc/webrtcsdp_unittest.cc
Source/ThirdParty/libwebrtc/Source/webrtc/system_wrappers/include/metrics.h
Source/ThirdParty/libwebrtc/Source/webrtc/video/BUILD.gn

index 7c6f04e..4db3d20 100644 (file)
@@ -1,3 +1,13 @@
+2019-01-22  Youenn Fablet  <youenn@apple.com>
+
+        Resync libwebrtc with latest M72 branch
+        https://bugs.webkit.org/show_bug.cgi?id=193693
+        <rdar://problem/47463803>
+
+        Reviewed by Eric Carlson.
+
+        * web-platform-tests/webrtc/RTCRtpTransceiver.https-expected.txt:
+
 2019-01-22  Oriol Brufau  <obrufau@igalia.com>
 
         [css-logical] Implement flow-relative margin, padding and border shorthands
index e8df9b1..92bcf0e 100644 (file)
@@ -8,7 +8,7 @@ PASS checkAddTransceiverWithDirection
 PASS checkAddTransceiverWithSetRemoteOfferSending 
 PASS checkAddTransceiverWithSetRemoteOfferNoSend 
 PASS checkAddTransceiverBadKind 
-FAIL checkNoMidOffer promise_test: Unhandled rejection with value: object "InvalidAccessError: Failed to set remote offer sdp: The BUNDLE group contains MID:0 matching no m= section."
+PASS checkNoMidOffer 
 PASS checkSetDirection 
 PASS checkCurrentDirection 
 PASS checkSendrecvWithNoSendTrack 
index af2aa3a..7e16131 100644 (file)
@@ -1,3 +1,35 @@
+2019-01-22  Youenn Fablet  <youenn@apple.com>
+
+        Resync libwebrtc with latest M72 branch
+        https://bugs.webkit.org/show_bug.cgi?id=193693
+
+        Reviewed by Eric Carlson.
+
+        Update libwebrtc up to latest M72 branch to fix some identified issues:
+        - Bad bandwidth estimation in case of multiple transceivers
+        - mid handling for legacy endpoints
+        - msid handling for updating mediastreams accordingly.
+
+        * Source/webrtc/modules/congestion_controller/goog_cc/delay_based_bwe.cc:
+        * Source/webrtc/modules/congestion_controller/goog_cc/delay_based_bwe.h:
+        * Source/webrtc/modules/congestion_controller/goog_cc/goog_cc_network_control.cc:
+        * Source/webrtc/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc:
+        * Source/webrtc/modules/congestion_controller/send_side_congestion_controller_unittest.cc:
+        * Source/webrtc/pc/jsepsessiondescription_unittest.cc:
+        * Source/webrtc/pc/mediasession.cc:
+        * Source/webrtc/pc/mediasession_unittest.cc:
+        * Source/webrtc/pc/peerconnection.cc:
+        * Source/webrtc/pc/peerconnection.h:
+        * Source/webrtc/pc/peerconnection_jsep_unittest.cc:
+        * Source/webrtc/pc/peerconnection_media_unittest.cc:
+        * Source/webrtc/pc/peerconnection_rtp_unittest.cc:
+        * Source/webrtc/pc/sessiondescription.cc:
+        * Source/webrtc/pc/sessiondescription.h:
+        * Source/webrtc/pc/webrtcsdp.cc:
+        * Source/webrtc/pc/webrtcsdp_unittest.cc:
+        * Source/webrtc/system_wrappers/include/metrics.h:
+        * Source/webrtc/video/BUILD.gn:
+
 2019-01-18  Jer Noble  <jer.noble@apple.com>
 
         SDK_VARIANT build destinations should be separate from non-SDK_VARIANT builds
index 4894c98..a76adb0 100644 (file)
@@ -46,8 +46,6 @@ constexpr size_t kDefaultTrendlineWindowSize = 20;
 constexpr double kDefaultTrendlineSmoothingCoeff = 0.9;
 constexpr double kDefaultTrendlineThresholdGain = 4.0;
 
-constexpr int kMaxConsecutiveFailedLookups = 5;
-
 const char kBweWindowSizeInPacketsExperiment[] =
     "WebRTC-BweWindowSizeInPackets";
 
@@ -94,7 +92,6 @@ DelayBasedBwe::DelayBasedBwe(RtcEventLog* event_log)
               : kDefaultTrendlineWindowSize),
       trendline_smoothing_coeff_(kDefaultTrendlineSmoothingCoeff),
       trendline_threshold_gain_(kDefaultTrendlineThresholdGain),
-      consecutive_delayed_feedbacks_(0),
       prev_bitrate_(DataRate::Zero()),
       prev_state_(BandwidthUsage::kBwNormal) {
   RTC_LOG(LS_INFO)
@@ -147,43 +144,12 @@ DelayBasedBwe::Result DelayBasedBwe::IncomingPacketFeedbackVector(
   }
 
   if (delayed_feedback) {
-    Timestamp arrival_time = Timestamp::PlusInfinity();
-    if (packet_feedback_vector.back().arrival_time_ms > 0)
-      arrival_time =
-          Timestamp::ms(packet_feedback_vector.back().arrival_time_ms);
-    return OnDelayedFeedback(arrival_time);
-
-  } else {
-    consecutive_delayed_feedbacks_ = 0;
-    return MaybeUpdateEstimate(acked_bitrate, probe_bitrate,
-                               recovered_from_overuse, at_time);
-  }
-  return Result();
-}
-
-DelayBasedBwe::Result DelayBasedBwe::OnDelayedFeedback(Timestamp receive_time) {
-  ++consecutive_delayed_feedbacks_;
-  if (consecutive_delayed_feedbacks_ >= kMaxConsecutiveFailedLookups) {
-    consecutive_delayed_feedbacks_ = 0;
-    return OnLongFeedbackDelay(receive_time);
+    // TODO(bugs.webrtc.org/10125): Design a better mechanism to safe-guard
+    // against building very large network queues.
+    return Result();
   }
-  return Result();
-}
-
-DelayBasedBwe::Result DelayBasedBwe::OnLongFeedbackDelay(
-    Timestamp arrival_time) {
-  // Estimate should always be valid since a start bitrate always is set in the
-  // Call constructor. An alternative would be to return an empty Result here,
-  // or to estimate the throughput based on the feedback we received.
-  RTC_DCHECK(rate_control_.ValidEstimate());
-  rate_control_.SetEstimate(rate_control_.LatestEstimate() / 2, arrival_time);
-  Result result;
-  result.updated = true;
-  result.probe = false;
-  result.target_bitrate = rate_control_.LatestEstimate();
-  RTC_LOG(LS_WARNING) << "Long feedback delay detected, reducing BWE to "
-                      << ToString(result.target_bitrate);
-  return result;
+  return MaybeUpdateEstimate(acked_bitrate, probe_bitrate,
+                             recovered_from_overuse, at_time);
 }
 
 void DelayBasedBwe::IncomingPacketFeedback(
index 616c5b0..ead60f3 100644 (file)
@@ -49,7 +49,6 @@ class DelayBasedBwe {
       absl::optional<DataRate> acked_bitrate,
       absl::optional<DataRate> probe_bitrate,
       Timestamp at_time);
-  Result OnDelayedFeedback(Timestamp receive_time);
   void OnRttUpdate(TimeDelta avg_rtt);
   bool LatestEstimate(std::vector<uint32_t>* ssrcs, DataRate* bitrate) const;
   void SetStartBitrate(DataRate start_bitrate);
@@ -60,7 +59,6 @@ class DelayBasedBwe {
   friend class GoogCcStatePrinter;
   void IncomingPacketFeedback(const PacketFeedback& packet_feedback,
                               Timestamp at_time);
-  Result OnLongFeedbackDelay(Timestamp arrival_time);
   Result MaybeUpdateEstimate(absl::optional<DataRate> acked_bitrate,
                              absl::optional<DataRate> probe_bitrate,
                              bool request_probe,
@@ -81,7 +79,6 @@ class DelayBasedBwe {
   size_t trendline_window_size_;
   double trendline_smoothing_coeff_;
   double trendline_threshold_gain_;
-  int consecutive_delayed_feedbacks_;
   DataRate prev_bitrate_;
   BandwidthUsage prev_state_;
 
index 4376953..982dd05 100644 (file)
@@ -394,15 +394,9 @@ NetworkControlUpdate GoogCcNetworkController::OnTransportLossReport(
 NetworkControlUpdate GoogCcNetworkController::OnTransportPacketsFeedback(
     TransportPacketsFeedback report) {
   if (report.packet_feedbacks.empty()) {
-    DelayBasedBwe::Result result = delay_based_bwe_->OnDelayedFeedback(
-        report.sendless_arrival_times.back());
-    NetworkControlUpdate update;
-    if (result.updated) {
-      bandwidth_estimation_->UpdateDelayBasedEstimate(report.feedback_time,
-                                                      result.target_bitrate);
-      MaybeTriggerOnNetworkChanged(&update, report.feedback_time);
-    }
-    return update;
+    // TODO(bugs.webrtc.org/10125): Design a better mechanism to safe-guard
+    // against building very large network queues.
+    return NetworkControlUpdate();
   }
 
   if (congestion_window_pushback_controller_) {
index fecd1b3..ee2a6c2 100644 (file)
@@ -193,74 +193,6 @@ TEST_F(GoogCcNetworkControllerTest, ProbeOnRouteChange) {
   update = controller_->OnProcessInterval(DefaultInterval());
 }
 
-// Estimated bitrate reduced when the feedbacks arrive with such a long delay,
-// that the send-time-history no longer holds the feedbacked packets.
-TEST_F(GoogCcNetworkControllerTest, LongFeedbackDelays) {
-  TargetBitrateTrackingSetup();
-  const webrtc::PacedPacketInfo kPacingInfo0(0, 5, 2000);
-  const webrtc::PacedPacketInfo kPacingInfo1(1, 8, 4000);
-  const int64_t kFeedbackTimeoutMs = 60001;
-  const int kMaxConsecutiveFailedLookups = 5;
-  for (int i = 0; i < kMaxConsecutiveFailedLookups; ++i) {
-    std::vector<PacketResult> packets;
-    packets.push_back(CreateResult(i * 100, 2 * i * 100, 1500, kPacingInfo0));
-    packets.push_back(
-        CreateResult(i * 100 + 10, 2 * i * 100 + 10, 1500, kPacingInfo0));
-    packets.push_back(
-        CreateResult(i * 100 + 20, 2 * i * 100 + 20, 1500, kPacingInfo0));
-    packets.push_back(
-        CreateResult(i * 100 + 30, 2 * i * 100 + 30, 1500, kPacingInfo1));
-    packets.push_back(
-        CreateResult(i * 100 + 40, 2 * i * 100 + 40, 1500, kPacingInfo1));
-
-    TransportPacketsFeedback feedback;
-    for (PacketResult& packet : packets) {
-      controller_->OnSentPacket(packet.sent_packet);
-      feedback.sendless_arrival_times.push_back(packet.receive_time);
-    }
-
-    feedback.feedback_time = packets[0].receive_time;
-    AdvanceTimeMilliseconds(kFeedbackTimeoutMs);
-    SentPacket later_packet;
-    later_packet.send_time = Timestamp::ms(kFeedbackTimeoutMs + i * 200 + 40);
-    later_packet.size = DataSize::bytes(1500);
-    later_packet.pacing_info = kPacingInfo1;
-    controller_->OnSentPacket(later_packet);
-
-    OnUpdate(controller_->OnTransportPacketsFeedback(feedback));
-  }
-  OnUpdate(controller_->OnProcessInterval(DefaultInterval()));
-
-  EXPECT_EQ(kInitialBitrateKbps / 2, target_bitrate_->kbps());
-
-  // Test with feedback that isn't late enough to time out.
-  {
-    std::vector<PacketResult> packets;
-    packets.push_back(CreateResult(100, 200, 1500, kPacingInfo0));
-    packets.push_back(CreateResult(110, 210, 1500, kPacingInfo0));
-    packets.push_back(CreateResult(120, 220, 1500, kPacingInfo0));
-    packets.push_back(CreateResult(130, 230, 1500, kPacingInfo1));
-    packets.push_back(CreateResult(140, 240, 1500, kPacingInfo1));
-
-    for (const PacketResult& packet : packets)
-      controller_->OnSentPacket(packet.sent_packet);
-
-    TransportPacketsFeedback feedback;
-    feedback.feedback_time = packets[0].receive_time;
-    feedback.packet_feedbacks = packets;
-
-    AdvanceTimeMilliseconds(kFeedbackTimeoutMs - 1);
-
-    SentPacket later_packet;
-    later_packet.send_time = Timestamp::ms(kFeedbackTimeoutMs + 240);
-    later_packet.size = DataSize::bytes(1500);
-    later_packet.pacing_info = kPacingInfo1;
-    controller_->OnSentPacket(later_packet);
-
-    OnUpdate(controller_->OnTransportPacketsFeedback(feedback));
-  }
-}
-
 // Bandwidth estimation is updated when feedbacks are received.
 // Feedbacks which show an increasing delay cause the estimation to be reduced.
 TEST_F(GoogCcNetworkControllerTest, UpdatesDelayBasedEstimate) {
index 5f58f21..0f37e0e 100644 (file)
@@ -344,97 +344,6 @@ TEST_F(LegacySendSideCongestionControllerTest, ProbeOnRouteChange) {
                                      20 * kInitialBitrateBps);
 }
 
-// Estimated bitrate reduced when the feedbacks arrive with such a long delay,
-// that the send-time-history no longer holds the feedbacked packets.
-TEST_F(LegacySendSideCongestionControllerTest, LongFeedbackDelays) {
-  TargetBitrateTrackingSetup();
-
-  const int64_t kFeedbackTimeoutMs = 60001;
-  const int kMaxConsecutiveFailedLookups = 5;
-  for (int i = 0; i < kMaxConsecutiveFailedLookups; ++i) {
-    std::vector<PacketFeedback> packets;
-    packets.push_back(
-        PacketFeedback(i * 100, 2 * i * 100, 0, 1500, kPacingInfo0));
-    packets.push_back(
-        PacketFeedback(i * 100 + 10, 2 * i * 100 + 10, 1, 1500, kPacingInfo0));
-    packets.push_back(
-        PacketFeedback(i * 100 + 20, 2 * i * 100 + 20, 2, 1500, kPacingInfo0));
-    packets.push_back(
-        PacketFeedback(i * 100 + 30, 2 * i * 100 + 30, 3, 1500, kPacingInfo1));
-    packets.push_back(
-        PacketFeedback(i * 100 + 40, 2 * i * 100 + 40, 4, 1500, kPacingInfo1));
-
-    for (const PacketFeedback& packet : packets)
-      OnSentPacket(packet);
-
-    rtcp::TransportFeedback feedback;
-    feedback.SetBase(packets[0].sequence_number,
-                     packets[0].arrival_time_ms * 1000);
-
-    for (const PacketFeedback& packet : packets) {
-      EXPECT_TRUE(feedback.AddReceivedPacket(packet.sequence_number,
-                                             packet.arrival_time_ms * 1000));
-    }
-
-    feedback.Build();
-
-    clock_.AdvanceTimeMilliseconds(kFeedbackTimeoutMs);
-    PacketFeedback later_packet(kFeedbackTimeoutMs + i * 100 + 40,
-                                kFeedbackTimeoutMs + i * 200 + 40, 5, 1500,
-                                kPacingInfo1);
-    OnSentPacket(later_packet);
-
-    controller_->OnTransportFeedback(feedback);
-
-    // Check that packets have timed out.
-    for (PacketFeedback& packet : packets) {
-      packet.send_time_ms = PacketFeedback::kNoSendTime;
-      packet.payload_size = 0;
-      packet.pacing_info = PacedPacketInfo();
-    }
-    ComparePacketFeedbackVectors(packets,
-                                 controller_->GetTransportFeedbackVector());
-  }
-
-  controller_->Process();
-
-  EXPECT_EQ(kInitialBitrateBps / 2, target_bitrate_bps_);
-
-  // Test with feedback that isn't late enough to time out.
-  {
-    std::vector<PacketFeedback> packets;
-    packets.push_back(PacketFeedback(100, 200, 0, 1500, kPacingInfo0));
-    packets.push_back(PacketFeedback(110, 210, 1, 1500, kPacingInfo0));
-    packets.push_back(PacketFeedback(120, 220, 2, 1500, kPacingInfo0));
-    packets.push_back(PacketFeedback(130, 230, 3, 1500, kPacingInfo1));
-    packets.push_back(PacketFeedback(140, 240, 4, 1500, kPacingInfo1));
-
-    for (const PacketFeedback& packet : packets)
-      OnSentPacket(packet);
-
-    rtcp::TransportFeedback feedback;
-    feedback.SetBase(packets[0].sequence_number,
-                     packets[0].arrival_time_ms * 1000);
-
-    for (const PacketFeedback& packet : packets) {
-      EXPECT_TRUE(feedback.AddReceivedPacket(packet.sequence_number,
-                                             packet.arrival_time_ms * 1000));
-    }
-
-    feedback.Build();
-
-    clock_.AdvanceTimeMilliseconds(kFeedbackTimeoutMs - 1);
-    PacketFeedback later_packet(kFeedbackTimeoutMs + 140,
-                                kFeedbackTimeoutMs + 240, 5, 1500,
-                                kPacingInfo1);
-    OnSentPacket(later_packet);
-
-    controller_->OnTransportFeedback(feedback);
-    ComparePacketFeedbackVectors(packets,
-                                 controller_->GetTransportFeedbackVector());
-  }
-}
-
 // Bandwidth estimation is updated when feedbacks are received.
 // Feedbacks which show an increasing delay cause the estimation to be reduced.
 TEST_F(LegacySendSideCongestionControllerTest, UpdatesDelayBasedEstimate) {
index d9e2588..1c82326 100644 (file)
@@ -60,16 +60,16 @@ static cricket::SessionDescription* CreateCricketSessionDescription() {
   video->AddCodec(cricket::VideoCodec(120, "VP8"));
   desc->AddContent(cricket::CN_VIDEO, MediaProtocolType::kRtp, video.release());
 
-  EXPECT_TRUE(desc->AddTransportInfo(cricket::TransportInfo(
+  desc->AddTransportInfo(cricket::TransportInfo(
       cricket::CN_AUDIO,
       cricket::TransportDescription(
           std::vector<std::string>(), kCandidateUfragVoice, kCandidatePwdVoice,
-          cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_NONE, NULL))));
-  EXPECT_TRUE(desc->AddTransportInfo(cricket::TransportInfo(
+          cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_NONE, NULL)));
+  desc->AddTransportInfo(cricket::TransportInfo(
       cricket::CN_VIDEO,
       cricket::TransportDescription(
           std::vector<std::string>(), kCandidateUfragVideo, kCandidatePwdVideo,
-          cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_NONE, NULL))));
+          cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_NONE, NULL)));
   return desc;
 }
 
index 263f369..cce7d22 100644 (file)
@@ -1799,14 +1799,12 @@ bool MediaSessionDescriptionFactory::AddTransportOffer(
   std::unique_ptr<TransportDescription> new_tdesc(
       transport_desc_factory_->CreateOffer(transport_options, current_tdesc,
                                            ice_credentials));
-  bool ret =
-      (new_tdesc.get() != NULL &&
-       offer_desc->AddTransportInfo(TransportInfo(content_name, *new_tdesc)));
-  if (!ret) {
+  if (!new_tdesc) {
     RTC_LOG(LS_ERROR) << "Failed to AddTransportOffer, content name="
                       << content_name;
   }
-  return ret;
+  offer_desc->AddTransportInfo(TransportInfo(content_name, *new_tdesc));
+  return true;
 }
 
 TransportDescription* MediaSessionDescriptionFactory::CreateTransportAnswer(
@@ -1831,12 +1829,7 @@ bool MediaSessionDescriptionFactory::AddTransportAnswer(
     const std::string& content_name,
     const TransportDescription& transport_desc,
     SessionDescription* answer_desc) const {
-  if (!answer_desc->AddTransportInfo(
-          TransportInfo(content_name, transport_desc))) {
-    RTC_LOG(LS_ERROR) << "Failed to AddTransportAnswer, content name="
-                      << content_name;
-    return false;
-  }
+  answer_desc->AddTransportInfo(TransportInfo(content_name, transport_desc));
   return true;
 }
 
index 2098ec0..fbc4035 100644 (file)
@@ -426,14 +426,14 @@ class MediaSessionDescriptionFactoryTest : public testing::Test {
     std::unique_ptr<SessionDescription> desc;
     if (has_current_desc) {
       current_desc.reset(new SessionDescription());
-      EXPECT_TRUE(current_desc->AddTransportInfo(TransportInfo(
+      current_desc->AddTransportInfo(TransportInfo(
           "audio",
-          TransportDescription(current_audio_ufrag, current_audio_pwd))));
-      EXPECT_TRUE(current_desc->AddTransportInfo(TransportInfo(
+          TransportDescription(current_audio_ufrag, current_audio_pwd)));
+      current_desc->AddTransportInfo(TransportInfo(
           "video",
-          TransportDescription(current_video_ufrag, current_video_pwd))));
-      EXPECT_TRUE(current_desc->AddTransportInfo(TransportInfo(
-          "data", TransportDescription(current_data_ufrag, current_data_pwd))));
+          TransportDescription(current_video_ufrag, current_video_pwd)));
+      current_desc->AddTransportInfo(TransportInfo(
+          "data", TransportDescription(current_data_ufrag, current_data_pwd)));
     }
     if (offer) {
       desc.reset(f1_.CreateOffer(options, current_desc.get()));
index ee8a909..1307ef6 100644 (file)
@@ -649,6 +649,25 @@ DataMessageType ToWebrtcDataMessageType(cricket::DataMessageType type) {
   return DataMessageType::kControl;
 }
 
+// Find a new MID that is not already in |used_mids|, then add it to |used_mids|
+// and return a reference to it.
+// Generated MIDs should be no more than 3 bytes long to take up less space in
+// the RTP packet.
+const std::string& AllocateMid(std::set<std::string>* used_mids) {
+  RTC_DCHECK(used_mids);
+  // We're boring: just generate MIDs 0, 1, 2, ...
+  size_t i = 0;
+  std::set<std::string>::iterator it;
+  bool inserted;
+  do {
+    std::string mid = rtc::ToString(i++);
+    auto insert_result = used_mids->insert(mid);
+    it = insert_result.first;
+    inserted = insert_result.second;
+  } while (!inserted);
+  return *it;
+}
+
 }  // namespace
 
 // Upon completion, posts a task to execute the callback of the
@@ -2231,6 +2250,56 @@ RTCError PeerConnection::ApplyLocalDescription(
   return RTCError::OK();
 }
 
+// The SDP parser used to populate these values by default for the 'content
+// name' if an a=mid line was absent.
+static absl::string_view GetDefaultMidForPlanB(cricket::MediaType media_type) {
+  switch (media_type) {
+    case cricket::MEDIA_TYPE_AUDIO:
+      return cricket::CN_AUDIO;
+    case cricket::MEDIA_TYPE_VIDEO:
+      return cricket::CN_VIDEO;
+    case cricket::MEDIA_TYPE_DATA:
+      return cricket::CN_DATA;
+  }
+  RTC_NOTREACHED();
+  return "";
+}
+
+void PeerConnection::FillInMissingRemoteMids(
+    cricket::SessionDescription* remote_description) {
+  RTC_DCHECK(remote_description);
+  const cricket::ContentInfos& local_contents =
+      (local_description() ? local_description()->description()->contents()
+                           : cricket::ContentInfos());
+  std::set<std::string> used_mids = seen_mids_;
+  for (size_t i = 0; i < remote_description->contents().size(); ++i) {
+    cricket::ContentInfo& content = remote_description->contents()[i];
+    if (!content.name.empty()) {
+      continue;
+    }
+    absl::string_view new_mid;
+    absl::string_view source_explanation;
+    if (IsUnifiedPlan()) {
+      if (i < local_contents.size()) {
+        new_mid = local_contents[i].name;
+        source_explanation = "from the matching local media section";
+      } else {
+        new_mid = AllocateMid(&used_mids);
+        source_explanation = "generated just now";
+      }
+    } else {
+      new_mid = GetDefaultMidForPlanB(content.media_description()->type());
+      source_explanation = "to match pre-existing behavior";
+    }
+    content.name = std::string(new_mid);
+    remote_description->transport_infos()[i].content_name =
+        std::string(new_mid);
+    RTC_LOG(LS_INFO) << "SetRemoteDescription: Remote media section at i=" << i
+                     << " is missing an a=mid line. Filling in the value '"
+                     << new_mid << "' " << source_explanation << ".";
+  }
+}
+
 void PeerConnection::SetRemoteDescription(
     SetSessionDescriptionObserver* observer,
     SessionDescriptionInterface* desc) {
@@ -2271,6 +2340,10 @@ void PeerConnection::SetRemoteDescription(
     ReportSdpFormatReceived(*desc);
   }
 
+  // Handle remote descriptions missing a=mid lines for interop with legacy end
+  // points.
+  FillInMissingRemoteMids(desc->description());
+
   RTCError error = ValidateSessionDescription(desc.get(), cricket::CS_REMOTE);
   if (!error.ok()) {
     std::string error_message = GetSetDescriptionErrorMessage(
@@ -2456,51 +2529,32 @@ RTCError PeerConnection::ApplyRemoteDescription(
       const MediaContentDescription* media_desc = content->media_description();
       RtpTransceiverDirection local_direction =
           RtpTransceiverDirectionReversed(media_desc->direction());
-      // From the WebRTC specification, steps 2.2.8.5/6 of section 4.4.1.6 "Set
-      // the RTCSessionDescription: If direction is sendrecv or recvonly, and
-      // transceiver's current direction is neither sendrecv nor recvonly,
-      // process the addition of a remote track for the media description.
-      std::vector<std::string> stream_ids;
-      if (!media_desc->streams().empty()) {
-        // The remote description has signaled the stream IDs.
-        stream_ids = media_desc->streams()[0].stream_ids();
-      }
-      if (RtpTransceiverDirectionHasRecv(local_direction) &&
-          (!transceiver->fired_direction() ||
-           !RtpTransceiverDirectionHasRecv(*transceiver->fired_direction()))) {
-        RTC_LOG(LS_INFO) << "Processing the addition of a new track for MID="
-                         << content->name << " (added to "
-                         << GetStreamIdsString(stream_ids) << ").";
-
-        std::vector<rtc::scoped_refptr<MediaStreamInterface>> media_streams;
-        for (const std::string& stream_id : stream_ids) {
-          rtc::scoped_refptr<MediaStreamInterface> stream =
-              remote_streams_->find(stream_id);
-          if (!stream) {
-            stream = MediaStreamProxy::Create(rtc::Thread::Current(),
-                                              MediaStream::Create(stream_id));
-            remote_streams_->AddStream(stream);
-            added_streams.push_back(stream);
-          }
-          media_streams.push_back(stream);
+      // Roughly the same as steps 2.2.8.6 of section 4.4.1.6 "Set the
+      // RTCSessionDescription: Set the associated remote streams given
+      // transceiver.[[Receiver]], msids, addList, and removeList".
+      // https://w3c.github.io/webrtc-pc/#set-the-rtcsessiondescription
+      if (RtpTransceiverDirectionHasRecv(local_direction)) {
+        std::vector<std::string> stream_ids;
+        if (!media_desc->streams().empty()) {
+          // The remote description has signaled the stream IDs.
+          stream_ids = media_desc->streams()[0].stream_ids();
         }
-        // Special case: "a=msid" missing, use random stream ID.
-        if (media_streams.empty() &&
-            !(remote_description()->description()->msid_signaling() &
-              cricket::kMsidSignalingMediaSection)) {
-          if (!missing_msid_default_stream_) {
-            missing_msid_default_stream_ = MediaStreamProxy::Create(
-                rtc::Thread::Current(),
-                MediaStream::Create(rtc::CreateRandomUuid()));
-            added_streams.push_back(missing_msid_default_stream_);
-          }
-          media_streams.push_back(missing_msid_default_stream_);
+        RTC_LOG(LS_INFO) << "Processing the MSIDs for MID=" << content->name
+                         << " (" << GetStreamIdsString(stream_ids) << ").";
+        SetAssociatedRemoteStreams(transceiver->internal()->receiver_internal(),
+                                   stream_ids, &added_streams,
+                                   &removed_streams);
+        // From the WebRTC specification, steps 2.2.8.5/6 of section 4.4.1.6
+        // "Set the RTCSessionDescription: If direction is sendrecv or recvonly,
+        // and transceiver's current direction is neither sendrecv nor recvonly,
+        // process the addition of a remote track for the media description.
+        if (!transceiver->fired_direction() ||
+            !RtpTransceiverDirectionHasRecv(*transceiver->fired_direction())) {
+          RTC_LOG(LS_INFO)
+              << "Processing the addition of a remote track for MID="
+              << content->name << ".";
+          now_receiving_transceivers.push_back(transceiver);
         }
-        // This will add the remote track to the streams.
-        // TODO(hbos): When we remove remote_streams(), use set_stream_ids()
-        // instead. https://crbug.com/webrtc/9480
-        transceiver->internal()->receiver_internal()->SetStreams(media_streams);
-        now_receiving_transceivers.push_back(transceiver);
       }
       // 2.2.8.1.7: If direction is "sendonly" or "inactive", and transceiver's
       // [[FiredDirection]] slot is either "sendrecv" or "recvonly", process the
@@ -2643,6 +2697,46 @@ RTCError PeerConnection::ApplyRemoteDescription(
   return RTCError::OK();
 }
 
+void PeerConnection::SetAssociatedRemoteStreams(
+    rtc::scoped_refptr<RtpReceiverInternal> receiver,
+    const std::vector<std::string>& stream_ids,
+    std::vector<rtc::scoped_refptr<MediaStreamInterface>>* added_streams,
+    std::vector<rtc::scoped_refptr<MediaStreamInterface>>* removed_streams) {
+  std::vector<rtc::scoped_refptr<MediaStreamInterface>> media_streams;
+  for (const std::string& stream_id : stream_ids) {
+    rtc::scoped_refptr<MediaStreamInterface> stream =
+        remote_streams_->find(stream_id);
+    if (!stream) {
+      stream = MediaStreamProxy::Create(rtc::Thread::Current(),
+                                        MediaStream::Create(stream_id));
+      remote_streams_->AddStream(stream);
+      added_streams->push_back(stream);
+    }
+    media_streams.push_back(stream);
+  }
+  // Special case: "a=msid" missing, use random stream ID.
+  if (media_streams.empty() &&
+      !(remote_description()->description()->msid_signaling() &
+        cricket::kMsidSignalingMediaSection)) {
+    if (!missing_msid_default_stream_) {
+      missing_msid_default_stream_ = MediaStreamProxy::Create(
+          rtc::Thread::Current(), MediaStream::Create(rtc::CreateRandomUuid()));
+      added_streams->push_back(missing_msid_default_stream_);
+    }
+    media_streams.push_back(missing_msid_default_stream_);
+  }
+  std::vector<rtc::scoped_refptr<MediaStreamInterface>> previous_streams =
+      receiver->streams();
+  // SetStreams() will add/remove the receiver's track to/from the streams. This
+  // differs from the spec - the spec uses an "addList" and "removeList" to
+  // update the stream-track relationships in a later step. We do this earlier,
+  // changing the order of things, but the end-result is the same.
+  // TODO(hbos): When we remove remote_streams(), use set_stream_ids()
+  // instead. https://crbug.com/webrtc/9480
+  receiver->SetStreams(media_streams);
+  RemoveRemoteStreamsIfEmpty(previous_streams, removed_streams);
+}
+
 void PeerConnection::ProcessRemovalOfRemoteTrack(
     rtc::scoped_refptr<RtpTransceiverProxyWithInternal<RtpTransceiver>>
         transceiver,
@@ -2651,19 +2745,25 @@ void PeerConnection::ProcessRemovalOfRemoteTrack(
   RTC_DCHECK(transceiver->mid());
   RTC_LOG(LS_INFO) << "Processing the removal of a track for MID="
                    << *transceiver->mid();
-  std::vector<rtc::scoped_refptr<MediaStreamInterface>> media_streams =
+  std::vector<rtc::scoped_refptr<MediaStreamInterface>> previous_streams =
       transceiver->internal()->receiver_internal()->streams();
   // This will remove the remote track from the streams.
   transceiver->internal()->receiver_internal()->set_stream_ids({});
   remove_list->push_back(transceiver);
-  // Remove any streams that no longer have tracks.
-  // TODO(https://crbug.com/webrtc/9480): When we use stream IDs instead
-  // of streams, see if the stream was removed by checking if this was the
-  // last receiver with that stream ID.
-  for (const auto& stream : media_streams) {
-    if (stream->GetAudioTracks().empty() && stream->GetVideoTracks().empty()) {
-      remote_streams_->RemoveStream(stream);
-      removed_streams->push_back(stream);
+  RemoveRemoteStreamsIfEmpty(previous_streams, removed_streams);
+}
+
+void PeerConnection::RemoveRemoteStreamsIfEmpty(
+    const std::vector<rtc::scoped_refptr<MediaStreamInterface>>& remote_streams,
+    std::vector<rtc::scoped_refptr<MediaStreamInterface>>* removed_streams) {
+  // TODO(https://crbug.com/webrtc/9480): When we use stream IDs instead of
+  // streams, see if the stream was removed by checking if this was the last
+  // receiver with that stream ID.
+  for (auto remote_stream : remote_streams) {
+    if (remote_stream->GetAudioTracks().empty() &&
+        remote_stream->GetVideoTracks().empty()) {
+      remote_streams_->RemoveStream(remote_stream);
+      removed_streams->push_back(remote_stream);
     }
   }
 }
@@ -3908,25 +4008,6 @@ void PeerConnection::GetOptionsForPlanBOffer(
                       offer_answer_options.num_simulcast_layers);
 }
 
-// Find a new MID that is not already in |used_mids|, then add it to |used_mids|
-// and return a reference to it.
-// Generated MIDs should be no more than 3 bytes long to take up less space in
-// the RTP packet.
-static const std::string& AllocateMid(std::set<std::string>* used_mids) {
-  RTC_DCHECK(used_mids);
-  // We're boring: just generate MIDs 0, 1, 2, ...
-  size_t i = 0;
-  std::set<std::string>::iterator it;
-  bool inserted;
-  do {
-    std::string mid = rtc::ToString(i++);
-    auto insert_result = used_mids->insert(mid);
-    it = insert_result.first;
-    inserted = insert_result.second;
-  } while (!inserted);
-  return *it;
-}
-
 static cricket::MediaDescriptionOptions
 GetMediaDescriptionOptionsForTransceiver(
     rtc::scoped_refptr<RtpTransceiverProxyWithInternal<RtpTransceiver>>
@@ -6086,6 +6167,21 @@ bool PeerConnection::HasRtcpMuxEnabled(const cricket::ContentInfo* content) {
   return content->media_description()->rtcp_mux();
 }
 
+static RTCError ValidateMids(const cricket::SessionDescription& description) {
+  std::set<std::string> mids;
+  for (const cricket::ContentInfo& content : description.contents()) {
+    if (content.name.empty()) {
+      LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER,
+                           "A media section is missing a MID attribute.");
+    }
+    if (!mids.insert(content.name).second) {
+      LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER,
+                           "Duplicate a=mid value '" + content.name + "'.");
+    }
+  }
+  return RTCError::OK();
+}
+
 RTCError PeerConnection::ValidateSessionDescription(
     const SessionDescriptionInterface* sdesc,
     cricket::ContentSource source) {
@@ -6105,6 +6201,11 @@ RTCError PeerConnection::ValidateSessionDescription(
         "Called in wrong state: " + GetSignalingStateString(signaling_state()));
   }
 
+  RTCError error = ValidateMids(*sdesc->description());
+  if (!error.ok()) {
+    return error;
+  }
+
   // Verify crypto settings.
   std::string crypto_error;
   if (webrtc_session_desc_factory_->SdesPolicy() == cricket::SEC_REQUIRED ||
index 1810f1c..6d7584d 100644 (file)
@@ -472,6 +472,14 @@ class PeerConnection : public PeerConnectionInternal,
           transceiver,
       const SessionDescriptionInterface* sdesc) const;
 
+  // Runs the algorithm **set the associated remote streams** specified in
+  // https://w3c.github.io/webrtc-pc/#set-associated-remote-streams.
+  void SetAssociatedRemoteStreams(
+      rtc::scoped_refptr<RtpReceiverInternal> receiver,
+      const std::vector<std::string>& stream_ids,
+      std::vector<rtc::scoped_refptr<MediaStreamInterface>>* added_streams,
+      std::vector<rtc::scoped_refptr<MediaStreamInterface>>* removed_streams);
+
   // Runs the algorithm **process the removal of a remote track** specified in
   // the WebRTC specification.
   // This method will update the following lists:
@@ -486,6 +494,11 @@ class PeerConnection : public PeerConnectionInternal,
       std::vector<rtc::scoped_refptr<RtpTransceiverInterface>>* remove_list,
       std::vector<rtc::scoped_refptr<MediaStreamInterface>>* removed_streams);
 
+  void RemoveRemoteStreamsIfEmpty(
+      const std::vector<rtc::scoped_refptr<MediaStreamInterface>>&
+          remote_streams,
+      std::vector<rtc::scoped_refptr<MediaStreamInterface>>* removed_streams);
+
   void OnNegotiationNeeded();
 
   bool IsClosed() const {
@@ -649,6 +662,12 @@ class PeerConnection : public PeerConnectionInternal,
     return configuration_.sdp_semantics == SdpSemantics::kUnifiedPlan;
   }
 
+  // The offer/answer machinery assumes the media section MID is present and
+  // unique. To support legacy end points that do not supply a=mid lines, this
+  // method will modify the session description to add MIDs generated according
+  // to the SDP semantics.
+  void FillInMissingRemoteMids(cricket::SessionDescription* remote_description);
+
   // Is there an RtpSender of the given type?
   bool HasRtpSender(cricket::MediaType type) const;
 
index f0c9111..7a04474 100644 (file)
@@ -1604,4 +1604,110 @@ TEST_F(PeerConnectionJsepTest, OneVideoUnifiedPlanToTwoVideoPlanBFails) {
   EXPECT_EQ(RTCErrorType::INVALID_PARAMETER, error.type());
 }
 
+// Removes the RTP header extension associated with the given URI from the media
+// description.
+static void RemoveRtpHeaderExtensionByUri(
+    MediaContentDescription* media_description,
+    absl::string_view uri) {
+  std::vector<RtpExtension> header_extensions =
+      media_description->rtp_header_extensions();
+  header_extensions.erase(std::remove_if(
+      header_extensions.begin(), header_extensions.end(),
+      [uri](const RtpExtension& extension) { return extension.uri == uri; }));
+  media_description->set_rtp_header_extensions(header_extensions);
+}
+
+// Transforms a session description to emulate a legacy endpoint which does not
+// support a=mid, BUNDLE, and the MID header extension.
+static void ClearMids(SessionDescriptionInterface* sdesc) {
+  cricket::SessionDescription* desc = sdesc->description();
+  desc->RemoveGroupByName(cricket::GROUP_TYPE_BUNDLE);
+  cricket::ContentInfo* audio_content = cricket::GetFirstAudioContent(desc);
+  if (audio_content) {
+    desc->GetTransportInfoByName(audio_content->name)->content_name = "";
+    audio_content->name = "";
+    RemoveRtpHeaderExtensionByUri(audio_content->media_description(),
+                                  RtpExtension::kMidUri);
+  }
+  cricket::ContentInfo* video_content = cricket::GetFirstVideoContent(desc);
+  if (video_content) {
+    desc->GetTransportInfoByName(video_content->name)->content_name = "";
+    video_content->name = "";
+    RemoveRtpHeaderExtensionByUri(video_content->media_description(),
+                                  RtpExtension::kMidUri);
+  }
+}
+
+// Test that negotiation works with legacy endpoints which do not support a=mid.
+TEST_F(PeerConnectionJsepTest, LegacyNoMidAudioOnlyOffer) {
+  auto caller = CreatePeerConnection();
+  caller->AddAudioTrack("audio");
+  auto callee = CreatePeerConnection();
+  callee->AddAudioTrack("audio");
+
+  auto offer = caller->CreateOffer();
+  ClearMids(offer.get());
+
+  ASSERT_TRUE(callee->SetRemoteDescription(std::move(offer)));
+  EXPECT_TRUE(callee->SetLocalDescription(callee->CreateAnswer()));
+}
+TEST_F(PeerConnectionJsepTest, LegacyNoMidAudioVideoOffer) {
+  auto caller = CreatePeerConnection();
+  caller->AddAudioTrack("audio");
+  caller->AddVideoTrack("video");
+  auto callee = CreatePeerConnection();
+  callee->AddAudioTrack("audio");
+  callee->AddVideoTrack("video");
+
+  auto offer = caller->CreateOffer();
+  ClearMids(offer.get());
+
+  ASSERT_TRUE(callee->SetRemoteDescription(std::move(offer)));
+  EXPECT_TRUE(callee->SetLocalDescription(callee->CreateAnswer()));
+}
+TEST_F(PeerConnectionJsepTest, LegacyNoMidAudioOnlyAnswer) {
+  auto caller = CreatePeerConnection();
+  caller->AddAudioTrack("audio");
+  auto callee = CreatePeerConnection();
+  callee->AddAudioTrack("audio");
+
+  ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
+
+  auto answer = callee->CreateAnswer();
+  ClearMids(answer.get());
+
+  EXPECT_TRUE(caller->SetRemoteDescription(std::move(answer)));
+}
+TEST_F(PeerConnectionJsepTest, LegacyNoMidAudioVideoAnswer) {
+  auto caller = CreatePeerConnection();
+  caller->AddAudioTrack("audio");
+  caller->AddVideoTrack("video");
+  auto callee = CreatePeerConnection();
+  callee->AddAudioTrack("audio");
+  callee->AddVideoTrack("video");
+
+  ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
+
+  auto answer = callee->CreateAnswer();
+  ClearMids(answer.get());
+
+  ASSERT_TRUE(caller->SetRemoteDescription(std::move(answer)));
+}
+
+// Test that SetLocalDescription fails if a=mid lines are missing.
+TEST_F(PeerConnectionJsepTest, SetLocalDescriptionFailsMissingMid) {
+  auto caller = CreatePeerConnection();
+  caller->AddAudioTrack("audio");
+
+  auto offer = caller->CreateOffer();
+  ClearMids(offer.get());
+
+  std::string error;
+  ASSERT_FALSE(caller->SetLocalDescription(std::move(offer), &error));
+  EXPECT_EQ(
+      "Failed to set local offer sdp: A media section is missing a MID "
+      "attribute.",
+      error);
+}
+
 }  // namespace webrtc
index 6af0c98..25aea98 100644 (file)
@@ -1084,6 +1084,22 @@ TEST_P(PeerConnectionMediaTest, ReOfferHasSameMidsAsFirstOffer) {
             cricket::GetFirstVideoContent(reoffer->description())->name);
 }
 
+// Test that SetRemoteDescription returns an error if there are two m= sections
+// with the same MID value.
+TEST_P(PeerConnectionMediaTest, SetRemoteDescriptionFailsWithDuplicateMids) {
+  auto caller = CreatePeerConnectionWithAudioVideo();
+  auto callee = CreatePeerConnectionWithAudioVideo();
+
+  auto offer = caller->CreateOffer();
+  RenameContent(offer->description(), cricket::MEDIA_TYPE_AUDIO, "same");
+  RenameContent(offer->description(), cricket::MEDIA_TYPE_VIDEO, "same");
+
+  std::string error;
+  EXPECT_FALSE(callee->SetRemoteDescription(std::move(offer), &error));
+  EXPECT_EQ(error,
+            "Failed to set remote offer sdp: Duplicate a=mid value 'same'.");
+}
+
 TEST_P(PeerConnectionMediaTest,
        CombinedAudioVideoBweConfigPropagatedToMediaEngine) {
   RTCConfiguration config;
index 7098470..0b1859a 100644 (file)
@@ -455,6 +455,33 @@ TEST_F(PeerConnectionRtpTestUnifiedPlan,
   EXPECT_EQ(1u, callee->observer()->remove_track_events_.size());
 }
 
+TEST_F(PeerConnectionRtpTestUnifiedPlan, ChangeMsidWhileReceiving) {
+  auto caller = CreatePeerConnection();
+  caller->AddAudioTrack("audio_track", {"stream1"});
+  auto callee = CreatePeerConnection();
+  ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
+
+  ASSERT_EQ(1u, callee->observer()->on_track_transceivers_.size());
+  auto transceiver = callee->observer()->on_track_transceivers_[0];
+  ASSERT_EQ(1u, transceiver->receiver()->streams().size());
+  EXPECT_EQ("stream1", transceiver->receiver()->streams()[0]->id());
+
+  ASSERT_TRUE(callee->SetLocalDescription(callee->CreateAnswer()));
+
+  // Change the stream ID in the offer.
+  // TODO(https://crbug.com/webrtc/10129): When RtpSenderInterface::SetStreams
+  // is supported, this can use that instead of munging the SDP.
+  auto offer = caller->CreateOffer();
+  auto contents = offer->description()->contents();
+  ASSERT_EQ(1u, contents.size());
+  auto& stream_params = contents[0].media_description()->mutable_streams();
+  ASSERT_EQ(1u, stream_params.size());
+  stream_params[0].set_stream_ids({"stream2"});
+  ASSERT_TRUE(callee->SetRemoteDescription(std::move(offer)));
+  ASSERT_EQ(1u, transceiver->receiver()->streams().size());
+  EXPECT_EQ("stream2", transceiver->receiver()->streams()[0]->id());
+}
+
 // These tests examine the state of the peer connection as a result of
 // performing SetRemoteDescription().
 
index 3a9b18e..3346012 100644 (file)
@@ -196,12 +196,8 @@ bool SessionDescription::RemoveContentByName(const std::string& name) {
   return false;
 }
 
-bool SessionDescription::AddTransportInfo(const TransportInfo& transport_info) {
-  if (GetTransportInfoByName(transport_info.content_name) != NULL) {
-    return false;
-  }
+void SessionDescription::AddTransportInfo(const TransportInfo& transport_info) {
   transport_infos_.push_back(transport_info);
-  return true;
 }
 
 bool SessionDescription::RemoveTransportInfoByName(const std::string& name) {
index 3829148..3a98dce 100644 (file)
@@ -447,8 +447,7 @@ class SessionDescription {
     transport_infos_ = transport_infos;
   }
   // Adds a TransportInfo to this description.
-  // Returns false if a TransportInfo with the same name already exists.
-  bool AddTransportInfo(const TransportInfo& transport_info);
+  void AddTransportInfo(const TransportInfo& transport_info);
   bool RemoveTransportInfoByName(const std::string& name);
 
   // Group accessors.
index 0cd6bec..0caaa12 100644 (file)
@@ -2363,20 +2363,6 @@ static C* ParseContentDescription(const std::string& message,
                                   std::vector<JsepIceCandidate*>* candidates,
                                   webrtc::SdpParseError* error) {
   C* media_desc = new C();
-  switch (media_type) {
-    case cricket::MEDIA_TYPE_AUDIO:
-      *content_name = cricket::CN_AUDIO;
-      break;
-    case cricket::MEDIA_TYPE_VIDEO:
-      *content_name = cricket::CN_VIDEO;
-      break;
-    case cricket::MEDIA_TYPE_DATA:
-      *content_name = cricket::CN_DATA;
-      break;
-    default:
-      RTC_NOTREACHED();
-      break;
-  }
   if (!ParseContent(message, media_type, mline_index, protocol, payload_types,
                     pos, content_name, bundle_only, msid_signaling, media_desc,
                     transport, candidates, error)) {
@@ -2559,14 +2545,7 @@ bool ParseMediaDescription(const std::string& message,
                                           : MediaProtocolType::kRtp,
                      content_rejected, bundle_only, content.release());
     // Create TransportInfo with the media level "ice-pwd" and "ice-ufrag".
-    TransportInfo transport_info(content_name, transport);
-
-    if (!desc->AddTransportInfo(transport_info)) {
-      rtc::StringBuilder description;
-      description << "Failed to AddTransportInfo with content name: "
-                  << content_name;
-      return ParseFailed("", description.str(), error);
-    }
+    desc->AddTransportInfo(TransportInfo(content_name, transport));
   }
 
   desc->set_msid_signaling(msid_signaling);
index 6775957..ff7f903 100644 (file)
@@ -25,6 +25,8 @@
 #include "rtc_base/messagedigest.h"
 #include "rtc_base/stringencode.h"
 #include "rtc_base/stringutils.h"
+#include "test/gmock.h"
+#include "test/gtest.h"
 
 #ifdef WEBRTC_ANDROID
 #include "pc/test/androidtestinitializer.h"
 using cricket::AudioCodec;
 using cricket::AudioContentDescription;
 using cricket::Candidate;
+using cricket::ContentGroup;
 using cricket::ContentInfo;
 using cricket::CryptoParams;
-using cricket::ContentGroup;
 using cricket::DataCodec;
 using cricket::DataContentDescription;
 using cricket::ICE_CANDIDATE_COMPONENT_RTCP;
 using cricket::ICE_CANDIDATE_COMPONENT_RTP;
 using cricket::kFecSsrcGroupSemantics;
 using cricket::LOCAL_PORT_TYPE;
+using cricket::MediaProtocolType;
 using cricket::RELAY_PORT_TYPE;
 using cricket::SessionDescription;
-using cricket::MediaProtocolType;
 using cricket::StreamParams;
 using cricket::STUN_PORT_TYPE;
 using cricket::TransportDescription;
 using cricket::TransportInfo;
 using cricket::VideoCodec;
 using cricket::VideoContentDescription;
+using ::testing::ElementsAre;
+using ::testing::Field;
 using webrtc::IceCandidateCollection;
 using webrtc::IceCandidateInterface;
 using webrtc::JsepIceCandidate;
@@ -915,10 +919,10 @@ class WebRtcSdpTest : public testing::Test {
     desc_.AddContent(kVideoContentName, MediaProtocolType::kRtp, video_desc_);
 
     // TransportInfo
-    EXPECT_TRUE(desc_.AddTransportInfo(TransportInfo(
-        kAudioContentName, TransportDescription(kUfragVoice, kPwdVoice))));
-    EXPECT_TRUE(desc_.AddTransportInfo(TransportInfo(
-        kVideoContentName, TransportDescription(kUfragVideo, kPwdVideo))));
+    desc_.AddTransportInfo(TransportInfo(
+        kAudioContentName, TransportDescription(kUfragVoice, kPwdVoice)));
+    desc_.AddTransportInfo(TransportInfo(
+        kVideoContentName, TransportDescription(kUfragVideo, kPwdVideo)));
 
     // v4 host
     int port = 1234;
@@ -1112,8 +1116,8 @@ class WebRtcSdpTest : public testing::Test {
     audio_track_2.ssrcs.push_back(kAudioTrack2Ssrc);
     audio_desc_2->AddStream(audio_track_2);
     desc_.AddContent(kAudioContentName2, MediaProtocolType::kRtp, audio_desc_2);
-    EXPECT_TRUE(desc_.AddTransportInfo(TransportInfo(
-        kAudioContentName2, TransportDescription(kUfragVoice2, kPwdVoice2))));
+    desc_.AddTransportInfo(TransportInfo(
+        kAudioContentName2, TransportDescription(kUfragVoice2, kPwdVoice2)));
     // Video track 2, in stream 2.
     VideoContentDescription* video_desc_2 = CreateVideoContentDescription();
     StreamParams video_track_2;
@@ -1123,8 +1127,8 @@ class WebRtcSdpTest : public testing::Test {
     video_track_2.ssrcs.push_back(kVideoTrack2Ssrc);
     video_desc_2->AddStream(video_track_2);
     desc_.AddContent(kVideoContentName2, MediaProtocolType::kRtp, video_desc_2);
-    EXPECT_TRUE(desc_.AddTransportInfo(TransportInfo(
-        kVideoContentName2, TransportDescription(kUfragVideo2, kPwdVideo2))));
+    desc_.AddTransportInfo(TransportInfo(
+        kVideoContentName2, TransportDescription(kUfragVideo2, kPwdVideo2)));
 
     // Video track 3, in stream 2.
     VideoContentDescription* video_desc_3 = CreateVideoContentDescription();
@@ -1135,8 +1139,8 @@ class WebRtcSdpTest : public testing::Test {
     video_track_3.ssrcs.push_back(kVideoTrack3Ssrc);
     video_desc_3->AddStream(video_track_3);
     desc_.AddContent(kVideoContentName3, MediaProtocolType::kRtp, video_desc_3);
-    EXPECT_TRUE(desc_.AddTransportInfo(TransportInfo(
-        kVideoContentName3, TransportDescription(kUfragVideo3, kPwdVideo3))));
+    desc_.AddTransportInfo(TransportInfo(
+        kVideoContentName3, TransportDescription(kUfragVideo3, kPwdVideo3)));
     desc_.set_msid_signaling(cricket::kMsidSignalingMediaSection);
 
     ASSERT_TRUE(jdesc_.Initialize(desc_.Copy(), jdesc_.session_id(),
@@ -1179,8 +1183,8 @@ class WebRtcSdpTest : public testing::Test {
     audio_track_2.ssrcs.push_back(kAudioTrack2Ssrc);
     audio_desc_2->AddStream(audio_track_2);
     desc_.AddContent(kAudioContentName2, MediaProtocolType::kRtp, audio_desc_2);
-    EXPECT_TRUE(desc_.AddTransportInfo(TransportInfo(
-        kAudioContentName2, TransportDescription(kUfragVoice2, kPwdVoice2))));
+    desc_.AddTransportInfo(TransportInfo(
+        kAudioContentName2, TransportDescription(kUfragVoice2, kPwdVoice2)));
 
     // Audio track 3 has no stream ids.
     AudioContentDescription* audio_desc_3 = CreateAudioContentDescription();
@@ -1191,8 +1195,8 @@ class WebRtcSdpTest : public testing::Test {
     audio_track_3.ssrcs.push_back(kAudioTrack3Ssrc);
     audio_desc_3->AddStream(audio_track_3);
     desc_.AddContent(kAudioContentName3, MediaProtocolType::kRtp, audio_desc_3);
-    EXPECT_TRUE(desc_.AddTransportInfo(TransportInfo(
-        kAudioContentName3, TransportDescription(kUfragVoice3, kPwdVoice3))));
+    desc_.AddTransportInfo(TransportInfo(
+        kAudioContentName3, TransportDescription(kUfragVoice3, kPwdVoice3)));
     desc_.set_msid_signaling(msid_signaling);
     ASSERT_TRUE(jdesc_.Initialize(desc_.Copy(), jdesc_.session_id(),
                                   jdesc_.session_version()));
@@ -1464,7 +1468,7 @@ class WebRtcSdpTest : public testing::Test {
     SessionDescription* desc =
         const_cast<SessionDescription*>(jdesc->description());
     desc->RemoveTransportInfoByName(content_name);
-    EXPECT_TRUE(desc->AddTransportInfo(transport_info));
+    desc->AddTransportInfo(transport_info);
     for (size_t i = 0; i < jdesc_.number_of_mediasections(); ++i) {
       const IceCandidateCollection* cc = jdesc_.candidates(i);
       for (size_t j = 0; j < cc->count(); ++j) {
@@ -1503,16 +1507,16 @@ class WebRtcSdpTest : public testing::Test {
     desc_.RemoveTransportInfoByName(kAudioContentName);
     desc_.RemoveTransportInfoByName(kVideoContentName);
     rtc::SSLFingerprint fingerprint(rtc::DIGEST_SHA_1, kIdentityDigest);
-    EXPECT_TRUE(desc_.AddTransportInfo(TransportInfo(
+    desc_.AddTransportInfo(TransportInfo(
         kAudioContentName,
         TransportDescription(std::vector<std::string>(), kUfragVoice, kPwdVoice,
                              cricket::ICEMODE_FULL,
-                             cricket::CONNECTIONROLE_NONE, &fingerprint))));
-    EXPECT_TRUE(desc_.AddTransportInfo(TransportInfo(
+                             cricket::CONNECTIONROLE_NONE, &fingerprint)));
+    desc_.AddTransportInfo(TransportInfo(
         kVideoContentName,
         TransportDescription(std::vector<std::string>(), kUfragVideo, kPwdVideo,
                              cricket::ICEMODE_FULL,
-                             cricket::CONNECTIONROLE_NONE, &fingerprint))));
+                             cricket::CONNECTIONROLE_NONE, &fingerprint)));
   }
 
   void AddExtmap(bool encrypted) {
@@ -1629,8 +1633,8 @@ class WebRtcSdpTest : public testing::Test {
     data_desc_->AddCodec(codec);
     desc_.AddContent(kDataContentName, MediaProtocolType::kSctp,
                      data.release());
-    EXPECT_TRUE(desc_.AddTransportInfo(TransportInfo(
-        kDataContentName, TransportDescription(kUfragData, kPwdData))));
+    desc_.AddTransportInfo(TransportInfo(
+        kDataContentName, TransportDescription(kUfragData, kPwdData)));
   }
 
   void AddRtpDataChannel() {
@@ -1649,8 +1653,8 @@ class WebRtcSdpTest : public testing::Test {
                      "inline:FvLcvU2P3ZWmQxgPAgcDu7Zl9vftYElFOjEzhWs5", ""));
     data_desc_->set_protocol(cricket::kMediaProtocolSavpf);
     desc_.AddContent(kDataContentName, MediaProtocolType::kRtp, data.release());
-    EXPECT_TRUE(desc_.AddTransportInfo(TransportInfo(
-        kDataContentName, TransportDescription(kUfragData, kPwdData))));
+    desc_.AddTransportInfo(TransportInfo(
+        kDataContentName, TransportDescription(kUfragData, kPwdData)));
   }
 
   bool TestDeserializeDirection(RtpTransceiverDirection direction) {
@@ -2300,9 +2304,7 @@ TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithoutRtpmap) {
   JsepSessionDescription jdesc(kDummyType);
   EXPECT_TRUE(SdpDeserialize(kSdpNoRtpmapString, &jdesc));
   cricket::AudioContentDescription* audio =
-      jdesc.description()
-          ->GetContentDescriptionByName(cricket::CN_AUDIO)
-          ->as_audio();
+      cricket::GetFirstAudioContentDescription(jdesc.description());
   AudioCodecs ref_codecs;
   // The codecs in the AudioContentDescription should be in the same order as
   // the payload types (<fmt>s) on the m= line.
@@ -2325,9 +2327,7 @@ TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithoutRtpmapButWithFmtp) {
   JsepSessionDescription jdesc(kDummyType);
   EXPECT_TRUE(SdpDeserialize(kSdpNoRtpmapString, &jdesc));
   cricket::AudioContentDescription* audio =
-      jdesc.description()
-          ->GetContentDescriptionByName(cricket::CN_AUDIO)
-          ->as_audio();
+      cricket::GetFirstAudioContentDescription(jdesc.description());
 
   cricket::AudioCodec g729 = audio->codecs()[0];
   EXPECT_EQ("G729", g729.name);
@@ -2924,15 +2924,11 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithConferenceFlag) {
 
   // Verify
   cricket::AudioContentDescription* audio =
-      jdesc.description()
-          ->GetContentDescriptionByName(cricket::CN_AUDIO)
-          ->as_audio();
+      cricket::GetFirstAudioContentDescription(jdesc.description());
   EXPECT_TRUE(audio->conference_mode());
 
   cricket::VideoContentDescription* video =
-      jdesc.description()
-          ->GetContentDescriptionByName(cricket::CN_VIDEO)
-          ->as_video();
+      cricket::GetFirstVideoContentDescription(jdesc.description());
   EXPECT_TRUE(video->conference_mode());
 }
 
@@ -2947,15 +2943,11 @@ TEST_F(WebRtcSdpTest, SerializeSdpWithConferenceFlag) {
 
   // Verify.
   cricket::AudioContentDescription* audio =
-      jdesc.description()
-          ->GetContentDescriptionByName(cricket::CN_AUDIO)
-          ->as_audio();
+      cricket::GetFirstAudioContentDescription(jdesc.description());
   EXPECT_TRUE(audio->conference_mode());
 
   cricket::VideoContentDescription* video =
-      jdesc.description()
-          ->GetContentDescriptionByName(cricket::CN_VIDEO)
-          ->as_video();
+      cricket::GetFirstVideoContentDescription(jdesc.description());
   EXPECT_TRUE(video->conference_mode());
 }
 
@@ -3909,3 +3901,19 @@ TEST_F(WebRtcSdpTest, DeserializeEmptySessionName) {
   Replace("s=-\r\n", "s= \r\n", &sdp);
   EXPECT_TRUE(SdpDeserialize(sdp, &jsep_desc));
 }
+
+// Test that the content name is empty if the media section does not have an
+// a=mid line.
+TEST_F(WebRtcSdpTest, ParseNoMid) {
+  std::string sdp = kSdpString;
+  Replace("a=mid:audio_content_name\r\n", "", &sdp);
+  Replace("a=mid:video_content_name\r\n", "", &sdp);
+
+  JsepSessionDescription output(kDummyType);
+  SdpParseError error;
+  ASSERT_TRUE(webrtc::SdpDeserialize(sdp, &output, &error));
+
+  EXPECT_THAT(output.description()->contents(),
+              ElementsAre(Field("name", &cricket::ContentInfo::name, ""),
+                          Field("name", &cricket::ContentInfo::name, "")));
+}
index f00ecf2..cbefa37 100644 (file)
 // TODO(qingsi): Refactor the default implementation given by RtcHistogram,
 // which is already sparse, and remove the boundary argument from the macro.
 #define RTC_HISTOGRAM_ENUMERATION_SPARSE(name, sample, boundary) \
-  RTC_HISTOGRAM_COMMON_BLOCK(                                    \
+  RTC_HISTOGRAM_COMMON_BLOCK_SLOW(                               \
       name, sample,                                              \
       webrtc::metrics::SparseHistogramFactoryGetEnumeration(name, boundary))
 
 // Histogram for enumerators (evenly spaced buckets).
 // |boundary| should be above the max enumerator sample.
 #define RTC_HISTOGRAM_ENUMERATION(name, sample, boundary) \
-  RTC_HISTOGRAM_COMMON_BLOCK(                             \
+  RTC_HISTOGRAM_COMMON_BLOCK_SLOW(                        \
       name, sample,                                       \
       webrtc::metrics::HistogramFactoryGetEnumeration(name, boundary))
 
     }                                                                      \
   } while (0)
 
-// Deprecated.
 // The histogram is constructed/found for each call.
 // May be used for histograms with infrequent updates.`
 #define RTC_HISTOGRAM_COMMON_BLOCK_SLOW(name, sample, factory_get_invocation) \
index b53d730..28fb690 100644 (file)
@@ -181,6 +181,7 @@ rtc_source_set("video_stream_encoder_impl") {
     "../common_video:common_video",
     "../modules/video_coding",
     "../modules/video_coding:video_coding_utility",
+    "../modules/video_coding:webrtc_vp9_helpers",
     "../rtc_base:checks",
     "../rtc_base:criticalsection",
     "../rtc_base:logging",