From 8f27fcce79584378da97f0d84574564799e138d6 Mon Sep 17 00:00:00 2001 From: "andrew@webrtc.org" Date: Fri, 9 Jan 2015 20:22:46 +0000 Subject: [PATCH] Revert 8028 "Support associated payload type when registering Rt..." Reasons for revert: 1. glaznev discovered potentially related problems using the Android AppRTCDemo. 2. We're trying to do an M41 webrtc roll in Chromium, and this CL is risky. > Support associated payload type when registering Rtx payload type. > > Major changes include, > - Add associated payload type for SetRtxSendPayloadType & SetRtxReceivePayloadType. > - Receiver: Restore RTP packets by the new RTX-APT map. > - Sender: Send RTP packets by checking RTX-APT map. > - Add RTX payload type for RED in the default codec list. > > BUG=4024 > R=pbos@webrtc.org, stefan@webrtc.org > TBR=mflodman@webrtc.org > > Review URL: https://webrtc-codereview.appspot.com/26259004 > > Patch from Changbin Shao . TBR=pbos@webrtc.org, stefan@webrtc.org Review URL: https://webrtc-codereview.appspot.com/33829004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@8033 4adac7df-926f-26a2-2b94-8c16560cd09d --- talk/libjingle_tests.gyp | 6 -- talk/media/webrtc/fakewebrtcvideoengine.h | 58 ++++--------- talk/media/webrtc/webrtcvideoengine.cc | 62 +++++++------- talk/media/webrtc/webrtcvideoengine.h | 3 +- talk/media/webrtc/webrtcvideoengine2.cc | 23 +---- talk/media/webrtc/webrtcvideoengine2.h | 1 + .../webrtc/webrtcvideoengine2_unittest.cc | 27 ++---- .../webrtc/webrtcvideoengine_unittest.cc | 37 +++----- webrtc/config.cc | 1 - webrtc/config.h | 6 +- .../rtp_rtcp/interface/rtp_payload_registry.h | 5 +- webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h | 3 +- webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h | 3 +- .../rtp_rtcp/source/nack_rtx_unittest.cc | 24 +++--- .../rtp_rtcp/source/rtp_payload_registry.cc | 33 +++---- .../modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 5 +- .../modules/rtp_rtcp/source/rtp_rtcp_impl.h | 3 +- .../rtp_rtcp/source/rtp_rtcp_impl_unittest.cc | 4 +- webrtc/modules/rtp_rtcp/source/rtp_sender.cc | 29 ++----- webrtc/modules/rtp_rtcp/source/rtp_sender.h | 5 +- .../rtp_rtcp/source/rtp_sender_unittest.cc | 4 +- .../modules/rtp_rtcp/test/testAPI/test_api.cc | 14 +-- webrtc/test/call_test.cc | 1 - webrtc/test/call_test.h | 1 - webrtc/video/end_to_end_tests.cc | 51 +++-------- webrtc/video/loopback.cc | 11 ++- webrtc/video/rampup_tests.cc | 85 +++++-------------- webrtc/video/rampup_tests.h | 9 +- webrtc/video/video_receive_stream.cc | 10 +-- webrtc/video/video_send_stream.cc | 9 +- webrtc/video_engine/include/vie_rtp_rtcp.h | 9 +- .../auto_test/source/vie_autotest_loopback.cc | 8 +- .../auto_test/source/vie_autotest_rtp_rtcp.cc | 9 +- webrtc/video_engine/vie_channel.cc | 12 ++- webrtc/video_engine/vie_channel.h | 4 +- webrtc/video_engine/vie_receiver.cc | 6 +- webrtc/video_engine/vie_receiver.h | 2 +- webrtc/video_engine/vie_rtp_rtcp_impl.cc | 17 ++-- webrtc/video_engine/vie_rtp_rtcp_impl.h | 6 +- 39 files changed, 206 insertions(+), 400 deletions(-) diff --git a/talk/libjingle_tests.gyp b/talk/libjingle_tests.gyp index 19ce82b67..ac13a4c54 100755 --- a/talk/libjingle_tests.gyp +++ b/talk/libjingle_tests.gyp @@ -88,16 +88,10 @@ 'target_name': 'libjingle_media_unittest', 'type': 'executable', 'dependencies': [ - '<(DEPTH)/testing/gmock.gyp:gmock', '<(webrtc_root)/base/base_tests.gyp:rtc_base_tests_utils', 'libjingle.gyp:libjingle_media', 'libjingle_unittest_main', ], - 'direct_dependent_settings': { - 'include_dirs': [ - '<(DEPTH)/testing/gmock/include', - ], - }, 'sources': [ # TODO(ronghuawu): Reenable this test. # 'media/base/capturemanager_unittest.cc', diff --git a/talk/media/webrtc/fakewebrtcvideoengine.h b/talk/media/webrtc/fakewebrtcvideoengine.h index dff6d0172..aaa2f3bc0 100644 --- a/talk/media/webrtc/fakewebrtcvideoengine.h +++ b/talk/media/webrtc/fakewebrtcvideoengine.h @@ -266,6 +266,8 @@ class FakeWebRtcVideoEngine receive_(false), can_transmit_(true), remote_rtx_ssrc_(-1), + rtx_send_payload_type(-1), + rtx_recv_payload_type(-1), rtcp_status_(webrtc::kRtcpNone), key_frame_request_method_(webrtc::kViEKeyFrameRequestNone), tmmbr_(false), @@ -303,8 +305,8 @@ class FakeWebRtcVideoEngine std::map ssrcs_; std::map rtx_ssrcs_; int remote_rtx_ssrc_; - std::map rtx_send_payload_types; - std::map rtx_recv_payload_types; + int rtx_send_payload_type; + int rtx_recv_payload_type; std::string cname_; webrtc::ViERTCPMode rtcp_status_; webrtc::ViEKeyFrameRequestMethod key_frame_request_method_; @@ -612,30 +614,14 @@ class FakeWebRtcVideoEngine WEBRTC_ASSERT_CHANNEL(channel); channels_[GetOriginalChannelId(channel)]->receive_bandwidth_ = receive_bandwidth; + }; + int GetRtxSendPayloadType(int channel) { + WEBRTC_CHECK_CHANNEL(channel); + return channels_[channel]->rtx_send_payload_type; } - std::set GetRtxSendPayloadType(int channel) { - std::set rtx_payload_types; - if (channels_.find(channel) == channels_.end()) - return rtx_payload_types; - - std::map::const_iterator it; - for (it = channels_[channel]->rtx_send_payload_types.begin(); - it != channels_[channel]->rtx_send_payload_types.end(); ++it) { - rtx_payload_types.insert(it->first); - } - return rtx_payload_types; - } - std::set GetRtxRecvPayloadType(int channel) { - std::set rtx_payload_types; - if (channels_.find(channel) == channels_.end()) - return rtx_payload_types; - - std::map::const_iterator it; - for (it = channels_[channel]->rtx_recv_payload_types.begin(); - it != channels_[channel]->rtx_recv_payload_types.end(); ++it) { - rtx_payload_types.insert(it->first); - } - return rtx_payload_types; + int GetRtxRecvPayloadType(int channel) { + WEBRTC_CHECK_CHANNEL(channel); + return channels_[channel]->rtx_recv_payload_type; } int GetRemoteRtxSsrc(int channel) { WEBRTC_CHECK_CHANNEL(channel); @@ -1020,27 +1006,17 @@ class FakeWebRtcVideoEngine WEBRTC_STUB_CONST(GetRemoteSSRC, (const int, unsigned int&)); WEBRTC_STUB_CONST(GetRemoteCSRCs, (const int, unsigned int*)); - WEBRTC_FUNC(SetRtxSendPayloadType, - (const int channel, - const uint8 payload_type, - const uint8 associated_payload_type)) { + WEBRTC_FUNC(SetRtxSendPayloadType, (const int channel, + const uint8 payload_type)) { WEBRTC_CHECK_CHANNEL(channel); - assert(payload_type >= 0); - assert(associated_payload_type >= 0); - channels_[channel]->rtx_send_payload_types[payload_type] = - associated_payload_type; + channels_[channel]->rtx_send_payload_type = payload_type; return 0; } - WEBRTC_FUNC(SetRtxReceivePayloadType, - (const int channel, - const uint8 payload_type, - const uint8 associated_payload_type)) { + WEBRTC_FUNC(SetRtxReceivePayloadType, (const int channel, + const uint8 payload_type)) { WEBRTC_CHECK_CHANNEL(channel); - assert(payload_type >= 0); - assert(associated_payload_type >= 0); - channels_[channel]->rtx_recv_payload_types[payload_type] = - associated_payload_type; + channels_[channel]->rtx_recv_payload_type = payload_type; return 0; } diff --git a/talk/media/webrtc/webrtcvideoengine.cc b/talk/media/webrtc/webrtcvideoengine.cc index 3cce243d3..689aef35d 100644 --- a/talk/media/webrtc/webrtcvideoengine.cc +++ b/talk/media/webrtc/webrtcvideoengine.cc @@ -180,13 +180,6 @@ const int kVideoRtpBufferSize = 65536; const char kVp8CodecName[] = "VP8"; const char kVp9CodecName[] = "VP9"; -const int kDefaultVp8PlType = 100; -const int kDefaultVp9PlType = 101; -const int kDefaultRedPlType = 116; -const int kDefaultUlpfecType = 117; -const int kDefaultRtxVp8PlType = 96; -const int kDefaultRtxRedPlType = 97; - // TODO(ronghuawu): Change to 640x360. const int kDefaultVideoMaxWidth = 640; const int kDefaultVideoMaxHeight = 400; @@ -312,16 +305,14 @@ bool CodecIsInternallySupported(const std::string& codec_name) { std::vector DefaultVideoCodecList() { std::vector codecs; if (CodecIsInternallySupported(kVp9CodecName)) { - codecs.push_back(MakeVideoCodecWithDefaultFeedbackParams(kDefaultVp9PlType, - kVp9CodecName)); + codecs.push_back( + MakeVideoCodecWithDefaultFeedbackParams(101, kVp9CodecName)); // TODO(andresp): Add rtx codec for vp9 and verify it works. } - codecs.push_back(MakeVideoCodecWithDefaultFeedbackParams(kDefaultVp8PlType, - kVp8CodecName)); - codecs.push_back(MakeRtxCodec(kDefaultRtxVp8PlType, kDefaultVp8PlType)); - codecs.push_back(MakeVideoCodec(kDefaultRedPlType, kRedCodecName)); - codecs.push_back(MakeRtxCodec(kDefaultRtxRedPlType, kDefaultRedPlType)); - codecs.push_back(MakeVideoCodec(kDefaultUlpfecType, kUlpfecCodecName)); + codecs.push_back(MakeVideoCodecWithDefaultFeedbackParams(100, kVp8CodecName)); + codecs.push_back(MakeRtxCodec(96, 100)); + codecs.push_back(MakeVideoCodec(116, kRedCodecName)); + codecs.push_back(MakeVideoCodec(117, kUlpfecCodecName)); return codecs; } @@ -1738,6 +1729,7 @@ WebRtcVideoMediaChannel::WebRtcVideoMediaChannel( first_receive_ssrc_(kSsrcUnset), receiver_report_ssrc_(kSsrcUnset), num_unsignalled_recv_channels_(0), + send_rtx_type_(-1), send_red_type_(-1), send_fec_type_(-1), sending_(false), @@ -1827,6 +1819,7 @@ bool WebRtcVideoMediaChannel::SetSendCodecs( std::vector send_codecs; VideoCodec checked_codec; VideoCodec dummy_current; // Will be ignored by CanSendCodec. + std::map primary_rtx_pt_mapping; bool nack_enabled = nack_enabled_; bool remb_enabled = remb_enabled_; for (std::vector::const_iterator iter = codecs.begin(); @@ -1839,7 +1832,7 @@ bool WebRtcVideoMediaChannel::SetSendCodecs( int rtx_type = iter->id; int rtx_primary_type = -1; if (iter->GetParam(kCodecParamAssociatedPayloadType, &rtx_primary_type)) { - send_rtx_apt_types_[rtx_type] = rtx_primary_type; + primary_rtx_pt_mapping[rtx_primary_type] = rtx_type; } } else if (engine()->CanSendCodec(*iter, dummy_current, &checked_codec)) { webrtc::VideoCodec wcodec; @@ -1904,6 +1897,14 @@ bool WebRtcVideoMediaChannel::SetSendCodecs( // Select the first matched codec. webrtc::VideoCodec& codec(send_codecs[0]); + // Set RTX payload type if primary now active. This value will be used in + // SetSendCodec. + std::map::const_iterator rtx_it = + primary_rtx_pt_mapping.find(static_cast(codec.plType)); + if (rtx_it != primary_rtx_pt_mapping.end()) { + send_rtx_type_ = rtx_it->second; + } + if (BitrateIsSet(codec.minBitrate) && BitrateIsSet(codec.maxBitrate) && codec.minBitrate > codec.maxBitrate) { // TODO(pthatcher): This behavior contradicts other behavior in @@ -3830,11 +3831,8 @@ void WebRtcVideoMediaChannel::LogSendCodecChange(const std::string& reason) { << vie_codec.codecSpecific.VP8.keyFrameInterval; } - std::map::const_iterator it; - for (it = send_rtx_apt_types_.begin(); it != send_rtx_apt_types_.end(); - ++it) { - LOG(LS_INFO) << "RTX payload type: " << it->first - << ", associated payload type:" << it->second; + if (send_rtx_type_ != -1) { + LOG(LS_INFO) << "RTX payload type: " << send_rtx_type_; } LogSimulcastSubstreams(vie_codec); @@ -3852,6 +3850,7 @@ bool WebRtcVideoMediaChannel::SetReceiveCodecs( it != receive_codecs_.end(); ++it) { pt_to_codec[it->plType] = &(*it); } + bool rtx_registered = false; for (std::vector::iterator it = receive_codecs_.begin(); it != receive_codecs_.end(); ++it) { if (it->codecType == webrtc::kVideoCodecRED) { @@ -3862,6 +3861,11 @@ bool WebRtcVideoMediaChannel::SetReceiveCodecs( // If this is an RTX codec we have to verify that it is associated with // a valid video codec which we have RTX support for. if (_stricmp(it->plName, kRtxCodecName) == 0) { + // WebRTC only supports one RTX codec at a time. + if (rtx_registered) { + LOG(LS_ERROR) << "Only one RTX codec at a time is supported."; + return false; + } std::map::iterator apt_it = associated_payload_types_.find( it->plType); bool valid_apt = false; @@ -3876,10 +3880,11 @@ bool WebRtcVideoMediaChannel::SetReceiveCodecs( return false; } if (engine()->vie()->rtp()->SetRtxReceivePayloadType( - channel_id, it->plType, apt_it->second) != 0) { + channel_id, it->plType) != 0) { LOG_RTCERR2(SetRtxReceivePayloadType, channel_id, it->plType); return false; } + rtx_registered = true; continue; } if (engine()->vie()->codec()->SetReceiveCodec(channel_id, *it) != 0) { @@ -4015,14 +4020,11 @@ bool WebRtcVideoMediaChannel::SetSendParams( // NOTE: SetRtxSendPayloadType must be called after all SSRCs are // configured. Otherwise ssrc's configured after this point will use // the primary PT for RTX. - std::map::const_iterator it; - for (it = send_rtx_apt_types_.begin(); it != send_rtx_apt_types_.end(); - ++it) { - if (engine()->vie()->rtp()->SetRtxSendPayloadType(channel_id, it->first, - it->second) != 0) { - LOG_RTCERR3(SetRtxSendPayloadType, channel_id, it->first, it->second); - return false; - } + if (send_rtx_type_ != -1 && + engine()->vie()->rtp()->SetRtxSendPayloadType(channel_id, + send_rtx_type_) != 0) { + LOG_RTCERR2(SetRtxSendPayloadType, channel_id, send_rtx_type_); + return false; } send_channel->set_send_params(send_params); diff --git a/talk/media/webrtc/webrtcvideoengine.h b/talk/media/webrtc/webrtcvideoengine.h index 85d4611b0..78a10e0bf 100644 --- a/talk/media/webrtc/webrtcvideoengine.h +++ b/talk/media/webrtc/webrtcvideoengine.h @@ -511,8 +511,7 @@ class WebRtcVideoMediaChannel : public rtc::MessageHandler, // Global send side state. SendChannelMap send_channels_; rtc::scoped_ptr send_codec_; - // A map from RTX payload types to their associated payload types. - std::map send_rtx_apt_types_; + int send_rtx_type_; int send_red_type_; int send_fec_type_; bool sending_; diff --git a/talk/media/webrtc/webrtcvideoengine2.cc b/talk/media/webrtc/webrtcvideoengine2.cc index 0239c8947..d066eb9c9 100644 --- a/talk/media/webrtc/webrtcvideoengine2.cc +++ b/talk/media/webrtc/webrtcvideoengine2.cc @@ -121,15 +121,6 @@ static void MergeFecConfig(const webrtc::FecConfig& other, } output->red_payload_type = other.red_payload_type; } - if (other.rtx_payload_type != -1) { - if (output->rtx_payload_type != -1 && - output->rtx_payload_type != other.rtx_payload_type) { - LOG(LS_WARNING) << "Conflict merging rtx_payload_type configs: " - << output->rtx_payload_type << " and " - << other.rtx_payload_type; - } - output->rtx_payload_type = other.rtx_payload_type; - } } } // namespace @@ -2079,7 +2070,6 @@ bool WebRtcVideoChannel2::VideoCodecSettings::operator==( return codec == other.codec && fec.ulpfec_payload_type == other.fec.ulpfec_payload_type && fec.red_payload_type == other.fec.red_payload_type && - fec.rtx_payload_type == other.fec.rtx_payload_type && rtx_payload_type == other.rtx_payload_type; } @@ -2152,24 +2142,17 @@ WebRtcVideoChannel2::MapCodecs(const std::vector& codecs) { LOG(LS_ERROR) << "RTX mapped to payload not in codec list."; return std::vector(); } - if (payload_codec_type[it->first] != VideoCodec::CODEC_VIDEO && - payload_codec_type[it->first] != VideoCodec::CODEC_RED) { - LOG(LS_ERROR) << "RTX not mapped to regular video codec or RED codec."; + if (payload_codec_type[it->first] != VideoCodec::CODEC_VIDEO) { + LOG(LS_ERROR) << "RTX not mapped to regular video codec."; return std::vector(); } - - if (it->first == fec_settings.red_payload_type) { - fec_settings.rtx_payload_type = it->second; - } } // TODO(pbos): Write tests that figure out that I have not verified that RTX // codecs aren't mapped to bogus payloads. for (size_t i = 0; i < video_codecs.size(); ++i) { video_codecs[i].fec = fec_settings; - if (rtx_mapping[video_codecs[i].codec.id] != 0 && - rtx_mapping[video_codecs[i].codec.id] != - fec_settings.red_payload_type) { + if (rtx_mapping[video_codecs[i].codec.id] != 0) { video_codecs[i].rtx_payload_type = rtx_mapping[video_codecs[i].codec.id]; } } diff --git a/talk/media/webrtc/webrtcvideoengine2.h b/talk/media/webrtc/webrtcvideoengine2.h index 52428d37b..2318971db 100644 --- a/talk/media/webrtc/webrtcvideoengine2.h +++ b/talk/media/webrtc/webrtcvideoengine2.h @@ -285,6 +285,7 @@ class WebRtcVideoChannel2 : public rtc::MessageHandler, struct VideoCodecSettings { VideoCodecSettings(); + bool operator ==(const VideoCodecSettings& other) const; VideoCodec codec; diff --git a/talk/media/webrtc/webrtcvideoengine2_unittest.cc b/talk/media/webrtc/webrtcvideoengine2_unittest.cc index 7ceabcabc..93660a572 100644 --- a/talk/media/webrtc/webrtcvideoengine2_unittest.cc +++ b/talk/media/webrtc/webrtcvideoengine2_unittest.cc @@ -34,7 +34,6 @@ #include "talk/media/webrtc/fakewebrtcvideoengine.h" #include "talk/media/webrtc/simulcast.h" #include "talk/media/webrtc/webrtcvideochannelfactory.h" -#include "talk/media/webrtc/webrtcvideoengine.h" #include "talk/media/webrtc/webrtcvideoengine2.h" #include "talk/media/webrtc/webrtcvideoengine2_unittest.h" #include "talk/media/webrtc/webrtcvoiceengine.h" @@ -74,19 +73,6 @@ void VerifyCodecHasDefaultFeedbackParams(const cricket::VideoCodec& codec) { cricket::kRtcpFbParamCcm, cricket::kRtcpFbCcmParamFir))); } -void VerifySendStreamHasRtxTypes(const webrtc::VideoSendStream::Config& config, - const std::map& rtx_types) { - std::map::const_iterator it; - it = rtx_types.find(config.encoder_settings.payload_type); - EXPECT_TRUE(it != rtx_types.end() && - it->second == config.rtp.rtx.payload_type); - - if (config.rtp.fec.rtx_payload_type != -1) { - it = rtx_types.find(config.rtp.fec.red_payload_type); - EXPECT_TRUE(it != rtx_types.end() && - it->second == config.rtp.fec.rtx_payload_type); - } -} } // namespace namespace cricket { @@ -363,11 +349,7 @@ class WebRtcVideoEngine2Test : public ::testing::Test { } else if (engine_codecs[i].name == "ulpfec") { default_ulpfec_codec_ = engine_codecs[i]; } else if (engine_codecs[i].name == "rtx") { - int associated_payload_type; - if (engine_codecs[i].GetParam(kCodecParamAssociatedPayloadType, - &associated_payload_type)) { - default_apt_rtx_types_[associated_payload_type] = engine_codecs[i].id; - } + default_rtx_codec_ = engine_codecs[i]; } else if (!codec_set) { default_codec_ = engine_codecs[i]; codec_set = true; @@ -406,7 +388,7 @@ class WebRtcVideoEngine2Test : public ::testing::Test { VideoCodec default_codec_; VideoCodec default_red_codec_; VideoCodec default_ulpfec_codec_; - std::map default_apt_rtx_types_; + VideoCodec default_rtx_codec_; }; TEST_F(WebRtcVideoEngine2Test, ConfiguresAvSyncForFirstReceiveChannel) { @@ -449,7 +431,7 @@ TEST_F(WebRtcVideoEngine2Test, ConfiguresAvSyncForFirstReceiveChannel) { TEST_F(WebRtcVideoEngine2Test, FindCodec) { const std::vector& c = engine_.codecs(); - EXPECT_EQ(cricket::DefaultVideoCodecList().size(), c.size()); + EXPECT_EQ(4U, c.size()); cricket::VideoCodec vp8(104, "VP8", 320, 200, 30, 0); EXPECT_TRUE(engine_.FindCodec(vp8)); @@ -1601,7 +1583,8 @@ TEST_F(WebRtcVideoChannel2Test, SetDefaultSendCodecs) { EXPECT_EQ(1u, config.rtp.rtx.ssrcs.size()); EXPECT_EQ(kRtxSsrcs1[0], config.rtp.rtx.ssrcs[0]); - VerifySendStreamHasRtxTypes(config, default_apt_rtx_types_); + EXPECT_EQ(static_cast(default_rtx_codec_.id), + config.rtp.rtx.payload_type); // TODO(juberti): Check RTCP, PLI, TMMBR. } diff --git a/talk/media/webrtc/webrtcvideoengine_unittest.cc b/talk/media/webrtc/webrtcvideoengine_unittest.cc index 93560a9b6..24e9af283 100644 --- a/talk/media/webrtc/webrtcvideoengine_unittest.cc +++ b/talk/media/webrtc/webrtcvideoengine_unittest.cc @@ -38,8 +38,6 @@ #include "talk/media/webrtc/webrtcvideoframe.h" #include "talk/media/webrtc/webrtcvoiceengine.h" #include "talk/session/media/mediasession.h" -#include "testing/gmock/include/gmock/gmock.h" -#include "testing/gtest/include/gtest/gtest.h" #include "webrtc/base/fakecpumonitor.h" #include "webrtc/base/gunit.h" #include "webrtc/base/logging.h" @@ -756,8 +754,7 @@ TEST_F(WebRtcVideoEngineTestFake, SetRecvCodecsWithRtx) { EXPECT_FALSE(vie_.ReceiveCodecRegistered(channel_num, wcodec)); // The RTX payload type should have been set. - EXPECT_THAT(vie_.GetRtxRecvPayloadType(channel_num), - ::testing::ElementsAre(rtx_codec.id)); + EXPECT_EQ(rtx_codec.id, vie_.GetRtxRecvPayloadType(channel_num)); } // Test that RTX packets are routed to the default video channel if @@ -2082,8 +2079,7 @@ TEST_F(WebRtcVideoEngineTestFake, SetSendCodecsWithExternalH264) { codecs.push_back(rtx_codec); EXPECT_TRUE(channel_->SetSendCodecs(codecs)); - EXPECT_THAT(vie_.GetRtxSendPayloadType(channel_num), - ::testing::ElementsAre(96)); + EXPECT_EQ(96, vie_.GetRtxSendPayloadType(channel_num)); cricket::StreamParams params = cricket::StreamParams::CreateLegacy(kSsrcs1[0]); @@ -2121,8 +2117,7 @@ TEST_F(WebRtcVideoEngineTestFake, SetSendCodecsWithVP8AndExternalH264) { // The first matched codec should be set, i.e., H.264. - EXPECT_THAT(vie_.GetRtxSendPayloadType(channel_num), - ::testing::ElementsAre(96, 97)); + EXPECT_EQ(96, vie_.GetRtxSendPayloadType(channel_num)); cricket::StreamParams params = cricket::StreamParams::CreateLegacy(kSsrcs1[0]); @@ -2159,8 +2154,7 @@ TEST_F(WebRtcVideoEngineTestFake, SetRecvCodecsWithExternalH264) { codecs.push_back(rtx_codec); EXPECT_TRUE(channel_->SetRecvCodecs(codecs)); - EXPECT_THAT(vie_.GetRtxRecvPayloadType(channel_num), - ::testing::ElementsAre(96)); + EXPECT_EQ(96, vie_.GetRtxRecvPayloadType(channel_num)); cricket::StreamParams params = cricket::StreamParams::CreateLegacy(kSsrcs1[0]); @@ -2195,21 +2189,17 @@ TEST_F(WebRtcVideoEngineTestFake, SetRecvCodecsWithVP8AndExternalH264) { cricket::VideoCodec rtx_codec2(96, "rtx", 0, 0, 0, 0); rtx_codec2.SetParam("apt", kVP8Codec.id); codecs.push_back(kVP8Codec); - codecs.push_back(rtx_codec2); - // Now WebRTC supports setting multiple RTX codecs at a time. - EXPECT_TRUE(channel_->SetRecvCodecs(codecs)); - EXPECT_THAT(vie_.GetRtxRecvPayloadType(channel_num), - ::testing::ElementsAre(96, 97)); + codecs.push_back(rtx_codec); + // Should fail since WebRTC only supports one RTX codec at a time. + EXPECT_FALSE(channel_->SetRecvCodecs(codecs)); codecs.pop_back(); + // One RTX codec should be fine. EXPECT_TRUE(channel_->SetRecvCodecs(codecs)); - // TODO(changbin): The Rtx key can still be found from the Rtx-Apt map - // if the new codec list doesn't assign it with a new value. - // Should pass a map to SetRtxRecvPayloadType in future. - EXPECT_THAT(vie_.GetRtxRecvPayloadType(channel_num), - ::testing::ElementsAre(96, 97)); + // The RTX payload type should have been set. + EXPECT_EQ(rtx_codec.id, vie_.GetRtxRecvPayloadType(channel_num)); } #endif @@ -2259,7 +2249,7 @@ TEST_F(WebRtcVideoEngineTestFake, CaptureFrameTimestampToNtpTimestamp) { TEST_F(WebRtcVideoEngineTest, FindCodec) { // We should not need to init engine in order to get codecs. const std::vector& c = engine_.codecs(); - EXPECT_EQ(cricket::DefaultVideoCodecList().size(), c.size()); + EXPECT_EQ(4U, c.size()); cricket::VideoCodec vp8(104, "VP8", 320, 200, 30, 0); EXPECT_TRUE(engine_.FindCodec(vp8)); @@ -2306,7 +2296,7 @@ TEST_F(WebRtcVideoEngineTest, RtxCodecHasAptSet) { std::vector::const_iterator it; bool apt_checked = false; for (it = engine_.codecs().begin(); it != engine_.codecs().end(); ++it) { - if (_stricmp(cricket::kRtxCodecName, it->name.c_str()) || it->id != 96) { + if (_stricmp(cricket::kRtxCodecName, it->name.c_str()) && it->id != 96) { continue; } int apt; @@ -3212,8 +3202,7 @@ TEST_F(WebRtcVideoEngineSimulcastTestFake, TestStreamWithRtx) { EXPECT_TRUE(channel_->SetSendCodecs(codec_list)); // RTX payload type should now be set. - EXPECT_THAT(vie_.GetRtxSendPayloadType(channel_num), - ::testing::ElementsAre(96)); + EXPECT_EQ(96, vie_.GetRtxSendPayloadType(channel_num)); // Verify all SSRCs are set after SetSendCodecs. EXPECT_EQ(3, vie_.GetNumSsrcs(channel_num)); diff --git a/webrtc/config.cc b/webrtc/config.cc index a8cb5234e..357f63670 100644 --- a/webrtc/config.cc +++ b/webrtc/config.cc @@ -17,7 +17,6 @@ std::string FecConfig::ToString() const { std::stringstream ss; ss << "{ulpfec_payload_type: " << ulpfec_payload_type; ss << ", red_payload_type: " << red_payload_type; - ss << ", rtx_payload_type: " << rtx_payload_type; ss << '}'; return ss.str(); } diff --git a/webrtc/config.h b/webrtc/config.h index f5a5144ab..cd5a23ffc 100644 --- a/webrtc/config.h +++ b/webrtc/config.h @@ -54,17 +54,13 @@ struct NackConfig { // Settings for forward error correction, see RFC 5109 for details. Set the // payload types to '-1' to disable. struct FecConfig { - FecConfig() - : ulpfec_payload_type(-1), red_payload_type(-1), rtx_payload_type(-1) {} + FecConfig() : ulpfec_payload_type(-1), red_payload_type(-1) {} std::string ToString() const; // Payload type used for ULPFEC packets. int ulpfec_payload_type; // Payload type used for RED packets. int red_payload_type; - - // Rtx payload type for RED payload. - int rtx_payload_type; }; // RTP header extension to use for the video stream, see RFC 5285. diff --git a/webrtc/modules/rtp_rtcp/interface/rtp_payload_registry.h b/webrtc/modules/rtp_rtcp/interface/rtp_payload_registry.h index e73bac3ce..f58eaead6 100644 --- a/webrtc/modules/rtp_rtcp/interface/rtp_payload_registry.h +++ b/webrtc/modules/rtp_rtcp/interface/rtp_payload_registry.h @@ -79,7 +79,7 @@ class RTPPayloadRegistry { bool GetRtxSsrc(uint32_t* ssrc) const; - void SetRtxPayloadType(int payload_type, int associated_payload_type); + void SetRtxPayloadType(int payload_type); bool IsRtx(const RTPHeader& header) const; @@ -158,8 +158,7 @@ class RTPPayloadRegistry { int8_t last_received_payload_type_; int8_t last_received_media_payload_type_; bool rtx_; - // Mapping rtx_apt_types_[rtx] = apt. - std::map rtx_apt_types_; + int8_t payload_type_rtx_; uint32_t ssrc_rtx_; }; diff --git a/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h b/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h index f1466ae41..6b9a5542e 100644 --- a/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h @@ -228,8 +228,7 @@ class RtpRtcp : public Module { // Sets the payload type to use when sending RTX packets. Note that this // doesn't enable RTX, only the payload type is set. - virtual void SetRtxSendPayloadType(int payload_type, - int associated_payload_type) = 0; + virtual void SetRtxSendPayloadType(int payload_type) = 0; /* * Get status of sending RTX (RFC 4588) on a specific SSRC. diff --git a/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h index 7c8171596..31fade479 100644 --- a/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h @@ -101,7 +101,8 @@ class MockRtpRtcp : public RtpRtcp { void(int* modes, uint32_t* ssrc, int* payload_type)); MOCK_METHOD1(SetRtxSsrc, void(uint32_t)); - MOCK_METHOD2(SetRtxSendPayloadType, void(int, int)); + MOCK_METHOD1(SetRtxSendPayloadType, + void(int)); MOCK_METHOD1(SetSendingStatus, int32_t(const bool sending)); MOCK_CONST_METHOD0(Sending, diff --git a/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc b/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc index ad553ac78..bfe0af076 100644 --- a/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc @@ -32,8 +32,6 @@ const uint16_t kTestSequenceNumber = 2345; const uint32_t kTestNumberOfPackets = 1350; const int kTestNumberOfRtxPackets = 149; const int kNumFrames = 30; -const int kPayloadType = 123; -const int kRtxPayloadType = 98; class VerifyingRtxReceiver : public NullRtpData { @@ -208,17 +206,15 @@ class RtpRtcpRtxNackTest : public ::testing::Test { VideoCodec video_codec; memset(&video_codec, 0, sizeof(video_codec)); - video_codec.plType = kPayloadType; + video_codec.plType = 123; memcpy(video_codec.plName, "I420", 5); EXPECT_EQ(0, rtp_rtcp_module_->RegisterSendPayload(video_codec)); - rtp_rtcp_module_->SetRtxSendPayloadType(kRtxPayloadType, kPayloadType); EXPECT_EQ(0, rtp_receiver_->RegisterReceivePayload(video_codec.plName, video_codec.plType, 90000, 0, video_codec.maxBitrate)); - rtp_payload_registry_.SetRtxPayloadType(kRtxPayloadType, kPayloadType); for (size_t n = 0; n < payload_data_length; n++) { payload_data[n] = n % 10; @@ -269,9 +265,12 @@ class RtpRtcpRtxNackTest : public ::testing::Test { uint32_t timestamp = 3000; uint16_t nack_list[kVideoNackListSize]; for (int frame = 0; frame < kNumFrames; ++frame) { - EXPECT_EQ(0, rtp_rtcp_module_->SendOutgoingData( - webrtc::kVideoFrameDelta, kPayloadType, timestamp, - timestamp / 90, payload_data, payload_data_length)); + EXPECT_EQ(0, rtp_rtcp_module_->SendOutgoingData(webrtc::kVideoFrameDelta, + 123, + timestamp, + timestamp / 90, + payload_data, + payload_data_length)); int length = BuildNackList(nack_list); if (length > 0) rtp_rtcp_module_->SendNACK(nack_list, length); @@ -314,9 +313,12 @@ TEST_F(RtpRtcpRtxNackTest, LongNackList) { // Send 30 frames which at the default size is roughly what we need to get // enough packets. for (int frame = 0; frame < kNumFrames; ++frame) { - EXPECT_EQ(0, rtp_rtcp_module_->SendOutgoingData( - webrtc::kVideoFrameDelta, kPayloadType, timestamp, - timestamp / 90, payload_data, payload_data_length)); + EXPECT_EQ(0, rtp_rtcp_module_->SendOutgoingData(webrtc::kVideoFrameDelta, + 123, + timestamp, + timestamp / 90, + payload_data, + payload_data_length)); // Prepare next frame. timestamp += 3000; fake_clock.AdvanceTimeMilliseconds(33); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc b/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc index 18aeb1167..727a4d3ac 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc @@ -24,6 +24,7 @@ RTPPayloadRegistry::RTPPayloadRegistry( last_received_payload_type_(-1), last_received_media_payload_type_(-1), rtx_(false), + payload_type_rtx_(-1), ssrc_rtx_(0) {} RTPPayloadRegistry::~RTPPayloadRegistry() { @@ -263,25 +264,19 @@ bool RTPPayloadRegistry::RestoreOriginalPacket(uint8_t** restored_packet, RtpUtility::AssignUWord32ToBuffer(*restored_packet + 8, original_ssrc); CriticalSectionScoped cs(crit_sect_.get()); - if (!rtx_) - return true; - if (rtx_apt_types_.empty()) { - LOG(LS_WARNING) << "No RTX is set, dropping packet."; - return false; - } - std::map::const_iterator it = - rtx_apt_types_.find(header.payloadType); - if (it != rtx_apt_types_.end()) { - (*restored_packet)[1] = static_cast(it->second); - if (header.markerBit) { - (*restored_packet)[1] |= kRtpMarkerBitMask; // Marker bit is set. + if (payload_type_rtx_ != -1) { + if (header.payloadType == payload_type_rtx_ && + incoming_payload_type_ != -1) { + (*restored_packet)[1] = static_cast(incoming_payload_type_); + if (header.markerBit) { + (*restored_packet)[1] |= kRtpMarkerBitMask; // Marker bit is set. + } + } else { + LOG(LS_WARNING) << "Incorrect RTX configuration, dropping packet."; + return false; } - } else { - LOG(LS_WARNING) << "Incorrect RTX configuration, dropping packet."; - return false; } - return true; } @@ -297,12 +292,10 @@ bool RTPPayloadRegistry::GetRtxSsrc(uint32_t* ssrc) const { return rtx_; } -void RTPPayloadRegistry::SetRtxPayloadType(int payload_type, - int associated_payload_type) { +void RTPPayloadRegistry::SetRtxPayloadType(int payload_type) { CriticalSectionScoped cs(crit_sect_.get()); assert(payload_type >= 0); - assert(associated_payload_type >= 0); - rtx_apt_types_[payload_type] = associated_payload_type; + payload_type_rtx_ = payload_type; rtx_ = true; } diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 15286cb13..4dd74cee4 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -257,9 +257,8 @@ void ModuleRtpRtcpImpl::SetRtxSsrc(uint32_t ssrc) { rtp_sender_.SetRtxSsrc(ssrc); } -void ModuleRtpRtcpImpl::SetRtxSendPayloadType(int payload_type, - int associated_payload_type) { - rtp_sender_.SetRtxPayloadType(payload_type, associated_payload_type); +void ModuleRtpRtcpImpl::SetRtxSendPayloadType(int payload_type) { + rtp_sender_.SetRtxPayloadType(payload_type); } int32_t ModuleRtpRtcpImpl::IncomingRtcpPacket( diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h index 6e0e90756..ec067832f 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -94,8 +94,7 @@ class ModuleRtpRtcpImpl : public RtpRtcp { virtual void SetRtxSsrc(uint32_t ssrc) OVERRIDE; - virtual void SetRtxSendPayloadType(int payload_type, - int associated_payload_type) OVERRIDE; + virtual void SetRtxSendPayloadType(int payload_type) OVERRIDE; // Sends kRtcpByeCode when going from true to false. virtual int32_t SetSendingStatus(bool sending) OVERRIDE; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc index 726d6f1bb..3f9e95fa0 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc @@ -691,7 +691,7 @@ TEST_F(RtpSendingTest, DISABLED_RoundRobinPaddingRtx) { // as otherwise the timestmap used for BWE will be broken. senders_[i]->RegisterSendRtpHeaderExtension(kRtpExtensionAbsoluteSendTime, 1); - senders_[i]->SetRtxSendPayloadType(96, 100); + senders_[i]->SetRtxSendPayloadType(96); senders_[i]->SetRtxSsrc(kSenderRtxSsrc + i); senders_[i]->SetRTXSendStatus(kRtxRetransmitted); } @@ -716,7 +716,7 @@ TEST_F(RtpSendingTest, DISABLED_RoundRobinPaddingRtx) { TEST_F(RtpSendingTest, DISABLED_RoundRobinPaddingRtxRedundantPayloads) { for (int i = 1; i < codec_.numberOfSimulcastStreams + 1; ++i) { - senders_[i]->SetRtxSendPayloadType(96, 100); + senders_[i]->SetRtxSendPayloadType(96); senders_[i]->SetRtxSsrc(kSenderRtxSsrc + i); senders_[i]->SetRTXSendStatus(kRtxRetransmitted | kRtxRedundantPayloads); senders_[i]->SetStorePacketsStatus(true, 100); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc index 42df57953..a1810f24f 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc @@ -147,6 +147,7 @@ RTPSender::RTPSender(int32_t id, last_packet_marker_bit_(false), csrcs_(), rtx_(kRtxOff), + payload_type_rtx_(-1), target_bitrate_critsect_(CriticalSectionWrapper::CreateCriticalSection()), target_bitrate_(0) { memset(nack_byte_count_times_, 0, sizeof(nack_byte_count_times_)); @@ -402,17 +403,12 @@ void RTPSender::RTXStatus(int* mode, uint32_t* ssrc, CriticalSectionScoped cs(send_critsect_); *mode = rtx_; *ssrc = ssrc_rtx_; - - std::map::const_iterator it = apt_rtx_types_.find(payload_type_); - *payload_type = (it == apt_rtx_types_.end()) ? -1 : it->second; + *payload_type = payload_type_rtx_; } -void RTPSender::SetRtxPayloadType(int payload_type, - int associated_payload_type) { +void RTPSender::SetRtxPayloadType(int payload_type) { CriticalSectionScoped cs(send_critsect_); - assert(payload_type >= 0); - assert(associated_payload_type >= 0); - apt_rtx_types_[associated_payload_type] = payload_type; + payload_type_rtx_ = payload_type; } int32_t RTPSender::CheckPayloadType(int8_t payload_type, @@ -614,14 +610,8 @@ size_t RTPSender::SendPadData(uint32_t timestamp, ssrc = ssrc_rtx_; sequence_number = sequence_number_rtx_; ++sequence_number_rtx_; - - if (apt_rtx_types_.empty()) - return 0; - std::map::const_iterator it = - apt_rtx_types_.find(payload_type_); - payload_type = it != apt_rtx_types_.end() - ? it->second - : apt_rtx_types_.begin()->second; + payload_type = ((rtx_ & kRtxRedundantPayloads) > 0) ? payload_type_rtx_ + : payload_type_; over_rtx = true; } } @@ -1670,10 +1660,9 @@ void RTPSender::BuildRtxPacket(uint8_t* buffer, size_t* length, // Add original RTP header. memcpy(data_buffer_rtx, buffer, rtp_header.headerLength); - std::map::const_iterator it = - apt_rtx_types_.find(rtp_header.payloadType); - if (it != apt_rtx_types_.end()) { - data_buffer_rtx[1] = static_cast(it->second); + // Replace payload type, if a specific type is set for RTX. + if (payload_type_rtx_ != -1) { + data_buffer_rtx[1] = static_cast(payload_type_rtx_); if (rtp_header.markerBit) data_buffer_rtx[1] |= kRtpMarkerBitMask; } diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.h b/webrtc/modules/rtp_rtcp/source/rtp_sender.h index c807a5baa..2703fea76 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.h @@ -191,7 +191,7 @@ class RTPSender : public RTPSenderInterface { uint32_t RtxSsrc() const; void SetRtxSsrc(uint32_t ssrc); - void SetRtxPayloadType(int payloadType, int associated_payload_type); + void SetRtxPayloadType(int payloadType); // Functions wrapping RTPSenderInterface. virtual int32_t BuildRTPheader( @@ -391,8 +391,7 @@ class RTPSender : public RTPSenderInterface { std::vector csrcs_ GUARDED_BY(send_critsect_); int rtx_ GUARDED_BY(send_critsect_); uint32_t ssrc_rtx_ GUARDED_BY(send_critsect_); - // Mapping apt_rtx_types_[apt] = rtx. - std::map apt_rtx_types_ GUARDED_BY(send_critsect_); + int payload_type_rtx_ GUARDED_BY(send_critsect_); // Note: Don't access this variable directly, always go through // SetTargetBitrateKbps or GetTargetBitrateKbps. Also remember diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc index 0a95807a5..3946c449c 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -30,7 +30,6 @@ namespace { const int kTransmissionTimeOffsetExtensionId = 1; const int kAbsoluteSendTimeExtensionId = 14; const int kPayload = 100; -const int kRtxPayload = 98; const uint32_t kTimestamp = 10; const uint16_t kSeqNum = 33; const int kTimeOffset = 22222; @@ -655,7 +654,6 @@ TEST_F(RtpSenderTest, SendRedundantPayloads) { rtp_sender_.reset(new RTPSender(0, false, &fake_clock_, &transport, NULL, &mock_paced_sender_, NULL, NULL, NULL)); rtp_sender_->SetSequenceNumber(kSeqNum); - rtp_sender_->SetRtxPayloadType(kRtxPayload, kPayload); // Make all packets go through the pacer. EXPECT_CALL(mock_paced_sender_, SendPacket(PacedSender::kNormalPriority, _, _, _, _, _)). @@ -1125,7 +1123,7 @@ TEST_F(RtpSenderTest, BytesReportedCorrectly) { const uint8_t kPayloadType = 127; rtp_sender_->SetSSRC(1234); rtp_sender_->SetRtxSsrc(4321); - rtp_sender_->SetRtxPayloadType(kPayloadType - 1, kPayloadType); + rtp_sender_->SetRtxPayloadType(kPayloadType - 1); rtp_sender_->SetRTXStatus(kRtxRetransmitted | kRtxRedundantPayloads); ASSERT_EQ( diff --git a/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc b/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc index f8b601180..ccc8cf26a 100644 --- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc +++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc @@ -106,31 +106,33 @@ TEST_F(RtpRtcpAPITest, RtxSender) { unsigned int ssrc = 0; int rtx_mode = kRtxOff; const int kRtxPayloadType = 119; - const int kPayloadType = 100; + int payload_type = -1; module->SetRTXSendStatus(kRtxRetransmitted); - module->SetRtxSendPayloadType(kRtxPayloadType, kPayloadType); + module->SetRtxSendPayloadType(kRtxPayloadType); module->SetRtxSsrc(1); - int payload_type; module->RTXSendStatus(&rtx_mode, &ssrc, &payload_type); EXPECT_EQ(kRtxRetransmitted, rtx_mode); EXPECT_EQ(1u, ssrc); + EXPECT_EQ(kRtxPayloadType, payload_type); rtx_mode = kRtxOff; module->SetRTXSendStatus(kRtxOff); - module->SetRtxSendPayloadType(kRtxPayloadType, kPayloadType); + payload_type = -1; + module->SetRtxSendPayloadType(kRtxPayloadType); module->RTXSendStatus(&rtx_mode, &ssrc, &payload_type); EXPECT_EQ(kRtxOff, rtx_mode); + EXPECT_EQ(kRtxPayloadType, payload_type); module->SetRTXSendStatus(kRtxRetransmitted); module->RTXSendStatus(&rtx_mode, &ssrc, &payload_type); EXPECT_EQ(kRtxRetransmitted, rtx_mode); + EXPECT_EQ(kRtxPayloadType, payload_type); } TEST_F(RtpRtcpAPITest, RtxReceiver) { const uint32_t kRtxSsrc = 1; const int kRtxPayloadType = 119; - const int kPayloadType = 100; EXPECT_FALSE(rtp_payload_registry_->RtxEnabled()); rtp_payload_registry_->SetRtxSsrc(kRtxSsrc); - rtp_payload_registry_->SetRtxPayloadType(kRtxPayloadType, kPayloadType); + rtp_payload_registry_->SetRtxPayloadType(kRtxPayloadType); EXPECT_TRUE(rtp_payload_registry_->RtxEnabled()); RTPHeader rtx_header; rtx_header.ssrc = kRtxSsrc; diff --git a/webrtc/test/call_test.cc b/webrtc/test/call_test.cc index dbd514eae..126c71635 100644 --- a/webrtc/test/call_test.cc +++ b/webrtc/test/call_test.cc @@ -151,7 +151,6 @@ const uint8_t CallTest::kSendPayloadType = 100; const uint8_t CallTest::kFakeSendPayloadType = 125; const uint8_t CallTest::kSendRtxPayloadType = 98; const uint8_t CallTest::kRedPayloadType = 118; -const uint8_t CallTest::kRtxRedPayloadType = 99; const uint8_t CallTest::kUlpfecPayloadType = 119; const uint32_t CallTest::kSendRtxSsrcs[kNumSsrcs] = {0xBADCAFD, 0xBADCAFE, 0xBADCAFF}; diff --git a/webrtc/test/call_test.h b/webrtc/test/call_test.h index 1e0173364..b9a091b45 100644 --- a/webrtc/test/call_test.h +++ b/webrtc/test/call_test.h @@ -37,7 +37,6 @@ class CallTest : public ::testing::Test { static const uint8_t kSendRtxPayloadType; static const uint8_t kFakeSendPayloadType; static const uint8_t kRedPayloadType; - static const uint8_t kRtxRedPayloadType; static const uint8_t kUlpfecPayloadType; static const uint32_t kSendRtxSsrcs[kNumSsrcs]; static const uint32_t kSendSsrcs[kNumSsrcs]; diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc index 65d9568b5..f82055456 100644 --- a/webrtc/video/end_to_end_tests.cc +++ b/webrtc/video/end_to_end_tests.cc @@ -69,7 +69,7 @@ class EndToEndTest : public test::CallTest { } }; - void DecodesRetransmittedFrame(bool use_rtx, bool use_red); + void DecodesRetransmittedFrame(bool retransmit_over_rtx); void ReceivesPliAndRecovers(int rtp_history_ms); void RespectsRtcpMode(newapi::RtcpMode rtcp_mode); void TestXrReceiverReferenceTimeReport(bool enable_rrtr); @@ -538,16 +538,16 @@ TEST_F(EndToEndTest, CanReceiveFec) { // This test drops second RTP packet with a marker bit set, makes sure it's // retransmitted and renders. Retransmission SSRCs are also checked. -void EndToEndTest::DecodesRetransmittedFrame(bool use_rtx, bool use_red) { +void EndToEndTest::DecodesRetransmittedFrame(bool retransmit_over_rtx) { static const int kDroppedFrameNumber = 2; class RetransmissionObserver : public test::EndToEndTest, public I420FrameCallback { public: - explicit RetransmissionObserver(bool use_rtx, bool use_red) + explicit RetransmissionObserver(bool expect_rtx) : EndToEndTest(kDefaultTimeoutMs), - payload_type_(GetPayloadType(false, use_red)), - retransmission_ssrc_(use_rtx ? kSendRtxSsrcs[0] : kSendSsrcs[0]), - retransmission_payload_type_(GetPayloadType(use_rtx, use_red)), + retransmission_ssrc_(expect_rtx ? kSendRtxSsrcs[0] : kSendSsrcs[0]), + retransmission_payload_type_(expect_rtx ? kSendRtxPayloadType + : kFakeSendPayloadType), marker_bits_observed_(0), retransmitted_timestamp_(0), frame_retransmitted_(false) {} @@ -565,7 +565,7 @@ void EndToEndTest::DecodesRetransmittedFrame(bool use_rtx, bool use_red) { } EXPECT_EQ(kSendSsrcs[0], header.ssrc); - EXPECT_EQ(payload_type_, header.payloadType); + EXPECT_EQ(kFakeSendPayloadType, header.payloadType); // Found the second frame's final packet, drop this and expect a // retransmission. @@ -592,25 +592,13 @@ void EndToEndTest::DecodesRetransmittedFrame(bool use_rtx, bool use_red) { send_config->rtp.nack.rtp_history_ms = kNackRtpHistoryMs; (*receive_configs)[0].pre_render_callback = this; (*receive_configs)[0].rtp.nack.rtp_history_ms = kNackRtpHistoryMs; - - if (payload_type_ == kRedPayloadType) { - send_config->rtp.fec.ulpfec_payload_type = kUlpfecPayloadType; - send_config->rtp.fec.red_payload_type = kRedPayloadType; - (*receive_configs)[0].rtp.fec.red_payload_type = kRedPayloadType; - (*receive_configs)[0].rtp.fec.ulpfec_payload_type = kUlpfecPayloadType; - } - 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[kFakeSendPayloadType].ssrc = + (*receive_configs)[0].rtp.rtx[kSendRtxPayloadType].ssrc = kSendRtxSsrcs[0]; - (*receive_configs)[0].rtp.rtx[kFakeSendPayloadType].payload_type = + (*receive_configs)[0].rtp.rtx[kSendRtxPayloadType].payload_type = kSendRtxPayloadType; - if (payload_type_ == kRedPayloadType) { - send_config->rtp.fec.rtx_payload_type = kRtxRedPayloadType; - (*receive_configs)[0].rtp.fec.rtx_payload_type = kRtxRedPayloadType; - } } } @@ -619,36 +607,22 @@ void EndToEndTest::DecodesRetransmittedFrame(bool use_rtx, bool use_red) { << "Timed out while waiting for retransmission to render."; } - int GetPayloadType(bool use_rtx, bool use_red) { - return use_red ? (use_rtx ? kRtxRedPayloadType : kRedPayloadType) - : (use_rtx ? kSendRtxPayloadType : kFakeSendPayloadType); - } - - const int payload_type_; const uint32_t retransmission_ssrc_; const int retransmission_payload_type_; int marker_bits_observed_; uint32_t retransmitted_timestamp_; bool frame_retransmitted_; - } test(use_rtx, use_red); + } test(retransmit_over_rtx); RunBaseTest(&test); } TEST_F(EndToEndTest, DecodesRetransmittedFrame) { - DecodesRetransmittedFrame(false, false); + DecodesRetransmittedFrame(false); } TEST_F(EndToEndTest, DecodesRetransmittedFrameOverRtx) { - DecodesRetransmittedFrame(true, false); -} - -TEST_F(EndToEndTest, DecodesRetransmittedFrameByRed) { - DecodesRetransmittedFrame(false, true); -} - -TEST_F(EndToEndTest, DecodesRetransmittedFrameByRedOverRtx) { - DecodesRetransmittedFrame(true, true); + DecodesRetransmittedFrame(true); } TEST_F(EndToEndTest, UsesFrameCallbacks) { @@ -2294,4 +2268,5 @@ TEST_F(EndToEndTest, CanCreateAndDestroyManyVideoStreams) { call->DestroyVideoReceiveStream(receive_stream); } } + } // namespace webrtc diff --git a/webrtc/video/loopback.cc b/webrtc/video/loopback.cc index 52cfaa36f..a1ebed136 100644 --- a/webrtc/video/loopback.cc +++ b/webrtc/video/loopback.cc @@ -103,8 +103,7 @@ static const uint32_t kSendSsrc = 0x654321; static const uint32_t kSendRtxSsrc = 0x654322; static const uint32_t kReceiverLocalSsrc = 0x123456; -static const uint8_t kRtxVideoPayloadType = 96; -static const uint8_t kVideoPayloadType = 124; +static const uint8_t kRtxPayloadType = 96; void Loopback() { scoped_ptr trace_to_stderr_; @@ -138,7 +137,7 @@ void Loopback() { VideoSendStream::Config send_config; send_config.rtp.ssrcs.push_back(kSendSsrc); send_config.rtp.rtx.ssrcs.push_back(kSendRtxSsrc); - send_config.rtp.rtx.payload_type = kRtxVideoPayloadType; + send_config.rtp.rtx.payload_type = kRtxPayloadType; send_config.rtp.nack.rtp_history_ms = 1000; send_config.rtp.extensions.push_back( RtpExtension(RtpExtension::kAbsSendTime, kAbsSendTimeExtensionId)); @@ -156,7 +155,7 @@ void Loopback() { } send_config.encoder_settings.encoder = encoder.get(); send_config.encoder_settings.payload_name = flags::Codec(); - send_config.encoder_settings.payload_type = kVideoPayloadType; + send_config.encoder_settings.payload_type = 124; VideoEncoderConfig encoder_config; encoder_config.streams = test::CreateVideoStreams(1); VideoStream* stream = &encoder_config.streams[0]; @@ -184,8 +183,8 @@ void Loopback() { receive_config.rtp.remote_ssrc = send_config.rtp.ssrcs[0]; receive_config.rtp.local_ssrc = kReceiverLocalSsrc; receive_config.rtp.nack.rtp_history_ms = 1000; - receive_config.rtp.rtx[kVideoPayloadType].ssrc = kSendRtxSsrc; - receive_config.rtp.rtx[kVideoPayloadType].payload_type = kRtxVideoPayloadType; + receive_config.rtp.rtx[kRtxPayloadType].ssrc = kSendRtxSsrc; + receive_config.rtp.rtx[kRtxPayloadType].payload_type = kRtxPayloadType; receive_config.rtp.extensions.push_back( RtpExtension(RtpExtension::kAbsSendTime, kAbsSendTimeExtensionId)); receive_config.renderer = loopback_video.get(); diff --git a/webrtc/video/rampup_tests.cc b/webrtc/video/rampup_tests.cc index b3f052230..4b26d055c 100644 --- a/webrtc/video/rampup_tests.cc +++ b/webrtc/video/rampup_tests.cc @@ -76,10 +76,6 @@ StreamObserver::StreamObserver(const SsrcMap& rtx_media_ssrcs, remote_bitrate_estimator_.reset( rbe_factory->Create(this, clock, control_type, kRemoteBitrateEstimatorMinBitrateBps)); - payload_registry_->SetRtxPayloadType(RampUpTest::kSendRtxPayloadType, - RampUpTest::kFakeSendPayloadType); - payload_registry_->SetRtxPayloadType(RampUpTest::kRtxRedPayloadType, - RampUpTest::kRedPayloadType); } void StreamObserver::set_expected_bitrate_bps( @@ -122,6 +118,7 @@ bool StreamObserver::SendRtp(const uint8_t* packet, size_t length) { RTPHeader header; EXPECT_TRUE(rtp_parser_->Parse(packet, length, &header)); receive_stats_->IncomingPacket(header, length, false); + payload_registry_->SetIncomingPayloadType(header); remote_bitrate_estimator_->IncomingPacket( clock_->TimeInMilliseconds(), length - 12, header); if (remote_bitrate_estimator_->TimeUntilNextProcess() <= 0) { @@ -139,9 +136,11 @@ bool StreamObserver::SendRtp(const uint8_t* packet, size_t length) { uint8_t restored_packet[kMaxPacketSize]; uint8_t* restored_packet_ptr = restored_packet; size_t restored_length = length; - EXPECT_TRUE(payload_registry_->RestoreOriginalPacket( - &restored_packet_ptr, packet, &restored_length, - rtx_media_ssrcs_[header.ssrc], header)); + payload_registry_->RestoreOriginalPacket(&restored_packet_ptr, + packet, + &restored_length, + rtx_media_ssrcs_[header.ssrc], + header); length = restored_length; EXPECT_TRUE(rtp_parser_->Parse( restored_packet, static_cast(length), &header)); @@ -367,11 +366,10 @@ EventTypeWrapper LowRateStreamObserver::Wait() { return test_done_->Wait(test::CallTest::kLongTimeoutMs); } -void RampUpTest::RunRampUpTest(size_t num_streams, +void RampUpTest::RunRampUpTest(bool rtx, + size_t num_streams, unsigned int start_bitrate_bps, - const std::string& extension_type, - bool rtx, - bool red) { + const std::string& extension_type) { std::vector ssrcs(GenerateSsrcs(num_streams, 100)); std::vector rtx_ssrcs(GenerateSsrcs(num_streams, 200)); StreamObserver::SsrcMap rtx_ssrc_map; @@ -424,11 +422,6 @@ void RampUpTest::RunRampUpTest(size_t num_streams, send_config_.rtp.rtx.payload_type = kSendRtxPayloadType; send_config_.rtp.rtx.ssrcs = rtx_ssrcs; } - if (red) { - send_config_.rtp.fec.ulpfec_payload_type = kUlpfecPayloadType; - send_config_.rtp.fec.red_payload_type = kRedPayloadType; - send_config_.rtp.fec.rtx_payload_type = kRtxRedPayloadType; - } if (num_streams == 1) { // For single stream rampup until 1mbps @@ -455,9 +448,7 @@ void RampUpTest::RunRampUpTest(size_t num_streams, DestroyStreams(); } -void RampUpTest::RunRampUpDownUpTest(size_t number_of_streams, - bool rtx, - bool red) { +void RampUpTest::RunRampUpDownUpTest(size_t number_of_streams, bool rtx) { test::DirectTransport receiver_transport; LowRateStreamObserver stream_observer( &receiver_transport, Clock::GetRealTimeClock(), number_of_streams, rtx); @@ -476,11 +467,6 @@ void RampUpTest::RunRampUpDownUpTest(size_t number_of_streams, send_config_.rtp.rtx.payload_type = kSendRtxPayloadType; send_config_.rtp.rtx.ssrcs = GenerateSsrcs(number_of_streams, 200); } - if (red) { - send_config_.rtp.fec.ulpfec_payload_type = kUlpfecPayloadType; - send_config_.rtp.fec.red_payload_type = kRedPayloadType; - send_config_.rtp.fec.rtx_payload_type = kRtxRedPayloadType; - } CreateStreams(); stream_observer.SetSendStream(send_stream_); @@ -496,68 +482,43 @@ void RampUpTest::RunRampUpDownUpTest(size_t number_of_streams, } TEST_F(RampUpTest, SingleStream) { - RunRampUpTest(1, 0, RtpExtension::kTOffset, false, false); + RunRampUpTest(false, 1, 0, RtpExtension::kTOffset); } TEST_F(RampUpTest, Simulcast) { - RunRampUpTest(3, 0, RtpExtension::kTOffset, false, false); + RunRampUpTest(false, 3, 0, RtpExtension::kTOffset); } TEST_F(RampUpTest, SimulcastWithRtx) { - RunRampUpTest(3, 0, RtpExtension::kTOffset, true, false); -} - -TEST_F(RampUpTest, SimulcastByRedWithRtx) { - RunRampUpTest(3, 0, RtpExtension::kTOffset, true, true); + RunRampUpTest(true, 3, 0, RtpExtension::kTOffset); } TEST_F(RampUpTest, SingleStreamWithHighStartBitrate) { - RunRampUpTest(1, 0.9 * kSingleStreamTargetBps, RtpExtension::kTOffset, false, - false); + RunRampUpTest(false, 1, 0.9 * kSingleStreamTargetBps, RtpExtension::kTOffset); } -TEST_F(RampUpTest, UpDownUpOneStream) { - RunRampUpDownUpTest(1, false, false); -} +TEST_F(RampUpTest, UpDownUpOneStream) { RunRampUpDownUpTest(1, false); } -TEST_F(RampUpTest, UpDownUpThreeStreams) { - RunRampUpDownUpTest(3, false, false); -} +TEST_F(RampUpTest, UpDownUpThreeStreams) { RunRampUpDownUpTest(3, false); } -TEST_F(RampUpTest, UpDownUpOneStreamRtx) { - RunRampUpDownUpTest(1, true, false); -} +TEST_F(RampUpTest, UpDownUpOneStreamRtx) { RunRampUpDownUpTest(1, true); } -TEST_F(RampUpTest, UpDownUpThreeStreamsRtx) { - RunRampUpDownUpTest(3, true, false); -} - -TEST_F(RampUpTest, UpDownUpOneStreamByRedRtx) { - RunRampUpDownUpTest(1, true, true); -} - -TEST_F(RampUpTest, UpDownUpThreeStreamsByRedRtx) { - RunRampUpDownUpTest(3, true, true); -} +TEST_F(RampUpTest, UpDownUpThreeStreamsRtx) { RunRampUpDownUpTest(3, true); } TEST_F(RampUpTest, AbsSendTimeSingleStream) { - RunRampUpTest(1, 0, RtpExtension::kAbsSendTime, false, false); + RunRampUpTest(false, 1, 0, RtpExtension::kAbsSendTime); } TEST_F(RampUpTest, AbsSendTimeSimulcast) { - RunRampUpTest(3, 0, RtpExtension::kAbsSendTime, false, false); + RunRampUpTest(false, 3, 0, RtpExtension::kAbsSendTime); } TEST_F(RampUpTest, AbsSendTimeSimulcastWithRtx) { - RunRampUpTest(3, 0, RtpExtension::kAbsSendTime, true, false); -} - -TEST_F(RampUpTest, AbsSendTimeSimulcastByRedWithRtx) { - RunRampUpTest(3, 0, RtpExtension::kAbsSendTime, true, true); + RunRampUpTest(true, 3, 0, RtpExtension::kAbsSendTime); } TEST_F(RampUpTest, AbsSendTimeSingleStreamWithHighStartBitrate) { - RunRampUpTest(1, 0.9 * kSingleStreamTargetBps, RtpExtension::kAbsSendTime, - false, false); + RunRampUpTest(false, 1, 0.9 * kSingleStreamTargetBps, + RtpExtension::kAbsSendTime); } } // namespace webrtc diff --git a/webrtc/video/rampup_tests.h b/webrtc/video/rampup_tests.h index b09de9b85..e506cd499 100644 --- a/webrtc/video/rampup_tests.h +++ b/webrtc/video/rampup_tests.h @@ -148,13 +148,12 @@ class LowRateStreamObserver : public test::DirectTransport, class RampUpTest : public test::CallTest { protected: - void RunRampUpTest(size_t num_streams, + void RunRampUpTest(bool rtx, + size_t num_streams, unsigned int start_bitrate_bps, - const std::string& extension_type, - bool rtx, - bool red); + const std::string& extension_type); - void RunRampUpDownUpTest(size_t number_of_streams, bool rtx, bool red); + void RunRampUpDownUpTest(size_t number_of_streams, bool rtx); }; } // namespace webrtc #endif // WEBRTC_VIDEO_RAMPUP_TESTS_H_ diff --git a/webrtc/video/video_receive_stream.cc b/webrtc/video/video_receive_stream.cc index 9c4a687af..73f6e7c83 100644 --- a/webrtc/video/video_receive_stream.cc +++ b/webrtc/video/video_receive_stream.cc @@ -92,13 +92,12 @@ VideoReceiveStream::VideoReceiveStream(webrtc::VideoEngine* video_engine, rtp_rtcp_->SetLocalSSRC(channel_, config_.rtp.local_ssrc); // TODO(pbos): Support multiple RTX, per video payload. Config::Rtp::RtxMap::const_iterator it = config_.rtp.rtx.begin(); - for (; it != config_.rtp.rtx.end(); ++it) { + if (it != config_.rtp.rtx.end()) { assert(it->second.ssrc != 0); assert(it->second.payload_type != 0); rtp_rtcp_->SetRemoteSSRCType(channel_, kViEStreamTypeRtx, it->second.ssrc); - rtp_rtcp_->SetRtxReceivePayloadType(channel_, it->second.payload_type, - it->first); + rtp_rtcp_->SetRtxReceivePayloadType(channel_, it->second.payload_type); } rtp_rtcp_->SetRembStatus(channel_, false, config_.rtp.remb); @@ -147,11 +146,6 @@ VideoReceiveStream::VideoReceiveStream(webrtc::VideoEngine* video_engine, LOG(LS_ERROR) << "Could not set RED codec. This shouldn't happen."; abort(); } - if (config_.rtp.fec.rtx_payload_type != -1) { - rtp_rtcp_->SetRtxReceivePayloadType(channel_, - config_.rtp.fec.rtx_payload_type, - config_.rtp.fec.red_payload_type); - } } stats_proxy_.reset( diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index f531f111c..b8ef4d349 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -49,6 +49,7 @@ std::string VideoSendStream::Config::Rtp::Rtx::ToString() ss << "}, {"; } ss << '}'; + ss << ", payload_type: " << payload_type; ss << '}'; return ss.str(); @@ -172,11 +173,6 @@ VideoSendStream::VideoSendStream( static_cast(config_.rtp.fec.red_payload_type), static_cast(config_.rtp.fec.ulpfec_payload_type)); } - if (config_.rtp.fec.rtx_payload_type != -1) { - rtp_rtcp_->SetRtxSendPayloadType(channel_, - config_.rtp.fec.rtx_payload_type, - config_.rtp.fec.red_payload_type); - } } else { rtp_rtcp_->SetNACKStatus(channel_, config_.rtp.nack.rtp_history_ms > 0); } @@ -479,8 +475,7 @@ void VideoSendStream::ConfigureSsrcs() { } assert(config_.rtp.rtx.payload_type >= 0); - rtp_rtcp_->SetRtxSendPayloadType(channel_, config_.rtp.rtx.payload_type, - config_.encoder_settings.payload_type); + rtp_rtcp_->SetRtxSendPayloadType(channel_, config_.rtp.rtx.payload_type); } std::map VideoSendStream::GetRtpStates() const { diff --git a/webrtc/video_engine/include/vie_rtp_rtcp.h b/webrtc/video_engine/include/vie_rtp_rtcp.h index 2c534754d..103a196f6 100644 --- a/webrtc/video_engine/include/vie_rtp_rtcp.h +++ b/webrtc/video_engine/include/vie_rtp_rtcp.h @@ -114,13 +114,10 @@ class WEBRTC_DLLEXPORT ViERTP_RTCP { // This sets a specific payload type for the RTX stream. Note that this // doesn't enable RTX, SetLocalSSRC must still be called to enable RTX. virtual int SetRtxSendPayloadType(const int video_channel, - const uint8_t payload_type, - const uint8_t associated_payload_type) = 0; + const uint8_t payload_type) = 0; - virtual int SetRtxReceivePayloadType( - const int video_channel, - const uint8_t payload_type, - const uint8_t associated_payload_type) = 0; + virtual int SetRtxReceivePayloadType(const int video_channel, + const uint8_t payload_type) = 0; // This function enables manual initialization of the sequence number. The // start sequence number is normally a random number. diff --git a/webrtc/video_engine/test/auto_test/source/vie_autotest_loopback.cc b/webrtc/video_engine/test/auto_test/source/vie_autotest_loopback.cc index ce4b71fd9..471112251 100644 --- a/webrtc/video_engine/test/auto_test/source/vie_autotest_loopback.cc +++ b/webrtc/video_engine/test/auto_test/source/vie_autotest_loopback.cc @@ -40,7 +40,6 @@ const uint32_t kSsrc = 0x01234567; const uint32_t kRtxSsrc = 0x01234568; const int kRtxPayloadType = 98; -const int kPayloadType = 100; #define VCM_RED_PAYLOAD_TYPE 96 #define VCM_ULPFEC_PAYLOAD_TYPE 97 @@ -245,15 +244,14 @@ int VideoEngineSampleCode(void* window1, void* window2) return -1; } - error = ptrViERtpRtcp->SetRtxSendPayloadType(videoChannel, kRtxPayloadType, - kPayloadType); + error = ptrViERtpRtcp->SetRtxSendPayloadType(videoChannel, kRtxPayloadType); if (error == -1) { printf("ERROR in ViERTP_RTCP::SetRtxSendPayloadType\n"); return -1; } - error = ptrViERtpRtcp->SetRtxReceivePayloadType( - videoChannel, kRtxPayloadType, kPayloadType); + error = ptrViERtpRtcp->SetRtxReceivePayloadType(videoChannel, + kRtxPayloadType); if (error == -1) { printf("ERROR in ViERTP_RTCP::SetRtxReceivePayloadType\n"); return -1; diff --git a/webrtc/video_engine/test/auto_test/source/vie_autotest_rtp_rtcp.cc b/webrtc/video_engine/test/auto_test/source/vie_autotest_rtp_rtcp.cc index ae6355815..923fe41b1 100644 --- a/webrtc/video_engine/test/auto_test/source/vie_autotest_rtp_rtcp.cc +++ b/webrtc/video_engine/test/auto_test/source/vie_autotest_rtp_rtcp.cc @@ -172,15 +172,14 @@ void ViEAutoTest::ViERtpRtcpStandardTest() myTransport.ClearStats(); const uint8_t kRtxPayloadType = 96; - const uint8_t kPayloadType = 100; // Temporarily disable pacing. EXPECT_EQ(0, ViE.rtp_rtcp->SetTransmissionSmoothingStatus( tbChannel.videoChannel, false)); EXPECT_EQ(0, ViE.rtp_rtcp->SetNACKStatus(tbChannel.videoChannel, true)); - EXPECT_EQ(0, ViE.rtp_rtcp->SetRtxSendPayloadType( - tbChannel.videoChannel, kRtxPayloadType, kPayloadType)); - EXPECT_EQ(0, ViE.rtp_rtcp->SetRtxReceivePayloadType( - tbChannel.videoChannel, kRtxPayloadType, kPayloadType)); + EXPECT_EQ(0, ViE.rtp_rtcp->SetRtxSendPayloadType(tbChannel.videoChannel, + kRtxPayloadType)); + EXPECT_EQ(0, ViE.rtp_rtcp->SetRtxReceivePayloadType(tbChannel.videoChannel, + kRtxPayloadType)); EXPECT_EQ(0, ViE.rtp_rtcp->SetLocalSSRC(tbChannel.videoChannel, 1234, webrtc::kViEStreamTypeRtx, 0)); EXPECT_EQ(0, ViE.rtp_rtcp->SetRemoteSSRCType(tbChannel.videoChannel, diff --git a/webrtc/video_engine/vie_channel.cc b/webrtc/video_engine/vie_channel.cc index eb28bd02b..79ebe7081 100644 --- a/webrtc/video_engine/vie_channel.cc +++ b/webrtc/video_engine/vie_channel.cc @@ -899,12 +899,11 @@ int32_t ViEChannel::GetRemoteCSRC(uint32_t CSRCs[kRtpCsrcSize]) { return 0; } -int ViEChannel::SetRtxSendPayloadType(int payload_type, - int associated_payload_type) { - rtp_rtcp_->SetRtxSendPayloadType(payload_type, associated_payload_type); +int ViEChannel::SetRtxSendPayloadType(int payload_type) { + rtp_rtcp_->SetRtxSendPayloadType(payload_type); for (std::list::iterator it = simulcast_rtp_rtcp_.begin(); it != simulcast_rtp_rtcp_.end(); it++) { - (*it)->SetRtxSendPayloadType(payload_type, associated_payload_type); + (*it)->SetRtxSendPayloadType(payload_type); } SetRtxSendStatus(true); return 0; @@ -921,9 +920,8 @@ void ViEChannel::SetRtxSendStatus(bool enable) { } } -void ViEChannel::SetRtxReceivePayloadType(int payload_type, - int associated_payload_type) { - vie_receiver_.SetRtxPayloadType(payload_type, associated_payload_type); +void ViEChannel::SetRtxReceivePayloadType(int payload_type) { + vie_receiver_.SetRtxPayloadType(payload_type); } int32_t ViEChannel::SetStartSequenceNumber(uint16_t sequence_number) { diff --git a/webrtc/video_engine/vie_channel.h b/webrtc/video_engine/vie_channel.h index 0096c705c..7a46d1f75 100644 --- a/webrtc/video_engine/vie_channel.h +++ b/webrtc/video_engine/vie_channel.h @@ -145,8 +145,8 @@ class ViEChannel // Gets the CSRC for the incoming stream. int32_t GetRemoteCSRC(uint32_t CSRCs[kRtpCsrcSize]); - int SetRtxSendPayloadType(int payload_type, int associated_payload_type); - void SetRtxReceivePayloadType(int payload_type, int associated_payload_type); + int SetRtxSendPayloadType(int payload_type); + void SetRtxReceivePayloadType(int payload_type); // Sets the starting sequence number, must be called before StartSend. int32_t SetStartSequenceNumber(uint16_t sequence_number); diff --git a/webrtc/video_engine/vie_receiver.cc b/webrtc/video_engine/vie_receiver.cc index 048eecda4..6dec98556 100644 --- a/webrtc/video_engine/vie_receiver.cc +++ b/webrtc/video_engine/vie_receiver.cc @@ -102,10 +102,8 @@ void ViEReceiver::SetNackStatus(bool enable, rtp_receiver_->SetNACKStatus(enable ? kNackRtcp : kNackOff); } -void ViEReceiver::SetRtxPayloadType(int payload_type, - int associated_payload_type) { - rtp_payload_registry_->SetRtxPayloadType(payload_type, - associated_payload_type); +void ViEReceiver::SetRtxPayloadType(int payload_type) { + rtp_payload_registry_->SetRtxPayloadType(payload_type); } void ViEReceiver::SetRtxSsrc(uint32_t ssrc) { diff --git a/webrtc/video_engine/vie_receiver.h b/webrtc/video_engine/vie_receiver.h index aaeba17be..39a85e4c2 100644 --- a/webrtc/video_engine/vie_receiver.h +++ b/webrtc/video_engine/vie_receiver.h @@ -47,7 +47,7 @@ class ViEReceiver : public RtpData { bool RegisterPayload(const VideoCodec& video_codec); void SetNackStatus(bool enable, int max_nack_reordering_threshold); - void SetRtxPayloadType(int payload_type, int associated_payload_type); + void SetRtxPayloadType(int payload_type); void SetRtxSsrc(uint32_t ssrc); bool GetRtxSsrc(uint32_t* ssrc) const; diff --git a/webrtc/video_engine/vie_rtp_rtcp_impl.cc b/webrtc/video_engine/vie_rtp_rtcp_impl.cc index 354e1d69e..ae213079d 100644 --- a/webrtc/video_engine/vie_rtp_rtcp_impl.cc +++ b/webrtc/video_engine/vie_rtp_rtcp_impl.cc @@ -191,10 +191,8 @@ int ViERTP_RTCPImpl::GetRemoteCSRCs(const int video_channel, return 0; } -int ViERTP_RTCPImpl::SetRtxSendPayloadType( - const int video_channel, - const uint8_t payload_type, - const uint8_t associated_payload_type) { +int ViERTP_RTCPImpl::SetRtxSendPayloadType(const int video_channel, + const uint8_t payload_type) { LOG_F(LS_INFO) << "channel: " << video_channel << " payload_type: " << static_cast(payload_type); ViEChannelManagerScoped cs(*(shared_data_->channel_manager())); @@ -203,17 +201,14 @@ int ViERTP_RTCPImpl::SetRtxSendPayloadType( shared_data_->SetLastError(kViERtpRtcpInvalidChannelId); return -1; } - if (vie_channel->SetRtxSendPayloadType(payload_type, - associated_payload_type) != 0) { + if (vie_channel->SetRtxSendPayloadType(payload_type) != 0) { return -1; } return 0; } -int ViERTP_RTCPImpl::SetRtxReceivePayloadType( - const int video_channel, - const uint8_t payload_type, - const uint8_t associated_payload_type) { +int ViERTP_RTCPImpl::SetRtxReceivePayloadType(const int video_channel, + const uint8_t payload_type) { LOG_F(LS_INFO) << "channel: " << video_channel << " payload_type: " << static_cast(payload_type); ViEChannelManagerScoped cs(*(shared_data_->channel_manager())); @@ -222,7 +217,7 @@ int ViERTP_RTCPImpl::SetRtxReceivePayloadType( shared_data_->SetLastError(kViERtpRtcpInvalidChannelId); return -1; } - vie_channel->SetRtxReceivePayloadType(payload_type, associated_payload_type); + vie_channel->SetRtxReceivePayloadType(payload_type); return 0; } diff --git a/webrtc/video_engine/vie_rtp_rtcp_impl.h b/webrtc/video_engine/vie_rtp_rtcp_impl.h index 7374d1a11..8e9f09735 100644 --- a/webrtc/video_engine/vie_rtp_rtcp_impl.h +++ b/webrtc/video_engine/vie_rtp_rtcp_impl.h @@ -40,11 +40,9 @@ class ViERTP_RTCPImpl virtual int GetRemoteCSRCs(const int video_channel, unsigned int CSRCs[kRtpCsrcSize]) const; virtual int SetRtxSendPayloadType(const int video_channel, - const uint8_t payload_type, - const uint8_t associated_payload_type); + const uint8_t payload_type); virtual int SetRtxReceivePayloadType(const int video_channel, - const uint8_t payload_type, - const uint8_t associated_payload_type); + const uint8_t payload_type); virtual int SetStartSequenceNumber(const int video_channel, uint16_t sequence_number); virtual void SetRtpStateForSsrc(int video_channel,