PT: Fix test failures and PT mapping conflicts under WebRTC-PayloadTypesInTransport This CL addresses multiple test failures in rtc_pc_unittests when the WebRTC-PayloadTypesInTransport field trial is enabled: 1. Reverts a too-permissive directionality check bypass in pc/codec_vendor.cc to restore correct negotiation of kInactive via reversed offer direction, fixing legacy AudioCodecsAnswerTest failures. 2. Adds BUNDLE support to FakePayloadTypeSuggester by mapping bundled MIDs to share the same PayloadTypeRecorder instance. 3. Fixes TestBundleOfferWithSameCodecPlType by configuring bundle groups on the fake suggester. 4. Avoids force-registering conflicting preferred payload types in RegisterExpectations inside CodecLookupHelperForTesting if they are already mapped. This allows fake suggester conflict resolution to work correctly in tests. 5. Updates 15 asymmetric H265 level negotiation tests to use CodecListsMatch instead of strict EXPECT_EQ, as TypedCodecVendor does not pre-assign payload types in video_sendrecv_codecs(). Bug: webrtc:360058654 Change-Id: I9431d77b2e3eceba25c2df71f5fef69f70480495 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/477860 Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org> Commit-Queue: Harald Alvestrand <hta@webrtc.org> Cr-Commit-Position: refs/heads/main@{#47887}
diff --git a/call/fake_payload_type_suggester.h b/call/fake_payload_type_suggester.h index 2cedb07..25d356b 100644 --- a/call/fake_payload_type_suggester.h +++ b/call/fake_payload_type_suggester.h
@@ -11,10 +11,12 @@ #ifndef CALL_FAKE_PAYLOAD_TYPE_SUGGESTER_H_ #define CALL_FAKE_PAYLOAD_TYPE_SUGGESTER_H_ +#include <algorithm> #include <memory> #include <optional> #include <string> #include <utility> +#include <vector> #include "absl/strings/string_view.h" #include "api/payload_type.h" @@ -77,6 +79,15 @@ suggestion; } + void SetBundleGroups( + const std::vector<std::vector<std::string>>& bundle_groups) { + bundle_groups_ = bundle_groups; + } + + bool HasMapping(PayloadType payload_type) const { + return pt_picker_.LookupCodec(payload_type).has_value(); + } + RTCError AddLocalMapping(absl::string_view mid, PayloadType payload_type, const Codec& codec) override { @@ -123,10 +134,17 @@ PayloadTypeRecorder& LookupRecorder(absl::string_view mid) { RTC_CHECK(!mid.empty()); - auto it = recorders_.find(mid); + std::string transport_mapped_name = std::string(mid); + for (const std::vector<std::string>& group : bundle_groups_) { + if (std::find(group.begin(), group.end(), mid) != group.end()) { + transport_mapped_name = group[0]; + break; + } + } + auto it = recorders_.find(transport_mapped_name); if (it == recorders_.end()) { it = recorders_ - .emplace(std::string(mid), + .emplace(transport_mapped_name, std::make_unique<PayloadTypeRecorder>(pt_picker_)) .first; } @@ -138,6 +156,7 @@ flat_map<std::pair<std::string, std::string>, PayloadType> fallback_suggestions_; flat_map<std::string, std::unique_ptr<PayloadTypeRecorder>> recorders_; + std::vector<std::vector<std::string>> bundle_groups_; }; } // namespace webrtc
diff --git a/pc/codec_vendor.cc b/pc/codec_vendor.cc index 4896234..4562e4f 100644 --- a/pc/codec_vendor.cc +++ b/pc/codec_vendor.cc
@@ -1222,25 +1222,7 @@ } } if (payload_types_in_transport_) { - // TODO(webrtc:360058654): This is not according to the specification, - // which says we should only include codecs valid for the negotiated - // direction. We include all supported codecs to match legacy behavior. - // Make this more restrictive in the future. - const std::vector<CodecConfiguration>& send_configs = - (media_description_options.type == MediaType::AUDIO) - ? audio_send_codecs_.configurations() - : video_send_codecs_.configurations(); - const std::vector<CodecConfiguration>& recv_configs = - (media_description_options.type == MediaType::AUDIO) - ? audio_recv_codecs_.configurations() - : video_recv_codecs_.configurations(); - - MergeCodecsFromConfigurations(send_configs, mid, filtered_codecs, - pt_suggester, trials_, - /*pick_from_top_of_range=*/false); - MergeCodecsFromConfigurations(recv_configs, mid, filtered_codecs, - pt_suggester, trials_, - /*pick_from_top_of_range=*/false); + filtered_codecs = supported_codecs; } else { // Merge other_codecs into filtered_codecs, resolving PT conflicts. MergeCodecsLegacy(supported_codecs, mid, filtered_codecs, pt_suggester);
diff --git a/pc/media_session_unittest.cc b/pc/media_session_unittest.cc index f0b24bd..593dc61 100644 --- a/pc/media_session_unittest.cc +++ b/pc/media_session_unittest.cc
@@ -106,6 +106,10 @@ return &payload_type_suggester_; } + FakePayloadTypeSuggester* fake_payload_type_suggester() { + return &payload_type_suggester_; + } + CodecVendor* GetCodecVendor() override { if (!codec_vendor_) { codec_vendor_.reset( @@ -117,7 +121,7 @@ void RegisterExpectations(absl::string_view mid, std::span<const Codec> codecs) { for (const Codec& c : codecs) { - if (c.id.IsSet()) { + if (c.id.IsSet() && !payload_type_suggester_.HasMapping(c.id)) { RTC_CHECK(payload_type_suggester_.AddLocalMapping(mid, c.id, c).ok()); } } @@ -1204,6 +1208,8 @@ MediaSessionOptions opts; AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &opts); opts.bundle_enabled = true; + codec_lookup_helper_2_.fake_payload_type_suggester()->SetBundleGroups( + {{kAudioMid, kVideoMid}}); std::unique_ptr<SessionDescription> offer = f2_.CreateOfferOrError(opts, nullptr).MoveValue(); const VideoContentDescription* vcd = @@ -5756,10 +5762,10 @@ offerer_recv_codecs); codec_lookup_helper_answerer_.SetVideoCodecs(answerer_send_codecs, answerer_recv_codecs); - EXPECT_EQ(offerer_sendrecv_codecs, - codec_lookup_helper_offerer_.GetCodecVendor() - ->video_sendrecv_codecs() - .codecs()); + EXPECT_THAT(codec_lookup_helper_offerer_.GetCodecVendor() + ->video_sendrecv_codecs() + .codecs(), + CodecListsMatch(offerer_sendrecv_codecs, &env_.field_trials())); MediaSessionOptions opts; AddMediaDescriptionOptions(MediaType::VIDEO, kVideoMid, @@ -5819,10 +5825,10 @@ offerer_recv_codecs); codec_lookup_helper_answerer_.SetVideoCodecs(answerer_send_codecs, answerer_recv_codecs); - EXPECT_EQ(offerer_sendrecv_codecs, - codec_lookup_helper_offerer_.GetCodecVendor() - ->video_sendrecv_codecs() - .codecs()); + EXPECT_THAT(codec_lookup_helper_offerer_.GetCodecVendor() + ->video_sendrecv_codecs() + .codecs(), + CodecListsMatch(offerer_sendrecv_codecs, &env_.field_trials())); MediaSessionOptions opts; AddMediaDescriptionOptions(MediaType::VIDEO, kVideoMid, @@ -5892,10 +5898,10 @@ offerer_recv_codecs); codec_lookup_helper_answerer_.SetVideoCodecs(answerer_send_codecs, answerer_recv_codecs); - EXPECT_EQ(offerer_sendrecv_codecs, - codec_lookup_helper_offerer_.GetCodecVendor() - ->video_sendrecv_codecs() - .codecs()); + EXPECT_THAT(codec_lookup_helper_offerer_.GetCodecVendor() + ->video_sendrecv_codecs() + .codecs(), + CodecListsMatch(offerer_sendrecv_codecs, &env_.field_trials())); MediaSessionOptions opts; AddMediaDescriptionOptions(MediaType::VIDEO, kVideoMid, @@ -5955,10 +5961,10 @@ offerer_recv_codecs); codec_lookup_helper_answerer_.SetVideoCodecs(answerer_send_codecs, answerer_recv_codecs); - EXPECT_EQ(offerer_sendrecv_codecs, - codec_lookup_helper_offerer_.GetCodecVendor() - ->video_sendrecv_codecs() - .codecs()); + EXPECT_THAT(codec_lookup_helper_offerer_.GetCodecVendor() + ->video_sendrecv_codecs() + .codecs(), + CodecListsMatch(offerer_sendrecv_codecs, &env_.field_trials())); MediaSessionOptions opts; AddMediaDescriptionOptions(MediaType::VIDEO, kVideoMid, @@ -6018,10 +6024,10 @@ offerer_recv_codecs); codec_lookup_helper_answerer_.SetVideoCodecs(answerer_send_codecs, answerer_recv_codecs); - EXPECT_EQ(offerer_sendrecv_codecs, - codec_lookup_helper_offerer_.GetCodecVendor() - ->video_sendrecv_codecs() - .codecs()); + EXPECT_THAT(codec_lookup_helper_offerer_.GetCodecVendor() + ->video_sendrecv_codecs() + .codecs(), + CodecListsMatch(offerer_sendrecv_codecs, &env_.field_trials())); MediaSessionOptions opts; AddMediaDescriptionOptions(MediaType::VIDEO, kVideoMid, @@ -6078,10 +6084,10 @@ offerer_recv_codecs); codec_lookup_helper_answerer_.SetVideoCodecs(answerer_send_codecs, answerer_recv_codecs); - EXPECT_EQ(offerer_sendrecv_codecs, - codec_lookup_helper_offerer_.GetCodecVendor() - ->video_sendrecv_codecs() - .codecs()); + EXPECT_THAT(codec_lookup_helper_offerer_.GetCodecVendor() + ->video_sendrecv_codecs() + .codecs(), + CodecListsMatch(offerer_sendrecv_codecs, &env_.field_trials())); MediaSessionOptions opts; AddMediaDescriptionOptions(MediaType::VIDEO, kVideoMid, @@ -6138,10 +6144,10 @@ offerer_recv_codecs); codec_lookup_helper_answerer_.SetVideoCodecs(answerer_send_codecs, answerer_recv_codecs); - EXPECT_EQ(offerer_sendrecv_codecs, - codec_lookup_helper_offerer_.GetCodecVendor() - ->video_sendrecv_codecs() - .codecs()); + EXPECT_THAT(codec_lookup_helper_offerer_.GetCodecVendor() + ->video_sendrecv_codecs() + .codecs(), + CodecListsMatch(offerer_sendrecv_codecs, &env_.field_trials())); MediaSessionOptions opts; AddMediaDescriptionOptions(MediaType::VIDEO, kVideoMid, @@ -6198,10 +6204,10 @@ offerer_recv_codecs); codec_lookup_helper_answerer_.SetVideoCodecs(answerer_send_codecs, answerer_recv_codecs); - EXPECT_EQ(offerer_sendrecv_codecs, - codec_lookup_helper_offerer_.GetCodecVendor() - ->video_sendrecv_codecs() - .codecs()); + EXPECT_THAT(codec_lookup_helper_offerer_.GetCodecVendor() + ->video_sendrecv_codecs() + .codecs(), + CodecListsMatch(offerer_sendrecv_codecs, &env_.field_trials())); MediaSessionOptions opts; AddMediaDescriptionOptions(MediaType::VIDEO, kVideoMid, @@ -6258,10 +6264,10 @@ offerer_recv_codecs); codec_lookup_helper_answerer_.SetVideoCodecs(answerer_send_codecs, answerer_recv_codecs); - EXPECT_EQ(offerer_sendrecv_codecs, - codec_lookup_helper_offerer_.GetCodecVendor() - ->video_sendrecv_codecs() - .codecs()); + EXPECT_THAT(codec_lookup_helper_offerer_.GetCodecVendor() + ->video_sendrecv_codecs() + .codecs(), + CodecListsMatch(offerer_sendrecv_codecs, &env_.field_trials())); MediaSessionOptions opts; AddMediaDescriptionOptions(MediaType::VIDEO, kVideoMid, @@ -6318,10 +6324,10 @@ offerer_recv_codecs); codec_lookup_helper_answerer_.SetVideoCodecs(answerer_send_codecs, answerer_recv_codecs); - EXPECT_EQ(offerer_sendrecv_codecs, - codec_lookup_helper_offerer_.GetCodecVendor() - ->video_sendrecv_codecs() - .codecs()); + EXPECT_THAT(codec_lookup_helper_offerer_.GetCodecVendor() + ->video_sendrecv_codecs() + .codecs(), + CodecListsMatch(offerer_sendrecv_codecs, &env_.field_trials())); MediaSessionOptions opts; AddMediaDescriptionOptions(MediaType::VIDEO, kVideoMid, @@ -6378,10 +6384,10 @@ offerer_recv_codecs); codec_lookup_helper_answerer_.SetVideoCodecs(answerer_send_codecs, answerer_recv_codecs); - EXPECT_EQ(offerer_sendrecv_codecs, - codec_lookup_helper_offerer_.GetCodecVendor() - ->video_sendrecv_codecs() - .codecs()); + EXPECT_THAT(codec_lookup_helper_offerer_.GetCodecVendor() + ->video_sendrecv_codecs() + .codecs(), + CodecListsMatch(offerer_sendrecv_codecs, &env_.field_trials())); MediaSessionOptions opts; AddMediaDescriptionOptions(MediaType::VIDEO, kVideoMid, @@ -6438,10 +6444,10 @@ offerer_recv_codecs); codec_lookup_helper_answerer_.SetVideoCodecs(answerer_send_codecs, answerer_recv_codecs); - EXPECT_EQ(offerer_sendrecv_codecs, - codec_lookup_helper_offerer_.GetCodecVendor() - ->video_sendrecv_codecs() - .codecs()); + EXPECT_THAT(codec_lookup_helper_offerer_.GetCodecVendor() + ->video_sendrecv_codecs() + .codecs(), + CodecListsMatch(offerer_sendrecv_codecs, &env_.field_trials())); MediaSessionOptions opts; AddMediaDescriptionOptions(MediaType::VIDEO, kVideoMid, @@ -6498,10 +6504,10 @@ offerer_recv_codecs); codec_lookup_helper_answerer_.SetVideoCodecs(answerer_send_codecs, answerer_recv_codecs); - EXPECT_EQ(offerer_sendrecv_codecs, - codec_lookup_helper_offerer_.GetCodecVendor() - ->video_sendrecv_codecs() - .codecs()); + EXPECT_THAT(codec_lookup_helper_offerer_.GetCodecVendor() + ->video_sendrecv_codecs() + .codecs(), + CodecListsMatch(offerer_sendrecv_codecs, &env_.field_trials())); MediaSessionOptions opts; AddMediaDescriptionOptions(MediaType::VIDEO, kVideoMid, @@ -6558,10 +6564,10 @@ offerer_recv_codecs); codec_lookup_helper_answerer_.SetVideoCodecs(answerer_send_codecs, answerer_recv_codecs); - EXPECT_EQ(offerer_sendrecv_codecs, - codec_lookup_helper_offerer_.GetCodecVendor() - ->video_sendrecv_codecs() - .codecs()); + EXPECT_THAT(codec_lookup_helper_offerer_.GetCodecVendor() + ->video_sendrecv_codecs() + .codecs(), + CodecListsMatch(offerer_sendrecv_codecs, &env_.field_trials())); MediaSessionOptions opts; AddMediaDescriptionOptions(MediaType::VIDEO, kVideoMid, @@ -6614,10 +6620,10 @@ offerer_recv_codecs); codec_lookup_helper_answerer_.SetVideoCodecs(answerer_send_codecs, answerer_recv_codecs); - EXPECT_EQ(offerer_sendrecv_codecs, - codec_lookup_helper_offerer_.GetCodecVendor() - ->video_sendrecv_codecs() - .codecs()); + EXPECT_THAT(codec_lookup_helper_offerer_.GetCodecVendor() + ->video_sendrecv_codecs() + .codecs(), + CodecListsMatch(offerer_sendrecv_codecs, &env_.field_trials())); MediaSessionOptions opts; AddMediaDescriptionOptions(MediaType::VIDEO, kVideoMid,