From e62202fedf57b74cc263246c0586ee353978caf8 Mon Sep 17 00:00:00 2001 From: Shao Changbin Date: Tue, 21 Apr 2015 20:24:50 +0800 Subject: [PATCH] Support handling multiple RTX but only generate SDP with RTX associated with VP8. This implementation registers RTX-APT map inside RTP sender and receiver. While it only generates SDP with RTX associated with VP8 to make it compatible with previous Chrome versions. Should add following changes after reaches stable, * Use RTX-APT map for building and restoring RTP packets. * Add RTX support for RED or VP9 in Video engine. * Set RTX payload type for RED inside FecConfig in EndToEndTest. BUG=4024 R=mflodman@webrtc.org, pbos@webrtc.org, pthatcher@webrtc.org, stefan@webrtc.org Review URL: https://webrtc-codereview.appspot.com/36889004 Cr-Commit-Position: refs/heads/master@{#9040} --- talk/media/base/codec.cc | 21 +++++ talk/media/base/codec.h | 5 ++ talk/media/base/constants.cc | 11 +++ talk/media/base/constants.h | 13 +++ talk/media/webrtc/fakewebrtcvideoengine.h | 58 +++++++++---- talk/media/webrtc/webrtcvideoengine.cc | 82 ++++++------------- talk/media/webrtc/webrtcvideoengine.h | 6 +- talk/media/webrtc/webrtcvideoengine2.cc | 47 +++++++---- talk/media/webrtc/webrtcvideoengine2.h | 1 - .../webrtc/webrtcvideoengine2_unittest.cc | 26 +++++- .../webrtc/webrtcvideoengine_unittest.cc | 39 +++++---- webrtc/config.h | 8 +- .../rtp_rtcp/interface/rtp_payload_registry.h | 8 +- webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h | 8 +- webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h | 6 +- .../rtp_rtcp/source/nack_rtx_unittest.cc | 27 +++--- .../rtp_rtcp/source/rtp_payload_registry.cc | 42 ++++++---- .../modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 7 +- .../modules/rtp_rtcp/source/rtp_rtcp_impl.h | 5 +- webrtc/modules/rtp_rtcp/source/rtp_sender.cc | 33 ++++++-- webrtc/modules/rtp_rtcp/source/rtp_sender.h | 11 ++- .../rtp_rtcp/source/rtp_sender_unittest.cc | 4 +- .../modules/rtp_rtcp/test/testAPI/test_api.cc | 3 +- webrtc/test/call_test.cc | 1 + webrtc/test/call_test.h | 1 + webrtc/video/end_to_end_tests.cc | 47 ++++++++--- webrtc/video/loopback.cc | 11 +-- webrtc/video/rampup_tests.cc | 80 +++++++++++++----- webrtc/video/rampup_tests.h | 9 +- webrtc/video/video_receive_stream.cc | 9 +- webrtc/video/video_send_stream.cc | 4 +- 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 | 18 ++-- 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 +- 40 files changed, 461 insertions(+), 251 deletions(-) diff --git a/talk/media/base/codec.cc b/talk/media/base/codec.cc index e22dce2455..5b747d1917 100644 --- a/talk/media/base/codec.cc +++ b/talk/media/base/codec.cc @@ -231,6 +231,13 @@ VideoCodec::VideoCodec(int pt, framerate(fr) { } +VideoCodec::VideoCodec(int pt, const std::string& nm) + : Codec(pt, nm, kVideoCodecClockrate, 0), + width(0), + height(0), + framerate(0) { +} + VideoCodec::VideoCodec() : Codec(), width(0), height(0), framerate(0) { clockrate = kVideoCodecClockrate; } @@ -317,4 +324,18 @@ std::string DataCodec::ToString() const { return os.str(); } +bool HasNack(const VideoCodec& codec) { + return codec.HasFeedbackParam( + FeedbackParam(kRtcpFbParamNack, kParamValueEmpty)); +} + +bool HasRemb(const VideoCodec& codec) { + return codec.HasFeedbackParam( + FeedbackParam(kRtcpFbParamRemb, kParamValueEmpty)); +} + +bool CodecNamesEq(const std::string& name1, const std::string& name2) { + return _stricmp(name1.c_str(), name2.c_str()) == 0; +} + } // namespace cricket diff --git a/talk/media/base/codec.h b/talk/media/base/codec.h index a04f6b994a..3bb08e7c7a 100644 --- a/talk/media/base/codec.h +++ b/talk/media/base/codec.h @@ -162,6 +162,7 @@ struct VideoCodec : public Codec { // Creates a codec with the given parameters. VideoCodec(int pt, const std::string& nm, int w, int h, int fr, int pr); + VideoCodec(int pt, const std::string& nm); // Creates an empty codec. VideoCodec(); VideoCodec(const VideoCodec& c); @@ -267,6 +268,10 @@ bool FindCodecById(const std::vector& codecs, return false; } +bool CodecNamesEq(const std::string& name1, const std::string& name2); +bool HasNack(const VideoCodec& codec); +bool HasRemb(const VideoCodec& codec); + } // namespace cricket #endif // TALK_MEDIA_BASE_CODEC_H_ diff --git a/talk/media/base/constants.cc b/talk/media/base/constants.cc index 6712a3f72e..562dad4a3e 100644 --- a/talk/media/base/constants.cc +++ b/talk/media/base/constants.cc @@ -126,6 +126,17 @@ const char kRtpVideoRotation6BitsHeaderExtensionForTesting[] = const int kNumDefaultUnsignalledVideoRecvStreams = 0; +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 kDefaultVideoMaxWidth = 640; +const int kDefaultVideoMaxHeight = 400; +const int kDefaultVideoMaxFramerate = 30; } // namespace cricket diff --git a/talk/media/base/constants.h b/talk/media/base/constants.h index a67c6679f4..84216fb534 100644 --- a/talk/media/base/constants.h +++ b/talk/media/base/constants.h @@ -155,6 +155,19 @@ extern const char kRtpVideoRotationHeaderExtension[]; extern const char kRtpVideoRotation6BitsHeaderExtensionForTesting[]; extern const int kNumDefaultUnsignalledVideoRecvStreams; + +extern const char kVp8CodecName[]; +extern const char kVp9CodecName[]; + +extern const int kDefaultVp8PlType; +extern const int kDefaultVp9PlType; +extern const int kDefaultRedPlType; +extern const int kDefaultUlpfecType; +extern const int kDefaultRtxVp8PlType; + +extern const int kDefaultVideoMaxWidth; +extern const int kDefaultVideoMaxHeight; +extern const int kDefaultVideoMaxFramerate; } // namespace cricket #endif // TALK_MEDIA_BASE_CONSTANTS_H_ diff --git a/talk/media/webrtc/fakewebrtcvideoengine.h b/talk/media/webrtc/fakewebrtcvideoengine.h index ee22a42d72..15e2ba0d07 100644 --- a/talk/media/webrtc/fakewebrtcvideoengine.h +++ b/talk/media/webrtc/fakewebrtcvideoengine.h @@ -288,8 +288,6 @@ 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), @@ -329,8 +327,10 @@ class FakeWebRtcVideoEngine std::map ssrcs_; std::map rtx_ssrcs_; int remote_rtx_ssrc_; - int rtx_send_payload_type; - int rtx_recv_payload_type; + // Mapping rtx_send_payload_types_[rtx] = associated. + std::map rtx_send_payload_types_; + // Mapping rtx_recv_payload_types_[rtx] = associated. + std::map rtx_recv_payload_types_; std::string cname_; webrtc::ViERTCPMode rtcp_status_; webrtc::ViEKeyFrameRequestMethod key_frame_request_method_; @@ -640,14 +640,28 @@ 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; } - int GetRtxRecvPayloadType(int channel) { - WEBRTC_CHECK_CHANNEL(channel); - return channels_[channel]->rtx_recv_payload_type; + std::vector GetRtxSendPayloadTypes(int channel) { + std::vector rtx_payload_types; + if (channels_.find(channel) == channels_.end()) + return rtx_payload_types; + + for (const auto& payload_type : + channels_[channel]->rtx_send_payload_types_) { + rtx_payload_types.push_back(payload_type.first); + } + return rtx_payload_types; + } + std::vector GetRtxRecvPayloadTypes(int channel) { + std::vector rtx_payload_types; + if (channels_.find(channel) == channels_.end()) + return rtx_payload_types; + + for (const auto& payload_type : + channels_[channel]->rtx_recv_payload_types_) { + rtx_payload_types.push_back(payload_type.first); + } + return rtx_payload_types; } int GetRemoteRtxSsrc(int channel) { WEBRTC_CHECK_CHANNEL(channel); @@ -1040,17 +1054,27 @@ 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)) { + WEBRTC_FUNC(SetRtxSendPayloadType, + (const int channel, + const uint8 payload_type, + const uint8 associated_payload_type)) { WEBRTC_CHECK_CHANNEL(channel); - channels_[channel]->rtx_send_payload_type = payload_type; + assert(payload_type >= 0); + assert(associated_payload_type >= 0); + channels_[channel]->rtx_send_payload_types_[payload_type] = + associated_payload_type; return 0; } - WEBRTC_FUNC(SetRtxReceivePayloadType, (const int channel, - const uint8 payload_type)) { + WEBRTC_FUNC(SetRtxReceivePayloadType, + (const int channel, + const uint8 payload_type, + const uint8 associated_payload_type)) { WEBRTC_CHECK_CHANNEL(channel); - channels_[channel]->rtx_recv_payload_type = payload_type; + assert(payload_type >= 0); + assert(associated_payload_type >= 0); + channels_[channel]->rtx_recv_payload_types_[payload_type] = + associated_payload_type; return 0; } diff --git a/talk/media/webrtc/webrtcvideoengine.cc b/talk/media/webrtc/webrtcvideoengine.cc index 090ad75e23..98cde33890 100644 --- a/talk/media/webrtc/webrtcvideoengine.cc +++ b/talk/media/webrtc/webrtcvideoengine.cc @@ -147,13 +147,7 @@ namespace cricket { const int kVideoMtu = 1200; const int kVideoRtpBufferSize = 65536; -const char kVp8CodecName[] = "VP8"; -const char kVp9CodecName[] = "VP9"; - // TODO(ronghuawu): Change to 640x360. -const int kDefaultVideoMaxWidth = 640; -const int kDefaultVideoMaxHeight = 400; -const int kDefaultVideoMaxFramerate = 30; const int kMinVideoBitrate = 30; const int kStartVideoBitrate = 300; const int kMaxVideoBitrate = 2000; @@ -220,16 +214,6 @@ static const bool kNotSending = false; static const rtc::DiffServCodePoint kVideoDscpValue = rtc::DSCP_AF41; -bool IsNackEnabled(const VideoCodec& codec) { - return codec.HasFeedbackParam( - FeedbackParam(kRtcpFbParamNack, kParamValueEmpty)); -} - -bool IsRembEnabled(const VideoCodec& codec) { - return codec.HasFeedbackParam( - FeedbackParam(kRtcpFbParamRemb, kParamValueEmpty)); -} - void AddDefaultFeedbackParams(VideoCodec* codec) { codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamCcm, kRtcpFbCcmParamFir)); codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamNack, kParamValueEmpty)); @@ -237,10 +221,6 @@ void AddDefaultFeedbackParams(VideoCodec* codec) { codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamRemb, kParamValueEmpty)); } -bool CodecNameMatches(const std::string& name1, const std::string& name2) { - return _stricmp(name1.c_str(), name2.c_str()) == 0; -} - static VideoCodec MakeVideoCodecWithDefaultFeedbackParams(int payload_type, const char* name) { VideoCodec codec(payload_type, name, kDefaultVideoMaxWidth, @@ -249,19 +229,11 @@ static VideoCodec MakeVideoCodecWithDefaultFeedbackParams(int payload_type, return codec; } -static VideoCodec MakeVideoCodec(int payload_type, const char* name) { - return VideoCodec(payload_type, name, 0, 0, 0, 0); -} - -static VideoCodec MakeRtxCodec(int payload_type, int associated_payload_type) { - return VideoCodec::CreateRtxCodec(payload_type, associated_payload_type); -} - bool CodecIsInternallySupported(const std::string& codec_name) { - if (CodecNameMatches(codec_name, kVp8CodecName)) { + if (CodecNamesEq(codec_name, kVp8CodecName)) { return true; } - if (CodecNameMatches(codec_name, kVp9CodecName)) { + if (CodecNamesEq(codec_name, kVp9CodecName)) { const std::string group_name = webrtc::field_trial::FindFullName("WebRTC-SupportVP9"); return group_name == "Enabled" || group_name == "EnabledByFlag"; @@ -272,14 +244,16 @@ bool CodecIsInternallySupported(const std::string& codec_name) { std::vector DefaultVideoCodecList() { std::vector codecs; if (CodecIsInternallySupported(kVp9CodecName)) { - codecs.push_back( - MakeVideoCodecWithDefaultFeedbackParams(101, kVp9CodecName)); + codecs.push_back(MakeVideoCodecWithDefaultFeedbackParams(kDefaultVp9PlType, + kVp9CodecName)); // TODO(andresp): Add rtx codec for vp9 and verify it works. } - codecs.push_back(MakeVideoCodecWithDefaultFeedbackParams(100, kVp8CodecName)); - codecs.push_back(MakeRtxCodec(96, 100)); - codecs.push_back(MakeVideoCodec(116, kRedCodecName)); - codecs.push_back(MakeVideoCodec(117, kUlpfecCodecName)); + codecs.push_back(MakeVideoCodecWithDefaultFeedbackParams(kDefaultVp8PlType, + kVp8CodecName)); + codecs.push_back( + VideoCodec::CreateRtxCodec(kDefaultRtxVp8PlType, kDefaultVp8PlType)); + codecs.push_back(VideoCodec(kDefaultRedPlType, kRedCodecName)); + codecs.push_back(VideoCodec(kDefaultUlpfecType, kUlpfecCodecName)); return codecs; } @@ -1713,7 +1687,6 @@ 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), @@ -1826,8 +1799,8 @@ bool WebRtcVideoMediaChannel::SetSendCodecs( webrtc::VideoCodec wcodec; if (engine()->ConvertFromCricketVideoCodec(checked_codec, &wcodec)) { if (send_codecs.empty()) { - nack_enabled = IsNackEnabled(checked_codec); - remb_enabled = IsRembEnabled(checked_codec); + nack_enabled = HasNack(checked_codec); + remb_enabled = HasRemb(checked_codec); } send_codecs.push_back(wcodec); } @@ -1885,12 +1858,12 @@ 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. + // TODO(changbin): Add support for more than on RTX payload type. Currently + // using only one to interop with previous versions of Chrome. std::map::const_iterator rtx_it = primary_rtx_pt_mapping.find(codec.plType); if (rtx_it != primary_rtx_pt_mapping.end()) { - send_rtx_type_ = rtx_it->second; + send_rtx_associated_types_[rtx_it->second] = rtx_it->first; } if (BitrateIsSet(codec.minBitrate) && BitrateIsSet(codec.maxBitrate) && @@ -3807,8 +3780,9 @@ void WebRtcVideoMediaChannel::LogSendCodecChange(const std::string& reason) { << vie_codec.codecSpecific.VP8.keyFrameInterval; } - if (send_rtx_type_ != -1) { - LOG(LS_INFO) << "RTX payload type: " << send_rtx_type_; + for (const auto& kv : send_rtx_associated_types_) { + LOG(LS_INFO) << "RTX payload type: " << kv.first + << ", associated payload type:" << kv.second; } LogSimulcastSubstreams(vie_codec); @@ -3826,7 +3800,6 @@ 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) { @@ -3837,11 +3810,6 @@ 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; @@ -3856,12 +3824,11 @@ bool WebRtcVideoMediaChannel::SetReceiveCodecs( return false; } if (engine()->vie()->rtp()->SetRtxReceivePayloadType( - channel_id, it->plType) != 0) { + channel_id, it->plType, apt_it->second) != 0) { LOG_RTCERR2(SetRtxReceivePayloadType, channel_id, static_cast(it->plType)); return false; } - rtx_registered = true; continue; } if (engine()->vie()->codec()->SetReceiveCodec(channel_id, *it) != 0) { @@ -3998,11 +3965,12 @@ 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. - 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; + for (const auto& kv : send_rtx_associated_types_) { + if (engine()->vie()->rtp()->SetRtxSendPayloadType(channel_id, kv.first, + kv.second) != 0) { + LOG_RTCERR3(SetRtxSendPayloadType, channel_id, kv.first, kv.second); + return false; + } } send_channel->set_send_params(send_params); diff --git a/talk/media/webrtc/webrtcvideoengine.h b/talk/media/webrtc/webrtcvideoengine.h index cb7f891018..f48d779068 100644 --- a/talk/media/webrtc/webrtcvideoengine.h +++ b/talk/media/webrtc/webrtcvideoengine.h @@ -88,10 +88,7 @@ struct Device; // This set of methods is declared here for the sole purpose of sharing code // between webrtc video engine v1 and v2. std::vector DefaultVideoCodecList(); -bool CodecNameMatches(const std::string& name1, const std::string& name2); bool CodecIsInternallySupported(const std::string& codec_name); -bool IsNackEnabled(const VideoCodec& codec); -bool IsRembEnabled(const VideoCodec& codec); void AddDefaultFeedbackParams(VideoCodec* codec); class WebRtcVideoEngine : public sigslot::has_slots<> { @@ -504,7 +501,8 @@ class WebRtcVideoMediaChannel : public rtc::MessageHandler, // Global send side state. SendChannelMap send_channels_; rtc::scoped_ptr send_codec_; - int send_rtx_type_; + // A map from RTX payload types to their associated payload types. + std::map send_rtx_associated_types_; 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 d1934327f3..14ab8a85b6 100644 --- a/talk/media/webrtc/webrtcvideoengine2.cc +++ b/talk/media/webrtc/webrtcvideoengine2.cc @@ -146,7 +146,7 @@ inline const webrtc::RtpExtension* FindHeaderExtension( } // Merges two fec configs and logs an error if a conflict arises -// such that merging in diferent order would trigger a diferent output. +// such that merging in different order would trigger a different output. static void MergeFecConfig(const webrtc::FecConfig& other, webrtc::FecConfig* output) { if (other.ulpfec_payload_type != -1) { @@ -167,6 +167,15 @@ static void MergeFecConfig(const webrtc::FecConfig& other, } output->red_payload_type = other.red_payload_type; } + if (other.red_rtx_payload_type != -1) { + if (output->red_rtx_payload_type != -1 && + output->red_rtx_payload_type != other.red_rtx_payload_type) { + LOG(LS_WARNING) << "Conflict merging red_rtx_payload_type configs: " + << output->red_rtx_payload_type << " and " + << other.red_rtx_payload_type; + } + output->red_rtx_payload_type = other.red_rtx_payload_type; + } } } // namespace @@ -304,12 +313,12 @@ WebRtcVideoChannel2::WebRtcVideoSendStream::CreateVideoStreams( void* WebRtcVideoChannel2::WebRtcVideoSendStream::ConfigureVideoEncoderSettings( const VideoCodec& codec, const VideoOptions& options) { - if (CodecNameMatches(codec.name, kVp8CodecName)) { + if (CodecNamesEq(codec.name, kVp8CodecName)) { encoder_settings_.vp8 = webrtc::VideoEncoder::GetDefaultVp8Settings(); options.video_noise_reduction.Get(&encoder_settings_.vp8.denoisingOn); return &encoder_settings_.vp8; } - if (CodecNameMatches(codec.name, kVp9CodecName)) { + if (CodecNamesEq(codec.name, kVp9CodecName)) { encoder_settings_.vp9 = webrtc::VideoEncoder::GetDefaultVp9Settings(); options.video_noise_reduction.Get(&encoder_settings_.vp9.denoisingOn); return &encoder_settings_.vp9; @@ -415,7 +424,7 @@ bool WebRtcVideoEngine2::SetDefaultEncoderConfig( const VideoCodec& codec = config.max_codec; bool supports_codec = false; for (size_t i = 0; i < video_codecs_.size(); ++i) { - if (CodecNameMatches(video_codecs_[i].name, codec.name)) { + if (CodecNamesEq(video_codecs_[i].name, codec.name)) { video_codecs_[i].width = codec.width; video_codecs_[i].height = codec.height; video_codecs_[i].framerate = codec.framerate; @@ -666,7 +675,7 @@ bool WebRtcVideoChannel2::CodecIsExternallySupported( const std::vector external_codecs = external_encoder_factory_->codecs(); for (size_t c = 0; c < external_codecs.size(); ++c) { - if (CodecNameMatches(name, external_codecs[c].name)) { + if (CodecNamesEq(name, external_codecs[c].name)) { return true; } } @@ -1639,11 +1648,11 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::SetCodec( } webrtc::VideoCodecType CodecTypeFromName(const std::string& name) { - if (CodecNameMatches(name, kVp8CodecName)) { + if (CodecNamesEq(name, kVp8CodecName)) { return webrtc::kVideoCodecVP8; - } else if (CodecNameMatches(name, kVp9CodecName)) { + } else if (CodecNamesEq(name, kVp9CodecName)) { return webrtc::kVideoCodecVP9; - } else if (CodecNameMatches(name, kH264CodecName)) { + } else if (CodecNamesEq(name, kH264CodecName)) { return webrtc::kVideoCodecH264; } return webrtc::kVideoCodecUnknown; @@ -1720,7 +1729,7 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::SetCodecAndOptions( } } - if (IsNackEnabled(codec_settings.codec)) { + if (HasNack(codec_settings.codec)) { parameters_.config.rtp.nack.rtp_history_ms = kNackHistoryMs; } @@ -2087,8 +2096,8 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetRecvCodecs( // TODO(pbos): Reconfigure RTX based on incoming recv_codecs. config_.rtp.fec = recv_codecs.front().fec; config_.rtp.nack.rtp_history_ms = - IsNackEnabled(recv_codecs.begin()->codec) ? kNackHistoryMs : 0; - config_.rtp.remb = IsRembEnabled(recv_codecs.begin()->codec); + HasNack(recv_codecs.begin()->codec) ? kNackHistoryMs : 0; + config_.rtp.remb = HasRemb(recv_codecs.begin()->codec); ClearDecoders(&old_decoders); RecreateWebRtcStream(); @@ -2234,6 +2243,7 @@ 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.red_rtx_payload_type == other.fec.red_rtx_payload_type && rtx_payload_type == other.rtx_payload_type; } @@ -2309,17 +2319,22 @@ 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) { - LOG(LS_ERROR) << "RTX not mapped to regular video codec."; + 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."; return std::vector(); } + + if (it->first == fec_settings.red_payload_type) { + fec_settings.red_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) { + if (rtx_mapping[video_codecs[i].codec.id] != 0 && + rtx_mapping[video_codecs[i].codec.id] != + fec_settings.red_payload_type) { 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 446e984a4e..927e699e3e 100644 --- a/talk/media/webrtc/webrtcvideoengine2.h +++ b/talk/media/webrtc/webrtcvideoengine2.h @@ -246,7 +246,6 @@ 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 7add66c13d..7b967f89f8 100644 --- a/talk/media/webrtc/webrtcvideoengine2_unittest.cc +++ b/talk/media/webrtc/webrtcvideoengine2_unittest.cc @@ -34,6 +34,7 @@ #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" @@ -87,6 +88,19 @@ static void CreateBlackFrame(webrtc::I420VideoFrame* video_frame, video_frame->allocated_size(webrtc::kVPlane)); } +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.red_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.red_rtx_payload_type); + } +} } // namespace namespace cricket { @@ -342,7 +356,11 @@ 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") { - default_rtx_codec_ = engine_codecs[i]; + int associated_payload_type; + if (engine_codecs[i].GetParam(kCodecParamAssociatedPayloadType, + &associated_payload_type)) { + default_apt_rtx_types_[associated_payload_type] = engine_codecs[i].id; + } } else if (!codec_set) { default_codec_ = engine_codecs[i]; codec_set = true; @@ -383,7 +401,7 @@ class WebRtcVideoEngine2Test : public ::testing::Test { VideoCodec default_codec_; VideoCodec default_red_codec_; VideoCodec default_ulpfec_codec_; - VideoCodec default_rtx_codec_; + std::map default_apt_rtx_types_; }; class WebRtcVideoEngine2VoiceTest : public WebRtcVideoEngine2Test { @@ -429,7 +447,7 @@ TEST_F(WebRtcVideoEngine2VoiceTest, ConfiguresAvSyncForFirstReceiveChannel) { TEST_F(WebRtcVideoEngine2Test, FindCodec) { const std::vector& c = engine_.codecs(); - EXPECT_EQ(4U, c.size()); + EXPECT_EQ(cricket::DefaultVideoCodecList().size(), c.size()); cricket::VideoCodec vp8(104, "VP8", 320, 200, 30, 0); EXPECT_TRUE(engine_.FindCodec(vp8)); @@ -1840,7 +1858,7 @@ TEST_F(WebRtcVideoChannel2Test, SetDefaultSendCodecs) { EXPECT_EQ(1u, config.rtp.rtx.ssrcs.size()); EXPECT_EQ(kRtxSsrcs1[0], config.rtp.rtx.ssrcs[0]); - EXPECT_EQ(default_rtx_codec_.id, config.rtp.rtx.payload_type); + VerifySendStreamHasRtxTypes(config, default_apt_rtx_types_); // TODO(juberti): Check RTCP, PLI, TMMBR. } diff --git a/talk/media/webrtc/webrtcvideoengine_unittest.cc b/talk/media/webrtc/webrtcvideoengine_unittest.cc index e6ac7c1f4e..60e7e364a8 100644 --- a/talk/media/webrtc/webrtcvideoengine_unittest.cc +++ b/talk/media/webrtc/webrtcvideoengine_unittest.cc @@ -76,6 +76,9 @@ static const uint32 kSsrcs3[] = {1, 2, 3}; static const uint32 kRtxSsrcs1[] = {4}; static const uint32 kRtxSsrcs3[] = {4, 5, 6}; +static const int kPayloadTypes1[] = {96}; +static const int kPayloadTypes2[] = {96, 97}; + class FakeViEWrapper : public cricket::ViEWrapper { public: explicit FakeViEWrapper(cricket::FakeWebRtcVideoEngine* engine) @@ -755,8 +758,8 @@ TEST_F(WebRtcVideoEngineTestFake, SetRecvCodecsWithRtx) { EXPECT_STREQ("rtx", wcodec.plName); EXPECT_FALSE(vie_.ReceiveCodecRegistered(channel_num, wcodec)); - // The RTX payload type should have been set. - EXPECT_EQ(rtx_codec.id, vie_.GetRtxRecvPayloadType(channel_num)); + EXPECT_EQ(vie_.GetRtxRecvPayloadTypes(channel_num), + MAKE_VECTOR(kPayloadTypes1)); } // Test that RTX packets are routed to the default video channel if @@ -2144,7 +2147,8 @@ TEST_F(WebRtcVideoEngineTestFake, SetSendCodecsWithExternalH264) { codecs.push_back(rtx_codec); EXPECT_TRUE(channel_->SetSendCodecs(codecs)); - EXPECT_EQ(96, vie_.GetRtxSendPayloadType(channel_num)); + EXPECT_EQ(vie_.GetRtxSendPayloadTypes(channel_num), + MAKE_VECTOR(kPayloadTypes1)); cricket::StreamParams params = cricket::StreamParams::CreateLegacy(kSsrcs1[0]); @@ -2181,8 +2185,8 @@ TEST_F(WebRtcVideoEngineTestFake, SetSendCodecsWithVP8AndExternalH264) { EXPECT_TRUE(channel_->SetSendCodecs(codecs)); // The first matched codec should be set, i.e., H.264. - - EXPECT_EQ(96, vie_.GetRtxSendPayloadType(channel_num)); + EXPECT_EQ(vie_.GetRtxSendPayloadTypes(channel_num), + MAKE_VECTOR(kPayloadTypes1)); cricket::StreamParams params = cricket::StreamParams::CreateLegacy(kSsrcs1[0]); @@ -2219,7 +2223,8 @@ TEST_F(WebRtcVideoEngineTestFake, SetRecvCodecsWithExternalH264) { codecs.push_back(rtx_codec); EXPECT_TRUE(channel_->SetRecvCodecs(codecs)); - EXPECT_EQ(96, vie_.GetRtxRecvPayloadType(channel_num)); + EXPECT_EQ(vie_.GetRtxRecvPayloadTypes(channel_num), + MAKE_VECTOR(kPayloadTypes1)); cricket::StreamParams params = cricket::StreamParams::CreateLegacy(kSsrcs1[0]); @@ -2254,17 +2259,20 @@ 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_codec); - // Should fail since WebRTC only supports one RTX codec at a time. - EXPECT_FALSE(channel_->SetRecvCodecs(codecs)); + codecs.push_back(rtx_codec2); + EXPECT_TRUE(channel_->SetRecvCodecs(codecs)); + EXPECT_EQ(vie_.GetRtxRecvPayloadTypes(channel_num), + MAKE_VECTOR(kPayloadTypes2)); codecs.pop_back(); - // One RTX codec should be fine. EXPECT_TRUE(channel_->SetRecvCodecs(codecs)); - // The RTX payload type should have been set. - EXPECT_EQ(rtx_codec.id, vie_.GetRtxRecvPayloadType(channel_num)); + // 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_EQ(vie_.GetRtxRecvPayloadTypes(channel_num), + MAKE_VECTOR(kPayloadTypes2)); } // Tests that OnReadyToSend will be propagated into ViE. @@ -2313,7 +2321,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(4U, c.size()); + EXPECT_EQ(cricket::DefaultVideoCodecList().size(), c.size()); cricket::VideoCodec vp8(104, "VP8", 320, 200, 30, 0); EXPECT_TRUE(engine_.FindCodec(vp8)); @@ -2360,7 +2368,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; @@ -3264,7 +3272,8 @@ TEST_F(WebRtcVideoEngineSimulcastTestFake, TestStreamWithRtx) { EXPECT_TRUE(channel_->SetSendCodecs(codec_list)); // RTX payload type should now be set. - EXPECT_EQ(96, vie_.GetRtxSendPayloadType(channel_num)); + EXPECT_EQ(vie_.GetRtxSendPayloadTypes(channel_num), + MAKE_VECTOR(kPayloadTypes1)); // Verify all SSRCs are set after SetSendCodecs. EXPECT_EQ(3, vie_.GetNumSsrcs(channel_num)); diff --git a/webrtc/config.h b/webrtc/config.h index e43a58805d..aae60e7fa6 100644 --- a/webrtc/config.h +++ b/webrtc/config.h @@ -34,13 +34,19 @@ 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) {} + FecConfig() + : ulpfec_payload_type(-1), + red_payload_type(-1), + red_rtx_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 red_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 bc1ba2be41..bcafdfb7f6 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); + void SetRtxPayloadType(int payload_type, int associated_payload_type); bool IsRtx(const RTPHeader& header) const; @@ -158,7 +158,11 @@ class RTPPayloadRegistry { int8_t last_received_payload_type_; int8_t last_received_media_payload_type_; bool rtx_; - int8_t payload_type_rtx_; + // TODO(changbin): Remove rtx_payload_type_ once interop with old clients that + // only understand one RTX PT is no longer needed. + int rtx_payload_type_; + // Mapping rtx_payload_type_map_[rtx] = associated. + std::map rtx_payload_type_map_; 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 1ece5e559d..1623554eac 100644 --- a/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h @@ -235,10 +235,12 @@ 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) = 0; + virtual void SetRtxSendPayloadType(int payload_type, + int associated_payload_type) = 0; - // Gets the payload type to use when sending RTX packets. - virtual int RtxSendPayloadType() const = 0; + // Gets the payload type pair of (RTX, associated) to use when sending RTX + // packets. + virtual std::pair RtxSendPayloadType() const = 0; /* * sends kRtcpByeCode when going from true to false diff --git a/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h index 86a792ae27..0097d56e97 100644 --- a/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h @@ -97,10 +97,8 @@ class MockRtpRtcp : public RtpRtcp { MOCK_CONST_METHOD0(RtxSendStatus, int()); MOCK_METHOD1(SetRtxSsrc, void(uint32_t)); - MOCK_METHOD1(SetRtxSendPayloadType, - void(int)); - MOCK_CONST_METHOD0(RtxSendPayloadType, - int()); + MOCK_METHOD2(SetRtxSendPayloadType, void(int, int)); + MOCK_CONST_METHOD0(RtxSendPayloadType, std::pair()); 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 e5c4faff34..0dc3322771 100644 --- a/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc @@ -32,6 +32,8 @@ 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 { @@ -129,7 +131,10 @@ class RtxLoopBackTransport : public webrtc::Transport { if (!parser->Parse(restored_packet_ptr, packet_length, &header)) { return -1; } + } else { + rtp_payload_registry_->SetIncomingPayloadType(header); } + restored_packet_ptr += header.headerLength; packet_length -= header.headerLength; PayloadUnion payload_specific; @@ -203,15 +208,17 @@ class RtpRtcpRtxNackTest : public ::testing::Test { VideoCodec video_codec; memset(&video_codec, 0, sizeof(video_codec)); - video_codec.plType = 123; + video_codec.plType = kPayloadType; 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; @@ -262,12 +269,9 @@ 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, - 123, - timestamp, - timestamp / 90, - payload_data, - payload_data_length)); + EXPECT_EQ(0, rtp_rtcp_module_->SendOutgoingData( + webrtc::kVideoFrameDelta, kPayloadType, timestamp, + timestamp / 90, payload_data, payload_data_length)); // Min required delay until retransmit = 5 + RTT ms (RTT = 0). fake_clock.AdvanceTimeMilliseconds(5); int length = BuildNackList(nack_list); @@ -310,12 +314,9 @@ 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, - 123, - timestamp, - timestamp / 90, - payload_data, - payload_data_length)); + EXPECT_EQ(0, rtp_rtcp_module_->SendOutgoingData( + webrtc::kVideoFrameDelta, kPayloadType, 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 24f6d24db6..df3067ac6f 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc @@ -15,8 +15,7 @@ namespace webrtc { -RTPPayloadRegistry::RTPPayloadRegistry( - RTPPayloadStrategy* rtp_payload_strategy) +RTPPayloadRegistry::RTPPayloadRegistry(RTPPayloadStrategy* rtp_payload_strategy) : crit_sect_(CriticalSectionWrapper::CreateCriticalSection()), rtp_payload_strategy_(rtp_payload_strategy), red_payload_type_(-1), @@ -25,8 +24,9 @@ RTPPayloadRegistry::RTPPayloadRegistry( last_received_payload_type_(-1), last_received_media_payload_type_(-1), rtx_(false), - payload_type_rtx_(-1), - ssrc_rtx_(0) {} + rtx_payload_type_(-1), + ssrc_rtx_(0) { +} RTPPayloadRegistry::~RTPPayloadRegistry() { while (!payload_type_map_.empty()) { @@ -256,18 +256,18 @@ bool RTPPayloadRegistry::RestoreOriginalPacket(uint8_t** restored_packet, ByteWriter::WriteBigEndian(*restored_packet + 8, original_ssrc); CriticalSectionScoped cs(crit_sect_.get()); + if (!rtx_) + return true; - 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; - } + if (rtx_payload_type_ == -1 || incoming_payload_type_ == -1) { + LOG(LS_WARNING) << "Incorrect RTX configuration, dropping packet."; + return false; + } + // TODO(changbin): Will use RTX APT map for restoring packets, + // thus incoming_payload_type_ should be removed in future. + (*restored_packet)[1] = static_cast(incoming_payload_type_); + if (header.markerBit) { + (*restored_packet)[1] |= kRtpMarkerBitMask; // Marker bit is set. } return true; } @@ -284,11 +284,17 @@ bool RTPPayloadRegistry::GetRtxSsrc(uint32_t* ssrc) const { return rtx_; } -void RTPPayloadRegistry::SetRtxPayloadType(int payload_type) { +void RTPPayloadRegistry::SetRtxPayloadType(int payload_type, + int associated_payload_type) { CriticalSectionScoped cs(crit_sect_.get()); - assert(payload_type >= 0); - payload_type_rtx_ = payload_type; + if (payload_type < 0) { + LOG(LS_ERROR) << "Invalid RTX payload type: " << payload_type; + return; + } + + rtx_payload_type_map_[payload_type] = associated_payload_type; rtx_ = true; + rtx_payload_type_ = payload_type; } bool RTPPayloadRegistry::IsRed(const RTPHeader& header) const { diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 4f76ef2767..93b5f545b0 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -206,11 +206,12 @@ void ModuleRtpRtcpImpl::SetRtxSsrc(uint32_t ssrc) { rtp_sender_.SetRtxSsrc(ssrc); } -void ModuleRtpRtcpImpl::SetRtxSendPayloadType(int payload_type) { - rtp_sender_.SetRtxPayloadType(payload_type); +void ModuleRtpRtcpImpl::SetRtxSendPayloadType(int payload_type, + int associated_payload_type) { + rtp_sender_.SetRtxPayloadType(payload_type, associated_payload_type); } -int ModuleRtpRtcpImpl::RtxSendPayloadType() const { +std::pair ModuleRtpRtcpImpl::RtxSendPayloadType() const { return rtp_sender_.RtxPayloadType(); } diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h index 3390db3a24..77256b7fcd 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -88,8 +88,9 @@ class ModuleRtpRtcpImpl : public RtpRtcp { void SetRtxSsrc(uint32_t ssrc) override; - void SetRtxSendPayloadType(int payload_type) override; - int RtxSendPayloadType() const override; + void SetRtxSendPayloadType(int payload_type, + int associated_payload_type) override; + std::pair RtxSendPayloadType() const override; // Sends kRtcpByeCode when going from true to false. int32_t SetSendingStatus(bool sending) override; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc index b8d44f1332..508c0f545d 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc @@ -11,7 +11,9 @@ #include "webrtc/modules/rtp_rtcp/source/rtp_sender.h" #include // srand +#include +#include "webrtc/base/checks.h" #include "webrtc/modules/rtp_rtcp/interface/rtp_cvo.h" #include "webrtc/modules/rtp_rtcp/source/byte_io.h" #include "webrtc/modules/rtp_rtcp/source/rtp_sender_audio.h" @@ -155,7 +157,7 @@ RTPSender::RTPSender(int32_t id, last_packet_marker_bit_(false), csrcs_(), rtx_(kRtxOff), - payload_type_rtx_(-1), + rtx_payload_type_(-1), target_bitrate_critsect_(CriticalSectionWrapper::CreateCriticalSection()), target_bitrate_(0) { memset(nack_byte_count_times_, 0, sizeof(nack_byte_count_times_)); @@ -421,14 +423,28 @@ uint32_t RTPSender::RtxSsrc() const { return ssrc_rtx_; } -void RTPSender::SetRtxPayloadType(int payload_type) { +void RTPSender::SetRtxPayloadType(int payload_type, + int associated_payload_type) { CriticalSectionScoped cs(send_critsect_.get()); - payload_type_rtx_ = payload_type; + DCHECK_LE(payload_type, 127); + DCHECK_LE(associated_payload_type, 127); + if (payload_type < 0) { + LOG(LS_ERROR) << "Invalid RTX payload type: " << payload_type; + return; + } + + rtx_payload_type_map_[associated_payload_type] = payload_type; + rtx_payload_type_ = payload_type; } -int RTPSender::RtxPayloadType() const { +std::pair RTPSender::RtxPayloadType() const { CriticalSectionScoped cs(send_critsect_.get()); - return payload_type_rtx_; + for (const auto& kv : rtx_payload_type_map_) { + if (kv.second == rtx_payload_type_) { + return std::make_pair(rtx_payload_type_, kv.first); + } + } + return std::make_pair(-1, -1); } int32_t RTPSender::CheckPayloadType(int8_t payload_type, @@ -637,8 +653,7 @@ size_t RTPSender::SendPadData(uint32_t timestamp, ssrc = ssrc_rtx_; sequence_number = sequence_number_rtx_; ++sequence_number_rtx_; - payload_type = ((rtx_ & kRtxRedundantPayloads) > 0) ? payload_type_rtx_ - : payload_type_; + payload_type = rtx_payload_type_; over_rtx = true; } } @@ -1795,8 +1810,8 @@ void RTPSender::BuildRtxPacket(uint8_t* buffer, size_t* length, memcpy(data_buffer_rtx, buffer, rtp_header.headerLength); // 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 (rtx_payload_type_ != -1) { + data_buffer_rtx[1] = static_cast(rtx_payload_type_); 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 060dc648e1..ffe8ce5bec 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.h @@ -10,7 +10,6 @@ #ifndef WEBRTC_MODULES_RTP_RTCP_SOURCE_RTP_SENDER_H_ #define WEBRTC_MODULES_RTP_RTCP_SOURCE_RTP_SENDER_H_ - #include #include @@ -216,8 +215,8 @@ class RTPSender : public RTPSenderInterface { uint32_t RtxSsrc() const; void SetRtxSsrc(uint32_t ssrc); - void SetRtxPayloadType(int payloadType); - int RtxPayloadType() const; + void SetRtxPayloadType(int payload_type, int associated_payload_type); + std::pair RtxPayloadType() const; // Functions wrapping RTPSenderInterface. int32_t BuildRTPheader(uint8_t* data_buffer, @@ -426,7 +425,11 @@ 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_); - int payload_type_rtx_ GUARDED_BY(send_critsect_); + // TODO(changbin): Remove rtx_payload_type_ once interop with old clients that + // only understand one RTX PT is no longer needed. + int rtx_payload_type_ GUARDED_BY(send_critsect_); + // Mapping rtx_payload_type_map_[associated] = rtx. + std::map rtx_payload_type_map_ 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 07fc5e347d..9b168feffc 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -36,6 +36,7 @@ const int kTransmissionTimeOffsetExtensionId = 1; const int kAbsoluteSendTimeExtensionId = 14; const int kTransportSequenceNumberExtensionId = 13; const int kPayload = 100; +const int kRtxPayload = 98; const uint32_t kTimestamp = 10; const uint16_t kSeqNum = 33; const int kTimeOffset = 22222; @@ -822,6 +823,7 @@ 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, _, _, _, _, _)). @@ -1291,7 +1293,7 @@ TEST_F(RtpSenderTest, BytesReportedCorrectly) { const uint8_t kPayloadType = 127; rtp_sender_->SetSSRC(1234); rtp_sender_->SetRtxSsrc(4321); - rtp_sender_->SetRtxPayloadType(kPayloadType - 1); + rtp_sender_->SetRtxPayloadType(kPayloadType - 1, kPayloadType); 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 6b4e55df66..5731efde9e 100644 --- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc +++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc @@ -174,9 +174,10 @@ TEST_F(RtpRtcpAPITest, RtxSender) { 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); + rtp_payload_registry_->SetRtxPayloadType(kRtxPayloadType, kPayloadType); 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 5d611432f4..e9129824cd 100644 --- a/webrtc/test/call_test.cc +++ b/webrtc/test/call_test.cc @@ -160,6 +160,7 @@ 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 b8d13c4fd9..ee1bafddfb 100644 --- a/webrtc/test/call_test.h +++ b/webrtc/test/call_test.h @@ -37,6 +37,7 @@ 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 956e8219fa..389d773d7e 100644 --- a/webrtc/video/end_to_end_tests.cc +++ b/webrtc/video/end_to_end_tests.cc @@ -71,7 +71,7 @@ class EndToEndTest : public test::CallTest { } }; - void DecodesRetransmittedFrame(bool retransmit_over_rtx); + void DecodesRetransmittedFrame(bool use_rtx, bool use_red); void ReceivesPliAndRecovers(int rtp_history_ms); void RespectsRtcpMode(newapi::RtcpMode rtcp_mode); void TestXrReceiverReferenceTimeReport(bool enable_rrtr); @@ -671,16 +671,16 @@ void EndToEndTest::TestReceivedFecPacketsNotNacked( // 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 retransmit_over_rtx) { +void EndToEndTest::DecodesRetransmittedFrame(bool use_rtx, bool use_red) { static const int kDroppedFrameNumber = 2; class RetransmissionObserver : public test::EndToEndTest, public I420FrameCallback { public: - explicit RetransmissionObserver(bool expect_rtx) + explicit RetransmissionObserver(bool use_rtx, bool use_red) : EndToEndTest(kDefaultTimeoutMs), - retransmission_ssrc_(expect_rtx ? kSendRtxSsrcs[0] : kSendSsrcs[0]), - retransmission_payload_type_(expect_rtx ? kSendRtxPayloadType - : kFakeSendPayloadType), + payload_type_(GetPayloadType(false, use_red)), + retransmission_ssrc_(use_rtx ? kSendRtxSsrcs[0] : kSendSsrcs[0]), + retransmission_payload_type_(GetPayloadType(use_rtx, use_red)), marker_bits_observed_(0), retransmitted_timestamp_(0), frame_retransmitted_(false) {} @@ -698,7 +698,7 @@ void EndToEndTest::DecodesRetransmittedFrame(bool retransmit_over_rtx) { } EXPECT_EQ(kSendSsrcs[0], header.ssrc); - EXPECT_EQ(kFakeSendPayloadType, header.payloadType); + EXPECT_EQ(payload_type_, header.payloadType); // Found the second frame's final packet, drop this and expect a // retransmission. @@ -724,12 +724,20 @@ void EndToEndTest::DecodesRetransmittedFrame(bool retransmit_over_rtx) { 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[kSendRtxPayloadType].ssrc = + (*receive_configs)[0].rtp.rtx[kFakeSendPayloadType].ssrc = kSendRtxSsrcs[0]; - (*receive_configs)[0].rtp.rtx[kSendRtxPayloadType].payload_type = + (*receive_configs)[0].rtp.rtx[kFakeSendPayloadType].payload_type = kSendRtxPayloadType; } } @@ -739,22 +747,36 @@ void EndToEndTest::DecodesRetransmittedFrame(bool retransmit_over_rtx) { << "Timed out while waiting for retransmission to render."; } + int GetPayloadType(bool use_rtx, bool use_red) { + return use_rtx ? kSendRtxPayloadType + : (use_red ? kRedPayloadType : 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(retransmit_over_rtx); + } test(use_rtx, use_red); RunBaseTest(&test); } TEST_F(EndToEndTest, DecodesRetransmittedFrame) { - DecodesRetransmittedFrame(false); + DecodesRetransmittedFrame(false, false); } TEST_F(EndToEndTest, DecodesRetransmittedFrameOverRtx) { - DecodesRetransmittedFrame(true); + DecodesRetransmittedFrame(true, false); +} + +TEST_F(EndToEndTest, DecodesRetransmittedFrameByRed) { + DecodesRetransmittedFrame(false, true); +} + +TEST_F(EndToEndTest, DecodesRetransmittedFrameByRedOverRtx) { + DecodesRetransmittedFrame(true, true); } TEST_F(EndToEndTest, UsesFrameCallbacks) { @@ -2589,5 +2611,4 @@ TEST_F(EndToEndTest, CanCreateAndDestroyManyVideoStreams) { call->DestroyVideoReceiveStream(receive_stream); } } - } // namespace webrtc diff --git a/webrtc/video/loopback.cc b/webrtc/video/loopback.cc index 9ef0ec4224..881fa49a1c 100644 --- a/webrtc/video/loopback.cc +++ b/webrtc/video/loopback.cc @@ -39,7 +39,8 @@ static const uint32_t kSendSsrc = 0x654321; static const uint32_t kSendRtxSsrc = 0x654322; static const uint32_t kReceiverLocalSsrc = 0x123456; -static const uint8_t kRtxPayloadType = 96; +static const uint8_t kRtxVideoPayloadType = 96; +static const uint8_t kVideoPayloadType = 124; Loopback::Loopback(const Config& config) : config_(config), clock_(Clock::GetRealTimeClock()) { @@ -83,7 +84,7 @@ void Loopback::Run() { 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 = kRtxPayloadType; + send_config.rtp.rtx.payload_type = kRtxVideoPayloadType; send_config.rtp.nack.rtp_history_ms = 1000; send_config.rtp.extensions.push_back( RtpExtension(RtpExtension::kAbsSendTime, kAbsSendTimeExtensionId)); @@ -101,7 +102,7 @@ void Loopback::Run() { } send_config.encoder_settings.encoder = encoder.get(); send_config.encoder_settings.payload_name = config_.codec; - send_config.encoder_settings.payload_type = 124; + send_config.encoder_settings.payload_type = kVideoPayloadType; VideoEncoderConfig encoder_config(CreateEncoderConfig()); @@ -114,8 +115,8 @@ void Loopback::Run() { 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[kRtxPayloadType].ssrc = kSendRtxSsrc; - receive_config.rtp.rtx[kRtxPayloadType].payload_type = kRtxPayloadType; + receive_config.rtp.rtx[kVideoPayloadType].ssrc = kSendRtxSsrc; + receive_config.rtp.rtx[kVideoPayloadType].payload_type = kRtxVideoPayloadType; 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 7d8f98143b..4ba4cbabe7 100644 --- a/webrtc/video/rampup_tests.cc +++ b/webrtc/video/rampup_tests.cc @@ -77,6 +77,8 @@ 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); } void StreamObserver::set_expected_bitrate_bps( @@ -137,11 +139,9 @@ 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; - payload_registry_->RestoreOriginalPacket(&restored_packet_ptr, - packet, - &restored_length, - rtx_media_ssrcs_[header.ssrc], - header); + EXPECT_TRUE(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,10 +367,11 @@ EventTypeWrapper LowRateStreamObserver::Wait() { return test_done_->Wait(test::CallTest::kLongTimeoutMs); } -void RampUpTest::RunRampUpTest(bool rtx, - size_t num_streams, +void RampUpTest::RunRampUpTest(size_t num_streams, unsigned int start_bitrate_bps, - const std::string& extension_type) { + const std::string& extension_type, + bool rtx, + bool red) { std::vector ssrcs(GenerateSsrcs(num_streams, 100)); std::vector rtx_ssrcs(GenerateSsrcs(num_streams, 200)); StreamObserver::SsrcMap rtx_ssrc_map; @@ -424,6 +425,10 @@ void RampUpTest::RunRampUpTest(bool rtx, 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; + } if (num_streams == 1) { // For single stream rampup until 1mbps @@ -450,7 +455,9 @@ void RampUpTest::RunRampUpTest(bool rtx, DestroyStreams(); } -void RampUpTest::RunRampUpDownUpTest(size_t number_of_streams, bool rtx) { +void RampUpTest::RunRampUpDownUpTest(size_t number_of_streams, + bool rtx, + bool red) { test::DirectTransport receiver_transport; LowRateStreamObserver stream_observer( &receiver_transport, Clock::GetRealTimeClock(), number_of_streams, rtx); @@ -469,6 +476,10 @@ void RampUpTest::RunRampUpDownUpTest(size_t number_of_streams, bool rtx) { 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; + } CreateStreams(); stream_observer.SetSendStream(send_stream_); @@ -484,43 +495,68 @@ void RampUpTest::RunRampUpDownUpTest(size_t number_of_streams, bool rtx) { } TEST_F(RampUpTest, SingleStream) { - RunRampUpTest(false, 1, 0, RtpExtension::kTOffset); + RunRampUpTest(1, 0, RtpExtension::kTOffset, false, false); } TEST_F(RampUpTest, Simulcast) { - RunRampUpTest(false, 3, 0, RtpExtension::kTOffset); + RunRampUpTest(3, 0, RtpExtension::kTOffset, false, false); } TEST_F(RampUpTest, SimulcastWithRtx) { - RunRampUpTest(true, 3, 0, RtpExtension::kTOffset); + RunRampUpTest(3, 0, RtpExtension::kTOffset, true, false); +} + +TEST_F(RampUpTest, SimulcastByRedWithRtx) { + RunRampUpTest(3, 0, RtpExtension::kTOffset, true, true); } TEST_F(RampUpTest, SingleStreamWithHighStartBitrate) { - RunRampUpTest(false, 1, 0.9 * kSingleStreamTargetBps, RtpExtension::kTOffset); + RunRampUpTest(1, 0.9 * kSingleStreamTargetBps, RtpExtension::kTOffset, false, + false); } -TEST_F(RampUpTest, UpDownUpOneStream) { RunRampUpDownUpTest(1, false); } +TEST_F(RampUpTest, UpDownUpOneStream) { + RunRampUpDownUpTest(1, false, false); +} -TEST_F(RampUpTest, UpDownUpThreeStreams) { RunRampUpDownUpTest(3, false); } +TEST_F(RampUpTest, UpDownUpThreeStreams) { + RunRampUpDownUpTest(3, false, false); +} -TEST_F(RampUpTest, UpDownUpOneStreamRtx) { RunRampUpDownUpTest(1, true); } +TEST_F(RampUpTest, UpDownUpOneStreamRtx) { + RunRampUpDownUpTest(1, true, false); +} -TEST_F(RampUpTest, UpDownUpThreeStreamsRtx) { RunRampUpDownUpTest(3, 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, AbsSendTimeSingleStream) { - RunRampUpTest(false, 1, 0, RtpExtension::kAbsSendTime); + RunRampUpTest(1, 0, RtpExtension::kAbsSendTime, false, false); } TEST_F(RampUpTest, AbsSendTimeSimulcast) { - RunRampUpTest(false, 3, 0, RtpExtension::kAbsSendTime); + RunRampUpTest(3, 0, RtpExtension::kAbsSendTime, false, false); } TEST_F(RampUpTest, AbsSendTimeSimulcastWithRtx) { - RunRampUpTest(true, 3, 0, RtpExtension::kAbsSendTime); + RunRampUpTest(3, 0, RtpExtension::kAbsSendTime, true, false); +} + +TEST_F(RampUpTest, AbsSendTimeSimulcastByRedWithRtx) { + RunRampUpTest(3, 0, RtpExtension::kAbsSendTime, true, true); } TEST_F(RampUpTest, AbsSendTimeSingleStreamWithHighStartBitrate) { - RunRampUpTest(false, 1, 0.9 * kSingleStreamTargetBps, - RtpExtension::kAbsSendTime); + RunRampUpTest(1, 0.9 * kSingleStreamTargetBps, RtpExtension::kAbsSendTime, + false, false); } } // namespace webrtc diff --git a/webrtc/video/rampup_tests.h b/webrtc/video/rampup_tests.h index 4335fc13a8..ed0d7951ec 100644 --- a/webrtc/video/rampup_tests.h +++ b/webrtc/video/rampup_tests.h @@ -147,12 +147,13 @@ class LowRateStreamObserver : public test::DirectTransport, class RampUpTest : public test::CallTest { protected: - void RunRampUpTest(bool rtx, - size_t num_streams, + void RunRampUpTest(size_t num_streams, unsigned int start_bitrate_bps, - const std::string& extension_type); + const std::string& extension_type, + bool rtx, + bool red); - void RunRampUpDownUpTest(size_t number_of_streams, bool rtx); + void RunRampUpDownUpTest(size_t number_of_streams, bool rtx, bool red); }; } // 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 a8dea5eb78..fd04079f65 100644 --- a/webrtc/video/video_receive_stream.cc +++ b/webrtc/video/video_receive_stream.cc @@ -164,12 +164,12 @@ VideoReceiveStream::VideoReceiveStream(webrtc::VideoEngine* video_engine, vie_channel_->SetSSRC(config_.rtp.local_ssrc, kViEStreamTypeNormal, 0); // TODO(pbos): Support multiple RTX, per video payload. Config::Rtp::RtxMap::const_iterator it = config_.rtp.rtx.begin(); - if (it != config_.rtp.rtx.end()) { + for (; it != config_.rtp.rtx.end(); ++it) { DCHECK(it->second.ssrc != 0); DCHECK(it->second.payload_type != 0); vie_channel_->SetRemoteSSRCType(kViEStreamTypeRtx, it->second.ssrc); - vie_channel_->SetRtxReceivePayloadType(it->second.payload_type); + vie_channel_->SetRtxReceivePayloadType(it->second.payload_type, it->first); } // TODO(pbos): Remove channel_group_ usage from VideoReceiveStream. This @@ -212,6 +212,11 @@ VideoReceiveStream::VideoReceiveStream(webrtc::VideoEngine* video_engine, strcpy(codec.plName, "red"); codec.plType = config_.rtp.fec.red_payload_type; CHECK_EQ(0, vie_channel_->SetReceiveCodec(codec)); + if (config_.rtp.fec.red_rtx_payload_type != -1) { + vie_channel_->SetRtxReceivePayloadType( + config_.rtp.fec.red_rtx_payload_type, + config_.rtp.fec.red_payload_type); + } } if (config.rtp.rtcp_xr.receiver_reference_time_report) diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index 9949f5017e..59111fa723 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -163,6 +163,7 @@ VideoSendStream::VideoSendStream( static_cast(config_.rtp.fec.red_payload_type), static_cast(config_.rtp.fec.ulpfec_payload_type)); } + // TODO(changbin): Should set RTX for RED mapping in RTP sender in future. } else { rtp_rtcp_->SetNACKStatus(channel_, config_.rtp.nack.rtp_history_ms > 0); } @@ -460,7 +461,8 @@ void VideoSendStream::ConfigureSsrcs() { } DCHECK_GE(config_.rtp.rtx.payload_type, 0); - rtp_rtcp_->SetRtxSendPayloadType(channel_, config_.rtp.rtx.payload_type); + rtp_rtcp_->SetRtxSendPayloadType(channel_, config_.rtp.rtx.payload_type, + config_.encoder_settings.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 a57e4f3628..7b72b80817 100644 --- a/webrtc/video_engine/include/vie_rtp_rtcp.h +++ b/webrtc/video_engine/include/vie_rtp_rtcp.h @@ -114,10 +114,13 @@ 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) = 0; + 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; + virtual int SetRtxReceivePayloadType( + const int video_channel, + const uint8_t payload_type, + const uint8_t associated_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 ffc9757984..52cf078d89 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,6 +40,7 @@ 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 @@ -244,14 +245,15 @@ int VideoEngineSampleCode(void* window1, void* window2) return -1; } - error = ptrViERtpRtcp->SetRtxSendPayloadType(videoChannel, kRtxPayloadType); + error = ptrViERtpRtcp->SetRtxSendPayloadType(videoChannel, kRtxPayloadType, + kPayloadType); if (error == -1) { printf("ERROR in ViERTP_RTCP::SetRtxSendPayloadType\n"); return -1; } - error = ptrViERtpRtcp->SetRtxReceivePayloadType(videoChannel, - kRtxPayloadType); + error = ptrViERtpRtcp->SetRtxReceivePayloadType( + videoChannel, kRtxPayloadType, kPayloadType); 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 47dbae4e42..ee9c6a0628 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,14 +172,15 @@ 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)); - EXPECT_EQ(0, ViE.rtp_rtcp->SetRtxReceivePayloadType(tbChannel.videoChannel, - kRtxPayloadType)); + 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->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 353dc12863..935e8853ec 100644 --- a/webrtc/video_engine/vie_channel.cc +++ b/webrtc/video_engine/vie_channel.cc @@ -428,7 +428,10 @@ int32_t ViEChannel::SetSendCodec(const VideoCodec& video_codec, } rtp_rtcp->SetSendingStatus(rtp_rtcp_->Sending()); rtp_rtcp->SetSendingMediaStatus(rtp_rtcp_->SendingMedia()); - rtp_rtcp->SetRtxSendPayloadType(rtp_rtcp_->RtxSendPayloadType()); + std::pair payload_type_value = + rtp_rtcp_->RtxSendPayloadType(); + rtp_rtcp->SetRtxSendPayloadType(payload_type_value.first, + payload_type_value.second); rtp_rtcp->SetRtxSendStatus(rtp_rtcp_->RtxSendStatus()); simulcast_rtp_rtcp_.push_back(rtp_rtcp); @@ -1038,11 +1041,13 @@ int32_t ViEChannel::GetRemoteCSRC(uint32_t CSRCs[kRtpCsrcSize]) { return 0; } -int ViEChannel::SetRtxSendPayloadType(int payload_type) { - rtp_rtcp_->SetRtxSendPayloadType(payload_type); +int ViEChannel::SetRtxSendPayloadType(int payload_type, + int associated_payload_type) { + rtp_rtcp_->SetRtxSendPayloadType(payload_type, associated_payload_type); + CriticalSectionScoped cs(rtp_rtcp_cs_.get()); for (std::list::iterator it = simulcast_rtp_rtcp_.begin(); it != simulcast_rtp_rtcp_.end(); it++) { - (*it)->SetRtxSendPayloadType(payload_type); + (*it)->SetRtxSendPayloadType(payload_type, associated_payload_type); } SetRtxSendStatus(true); return 0; @@ -1059,8 +1064,9 @@ void ViEChannel::SetRtxSendStatus(bool enable) { } } -void ViEChannel::SetRtxReceivePayloadType(int payload_type) { - vie_receiver_.SetRtxPayloadType(payload_type); +void ViEChannel::SetRtxReceivePayloadType(int payload_type, + int associated_payload_type) { + vie_receiver_.SetRtxPayloadType(payload_type, associated_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 a3ad72241a..4bd27de5e1 100644 --- a/webrtc/video_engine/vie_channel.h +++ b/webrtc/video_engine/vie_channel.h @@ -154,8 +154,8 @@ class ViEChannel // Gets the CSRC for the incoming stream. int32_t GetRemoteCSRC(uint32_t CSRCs[kRtpCsrcSize]); - int SetRtxSendPayloadType(int payload_type); - void SetRtxReceivePayloadType(int payload_type); + int SetRtxSendPayloadType(int payload_type, int associated_payload_type); + void SetRtxReceivePayloadType(int payload_type, int associated_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 e61c82bb9d..0fe12b4770 100644 --- a/webrtc/video_engine/vie_receiver.cc +++ b/webrtc/video_engine/vie_receiver.cc @@ -119,8 +119,10 @@ void ViEReceiver::SetNackStatus(bool enable, rtp_receiver_->SetNACKStatus(enable ? kNackRtcp : kNackOff); } -void ViEReceiver::SetRtxPayloadType(int payload_type) { - rtp_payload_registry_->SetRtxPayloadType(payload_type); +void ViEReceiver::SetRtxPayloadType(int payload_type, + int associated_payload_type) { + rtp_payload_registry_->SetRtxPayloadType(payload_type, + associated_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 5c09a3e482..0a5229321f 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); + void SetRtxPayloadType(int payload_type, int associated_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 4fbf11b6a5..9b2a4391b6 100644 --- a/webrtc/video_engine/vie_rtp_rtcp_impl.cc +++ b/webrtc/video_engine/vie_rtp_rtcp_impl.cc @@ -191,8 +191,10 @@ int ViERTP_RTCPImpl::GetRemoteCSRCs(const int video_channel, return 0; } -int ViERTP_RTCPImpl::SetRtxSendPayloadType(const int video_channel, - const uint8_t payload_type) { +int ViERTP_RTCPImpl::SetRtxSendPayloadType( + const int video_channel, + const uint8_t payload_type, + const uint8_t associated_payload_type) { LOG_F(LS_INFO) << "channel: " << video_channel << " payload_type: " << static_cast(payload_type); ViEChannelManagerScoped cs(*(shared_data_->channel_manager())); @@ -201,14 +203,17 @@ int ViERTP_RTCPImpl::SetRtxSendPayloadType(const int video_channel, shared_data_->SetLastError(kViERtpRtcpInvalidChannelId); return -1; } - if (vie_channel->SetRtxSendPayloadType(payload_type) != 0) { + if (vie_channel->SetRtxSendPayloadType(payload_type, + associated_payload_type) != 0) { return -1; } return 0; } -int ViERTP_RTCPImpl::SetRtxReceivePayloadType(const int video_channel, - const uint8_t payload_type) { +int ViERTP_RTCPImpl::SetRtxReceivePayloadType( + const int video_channel, + const uint8_t payload_type, + const uint8_t associated_payload_type) { LOG_F(LS_INFO) << "channel: " << video_channel << " payload_type: " << static_cast(payload_type); ViEChannelManagerScoped cs(*(shared_data_->channel_manager())); @@ -217,7 +222,7 @@ int ViERTP_RTCPImpl::SetRtxReceivePayloadType(const int video_channel, shared_data_->SetLastError(kViERtpRtcpInvalidChannelId); return -1; } - vie_channel->SetRtxReceivePayloadType(payload_type); + vie_channel->SetRtxReceivePayloadType(payload_type, associated_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 7894f59b83..e22747f918 100644 --- a/webrtc/video_engine/vie_rtp_rtcp_impl.h +++ b/webrtc/video_engine/vie_rtp_rtcp_impl.h @@ -40,9 +40,11 @@ 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 payload_type, + const uint8_t associated_payload_type); virtual int SetRtxReceivePayloadType(const int video_channel, - const uint8_t payload_type); + const uint8_t payload_type, + const uint8_t associated_payload_type); virtual int SetStartSequenceNumber(const int video_channel, uint16_t sequence_number); void SetRtpStateForSsrc(int video_channel,