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

This reverts commit 374b892.

Reason for revert: Racing CLs sorted out

Original change's description:
> 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}

Bug: webrtc:360058654
Change-Id: I80b59aab396abbc0afa60dab3516f2a22e822a5d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/383981
Reviewed-by: Mirko Bonadei <[email protected]>
Commit-Queue: Mirko Bonadei <[email protected]>
Commit-Queue: Harald Alvestrand <[email protected]>
Reviewed-by: Danil Chapovalov <[email protected]>
Cr-Commit-Position: refs/heads/main@{#44288}

Author: Harald Alvestrand

pc/BUILD.gn

@@ -428,6 +428,7 @@ rtc_library("codec_vendor") {
     "../rtc_base:unique_id_generator",
     "//third_party/abseil-cpp/absl/algorithm:container",
     "//third_party/abseil-cpp/absl/strings",
+    "//third_party/abseil-cpp/absl/strings:str_format",
     "//third_party/abseil-cpp/absl/strings:string_view",
   ]
 }

pc/codec_vendor.cc

@@ -47,22 +47,11 @@ 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,
-      const CodecList& codecs);
+      webrtc::PayloadTypeSuggester& pt_suggester);
 
   webrtc::RTCErrorOr<Codecs> GetNegotiatedCodecsForAnswer(
       const MediaDescriptionOptions& media_description_options,
@@ -71,8 +60,7 @@ class CodecVendor {
       webrtc::RtpTransceiverDirection answer_rtd,
       const webrtc::ContentInfo* current_content,
       std::vector<Codec> codecs_from_offer,
-      webrtc::PayloadTypeSuggester& pt_suggester,
-      const CodecList& codecs);
+      webrtc::PayloadTypeSuggester& pt_suggester);
 
   // Functions exposed for testing
   void set_audio_codecs(const CodecList& send_codecs,

pc/codec_vendor.h

@@ -18,6 +18,7 @@
 #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"

pc/jsep_transport_controller_unittest.cc

@@ -29,7 +29,6 @@
 #include "api/rtp_transceiver_direction.h"
 #include "api/sctp_transport_interface.h"
 #include "media/base/codec.h"
-#include "media/base/codec_list.h"
 #include "media/base/media_constants.h"
 #include "media/base/media_engine.h"
 #include "media/base/rid_description.h"
@@ -734,16 +733,6 @@ MediaSessionDescriptionFactory::CreateOfferOrError(
   cricket::StreamParamsVec current_streams =
       GetCurrentStreamParams(current_active_contents);
 
-  cricket::CodecList offer_audio_codecs;
-  cricket::CodecList offer_video_codecs;
-
-  // TODO: issues.webrtc.org/360058654 - Get codecs when we know the right mid.
-  RTCError error = codec_lookup_helper_->CodecVendor("")->GetCodecsForOffer(
-      current_active_contents, offer_audio_codecs, offer_video_codecs);
-  if (!error.ok()) {
-    return error;
-  }
-
   AudioVideoRtpHeaderExtensions extensions_with_ids =
       GetOfferedRtpHeaderExtensionsWithIds(
           current_active_contents, session_options.offer_extmap_allow_mixed,
@@ -762,6 +751,7 @@ MediaSessionDescriptionFactory::CreateOfferOrError(
       current_content = &current_description->contents()[msection_index];
       // Media type must match unless this media section is being recycled.
     }
+    RTCError error;
     switch (media_description_options.type) {
       case webrtc::MediaType::AUDIO:
       case webrtc::MediaType::VIDEO:
@@ -771,9 +761,6 @@ MediaSessionDescriptionFactory::CreateOfferOrError(
             media_description_options.type == webrtc::MediaType::AUDIO
                 ? extensions_with_ids.audio
                 : extensions_with_ids.video,
-            media_description_options.type == webrtc::MediaType::AUDIO
-                ? offer_audio_codecs
-                : offer_video_codecs,
             &current_streams, offer.get(), &ice_credentials);
         break;
       case webrtc::MediaType::DATA:
@@ -882,22 +869,6 @@ MediaSessionDescriptionFactory::CreateAnswerOrError(
     }
   }
 
-  // Get list of all possible codecs that respects existing payload type
-  // mappings and uses a single payload type space.
-  //
-  // Note that these lists may be further filtered for each m= section; this
-  // step is done just to establish the payload type mappings shared by all
-  // sections.
-  cricket::CodecList answer_audio_codecs;
-  cricket::CodecList answer_video_codecs;
-  // TODO: issues.webrtc.org/360058654 - do this when we have the MID.
-  RTCError error = codec_lookup_helper_->CodecVendor("")->GetCodecsForAnswer(
-      current_active_contents, *offer, answer_audio_codecs,
-      answer_video_codecs);
-  if (!error.ok()) {
-    return error;
-  }
-
   auto answer = std::make_unique<SessionDescription>();
 
   // If the offer supports BUNDLE, and we want to use it too, create a BUNDLE
@@ -958,15 +929,13 @@ MediaSessionDescriptionFactory::CreateAnswerOrError(
     cricket::RtpHeaderExtensions header_extensions =
         RtpHeaderExtensionsFromCapabilities(
             UnstoppedRtpHeaderExtensionCapabilities(header_extensions_in));
+    RTCError error;
     switch (media_description_options.type) {
       case webrtc::MediaType::AUDIO:
       case webrtc::MediaType::VIDEO:
         error = AddRtpContentForAnswer(
             media_description_options, session_options, offer_content, offer,
             current_content, current_description, bundle_transport,
-            media_description_options.type == webrtc::MediaType::AUDIO
-                ? answer_audio_codecs
-                : answer_video_codecs,
             header_extensions, &current_streams, answer.get(),
             &ice_credentials);
         break;
@@ -1195,7 +1164,6 @@ RTCError MediaSessionDescriptionFactory::AddRtpContentForOffer(
     const ContentInfo* current_content,
     const SessionDescription* current_description,
     const cricket::RtpHeaderExtensions& header_extensions,
-    const cricket::CodecList& codecs,
     cricket::StreamParamsVec* current_streams,
     SessionDescription* session_description,
     cricket::IceCredentialsIterator* ice_credentials) const {
@@ -1207,7 +1175,7 @@ RTCError MediaSessionDescriptionFactory::AddRtpContentForOffer(
   RTCErrorOr<std::vector<cricket::Codec>> error_or_filtered_codecs =
       codec_lookup_helper_->CodecVendor(mid)->GetNegotiatedCodecsForOffer(
           media_description_options, session_options, current_content,
-          *codec_lookup_helper_->PayloadTypeSuggester(), codecs);
+          *codec_lookup_helper_->PayloadTypeSuggester());
   if (!error_or_filtered_codecs.ok()) {
     return error_or_filtered_codecs.MoveError();
   }
@@ -1327,7 +1295,6 @@ RTCError MediaSessionDescriptionFactory::AddRtpContentForAnswer(
     const ContentInfo* current_content,
     const SessionDescription* current_description,
     const cricket::TransportInfo* bundle_transport,
-    const cricket::CodecList& codecs,
     const cricket::RtpHeaderExtensions& header_extensions,
     cricket::StreamParamsVec* current_streams,
     SessionDescription* answer,
@@ -1371,7 +1338,7 @@ RTCError MediaSessionDescriptionFactory::AddRtpContentForAnswer(
           ->GetNegotiatedCodecsForAnswer(
               media_description_options, session_options, offer_rtd, answer_rtd,
               current_content, offer_content_description->codecs(),
-              *codec_lookup_helper_->PayloadTypeSuggester(), codecs);
+              *codec_lookup_helper_->PayloadTypeSuggester());
   if (!error_or_filtered_codecs.ok()) {
     return error_or_filtered_codecs.MoveError();
   }

pc/media_session.cc

@@ -19,7 +19,6 @@
 
 #include "api/media_types.h"
 #include "api/rtc_error.h"
-#include "media/base/codec_list.h"
 #include "media/base/stream_params.h"
 #include "p2p/base/ice_credentials_iterator.h"
 #include "p2p/base/transport_description.h"
@@ -118,7 +117,6 @@ class MediaSessionDescriptionFactory {
       const ContentInfo* current_content,
       const SessionDescription* current_description,
       const cricket::RtpHeaderExtensions& header_extensions,
-      const cricket::CodecList& codecs,
       cricket::StreamParamsVec* current_streams,
       SessionDescription* desc,
       cricket::IceCredentialsIterator* ice_credentials) const;
@@ -148,7 +146,6 @@ class MediaSessionDescriptionFactory {
       const ContentInfo* current_content,
       const SessionDescription* current_description,
       const cricket::TransportInfo* bundle_transport,
-      const cricket::CodecList& codecs,
       const cricket::RtpHeaderExtensions& header_extensions,
       cricket::StreamParamsVec* current_streams,
       SessionDescription* answer,

pc/media_session.h

@@ -74,6 +74,7 @@ using ::testing::ElementsAre;
 using ::testing::ElementsAreArray;
 using ::testing::Eq;
 using ::testing::Field;
+using ::testing::Gt;
 using ::testing::IsEmpty;
 using ::testing::Not;
 using ::testing::Pointwise;
@@ -111,7 +112,7 @@ cricket::Codec CreateRedAudioCodec(absl::string_view encoding_id) {
 const cricket::Codec kAudioCodecs1[] = {
     cricket::CreateAudioCodec(111, "opus", 48000, 2),
     CreateRedAudioCodec("111"),
-    cricket::CreateAudioCodec(102, "G722", 16000, 1),
+    cricket::CreateAudioCodec(103, "G722", 16000, 1),
     cricket::CreateAudioCodec(0, "PCMU", 8000, 1),
     cricket::CreateAudioCodec(8, "PCMA", 8000, 1),
     cricket::CreateAudioCodec(107, "CN", 48000, 1)};
@@ -123,7 +124,7 @@ const cricket::Codec kAudioCodecs2[] = {
 };
 
 const cricket::Codec kAudioCodecsAnswer[] = {
-    cricket::CreateAudioCodec(102, "G722", 16000, 1),
+    cricket::CreateAudioCodec(103, "G722", 16000, 1),
     cricket::CreateAudioCodec(0, "PCMU", 8000, 1),
 };
 
@@ -3410,6 +3411,7 @@ TEST_F(MediaSessionDescriptionFactoryTest,
   AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &opts);
 
   std::vector<cricket::Codec> f2_codecs = MAKE_VECTOR(kVideoCodecs2);
+  ASSERT_THAT(acd->codecs().size(), Gt(0));
   int used_pl_type = acd->codecs()[0].id;
   f2_codecs[0].id = used_pl_type;  // Set the payload type for H264.
   AddRtxCodec(cricket::CreateVideoRtxCodec(125, used_pl_type), &f2_codecs);
@@ -4703,6 +4705,8 @@ TEST_F(MediaSessionDescriptionFactoryTest, CreateAnswerWithLocalCodecParams) {
   auto answer_acd = answer->contents()[0].media_description();
   auto answer_vcd = answer->contents()[1].media_description();
   // Use the parameters from the local codecs.
+  ASSERT_TRUE(answer_acd);
+  ASSERT_THAT(answer_acd->codecs().size(), Gt(0));
   EXPECT_TRUE(answer_acd->codecs()[0].GetParam(audio_param_name, &value));
   EXPECT_EQ(audio_value2, value);
   EXPECT_TRUE(answer_vcd->codecs()[0].GetParam(video_param_name, &value));

pc/media_session_unittest.cc

@@ -18,6 +18,7 @@
 #include <vector>
 
 #include "absl/strings/match.h"
+#include "absl/strings/str_cat.h"
 #include "absl/strings/str_replace.h"
 #include "api/audio_codecs/audio_format.h"
 #include "api/audio_codecs/builtin_audio_decoder_factory.h"
@@ -56,7 +57,6 @@
 #include "pc/test/fake_rtc_certificate_generator.h"
 #include "pc/test/integration_test_helpers.h"
 #include "pc/test/mock_peer_connection_observers.h"
-#include "rtc_base/string_encode.h"
 #include "rtc_base/thread.h"
 #include "system_wrappers/include/metrics.h"
 #include "test/gmock.h"
@@ -1726,7 +1726,7 @@ TEST_F(SdpOfferAnswerTest, PayloadTypeMatchingWithSubsequentOfferAnswer) {
   codecs = media_description2->codecs();
   ASSERT_EQ(codecs.size(), 2u);
   EXPECT_EQ(codecs[1].name, av1.name);
-  EXPECT_NE(codecs[1].id, av1.id);
+  // At this point, the value 127 may or may not have been chosen.
 
   // 4. O/A triggered by remote. This "locks in" the payload type.
   auto offer3 = callee->CreateOfferAndSetAsLocal();

pc/sdp_offer_answer_unittest.cc

@@ -11,11 +11,17 @@
 #ifndef RTC_BASE_NET_TEST_HELPERS_H_
 #define RTC_BASE_NET_TEST_HELPERS_H_
 
+#include "rtc_base/system/rtc_export.h"
+
 namespace webrtc {
 
 bool HasIPv4Enabled();
-bool HasIPv6Enabled();
+RTC_EXPORT bool HasIPv6Enabled();
 
 }  // namespace webrtc
 
+namespace rtc {
+using webrtc::HasIPv6Enabled;
+}
+
 #endif  // RTC_BASE_NET_TEST_HELPERS_H_