From de305014c62832a382d38144a9dc518cf1d02f88 Mon Sep 17 00:00:00 2001 From: "wu@webrtc.org" Date: Thu, 31 Oct 2013 15:40:38 +0000 Subject: [PATCH] Update talk to 55906045. Review URL: https://webrtc-codereview.appspot.com/3159005 git-svn-id: http://webrtc.googlecode.com/svn/trunk@5065 4adac7df-926f-26a2-2b94-8c16560cd09d --- talk/app/webrtc/mediaconstraintsinterface.h | 2 ++ talk/app/webrtc/webrtcsession.cc | 31 ++++++++++++++-- talk/app/webrtc/webrtcsession.h | 2 ++ talk/app/webrtc/webrtcsession_unittest.cc | 26 ++++++++++++++ talk/media/base/fakenetworkinterface.h | 8 ++++- talk/media/base/mediachannel.h | 29 +++++++++++++-- talk/media/webrtc/webrtcvideoengine.cc | 15 ++++++++ .../webrtc/webrtcvideoengine_unittest.cc | 15 ++++++++ talk/media/webrtc/webrtcvideoframe.cc | 35 ++++++++----------- talk/media/webrtc/webrtcvideoframe.h | 2 -- talk/media/webrtc/webrtcvoiceengine.cc | 15 ++++++++ .../webrtc/webrtcvoiceengine_unittest.cc | 18 ++++++++++ talk/p2p/base/p2ptransportchannel.cc | 1 + talk/session/media/channel.cc | 4 ++- 14 files changed, 174 insertions(+), 29 deletions(-) diff --git a/talk/app/webrtc/mediaconstraintsinterface.h b/talk/app/webrtc/mediaconstraintsinterface.h index 97d5cf63f5..ba6b09be91 100644 --- a/talk/app/webrtc/mediaconstraintsinterface.h +++ b/talk/app/webrtc/mediaconstraintsinterface.h @@ -110,6 +110,8 @@ class MediaConstraintsInterface { // TODO(perkj): Remove kEnableSctpDataChannels once Chrome use // PeerConnectionFactory::SetOptions. static const char kEnableSctpDataChannels[]; // Enable SCTP DataChannels + // Temporary pseudo-constraint for enabling DSCP through JS. + static const char kEnableDscp[]; // The prefix of internal-only constraints whose JS set values should be // stripped by Chrome before passed down to Libjingle. diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc index 3f14268376..a96c2da8cc 100644 --- a/talk/app/webrtc/webrtcsession.cc +++ b/talk/app/webrtc/webrtcsession.cc @@ -57,6 +57,8 @@ namespace webrtc { const char MediaConstraintsInterface::kInternalConstraintPrefix[] = "internal"; // Supported MediaConstraints. +// DSCP constraints. +const char MediaConstraintsInterface::kEnableDscp[] = "googDscp"; // DTLS-SRTP pseudo-constraints. const char MediaConstraintsInterface::kEnableDtlsSrtp[] = "DtlsSrtpKeyAgreement"; @@ -430,6 +432,7 @@ WebRtcSession::WebRtcSession( ice_connection_state_(PeerConnectionInterface::kIceConnectionNew), older_version_remote_peer_(false), dtls_enabled_(false), + dscp_enabled_(false), data_channel_type_(cricket::DCT_NONE), ice_restart_latch_(new IceRestartAnswerLatch) { } @@ -491,6 +494,14 @@ bool WebRtcSession::Initialize( mediastream_signaling_->SetDataChannelFactory(this); } + // Find DSCP constraint. + if (FindConstraint( + constraints, + MediaConstraintsInterface::kEnableDscp, + &value, NULL)) { + dscp_enabled_ = value; + } + const cricket::VideoCodec default_codec( JsepSessionDescription::kDefaultVideoCodecId, JsepSessionDescription::kDefaultVideoCodecName, @@ -1357,13 +1368,29 @@ bool WebRtcSession::CreateChannels(const SessionDescription* desc) { bool WebRtcSession::CreateVoiceChannel(const cricket::ContentInfo* content) { voice_channel_.reset(channel_manager_->CreateVoiceChannel( this, content->name, true)); - return (voice_channel_ != NULL); + if (!voice_channel_.get()) + return false; + + if (dscp_enabled_) { + cricket::AudioOptions options; + options.dscp.Set(true); + voice_channel_->SetChannelOptions(options); + } + return true; } bool WebRtcSession::CreateVideoChannel(const cricket::ContentInfo* content) { video_channel_.reset(channel_manager_->CreateVideoChannel( this, content->name, true, voice_channel_.get())); - return (video_channel_ != NULL); + if (!video_channel_.get()) + return false; + + if (dscp_enabled_) { + cricket::VideoOptions options; + options.dscp.Set(true); + video_channel_->SetChannelOptions(options); + } + return true; } bool WebRtcSession::CreateDataChannel(const cricket::ContentInfo* content) { diff --git a/talk/app/webrtc/webrtcsession.h b/talk/app/webrtc/webrtcsession.h index c4f6055919..893ab3fb43 100644 --- a/talk/app/webrtc/webrtcsession.h +++ b/talk/app/webrtc/webrtcsession.h @@ -313,6 +313,8 @@ class WebRtcSession : public cricket::BaseSession, // If the remote peer is using a older version of implementation. bool older_version_remote_peer_; bool dtls_enabled_; + // Flag will be set based on the constraint value. + bool dscp_enabled_; // Specifies which kind of data channel is allowed. This is controlled // by the chrome command-line flag and constraints: // 1. If chrome command-line switch 'enable-sctp-data-channels' is enabled, diff --git a/talk/app/webrtc/webrtcsession_unittest.cc b/talk/app/webrtc/webrtcsession_unittest.cc index ecbba1817c..0ddc16cf2f 100644 --- a/talk/app/webrtc/webrtcsession_unittest.cc +++ b/talk/app/webrtc/webrtcsession_unittest.cc @@ -2791,6 +2791,32 @@ TEST_F(WebRtcSessionTest, TestSetRemoteOfferFailIfDtlsDisabledAndNoCrypto) { offer); } +// This test verifies DSCP is properly applied on the media channels. +TEST_F(WebRtcSessionTest, TestDscpConstraint) { + constraints_.reset(new FakeConstraints()); + constraints_->AddOptional( + webrtc::MediaConstraintsInterface::kEnableDscp, true); + Init(NULL); + mediastream_signaling_.SendAudioVideoStream1(); + SessionDescriptionInterface* offer = CreateOffer(NULL); + + SetLocalDescriptionWithoutError(offer); + + video_channel_ = media_engine_->GetVideoChannel(0); + voice_channel_ = media_engine_->GetVoiceChannel(0); + + ASSERT_TRUE(video_channel_ != NULL); + ASSERT_TRUE(voice_channel_ != NULL); + cricket::AudioOptions audio_options; + EXPECT_TRUE(voice_channel_->GetOptions(&audio_options)); + cricket::VideoOptions video_options; + EXPECT_TRUE(video_channel_->GetOptions(&video_options)); + EXPECT_TRUE(audio_options.dscp.IsSet()); + EXPECT_TRUE(audio_options.dscp.GetWithDefaultIfUnset(false)); + EXPECT_TRUE(video_options.dscp.IsSet()); + EXPECT_TRUE(video_options.dscp.GetWithDefaultIfUnset(false)); +} + // TODO(bemasc): Add a TestIceStatesBundle with BUNDLE enabled. That test // currently fails because upon disconnection and reconnection OnIceComplete is // called more than once without returning to IceGatheringGathering. diff --git a/talk/media/base/fakenetworkinterface.h b/talk/media/base/fakenetworkinterface.h index d0f277e8f4..37679eb601 100644 --- a/talk/media/base/fakenetworkinterface.h +++ b/talk/media/base/fakenetworkinterface.h @@ -34,6 +34,7 @@ #include "talk/base/buffer.h" #include "talk/base/byteorder.h" #include "talk/base/criticalsection.h" +#include "talk/base/dscp.h" #include "talk/base/messagehandler.h" #include "talk/base/messagequeue.h" #include "talk/base/thread.h" @@ -51,7 +52,8 @@ class FakeNetworkInterface : public MediaChannel::NetworkInterface, dest_(NULL), conf_(false), sendbuf_size_(-1), - recvbuf_size_(-1) { + recvbuf_size_(-1), + dscp_(talk_base::DSCP_NO_CHANGE) { } void SetDestination(MediaChannel* dest) { dest_ = dest; } @@ -128,6 +130,7 @@ class FakeNetworkInterface : public MediaChannel::NetworkInterface, int sendbuf_size() const { return sendbuf_size_; } int recvbuf_size() const { return recvbuf_size_; } + talk_base::DiffServCodePoint dscp() const { return dscp_; } protected: virtual bool SendPacket(talk_base::Buffer* packet, @@ -182,6 +185,8 @@ class FakeNetworkInterface : public MediaChannel::NetworkInterface, sendbuf_size_ = option; } else if (opt == talk_base::Socket::OPT_RCVBUF) { recvbuf_size_ = option; + } else if (opt == talk_base::Socket::OPT_DSCP) { + dscp_ = static_cast(option); } return 0; } @@ -244,6 +249,7 @@ class FakeNetworkInterface : public MediaChannel::NetworkInterface, std::vector rtcp_packets_; int sendbuf_size_; int recvbuf_size_; + talk_base::DiffServCodePoint dscp_; }; } // namespace cricket diff --git a/talk/media/base/mediachannel.h b/talk/media/base/mediachannel.h index 49b3336243..919248f7f3 100644 --- a/talk/media/base/mediachannel.h +++ b/talk/media/base/mediachannel.h @@ -183,6 +183,7 @@ struct AudioOptions { rx_agc_limiter.SetFrom(change.rx_agc_limiter); recording_sample_rate.SetFrom(change.recording_sample_rate); playout_sample_rate.SetFrom(change.playout_sample_rate); + dscp.SetFrom(change.dscp); } bool operator==(const AudioOptions& o) const { @@ -206,7 +207,8 @@ struct AudioOptions { rx_agc_digital_compression_gain == o.rx_agc_digital_compression_gain && rx_agc_limiter == o.rx_agc_limiter && recording_sample_rate == o.recording_sample_rate && - playout_sample_rate == o.playout_sample_rate; + playout_sample_rate == o.playout_sample_rate && + dscp == o.dscp; } std::string ToString() const { @@ -235,6 +237,7 @@ struct AudioOptions { ost << ToStringIfSet("rx_agc_limiter", rx_agc_limiter); ost << ToStringIfSet("recording_sample_rate", recording_sample_rate); ost << ToStringIfSet("playout_sample_rate", playout_sample_rate); + ost << ToStringIfSet("dscp", dscp); ost << "}"; return ost.str(); } @@ -269,6 +272,8 @@ struct AudioOptions { Settable rx_agc_limiter; Settable recording_sample_rate; Settable playout_sample_rate; + // Set DSCP value for packet sent from audio channel. + Settable dscp; }; // Options that can be applied to a VideoMediaChannel or a VideoMediaEngine. @@ -307,6 +312,7 @@ struct VideoOptions { change.system_high_adaptation_threshhold); buffered_mode_latency.SetFrom(change.buffered_mode_latency); lower_min_bitrate.SetFrom(change.lower_min_bitrate); + dscp.SetFrom(change.dscp); } bool operator==(const VideoOptions& o) const { @@ -331,7 +337,8 @@ struct VideoOptions { system_high_adaptation_threshhold == o.system_high_adaptation_threshhold && buffered_mode_latency == o.buffered_mode_latency && - lower_min_bitrate == o.lower_min_bitrate; + lower_min_bitrate == o.lower_min_bitrate && + dscp == o.dscp; } std::string ToString() const { @@ -359,6 +366,7 @@ struct VideoOptions { ost << ToStringIfSet("high", system_high_adaptation_threshhold); ost << ToStringIfSet("buffered mode latency", buffered_mode_latency); ost << ToStringIfSet("lower min bitrate", lower_min_bitrate); + ost << ToStringIfSet("dscp", dscp); ost << "}"; return ost.str(); } @@ -405,6 +413,8 @@ struct VideoOptions { Settable buffered_mode_latency; // Make minimum configured send bitrate even lower than usual, at 30kbit. Settable lower_min_bitrate; + // Set DSCP value for packet sent from video channel. + Settable dscp; }; // A class for playing out soundclips. @@ -543,6 +553,21 @@ class MediaChannel : public sigslot::has_slots<> { return network_interface_->SetOption(type, opt, option); } + protected: + // This method sets DSCP |value| on both RTP and RTCP channels. + int SetDscp(talk_base::DiffServCodePoint value) { + int ret; + ret = SetOption(NetworkInterface::ST_RTP, + talk_base::Socket::OPT_DSCP, + value); + if (ret == 0) { + ret = SetOption(NetworkInterface::ST_RTCP, + talk_base::Socket::OPT_DSCP, + value); + } + return ret; + } + private: bool DoSendPacket(talk_base::Buffer* packet, bool rtcp) { talk_base::CritScope cs(&network_interface_crit_); diff --git a/talk/media/webrtc/webrtcvideoengine.cc b/talk/media/webrtc/webrtcvideoengine.cc index f2827db026..6067507dfc 100644 --- a/talk/media/webrtc/webrtcvideoengine.cc +++ b/talk/media/webrtc/webrtcvideoengine.cc @@ -156,6 +156,11 @@ static const int kRtpTimeOffsetExtensionId = 2; static const char kRtpAbsoluteSendTimeHeaderExtension[] = "http://www.webrtc.org/experiments/rtp-hdrext/abs-send-time"; static const int kRtpAbsoluteSendTimeExtensionId = 3; +// Default video dscp value. +// See http://tools.ietf.org/html/rfc2474 for details +// See also http://tools.ietf.org/html/draft-jennings-rtcweb-qos-00 +static const talk_base::DiffServCodePoint kVideoDscpValue = + talk_base::DSCP_AF41; static bool IsNackEnabled(const VideoCodec& codec) { return codec.HasFeedbackParam(FeedbackParam(kRtcpFbParamNack, @@ -2612,6 +2617,8 @@ bool WebRtcVideoMediaChannel::SetOptions(const VideoOptions &options) { bool cpu_overuse_detection_changed = options.cpu_overuse_detection.IsSet() && (options_.cpu_overuse_detection != options.cpu_overuse_detection); + bool dscp_option_changed = (options_.dscp != options.dscp); + bool conference_mode_turned_off = false; if (options_.conference_mode.IsSet() && options.conference_mode.IsSet() && options_.conference_mode.GetWithDefaultIfUnset(false) && @@ -2710,6 +2717,14 @@ bool WebRtcVideoMediaChannel::SetOptions(const VideoOptions &options) { send_channel->SetCpuOveruseDetection(cpu_overuse_detection); } } + if (dscp_option_changed) { + talk_base::DiffServCodePoint dscp = talk_base::DSCP_DEFAULT; + if (options.dscp.GetWithDefaultIfUnset(false)) + dscp = kVideoDscpValue; + if (MediaChannel::SetDscp(dscp) != 0) { + LOG(LS_WARNING) << "Failed to set DSCP settings for video channel"; + } + } return true; } diff --git a/talk/media/webrtc/webrtcvideoengine_unittest.cc b/talk/media/webrtc/webrtcvideoengine_unittest.cc index 0b8bdda0ec..05fac8e357 100644 --- a/talk/media/webrtc/webrtcvideoengine_unittest.cc +++ b/talk/media/webrtc/webrtcvideoengine_unittest.cc @@ -1508,6 +1508,21 @@ TEST_F(WebRtcVideoMediaChannelTest, AddRemoveCapturerMultipleSources) { Base::AddRemoveCapturerMultipleSources(); } +// This test verifies DSCP settings are properly applied on video media channel. +TEST_F(WebRtcVideoMediaChannelTest, TestSetDscpOptions) { + talk_base::scoped_ptr network_interface( + new cricket::FakeNetworkInterface); + channel_->SetInterface(network_interface.get()); + cricket::VideoOptions options; + options.dscp.Set(true); + EXPECT_TRUE(channel_->SetOptions(options)); + EXPECT_EQ(talk_base::DSCP_AF41, network_interface->dscp()); + options.dscp.Set(false); + EXPECT_TRUE(channel_->SetOptions(options)); + EXPECT_EQ(talk_base::DSCP_DEFAULT, network_interface->dscp()); + channel_->SetInterface(NULL); +} + TEST_F(WebRtcVideoMediaChannelTest, SetOptionsSucceedsWhenSending) { cricket::VideoOptions options; diff --git a/talk/media/webrtc/webrtcvideoframe.cc b/talk/media/webrtc/webrtcvideoframe.cc index 584aac0c3f..0938ae6ab6 100644 --- a/talk/media/webrtc/webrtcvideoframe.cc +++ b/talk/media/webrtc/webrtcvideoframe.cc @@ -42,45 +42,38 @@ static const int kWatermarkOffsetFromLeft = 8; static const int kWatermarkOffsetFromBottom = 8; static const unsigned char kWatermarkMaxYValue = 64; -FrameBuffer::FrameBuffer() : length_(0) {} +FrameBuffer::FrameBuffer() {} -FrameBuffer::FrameBuffer(size_t length) : length_(0) { +FrameBuffer::FrameBuffer(size_t length) { char* buffer = new char[length]; SetData(buffer, length); } -FrameBuffer::~FrameBuffer() { - // Make sure that the video_frame_ doesn't delete the buffer as it may be - // shared between multiple WebRtcVideoFrame. - uint8_t* new_memory = NULL; - uint32_t new_length = 0; - uint32_t new_size = 0; - video_frame_.Swap(new_memory, new_length, new_size); -} +FrameBuffer::~FrameBuffer() {} void FrameBuffer::SetData(char* data, size_t length) { - data_.reset(data); - length_ = length; uint8_t* new_memory = reinterpret_cast(data); - uint32_t new_length = static_cast(length); - uint32_t new_size = static_cast(length); + uint32_t new_length = static_cast(length); + uint32_t new_size = static_cast(length); video_frame_.Swap(new_memory, new_length, new_size); } void FrameBuffer::ReturnData(char** data, size_t* length) { - uint8_t* old_memory = NULL; + *data = NULL; uint32_t old_length = 0; uint32_t old_size = 0; - video_frame_.Swap(old_memory, old_length, old_size); - data_.release(); - length_ = 0; + video_frame_.Swap(reinterpret_cast(*data), + old_length, old_size); *length = old_length; - *data = reinterpret_cast(old_memory); } -char* FrameBuffer::data() { return data_.get(); } +char* FrameBuffer::data() { + return reinterpret_cast(video_frame_.Buffer()); +} -size_t FrameBuffer::length() const { return length_; } +size_t FrameBuffer::length() const { + return static_cast(video_frame_.Length()); +} webrtc::VideoFrame* FrameBuffer::frame() { return &video_frame_; } diff --git a/talk/media/webrtc/webrtcvideoframe.h b/talk/media/webrtc/webrtcvideoframe.h index e02323404c..ce13170538 100644 --- a/talk/media/webrtc/webrtcvideoframe.h +++ b/talk/media/webrtc/webrtcvideoframe.h @@ -55,8 +55,6 @@ class FrameBuffer { const webrtc::VideoFrame* frame() const; private: - talk_base::scoped_ptr data_; - size_t length_; webrtc::VideoFrame video_frame_; }; diff --git a/talk/media/webrtc/webrtcvoiceengine.cc b/talk/media/webrtc/webrtcvoiceengine.cc index 7f06009270..ac8e06743c 100644 --- a/talk/media/webrtc/webrtcvoiceengine.cc +++ b/talk/media/webrtc/webrtcvoiceengine.cc @@ -125,6 +125,10 @@ static const int kOpusStereoBitrate = 64000; // Opus bitrate should be in the range between 6000 and 510000. static const int kOpusMinBitrate = 6000; static const int kOpusMaxBitrate = 510000; +// Default audio dscp value. +// See http://tools.ietf.org/html/rfc2474 for details. +// See also http://tools.ietf.org/html/draft-jennings-rtcweb-qos-00 +static const talk_base::DiffServCodePoint kAudioDscpValue = talk_base::DSCP_EF; // Ensure we open the file in a writeable path on ChromeOS and Android. This // workaround can be removed when it's possible to specify a filename for audio @@ -1637,6 +1641,9 @@ bool WebRtcVoiceMediaChannel::SetOptions(const AudioOptions& options) { LOG(LS_INFO) << "Setting voice channel options: " << options.ToString(); + // Check if DSCP value is changed from previous. + bool dscp_option_changed = (options_.dscp != options.dscp); + // TODO(xians): Add support to set different options for different send // streams after we support multiple APMs. @@ -1704,6 +1711,14 @@ bool WebRtcVoiceMediaChannel::SetOptions(const AudioOptions& options) { return false; } } + if (dscp_option_changed) { + talk_base::DiffServCodePoint dscp = talk_base::DSCP_DEFAULT; + if (options.dscp.GetWithDefaultIfUnset(false)) + dscp = kAudioDscpValue; + if (MediaChannel::SetDscp(dscp) != 0) { + LOG(LS_WARNING) << "Failed to set DSCP settings for audio channel"; + } + } LOG(LS_INFO) << "Set voice channel options. Current options: " << options_.ToString(); diff --git a/talk/media/webrtc/webrtcvoiceengine_unittest.cc b/talk/media/webrtc/webrtcvoiceengine_unittest.cc index 2e52c8f03b..d0b06bbd09 100644 --- a/talk/media/webrtc/webrtcvoiceengine_unittest.cc +++ b/talk/media/webrtc/webrtcvoiceengine_unittest.cc @@ -12,6 +12,7 @@ #include "talk/media/base/constants.h" #include "talk/media/base/fakemediaengine.h" #include "talk/media/base/fakemediaprocessor.h" +#include "talk/media/base/fakenetworkinterface.h" #include "talk/media/base/fakertp.h" #include "talk/media/webrtc/fakewebrtcvoiceengine.h" #include "talk/media/webrtc/webrtcvoiceengine.h" @@ -2703,6 +2704,23 @@ TEST_F(WebRtcVoiceEngineTestFake, SetOptionOverridesViaChannels) { EXPECT_FALSE(ns_enabled); } +// This test verifies DSCP settings are properly applied on voice media channel. +TEST_F(WebRtcVoiceEngineTestFake, TestSetDscpOptions) { + EXPECT_TRUE(SetupEngine()); + talk_base::scoped_ptr channel( + engine_.CreateChannel()); + talk_base::scoped_ptr network_interface( + new cricket::FakeNetworkInterface); + channel->SetInterface(network_interface.get()); + cricket::AudioOptions options; + options.dscp.Set(true); + EXPECT_TRUE(channel->SetOptions(options)); + EXPECT_EQ(talk_base::DSCP_EF, network_interface->dscp()); + options.dscp.Set(false); + EXPECT_TRUE(channel->SetOptions(options)); + EXPECT_EQ(talk_base::DSCP_DEFAULT, network_interface->dscp()); +} + TEST(WebRtcVoiceEngineTest, TestDefaultOptionsBeforeInit) { cricket::WebRtcVoiceEngine engine; cricket::AudioOptions options = engine.GetOptions(); diff --git a/talk/p2p/base/p2ptransportchannel.cc b/talk/p2p/base/p2ptransportchannel.cc index 8bdaa9a30f..e8f53ada58 100644 --- a/talk/p2p/base/p2ptransportchannel.cc +++ b/talk/p2p/base/p2ptransportchannel.cc @@ -800,6 +800,7 @@ int P2PTransportChannel::SendPacket(const char *data, size_t len, error_ = EWOULDBLOCK; return -1; } + int sent = best_connection_->Send(data, len, dscp); if (sent <= 0) { ASSERT(sent < 0); diff --git a/talk/session/media/channel.cc b/talk/session/media/channel.cc index 9c9847fb2b..1a3063d44a 100644 --- a/talk/session/media/channel.cc +++ b/talk/session/media/channel.cc @@ -439,7 +439,6 @@ bool BaseChannel::Init(TransportChannel* transport_channel, return false; } - media_channel_->SetInterface(this); transport_channel_->SignalWritableState.connect( this, &BaseChannel::OnWritableState); transport_channel_->SignalReadPacket.connect( @@ -453,6 +452,9 @@ bool BaseChannel::Init(TransportChannel* transport_channel, this, &BaseChannel::OnNewRemoteDescription); set_rtcp_transport_channel(rtcp_transport_channel); + // Both RTP and RTCP channels are set, we can call SetInterface on + // media channel and it can set network options. + media_channel_->SetInterface(this); return true; }