Let SDP operations always look at all simulcast layers

This simplifies the logic of what simulcast layers to signal, and avoids
situations where the upper layers get confused about which layers exist.

Bug: chromium:1350245
Change-Id: I9edeb93cbb30e872c4d3f3429a85a1fccf17996a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/272902
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37905}

pc/rtp_sender.cc

@@ -169,6 +169,20 @@ RtpParameters RtpSenderBase::GetParametersInternal() const {
   });
 }
 
+RtpParameters RtpSenderBase::GetParametersInternalWithAllLayers() const {
+  RTC_DCHECK_RUN_ON(signaling_thread_);
+  if (stopped_) {
+    return RtpParameters();
+  }
+  if (!media_channel_ || !ssrc_) {
+    return init_parameters_;
+  }
+  return worker_thread_->Invoke<RtpParameters>(RTC_FROM_HERE, [&] {
+    RtpParameters result = media_channel_->GetRtpSendParameters(ssrc_);
+    return result;
+  });
+}
+
 RtpParameters RtpSenderBase::GetParameters() const {
   RTC_DCHECK_RUN_ON(signaling_thread_);
   RtpParameters result = GetParametersInternal();
@@ -207,6 +221,30 @@ RTCError RtpSenderBase::SetParametersInternal(const RtpParameters& parameters) {
   });
 }
 
