diff --git a/webrtc/call/call.cc b/webrtc/call/call.cc index d37d08ef8e..ac9866b314 100644 --- a/webrtc/call/call.cc +++ b/webrtc/call/call.cc @@ -649,11 +649,8 @@ webrtc::VideoReceiveStream* Call::CreateVideoReceiveStream( RTC_DCHECK(video_receive_ssrcs_.find(config.rtp.remote_ssrc) == video_receive_ssrcs_.end()); video_receive_ssrcs_[config.rtp.remote_ssrc] = receive_stream; - // TODO(pbos): Configure different RTX payloads per receive payload. - VideoReceiveStream::Config::Rtp::RtxMap::const_iterator it = - config.rtp.rtx.begin(); - if (it != config.rtp.rtx.end()) - video_receive_ssrcs_[it->second.ssrc] = receive_stream; + if (config.rtp.rtx_ssrc) + video_receive_ssrcs_[config.rtp.rtx_ssrc] = receive_stream; video_receive_streams_.insert(receive_stream); ConfigureSync(config.sync_group); } diff --git a/webrtc/call/rampup_tests.cc b/webrtc/call/rampup_tests.cc index f637f77577..d8f0592ec4 100644 --- a/webrtc/call/rampup_tests.cc +++ b/webrtc/call/rampup_tests.cc @@ -199,10 +199,10 @@ void RampUpTester::ModifyVideoConfigs( } if (rtx_) { - recv_config.rtp.rtx[send_config->encoder_settings.payload_type].ssrc = - video_rtx_ssrcs_[i]; - recv_config.rtp.rtx[send_config->encoder_settings.payload_type] - .payload_type = send_config->rtp.rtx.payload_type; + recv_config.rtp.rtx_ssrc = video_rtx_ssrcs_[i]; + recv_config.rtp + .rtx_payload_types[send_config->encoder_settings.payload_type] = + send_config->rtp.rtx.payload_type; } ++i; } diff --git a/webrtc/logging/rtc_event_log/rtc_event_log.cc b/webrtc/logging/rtc_event_log/rtc_event_log.cc index 65ee7d8881..b545d6453d 100644 --- a/webrtc/logging/rtc_event_log/rtc_event_log.cc +++ b/webrtc/logging/rtc_event_log/rtc_event_log.cc @@ -242,11 +242,11 @@ void RtcEventLogImpl::LogVideoReceiveStreamConfig( receiver_config->set_rtcp_mode(ConvertRtcpMode(config.rtp.rtcp_mode)); receiver_config->set_remb(config.rtp.remb); - for (const auto& kv : config.rtp.rtx) { + for (const auto& kv : config.rtp.rtx_payload_types) { rtclog::RtxMap* rtx = receiver_config->add_rtx_map(); rtx->set_payload_type(kv.first); - rtx->mutable_config()->set_rtx_ssrc(kv.second.ssrc); - rtx->mutable_config()->set_rtx_payload_type(kv.second.payload_type); + rtx->mutable_config()->set_rtx_ssrc(config.rtp.rtx_ssrc); + rtx->mutable_config()->set_rtx_payload_type(kv.second); } for (const auto& e : config.rtp.extensions) { diff --git a/webrtc/logging/rtc_event_log/rtc_event_log_parser.cc b/webrtc/logging/rtc_event_log/rtc_event_log_parser.cc index 3b808b2e43..c1bf94458d 100644 --- a/webrtc/logging/rtc_event_log/rtc_event_log_parser.cc +++ b/webrtc/logging/rtc_event_log/rtc_event_log_parser.cc @@ -10,8 +10,10 @@ #include "webrtc/logging/rtc_event_log/rtc_event_log_parser.h" +#include #include +#include #include #include #include @@ -314,17 +316,30 @@ void ParsedRtcEventLog::GetVideoReceiveConfig( RTC_CHECK(receiver_config.has_remb()); config->rtp.remb = receiver_config.remb(); // Get RTX map. - config->rtp.rtx.clear(); + std::vector rtx_ssrcs(receiver_config.rtx_map_size()); + config->rtp.rtx_payload_types.clear(); for (int i = 0; i < receiver_config.rtx_map_size(); i++) { const rtclog::RtxMap& map = receiver_config.rtx_map(i); RTC_CHECK(map.has_payload_type()); RTC_CHECK(map.has_config()); RTC_CHECK(map.config().has_rtx_ssrc()); + rtx_ssrcs[i] = map.config().rtx_ssrc(); RTC_CHECK(map.config().has_rtx_payload_type()); - webrtc::VideoReceiveStream::Config::Rtp::Rtx rtx_pair; - rtx_pair.ssrc = map.config().rtx_ssrc(); - rtx_pair.payload_type = map.config().rtx_payload_type(); - config->rtp.rtx.insert(std::make_pair(map.payload_type(), rtx_pair)); + config->rtp.rtx_payload_types.insert( + std::make_pair(map.payload_type(), map.config().rtx_payload_type())); + } + if (!rtx_ssrcs.empty()) { + config->rtp.rtx_ssrc = rtx_ssrcs[0]; + + auto pred = [&config](uint32_t ssrc) { + return ssrc == config->rtp.rtx_ssrc; + }; + if (!std::all_of(rtx_ssrcs.cbegin(), rtx_ssrcs.cend(), pred)) { + LOG(LS_WARNING) << "RtcEventLog protobuf contained different SSRCs for " + "different received RTX payload types. Will only use " + "rtx_ssrc = " + << config->rtp.rtx_ssrc << "."; + } } // Get header extensions. GetHeaderExtensions(&config->rtp.extensions, diff --git a/webrtc/logging/rtc_event_log/rtc_event_log_unittest.cc b/webrtc/logging/rtc_event_log/rtc_event_log_unittest.cc index f0ee973a13..97c82b3609 100644 --- a/webrtc/logging/rtc_event_log/rtc_event_log_unittest.cc +++ b/webrtc/logging/rtc_event_log/rtc_event_log_unittest.cc @@ -165,11 +165,10 @@ void GenerateVideoReceiveConfig(uint32_t extensions_bitvector, config->rtp.rtcp_mode = prng->Rand() ? RtcpMode::kCompound : RtcpMode::kReducedSize; config->rtp.remb = prng->Rand(); - // Add a map from a payload type to a new ssrc and a new payload type for RTX. - VideoReceiveStream::Config::Rtp::Rtx rtx_pair; - rtx_pair.ssrc = prng->Rand(); - rtx_pair.payload_type = prng->Rand(0, 127); - config->rtp.rtx.insert(std::make_pair(prng->Rand(0, 127), rtx_pair)); + config->rtp.rtx_ssrc = prng->Rand(); + // Add a map from a payload type to a new payload type for RTX. + config->rtp.rtx_payload_types.insert( + std::make_pair(prng->Rand(0, 127), prng->Rand(0, 127))); // Add header extensions. for (unsigned i = 0; i < kNumExtensions; i++) { if (extensions_bitvector & (1u << i)) { diff --git a/webrtc/logging/rtc_event_log/rtc_event_log_unittest_helper.cc b/webrtc/logging/rtc_event_log/rtc_event_log_unittest_helper.cc index 19ca8aa15c..5bd8e4d060 100644 --- a/webrtc/logging/rtc_event_log/rtc_event_log_unittest_helper.cc +++ b/webrtc/logging/rtc_event_log/rtc_event_log_unittest_helper.cc @@ -127,19 +127,18 @@ void RtcEventLogTestHelper::VerifyVideoReceiveStreamConfig( ASSERT_TRUE(receiver_config.has_remb()); EXPECT_EQ(config.rtp.remb, receiver_config.remb()); // Check RTX map. - ASSERT_EQ(static_cast(config.rtp.rtx.size()), + ASSERT_EQ(static_cast(config.rtp.rtx_payload_types.size()), receiver_config.rtx_map_size()); for (const rtclog::RtxMap& rtx_map : receiver_config.rtx_map()) { ASSERT_TRUE(rtx_map.has_payload_type()); ASSERT_TRUE(rtx_map.has_config()); - EXPECT_EQ(1u, config.rtp.rtx.count(rtx_map.payload_type())); + EXPECT_EQ(1u, config.rtp.rtx_payload_types.count(rtx_map.payload_type())); const rtclog::RtxConfig& rtx_config = rtx_map.config(); - const VideoReceiveStream::Config::Rtp::Rtx& rtx = - config.rtp.rtx.at(rtx_map.payload_type()); ASSERT_TRUE(rtx_config.has_rtx_ssrc()); ASSERT_TRUE(rtx_config.has_rtx_payload_type()); - EXPECT_EQ(rtx.ssrc, rtx_config.rtx_ssrc()); - EXPECT_EQ(rtx.payload_type, rtx_config.rtx_payload_type()); + EXPECT_EQ(config.rtp.rtx_ssrc, rtx_config.rtx_ssrc()); + EXPECT_EQ(config.rtp.rtx_payload_types.at(rtx_map.payload_type()), + rtx_config.rtx_payload_type()); } // Check header extensions. ASSERT_EQ(static_cast(config.rtp.extensions.size()), @@ -173,12 +172,13 @@ void RtcEventLogTestHelper::VerifyVideoReceiveStreamConfig( EXPECT_EQ(config.rtp.rtcp_mode, parsed_config.rtp.rtcp_mode); EXPECT_EQ(config.rtp.remb, parsed_config.rtp.remb); // Check RTX map. - EXPECT_EQ(config.rtp.rtx.size(), parsed_config.rtp.rtx.size()); - for (const auto& kv : config.rtp.rtx) { - auto parsed_kv = parsed_config.rtp.rtx.find(kv.first); + EXPECT_EQ(config.rtp.rtx_ssrc, parsed_config.rtp.rtx_ssrc); + EXPECT_EQ(config.rtp.rtx_payload_types.size(), + parsed_config.rtp.rtx_payload_types.size()); + for (const auto& kv : config.rtp.rtx_payload_types) { + auto parsed_kv = parsed_config.rtp.rtx_payload_types.find(kv.first); EXPECT_EQ(kv.first, parsed_kv->first); - EXPECT_EQ(kv.second.ssrc, parsed_kv->second.ssrc); - EXPECT_EQ(kv.second.payload_type, parsed_kv->second.payload_type); + EXPECT_EQ(kv.second, parsed_kv->second); } // Check header extensions. EXPECT_EQ(config.rtp.extensions.size(), parsed_config.rtp.extensions.size()); diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc index 0cd31937cc..40f486dc0f 100644 --- a/webrtc/media/engine/webrtcvideoengine2.cc +++ b/webrtc/media/engine/webrtcvideoengine2.cc @@ -1233,16 +1233,7 @@ void WebRtcVideoChannel2::ConfigureReceiverRtp( flexfec_config->rtp_header_extensions = config->rtp.extensions; } - for (size_t i = 0; i < recv_codecs_.size(); ++i) { - uint32_t rtx_ssrc; - if (recv_codecs_[i].rtx_payload_type != -1 && - sp.GetFidSsrc(ssrc, &rtx_ssrc)) { - webrtc::VideoReceiveStream::Config::Rtp::Rtx& rtx = - config->rtp.rtx[recv_codecs_[i].codec.id]; - rtx.ssrc = rtx_ssrc; - rtx.payload_type = recv_codecs_[i].rtx_payload_type; - } - } + sp.GetFidSsrc(ssrc, &config->rtp.rtx_ssrc); config->rtp.extensions = recv_rtp_extensions_; } @@ -2205,7 +2196,12 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::ConfigureCodecs( config_.decoders.push_back(decoder); } - // TODO(pbos): Reconfigure RTX based on incoming recv_codecs. + config_.rtp.rtx_payload_types.clear(); + for (const VideoCodecSettings& recv_codec : recv_codecs) { + config_.rtp.rtx_payload_types[recv_codec.codec.id] = + recv_codec.rtx_payload_type; + } + config_.rtp.ulpfec = recv_codecs.front().ulpfec; flexfec_config_.payload_type = recv_codecs.front().flexfec_payload_type; diff --git a/webrtc/media/engine/webrtcvideoengine2_unittest.cc b/webrtc/media/engine/webrtcvideoengine2_unittest.cc index a93d2d98c2..ef77e9e927 100644 --- a/webrtc/media/engine/webrtcvideoengine2_unittest.cc +++ b/webrtc/media/engine/webrtcvideoengine2_unittest.cc @@ -1228,13 +1228,11 @@ TEST_F(WebRtcVideoChannel2Test, RecvStreamWithSimAndRtx) { // Receiver side. FakeVideoReceiveStream* recv_stream = AddRecvStream( cricket::CreateSimWithRtxStreamParams("cname", ssrcs, rtx_ssrcs)); - EXPECT_FALSE(recv_stream->GetConfig().rtp.rtx.empty()); + EXPECT_FALSE(recv_stream->GetConfig().rtp.rtx_payload_types.empty()); EXPECT_EQ(recv_stream->GetConfig().decoders.size(), - recv_stream->GetConfig().rtp.rtx.size()) + recv_stream->GetConfig().rtp.rtx_payload_types.size()) << "RTX should be mapped for all decoders/payload types."; - for (const auto& kv : recv_stream->GetConfig().rtp.rtx) { - EXPECT_EQ(rtx_ssrcs[0], kv.second.ssrc); - } + EXPECT_EQ(rtx_ssrcs[0], recv_stream->GetConfig().rtp.rtx_ssrc); } TEST_F(WebRtcVideoChannel2Test, RecvStreamWithRtx) { @@ -1243,8 +1241,7 @@ TEST_F(WebRtcVideoChannel2Test, RecvStreamWithRtx) { cricket::StreamParams::CreateLegacy(kSsrcs1[0]); params.AddFidSsrc(kSsrcs1[0], kRtxSsrcs1[0]); FakeVideoReceiveStream* recv_stream = AddRecvStream(params); - EXPECT_EQ(kRtxSsrcs1[0], - recv_stream->GetConfig().rtp.rtx.begin()->second.ssrc); + EXPECT_EQ(kRtxSsrcs1[0], recv_stream->GetConfig().rtp.rtx_ssrc); } TEST_F(WebRtcVideoChannel2Test, RecvStreamNoRtx) { @@ -1252,7 +1249,7 @@ TEST_F(WebRtcVideoChannel2Test, RecvStreamNoRtx) { cricket::StreamParams params = cricket::StreamParams::CreateLegacy(kSsrcs1[0]); FakeVideoReceiveStream* recv_stream = AddRecvStream(params); - ASSERT_TRUE(recv_stream->GetConfig().rtp.rtx.empty()); + ASSERT_EQ(0U, recv_stream->GetConfig().rtp.rtx_ssrc); } TEST_F(WebRtcVideoChannel2Test, NoHeaderExtesionsByDefault) { @@ -2482,6 +2479,43 @@ TEST_F(WebRtcVideoChannel2Test, } } +TEST_F(WebRtcVideoChannel2Test, SetSendCodecsWithChangedRtxPayloadType) { + const int kUnusedPayloadType1 = 126; + const int kUnusedPayloadType2 = 127; + EXPECT_FALSE(FindCodecById(engine_.codecs(), kUnusedPayloadType1)); + EXPECT_FALSE(FindCodecById(engine_.codecs(), kUnusedPayloadType2)); + + // SSRCs for RTX. + cricket::StreamParams params = + cricket::StreamParams::CreateLegacy(kSsrcs1[0]); + params.AddFidSsrc(kSsrcs1[0], kRtxSsrcs1[0]); + AddSendStream(params); + + // Original payload type for RTX. + cricket::VideoSendParameters parameters; + parameters.codecs.push_back(GetEngineCodec("VP8")); + cricket::VideoCodec rtx_codec(kUnusedPayloadType1, "rtx"); + rtx_codec.SetParam("apt", GetEngineCodec("VP8").id); + parameters.codecs.push_back(rtx_codec); + EXPECT_TRUE(channel_->SetSendParameters(parameters)); + ASSERT_EQ(1U, fake_call_->GetVideoSendStreams().size()); + const webrtc::VideoSendStream::Config& config_before = + fake_call_->GetVideoSendStreams()[0]->GetConfig(); + EXPECT_EQ(kUnusedPayloadType1, config_before.rtp.rtx.payload_type); + ASSERT_EQ(1U, config_before.rtp.rtx.ssrcs.size()); + EXPECT_EQ(kRtxSsrcs1[0], config_before.rtp.rtx.ssrcs[0]); + + // Change payload type for RTX. + parameters.codecs[1].id = kUnusedPayloadType2; + EXPECT_TRUE(channel_->SetSendParameters(parameters)); + ASSERT_EQ(1U, fake_call_->GetVideoSendStreams().size()); + const webrtc::VideoSendStream::Config& config_after = + fake_call_->GetVideoSendStreams()[0]->GetConfig(); + EXPECT_EQ(kUnusedPayloadType2, config_after.rtp.rtx.payload_type); + ASSERT_EQ(1U, config_after.rtp.rtx.ssrcs.size()); + EXPECT_EQ(kRtxSsrcs1[0], config_after.rtp.rtx.ssrcs[0]); +} + TEST_F(WebRtcVideoChannel2Test, SetSendCodecsWithoutFecDisablesFec) { cricket::VideoSendParameters parameters; parameters.codecs.push_back(GetEngineCodec("VP8")); @@ -2812,6 +2846,49 @@ TEST_F(WebRtcVideoChannel2Test, SetRecvCodecsWithRtx) { "rejected."; } +TEST_F(WebRtcVideoChannel2Test, SetRecvCodecsWithChangedRtxPayloadType) { + const int kUnusedPayloadType1 = 126; + const int kUnusedPayloadType2 = 127; + EXPECT_FALSE(FindCodecById(engine_.codecs(), kUnusedPayloadType1)); + EXPECT_FALSE(FindCodecById(engine_.codecs(), kUnusedPayloadType2)); + + // SSRCs for RTX. + cricket::StreamParams params = + cricket::StreamParams::CreateLegacy(kSsrcs1[0]); + params.AddFidSsrc(kSsrcs1[0], kRtxSsrcs1[0]); + AddRecvStream(params); + + // Original payload type for RTX. + cricket::VideoRecvParameters parameters; + parameters.codecs.push_back(GetEngineCodec("VP8")); + cricket::VideoCodec rtx_codec(kUnusedPayloadType1, "rtx"); + rtx_codec.SetParam("apt", GetEngineCodec("VP8").id); + parameters.codecs.push_back(rtx_codec); + EXPECT_TRUE(channel_->SetRecvParameters(parameters)); + ASSERT_EQ(1U, fake_call_->GetVideoReceiveStreams().size()); + const webrtc::VideoReceiveStream::Config& config_before = + fake_call_->GetVideoReceiveStreams()[0]->GetConfig(); + EXPECT_EQ(1U, config_before.rtp.rtx_payload_types.size()); + auto it_before = + config_before.rtp.rtx_payload_types.find(GetEngineCodec("VP8").id); + ASSERT_NE(it_before, config_before.rtp.rtx_payload_types.end()); + EXPECT_EQ(kUnusedPayloadType1, it_before->second); + EXPECT_EQ(kRtxSsrcs1[0], config_before.rtp.rtx_ssrc); + + // Change payload type for RTX. + parameters.codecs[1].id = kUnusedPayloadType2; + EXPECT_TRUE(channel_->SetRecvParameters(parameters)); + ASSERT_EQ(1U, fake_call_->GetVideoReceiveStreams().size()); + const webrtc::VideoReceiveStream::Config& config_after = + fake_call_->GetVideoReceiveStreams()[0]->GetConfig(); + EXPECT_EQ(1U, config_after.rtp.rtx_payload_types.size()); + auto it_after = + config_after.rtp.rtx_payload_types.find(GetEngineCodec("VP8").id); + ASSERT_NE(it_after, config_after.rtp.rtx_payload_types.end()); + EXPECT_EQ(kUnusedPayloadType2, it_after->second); + EXPECT_EQ(kRtxSsrcs1[0], config_after.rtp.rtx_ssrc); +} + TEST_F(WebRtcVideoChannel2Test, SetRecvCodecsDifferentPayloadType) { cricket::VideoRecvParameters parameters; parameters.codecs.push_back(GetEngineCodec("VP8")); @@ -3441,21 +3518,19 @@ TEST_F(WebRtcVideoChannel2Test, DefaultReceiveStreamReconfiguresToUseRtx) { ASSERT_EQ(1u, fake_call_->GetVideoReceiveStreams().size()) << "No default receive stream created."; FakeVideoReceiveStream* recv_stream = fake_call_->GetVideoReceiveStreams()[0]; - EXPECT_TRUE(recv_stream->GetConfig().rtp.rtx.empty()) + EXPECT_EQ(0u, recv_stream->GetConfig().rtp.rtx_ssrc) << "Default receive stream should not have configured RTX"; EXPECT_TRUE(channel_->AddRecvStream( cricket::CreateSimWithRtxStreamParams("cname", ssrcs, rtx_ssrcs))); ASSERT_EQ(1u, fake_call_->GetVideoReceiveStreams().size()) - << "AddRecvStream should've reconfigured, not added a new receiver."; + << "AddRecvStream should have reconfigured, not added a new receiver."; recv_stream = fake_call_->GetVideoReceiveStreams()[0]; - EXPECT_FALSE(recv_stream->GetConfig().rtp.rtx.empty()); + EXPECT_FALSE(recv_stream->GetConfig().rtp.rtx_payload_types.empty()); EXPECT_EQ(recv_stream->GetConfig().decoders.size(), - recv_stream->GetConfig().rtp.rtx.size()) + recv_stream->GetConfig().rtp.rtx_payload_types.size()) << "RTX should be mapped for all decoders/payload types."; - for (const auto& kv : recv_stream->GetConfig().rtp.rtx) { - EXPECT_EQ(rtx_ssrcs[0], kv.second.ssrc); - } + EXPECT_EQ(rtx_ssrcs[0], recv_stream->GetConfig().rtp.rtx_ssrc); } TEST_F(WebRtcVideoChannel2Test, RejectsAddingStreamsWithMissingSsrcsForRtx) { diff --git a/webrtc/tools/event_log_visualizer/analyzer.cc b/webrtc/tools/event_log_visualizer/analyzer.cc index 3fd2c25fe8..9794ab811a 100644 --- a/webrtc/tools/event_log_visualizer/analyzer.cc +++ b/webrtc/tools/event_log_visualizer/analyzer.cc @@ -311,13 +311,11 @@ EventLogAnalyzer::EventLogAnalyzer(const ParsedRtcEventLog& log) StreamId stream(config.rtp.remote_ssrc, kIncomingPacket); extension_maps[stream] = RtpHeaderExtensionMap(config.rtp.extensions); video_ssrcs_.insert(stream); - for (auto kv : config.rtp.rtx) { - StreamId rtx_stream(kv.second.ssrc, kIncomingPacket); - extension_maps[rtx_stream] = - RtpHeaderExtensionMap(config.rtp.extensions); - video_ssrcs_.insert(rtx_stream); - rtx_ssrcs_.insert(rtx_stream); - } + StreamId rtx_stream(config.rtp.rtx_ssrc, kIncomingPacket); + extension_maps[rtx_stream] = + RtpHeaderExtensionMap(config.rtp.extensions); + video_ssrcs_.insert(rtx_stream); + rtx_ssrcs_.insert(rtx_stream); break; } case ParsedRtcEventLog::VIDEO_SENDER_CONFIG_EVENT: { diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc index fab6f8e186..6812467cb7 100644 --- a/webrtc/video/end_to_end_tests.cc +++ b/webrtc/video/end_to_end_tests.cc @@ -1091,8 +1091,8 @@ void EndToEndTest::DecodesRetransmittedFrame(bool enable_rtx, bool enable_red) { if (retransmission_ssrc_ == kSendRtxSsrcs[0]) { send_config->rtp.rtx.ssrcs.push_back(kSendRtxSsrcs[0]); send_config->rtp.rtx.payload_type = kSendRtxPayloadType; - (*receive_configs)[0].rtp.rtx[payload_type_].ssrc = kSendRtxSsrcs[0]; - (*receive_configs)[0].rtp.rtx[payload_type_].payload_type = + (*receive_configs)[0].rtp.rtx_ssrc = kSendRtxSsrcs[0]; + (*receive_configs)[0].rtp.rtx_payload_types[payload_type_] = kSendRtxPayloadType; } // Configure encoding and decoding with VP8, since generic packetization @@ -2331,9 +2331,8 @@ void EndToEndTest::VerifyHistogramStats(bool use_rtx, if (use_rtx_) { send_config->rtp.rtx.ssrcs.push_back(kSendRtxSsrcs[0]); send_config->rtp.rtx.payload_type = kSendRtxPayloadType; - (*receive_configs)[0].rtp.rtx[kFakeVideoSendPayloadType].ssrc = - kSendRtxSsrcs[0]; - (*receive_configs)[0].rtp.rtx[kFakeVideoSendPayloadType].payload_type = + (*receive_configs)[0].rtp.rtx_ssrc = kSendRtxSsrcs[0]; + (*receive_configs)[0].rtp.rtx_payload_types[kFakeVideoSendPayloadType] = kSendRtxPayloadType; } // RTT needed for RemoteNtpTimeEstimator for the receive stream. @@ -2953,9 +2952,8 @@ TEST_P(EndToEndTest, GetStats) { (*receive_configs)[i].renderer = &receive_stream_renderer_; (*receive_configs)[i].rtp.nack.rtp_history_ms = kNackRtpHistoryMs; - (*receive_configs)[i].rtp.rtx[kFakeVideoSendPayloadType].ssrc = - kSendRtxSsrcs[i]; - (*receive_configs)[i].rtp.rtx[kFakeVideoSendPayloadType].payload_type = + (*receive_configs)[i].rtp.rtx_ssrc = kSendRtxSsrcs[i]; + (*receive_configs)[i].rtp.rtx_payload_types[kFakeVideoSendPayloadType] = kSendRtxPayloadType; } @@ -3939,7 +3937,9 @@ TEST_P(EndToEndTest, VerifyDefaultVideoReceiveConfigParameters) { EXPECT_FALSE( default_receive_config.rtp.rtcp_xr.receiver_reference_time_report) << "RTCP XR settings require rtcp-xr to be negotiated."; - EXPECT_TRUE(default_receive_config.rtp.rtx.empty()) + EXPECT_EQ(0U, default_receive_config.rtp.rtx_ssrc) + << "Enabling RTX requires ssrc-group: FID negotiation"; + EXPECT_TRUE(default_receive_config.rtp.rtx_payload_types.empty()) << "Enabling RTX requires rtpmap: rtx negotiation."; EXPECT_TRUE(default_receive_config.rtp.extensions.empty()) << "Enabling RTP extensions require negotiation."; diff --git a/webrtc/video/receive_statistics_proxy.cc b/webrtc/video/receive_statistics_proxy.cc index 8ac2c43e9a..6fe85745c3 100644 --- a/webrtc/video/receive_statistics_proxy.cc +++ b/webrtc/video/receive_statistics_proxy.cc @@ -78,8 +78,11 @@ ReceiveStatisticsProxy::ReceiveStatisticsProxy( avg_rtt_ms_(0), frame_window_accumulated_bytes_(0) { stats_.ssrc = config_.rtp.remote_ssrc; - for (auto it : config_.rtp.rtx) - rtx_stats_[it.second.ssrc] = StreamDataCounters(); + // TODO(brandtr): Replace |rtx_stats_| with a single instance of + // StreamDataCounters. + if (config_.rtp.rtx_ssrc) { + rtx_stats_[config_.rtp.rtx_ssrc] = StreamDataCounters(); + } } ReceiveStatisticsProxy::~ReceiveStatisticsProxy() { diff --git a/webrtc/video/rtp_stream_receiver.cc b/webrtc/video/rtp_stream_receiver.cc index e8acf48b00..c4081f3a76 100644 --- a/webrtc/video/rtp_stream_receiver.cc +++ b/webrtc/video/rtp_stream_receiver.cc @@ -150,14 +150,13 @@ RtpStreamReceiver::RtpStreamReceiver( : kDefaultMaxReorderingThreshold; rtp_receive_statistics_->SetMaxReorderingThreshold(max_reordering_threshold); - // TODO(pbos): Support multiple RTX, per video payload. - for (const auto& kv : config_.rtp.rtx) { - RTC_DCHECK(kv.second.ssrc != 0); - RTC_DCHECK(kv.second.payload_type != 0); + if (config_.rtp.rtx_ssrc) { + rtp_payload_registry_.SetRtxSsrc(config_.rtp.rtx_ssrc); - rtp_payload_registry_.SetRtxSsrc(kv.second.ssrc); - rtp_payload_registry_.SetRtxPayloadType(kv.second.payload_type, - kv.first); + for (const auto& kv : config_.rtp.rtx_payload_types) { + RTC_DCHECK(kv.second != 0); + rtp_payload_registry_.SetRtxPayloadType(kv.second, kv.first); + } } if (IsUlpfecEnabled()) { diff --git a/webrtc/video/video_quality_test.cc b/webrtc/video/video_quality_test.cc index bd9c4f089c..c1e07864d2 100644 --- a/webrtc/video/video_quality_test.cc +++ b/webrtc/video/video_quality_test.cc @@ -1124,8 +1124,8 @@ void VideoQualityTest::SetupVideo(Transport* send_transport, for (size_t i = 0; i < num_video_streams; ++i) { video_receive_configs_[i].rtp.nack.rtp_history_ms = kNackRtpHistoryMs; - video_receive_configs_[i].rtp.rtx[payload_type].ssrc = kSendRtxSsrcs[i]; - video_receive_configs_[i].rtp.rtx[payload_type].payload_type = + video_receive_configs_[i].rtp.rtx_ssrc = kSendRtxSsrcs[i]; + video_receive_configs_[i].rtp.rtx_payload_types[payload_type] = kSendRtxPayloadType; video_receive_configs_[i].rtp.transport_cc = params_.call.send_side_bwe; } diff --git a/webrtc/video/video_receive_stream.cc b/webrtc/video/video_receive_stream.cc index 5e8754b6bf..0ab2707ea6 100644 --- a/webrtc/video/video_receive_stream.cc +++ b/webrtc/video/video_receive_stream.cc @@ -100,12 +100,10 @@ std::string VideoReceiveStream::Config::Rtp::ToString() const { ss << ", transport_cc: " << (transport_cc ? "on" : "off"); ss << ", nack: {rtp_history_ms: " << nack.rtp_history_ms << '}'; ss << ", ulpfec: " << ulpfec.ToString(); - ss << ", rtx: {"; - for (auto& kv : rtx) { - ss << kv.first << " -> "; - ss << "{ssrc: " << kv.second.ssrc; - ss << ", payload_type: " << kv.second.payload_type; - ss << '}'; + ss << ", rtx_ssrc: " << rtx_ssrc; + ss << ", rtx_payload_types: {"; + for (auto& kv : rtx_payload_types) { + ss << kv.first << " (apt) -> " << kv.second << " (pt), "; } ss << '}'; ss << ", extensions: ["; diff --git a/webrtc/video_receive_stream.h b/webrtc/video_receive_stream.h index 7d97a29c69..511db3462c 100644 --- a/webrtc/video_receive_stream.h +++ b/webrtc/video_receive_stream.h @@ -117,6 +117,7 @@ class VideoReceiveStream { // Synchronization source (stream identifier) to be received. uint32_t remote_ssrc = 0; + // Sender SSRC used for sending RTCP (such as receiver reports). uint32_t local_ssrc = 0; @@ -142,19 +143,12 @@ class VideoReceiveStream { // See UlpfecConfig for description. UlpfecConfig ulpfec; - // RTX settings for incoming video payloads that may be received. RTX is - // disabled if there's no config present. - struct Rtx { - // SSRCs to use for the RTX streams. - uint32_t ssrc = 0; + // SSRC for retransmissions. + uint32_t rtx_ssrc = 0; - // Payload type to use for the RTX stream. - int payload_type = 0; - }; - - // Map from video RTP payload type -> RTX config. - typedef std::map RtxMap; - RtxMap rtx; + // Map from video payload type (apt) -> RTX payload type (pt). + // For RTX to be enabled, both an SSRC and this mapping are needed. + std::map rtx_payload_types; // RTP header extensions used for the received stream. std::vector extensions;