Revert "Assign supported codecs per section, not globally."

This reverts commit c92fc76.

Reason for revert: Gerritt submitted patch set #10 not #12 - this broke

Original change's description:
> Assign supported codecs per section, not globally.
>
> This uses the PayloadTypeSuggester for PT disambiguation,
> ensuring that conficts are managed across the transport.
>
> Bug: webrtc:360058654
> Change-Id: Ic8f0569d0d96d97c13765169903929ec8a110104
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/383400
> Commit-Queue: Harald Alvestrand <[email protected]>
> Reviewed-by: Evan Shrubsole <[email protected]>
> Reviewed-by: Henrik Boström <[email protected]>
> Cr-Commit-Position: refs/heads/main@{#44284}

Bug: webrtc:360058654
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Change-Id: I16d0d92fb9a1ecc65eda060f81d41faefcca1e4c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/384000
Commit-Queue: Harald Alvestrand <[email protected]>
Reviewed-by: Danil Chapovalov <[email protected]>
Bot-Commit: [email protected] <[email protected]>
Reviewed-by: Mirko Bonadei <[email protected]>
Cr-Commit-Position: refs/heads/main@{#44285}

Author: Harald Alvestrand

pc/codec_vendor.cc

@@ -166,94 +166,6 @@ const Codec* GetAssociatedCodecForRed(const CodecList& codec_list,
 // Adds all codecs from `reference_codecs` to `offered_codecs` that don't
 // already exist in `offered_codecs` and ensure the payload types don't
 // collide.
-RTCError MergeCodecs(const CodecList& reference_codecs,
-                     const std::string& mid,
-                     CodecList& offered_codecs,
-                     PayloadTypeSuggester& pt_suggester) {
-  // Add all new codecs that are not RTX/RED codecs.
-  // The two-pass splitting of the loops means preferring payload types
-  // of actual codecs with respect to collisions.
-  for (const Codec& reference_codec : reference_codecs) {
-    if (reference_codec.GetResiliencyType() != Codec::ResiliencyType::kRtx &&
-        reference_codec.GetResiliencyType() != Codec::ResiliencyType::kRed &&
-        !FindMatchingCodec(reference_codecs, offered_codecs, reference_codec)) {
-      Codec codec = reference_codec;
-      RTCErrorOr<webrtc::PayloadType> suggestion =
-          pt_suggester.SuggestPayloadType(mid, codec);
-      if (!suggestion.ok()) {
-        return suggestion.MoveError();
-      }
-      codec.id = suggestion.value();
-      offered_codecs.push_back(codec);
-    }
-  }
-
-  // Add all new RTX or RED codecs.
-  for (const Codec& reference_codec : reference_codecs) {
-    if (reference_codec.GetResiliencyType() == Codec::ResiliencyType::kRtx &&
-        !FindMatchingCodec(reference_codecs, offered_codecs, reference_codec)) {
-      Codec rtx_codec = reference_codec;
-      const Codec* associated_codec =
-          GetAssociatedCodecForRtx(reference_codecs, rtx_codec);
-      if (!associated_codec) {
-        continue;
-      }
-      // Find a codec in the offered list that matches the reference codec.
-      // Its payload type may be different than the reference codec.
-      std::optional<Codec> matching_codec = FindMatchingCodec(
-          reference_codecs, offered_codecs, *associated_codec);
-      if (!matching_codec) {
-        RTC_LOG(LS_WARNING)
-            << "Couldn't find matching " << associated_codec->name << " codec.";
-        continue;
-      }
-
-      rtx_codec.params[kCodecParamAssociatedPayloadType] =
-          rtc::ToString(matching_codec->id);
-      RTCErrorOr<webrtc::PayloadType> suggestion =
-          pt_suggester.SuggestPayloadType(mid, rtx_codec);
-      if (!suggestion.ok()) {
-        return suggestion.MoveError();
-      }
-      rtx_codec.id = suggestion.value();
-      offered_codecs.push_back(rtx_codec);
-    } else if (reference_codec.GetResiliencyType() ==
-                   Codec::ResiliencyType::kRed &&
-               !FindMatchingCodec(reference_codecs, offered_codecs,
-                                  reference_codec)) {
-      Codec red_codec = reference_codec;
-      const Codec* associated_codec =
-          GetAssociatedCodecForRed(reference_codecs, red_codec);
-      if (associated_codec) {
-        std::optional<Codec> matching_codec = FindMatchingCodec(
-            reference_codecs, offered_codecs, *associated_codec);
-        if (!matching_codec) {
-          RTC_LOG(LS_WARNING) << "Couldn't find matching "
-                              << associated_codec->name << " codec.";
-          continue;
-        }
-
-        red_codec.params[kCodecParamNotInNameValueFormat] =
-            rtc::ToString(matching_codec->id) + "/" +
-            rtc::ToString(matching_codec->id);
-      }
-      RTCErrorOr<webrtc::PayloadType> suggestion =
-          pt_suggester.SuggestPayloadType(mid, red_codec);
-      if (!suggestion.ok()) {
-        return suggestion.MoveError();
-      }
-      red_codec.id = suggestion.value();
-      offered_codecs.push_back(red_codec);
-    }
-  }
-  offered_codecs.CheckConsistency();
-  return RTCError::OK();
-}
-
-// Adds all codecs from `reference_codecs` to `offered_codecs` that don't
-// already exist in `offered_codecs` and ensure the payload types don't
-// collide.
-// OLD VERSION - uses UsedPayloadTypes
 void MergeCodecs(const CodecList& reference_codecs,
                  CodecList& offered_codecs,
                  UsedPayloadTypes* used_pltypes) {
@@ -405,6 +317,43 @@ CodecList MatchCodecPreference(
   return filtered_codecs;
 }
 
+// Compute the union of `codecs1` and `codecs2`.
+CodecList ComputeCodecsUnion(const CodecList codecs1, const CodecList codecs2) {
+  CodecList all_codecs;
+  UsedPayloadTypes used_payload_types;
+  for (const Codec& codec : codecs1) {
+    Codec codec_mutable = codec;
+    used_payload_types.FindAndSetIdUsed(&codec_mutable);
+    all_codecs.push_back(codec_mutable);
+  }
+
+  // Use MergeCodecs to merge the second half of our list as it already checks
+  // and fixes problems with duplicate payload types.
+  MergeCodecs(codecs2, all_codecs, &used_payload_types);
+
+  return all_codecs;
+}
+
+RTCError MergeCodecsFromDescription(
+    const std::vector<const webrtc::ContentInfo*>& current_active_contents,
+    CodecList& audio_codecs,
+    CodecList& video_codecs,
+    UsedPayloadTypes* used_pltypes) {
+  for (const ContentInfo* content : current_active_contents) {
+    RTCErrorOr<CodecList> checked_codec_list =
+        CodecList::Create(content->media_description()->codecs());
+    if (!checked_codec_list.ok()) {
+      RTC_LOG(LS_ERROR) << checked_codec_list.error();
+    }
+    if (IsMediaContentOfType(content, webrtc::MediaType::AUDIO)) {
+      MergeCodecs(checked_codec_list.value(), audio_codecs, used_pltypes);
+    } else if (IsMediaContentOfType(content, webrtc::MediaType::VIDEO)) {
+      MergeCodecs(checked_codec_list.value(), video_codecs, used_pltypes);
+    }
+  }
+  return RTCError::OK();
+}
+
 void NegotiatePacketization(const Codec& local_codec,
                             const Codec& remote_codec,
                             Codec* negotiated_codec) {
@@ -624,25 +573,8 @@ webrtc::RTCErrorOr<std::vector<Codec>> CodecVendor::GetNegotiatedCodecsForOffer(
     const MediaDescriptionOptions& media_description_options,
     const MediaSessionOptions& session_options,
     const webrtc::ContentInfo* current_content,
-    PayloadTypeSuggester& pt_suggester) {
-  CodecList codecs;
-  std::string mid = media_description_options.mid;
-  // If current content exists and is not being recycled, use its codecs.
-  if (current_content && current_content->mid() == mid) {
-    RTCErrorOr<CodecList> checked_codec_list =
-        CodecList::Create(current_content->media_description()->codecs());
-    if (!checked_codec_list.ok()) {
-      return checked_codec_list.MoveError();
-    }
-    // Use MergeCodecs in order to handle PT clashes.
-    MergeCodecs(checked_codec_list.value(), mid, codecs, pt_suggester);
-  }
-  // Add our codecs that are not in the current description.
-  if (media_description_options.type == webrtc::MediaType::AUDIO) {
-    MergeCodecs(all_audio_codecs(), mid, codecs, pt_suggester);
-  } else {
-    MergeCodecs(all_video_codecs(), mid, codecs, pt_suggester);
-  }
+    PayloadTypeSuggester& pt_suggester,
+    const CodecList& codecs) {
   CodecList filtered_codecs;
   CodecList supported_codecs =
       media_description_options.type == webrtc::MediaType::AUDIO
@@ -743,7 +675,7 @@ webrtc::RTCErrorOr<std::vector<Codec>> CodecVendor::GetNegotiatedCodecsForOffer(
     }
     filtered_codecs = codecs_from_arg.MoveValue();
   }
-  AssignCodecIdsAndLinkRed(&pt_suggester, mid,
+  AssignCodecIdsAndLinkRed(&pt_suggester, media_description_options.mid,
                            filtered_codecs.writable_codecs());
   return filtered_codecs.codecs();
 }
@@ -755,23 +687,8 @@ webrtc::RTCErrorOr<Codecs> CodecVendor::GetNegotiatedCodecsForAnswer(
     webrtc::RtpTransceiverDirection answer_rtd,
     const webrtc::ContentInfo* current_content,
     const std::vector<Codec> codecs_from_offer,
-    PayloadTypeSuggester& pt_suggester) {
-  CodecList codecs;
-  std::string mid = media_description_options.mid;
-  if (current_content && current_content->mid() == mid) {
-    RTCErrorOr<CodecList> checked_codec_list =
-        CodecList::Create(current_content->media_description()->codecs());
-    if (!checked_codec_list.ok()) {
-      return checked_codec_list.MoveError();
-    }
-    MergeCodecs(checked_codec_list.value(), mid, codecs, pt_suggester);
-  }
-  // Add all our supported codecs
-  if (media_description_options.type == webrtc::MediaType::AUDIO) {
-    MergeCodecs(all_audio_codecs(), mid, codecs, pt_suggester);
-  } else {
-    MergeCodecs(all_video_codecs(), mid, codecs, pt_suggester);
-  }
+    PayloadTypeSuggester& pt_suggester,
+    const CodecList& codecs) {
   CodecList filtered_codecs;
   CodecList negotiated_codecs;
   if (media_description_options.codecs_to_include.empty()) {
@@ -805,8 +722,20 @@ webrtc::RTCErrorOr<Codecs> CodecVendor::GetNegotiatedCodecsForAnswer(
           }
         }
       }
-      // Merge other_codecs into filtered_codecs, resolving PT conflicts.
-      MergeCodecs(supported_codecs, mid, filtered_codecs, pt_suggester);
+      // Add other supported codecs.
+      CodecList other_codecs;
+      for (const Codec& codec : supported_codecs) {
+        if (FindMatchingCodec(supported_codecs, codecs, codec) &&
+            !FindMatchingCodec(supported_codecs, filtered_codecs, codec)) {
+          // We should use the local codec with local parameters and the codec
+          // id would be correctly mapped in `NegotiateCodecs`.
+          other_codecs.push_back(codec);
+        }
+      }
+
+      // Use ComputeCodecsUnion to avoid having duplicate payload IDs.
+      // This is a no-op for audio until RTX is added.
+      filtered_codecs = ComputeCodecsUnion(filtered_codecs, other_codecs);
     }
 
     if (media_description_options.type == webrtc::MediaType::AUDIO &&
@@ -892,6 +821,107 @@ void CodecVendor::set_video_codecs(const CodecList& send_codecs,
   video_send_codecs_.set_codecs(send_codecs);
   video_recv_codecs_.set_codecs(recv_codecs);
 }
+// Getting codecs for an offer involves these steps:
+//
+// 1. Construct payload type -> codec mappings for current description.
+// 2. Add any reference codecs that weren't already present
+// 3. For each individual media description (m= section), filter codecs based
+//    on the directional attribute (happens in another method).
+RTCError CodecVendor::GetCodecsForOffer(
+    const std::vector<const webrtc::ContentInfo*>& current_active_contents,
+    CodecList& audio_codecs,
+    CodecList& video_codecs) const {
+  // First - get all codecs from the current description if the media type
+  // is used. Add them to `used_pltypes` so the payload type is not reused if a
+  // new media type is added.
+  UsedPayloadTypes used_pltypes;
+  auto error = MergeCodecsFromDescription(current_active_contents, audio_codecs,
+                                          video_codecs, &used_pltypes);
+  if (!error.ok()) {
+    return error;
+  }
+  // Add our codecs that are not in the current description.
+  MergeCodecs(all_audio_codecs(), audio_codecs, &used_pltypes);
+  MergeCodecs(all_video_codecs(), video_codecs, &used_pltypes);
+  return RTCError::OK();
+}
+
+// Getting codecs for an answer involves these steps:
+//
+// 1. Construct payload type -> codec mappings for current description.
+// 2. Add any codecs from the offer that weren't already present.
+// 3. Add any remaining codecs that weren't already present.
+// 4. For each individual media description (m= section), filter codecs based
+//    on the directional attribute (happens in another method).
+RTCError CodecVendor::GetCodecsForAnswer(
+    const std::vector<const webrtc::ContentInfo*>& current_active_contents,
+    const webrtc::SessionDescription& remote_offer,
+    CodecList& audio_codecs,
+    CodecList& video_codecs) const {
+  // First - get all codecs from the current description if the media type
+  // is used. Add them to `used_pltypes` so the payload type is not reused if a
+  // new media type is added.
+  UsedPayloadTypes used_pltypes;
+  RTCError error = MergeCodecsFromDescription(
+      current_active_contents, audio_codecs, video_codecs, &used_pltypes);
+  if (!error.ok()) {
+    return error;
+  }
+  // Second - filter out codecs that we don't support at all and should ignore.
+  CodecList filtered_offered_audio_codecs;
+  CodecList filtered_offered_video_codecs;
+  for (const ContentInfo& content : remote_offer.contents()) {
+    RTCErrorOr<CodecList> offered_codecs =
+        CodecList::Create(content.media_description()->codecs());
+    if (!offered_codecs.ok()) {
+      return offered_codecs.MoveError();
+    }
+    if (IsMediaContentOfType(&content, webrtc::MediaType::AUDIO)) {
+      for (const Codec& offered_audio_codec : offered_codecs.value()) {
+        if (!FindMatchingCodec(offered_codecs.value(),
+                               filtered_offered_audio_codecs,
+                               offered_audio_codec) &&
+            FindMatchingCodec(offered_codecs.value(), all_audio_codecs(),
+                              offered_audio_codec)) {
+          filtered_offered_audio_codecs.push_back(offered_audio_codec);
+        }
+      }
+    } else if (IsMediaContentOfType(&content, webrtc::MediaType::VIDEO)) {
+      std::vector<Codec> pending_rtx_codecs;
+      for (const Codec& offered_video_codec : offered_codecs.value()) {
+        if (!FindMatchingCodec(offered_codecs.value(),
+                               filtered_offered_video_codecs,
+                               offered_video_codec) &&
+            FindMatchingCodec(offered_codecs.value(), all_video_codecs(),
+                              offered_video_codec)) {
+          // Special case: If it's an RTX codec, and the APT points to
+          // a codec that is not yet in the codec list, put it aside.
+          if (offered_video_codec.GetResiliencyType() ==
+                  Codec::ResiliencyType::kRtx &&
+              !GetAssociatedCodecForRtx(filtered_offered_video_codecs,
+                                        offered_video_codec)) {
+            pending_rtx_codecs.push_back(offered_video_codec);
+            continue;
+          }
+          filtered_offered_video_codecs.push_back(offered_video_codec);
+        }
+      }
+      // If the associated codec showed up later in the codec list,
+      // append the corresponding RTX codec.
+      for (const Codec& codec : pending_rtx_codecs) {
+        if (GetAssociatedCodecForRtx(filtered_offered_video_codecs, codec)) {
+          filtered_offered_video_codecs.push_back(codec);
+        }
+      }
+    }
+  }
+
+  // Add codecs that are not in the current description but were in
+  // `remote_offer`.
+  MergeCodecs(filtered_offered_audio_codecs, audio_codecs, &used_pltypes);
+  MergeCodecs(filtered_offered_video_codecs, video_codecs, &used_pltypes);
+  return RTCError::OK();
+}
 
 CodecList CodecVendor::GetVideoCodecsForOffer(
     const RtpTransceiverDirection& direction) const {
@@ -964,19 +994,9 @@ CodecList CodecVendor::GetAudioCodecsForAnswer(
 }
 
 CodecList CodecVendor::all_video_codecs() const {
-  CodecList all_codecs;
-  UsedPayloadTypes used_payload_types;
-  for (const Codec& codec : video_recv_codecs_.codecs()) {
-    Codec codec_mutable = codec;
-    used_payload_types.FindAndSetIdUsed(&codec_mutable);
-    all_codecs.push_back(codec_mutable);
-  }
-
-  // Use MergeCodecs to merge the second half of our list as it already checks
-  // and fixes problems with duplicate payload types.
-  MergeCodecs(video_send_codecs_.codecs(), all_codecs, &used_payload_types);
-
-  return all_codecs;
+  // Use ComputeCodecsUnion to avoid having duplicate payload IDs
+  return ComputeCodecsUnion(video_recv_codecs_.codecs(),
+                            video_send_codecs_.codecs());
 }
 
 CodecList CodecVendor::all_audio_codecs() const {

pc/codec_vendor.h

@@ -47,11 +47,22 @@ class CodecVendor {
               const webrtc::FieldTrialsView& trials);
 
  public:
+  webrtc::RTCError GetCodecsForOffer(
+      const std::vector<const webrtc::ContentInfo*>& current_active_contents,
+      CodecList& audio_codecs,
+      CodecList& video_codecs) const;
+  webrtc::RTCError GetCodecsForAnswer(
+      const std::vector<const webrtc::ContentInfo*>& current_active_contents,
+      const webrtc::SessionDescription& remote_offer,
+      CodecList& audio_codecs,
+      CodecList& video_codecs) const;
+
   webrtc::RTCErrorOr<std::vector<Codec>> GetNegotiatedCodecsForOffer(
       const MediaDescriptionOptions& media_description_options,
       const MediaSessionOptions& session_options,
       const webrtc::ContentInfo* current_content,
-      webrtc::PayloadTypeSuggester& pt_suggester);
+      webrtc::PayloadTypeSuggester& pt_suggester,
+      const CodecList& codecs);
 
   webrtc::RTCErrorOr<Codecs> GetNegotiatedCodecsForAnswer(
       const MediaDescriptionOptions& media_description_options,
@@ -60,7 +71,8 @@ class CodecVendor {
       webrtc::RtpTransceiverDirection answer_rtd,
       const webrtc::ContentInfo* current_content,
       std::vector<Codec> codecs_from_offer,
-      webrtc::PayloadTypeSuggester& pt_suggester);
+      webrtc::PayloadTypeSuggester& pt_suggester,
+      const CodecList& codecs);
 
   // Functions exposed for testing
   void set_audio_codecs(const CodecList& send_codecs,

pc/jsep_transport_controller_unittest.cc

@@ -18,7 +18,6 @@
 #include <utility>
 #include <vector>
 
-#include "api/candidate.h"
 #include "api/crypto/crypto_options.h"
 #include "api/dtls_transport_interface.h"
 #include "api/environment/environment.h"