+RTCError RtpSenderBase::SetParametersInternalWithAllLayers(
+    const RtpParameters& parameters) {
+  RTC_DCHECK_RUN_ON(signaling_thread_);
+  RTC_DCHECK(!stopped_);
+
+  if (UnimplementedRtpParameterHasValue(parameters)) {
+    LOG_AND_RETURN_ERROR(
+        RTCErrorType::UNSUPPORTED_PARAMETER,
+        "Attempted to set an unimplemented parameter of RtpParameters.");
+  }
+  if (!media_channel_ || !ssrc_) {
+    auto result = cricket::CheckRtpParametersInvalidModificationAndValues(
+        init_parameters_, parameters);
+    if (result.ok()) {
+      init_parameters_ = parameters;
+    }
+    return result;
+  }
+  return worker_thread_->Invoke<RTCError>(RTC_FROM_HERE, [&] {
+    RtpParameters rtp_parameters = parameters;
+    return media_channel_->SetRtpSendParameters(ssrc_, rtp_parameters);
+  });
+}
+
 RTCError RtpSenderBase::SetParameters(const RtpParameters& parameters) {
   RTC_DCHECK_RUN_ON(signaling_thread_);
   TRACE_EVENT0("webrtc", "RtpSenderBase::SetParameters");
@@ -377,7 +415,7 @@ RTCError RtpSenderBase::DisableEncodingLayers(
   }
 
   // Check that all the specified layers exist and disable them in the channel.
-  RtpParameters parameters = GetParametersInternal();
+  RtpParameters parameters = GetParametersInternalWithAllLayers();
   for (const std::string& rid : rids) {
     if (absl::c_none_of(parameters.encodings,
                         [&rid](const RtpEncodingParameters& encoding) {
@@ -402,7 +440,7 @@ RTCError RtpSenderBase::DisableEncodingLayers(
         [&encoding](const std::string& rid) { return encoding.rid == rid; });
   }
 
-  RTCError result = SetParametersInternal(parameters);
+  RTCError result = SetParametersInternalWithAllLayers(parameters);
   if (result.ok()) {
     disabled_rids_.insert(disabled_rids_.end(), rids.begin(), rids.end());
     // Invalidate any transaction upon success.

pc/rtp_sender.h

@@ -75,6 +75,13 @@ class RtpSenderInternal : public RtpSenderInterface {
   virtual RtpParameters GetParametersInternal() const = 0;
   virtual RTCError SetParametersInternal(const RtpParameters& parameters) = 0;
 
+  // GetParameters and SetParameters will remove deactivated simulcast layers
+  // and restore them on SetParameters. This is probably a Bad Idea, but we
+  // do not know who depends on this behavior
+  virtual RtpParameters GetParametersInternalWithAllLayers() const = 0;
+  virtual RTCError SetParametersInternalWithAllLayers(
+      const RtpParameters& parameters) = 0;
+
   // Returns an ID that changes every time SetTrack() is called, but
   // otherwise remains constant. Used to generate IDs for stats.
   // The special value zero means that no track is attached.
@@ -118,6 +125,9 @@ class RtpSenderBase : public RtpSenderInternal, public ObserverInterface {
   // Allow access to get/set parameters without invalidating transaction id.
   RtpParameters GetParametersInternal() const override;
   RTCError SetParametersInternal(const RtpParameters& parameters) override;
+  RtpParameters GetParametersInternalWithAllLayers() const override;
+  RTCError SetParametersInternalWithAllLayers(
+      const RtpParameters& parameters) override;
 
   // Used to set the SSRC of the sender, once a local description has been set.
   // If `ssrc` is 0, this indiates that the sender should disconnect from the

pc/sdp_offer_answer.cc

@@ -479,7 +479,7 @@ RTCError UpdateSimulcastLayerStatusInSender(
     const std::vector<SimulcastLayer>& layers,
     rtc::scoped_refptr<RtpSenderInternal> sender) {
   RTC_DCHECK(sender);
-  RtpParameters parameters = sender->GetParametersInternal();
+  RtpParameters parameters = sender->GetParametersInternalWithAllLayers();
   std::vector<std::string> disabled_layers;
 
   // The simulcast envelope cannot be changed, only the status of the streams.
@@ -498,7 +498,7 @@ RTCError UpdateSimulcastLayerStatusInSender(
     encoding.active = !iter->is_paused;
   }
 
-  RTCError result = sender->SetParametersInternal(parameters);
+  RTCError result = sender->SetParametersInternalWithAllLayers(parameters);
   if (result.ok()) {
     result = sender->DisableEncodingLayers(disabled_layers);
   }
@@ -524,7 +524,7 @@ bool SimulcastIsRejected(const ContentInfo* local_content,
 RTCError DisableSimulcastInSender(
     rtc::scoped_refptr<RtpSenderInternal> sender) {
   RTC_DCHECK(sender);
-  RtpParameters parameters = sender->GetParametersInternal();
+  RtpParameters parameters = sender->GetParametersInternalWithAllLayers();
   if (parameters.encodings.size() <= 1) {
     return RTCError::OK();
   }
@@ -610,7 +610,7 @@ cricket::MediaDescriptionOptions GetMediaDescriptionOptionsForTransceiver(
   // The following sets up RIDs and Simulcast.
   // RIDs are included if Simulcast is requested or if any RID was specified.
   RtpParameters send_parameters =
-      transceiver->sender_internal()->GetParametersInternal();
+      transceiver->sender_internal()->GetParametersInternalWithAllLayers();
   bool has_rids = std::any_of(send_parameters.encodings.begin(),
                               send_parameters.encodings.end(),
                               [](const RtpEncodingParameters& encoding) {

pc/test/mock_rtp_sender_internal.h

@@ -46,11 +46,19 @@ class MockRtpSenderInternal : public RtpSenderInternal {
               (override));
   MOCK_METHOD(RtpParameters, GetParameters, (), (const, override));
   MOCK_METHOD(RtpParameters, GetParametersInternal, (), (const, override));
+  MOCK_METHOD(RtpParameters,
+              GetParametersInternalWithAllLayers,
+              (),
+              (const, override));
   MOCK_METHOD(RTCError, SetParameters, (const RtpParameters&), (override));
   MOCK_METHOD(RTCError,
               SetParametersInternal,
               (const RtpParameters&),
               (override));
+  MOCK_METHOD(RTCError,
+              SetParametersInternalWithAllLayers,
+              (const RtpParameters&),
+              (override));
   MOCK_METHOD(rtc::scoped_refptr<DtmfSenderInterface>,
               GetDtmfSender,
               (),