diff --git a/talk/media/webrtc/webrtcvideoengine2.cc b/talk/media/webrtc/webrtcvideoengine2.cc index 21e51eb726..12fcec01d1 100644 --- a/talk/media/webrtc/webrtcvideoengine2.cc +++ b/talk/media/webrtc/webrtcvideoengine2.cc @@ -34,6 +34,7 @@ #include +#include #include #include "libyuv/convert_from.h" @@ -136,6 +137,34 @@ static std::vector DefaultVideoCodecs() { return codecs; } +static bool ValidateRtpHeaderExtensionIds( + const std::vector& extensions) { + std::set extensions_used; + for (size_t i = 0; i < extensions.size(); ++i) { + if (extensions[i].id < 0 || extensions[i].id >= 15 || + !extensions_used.insert(extensions[i].id).second) { + LOG(LS_ERROR) << "RTP extensions are with incorrect or duplicate ids."; + return false; + } + } + return true; +} + +static std::vector FilterRtpExtensions( + const std::vector& extensions) { + std::vector webrtc_extensions; + for (size_t i = 0; i < extensions.size(); ++i) { + // Unsupported extensions will be ignored. + if (webrtc::RtpExtension::IsSupported(extensions[i].uri)) { + webrtc_extensions.push_back(webrtc::RtpExtension( + extensions[i].uri, extensions[i].id)); + } else { + LOG(LS_WARNING) << "Unsupported RTP extension: " << extensions[i].uri; + } + } + return webrtc_extensions; +} + WebRtcVideoEncoderFactory2::~WebRtcVideoEncoderFactory2() { } @@ -1099,22 +1128,16 @@ bool WebRtcVideoChannel2::SetRecvRtpHeaderExtensions( const std::vector& extensions) { LOG(LS_INFO) << "SetRecvRtpHeaderExtensions: " << RtpExtensionsToString(extensions); - std::vector webrtc_extensions; - for (size_t i = 0; i < extensions.size(); ++i) { - // TODO(pbos): Make sure we don't pass unsupported extensions! - webrtc::RtpExtension webrtc_extension(extensions[i].uri.c_str(), - extensions[i].id); - webrtc_extensions.push_back(webrtc_extension); - } - recv_rtp_extensions_ = webrtc_extensions; + if (!ValidateRtpHeaderExtensionIds(extensions)) + return false; + recv_rtp_extensions_ = FilterRtpExtensions(extensions); for (std::map::iterator it = receive_streams_.begin(); it != receive_streams_.end(); ++it) { it->second->SetRtpExtensions(recv_rtp_extensions_); } - return true; } @@ -1122,14 +1145,10 @@ bool WebRtcVideoChannel2::SetSendRtpHeaderExtensions( const std::vector& extensions) { LOG(LS_INFO) << "SetSendRtpHeaderExtensions: " << RtpExtensionsToString(extensions); - std::vector webrtc_extensions; - for (size_t i = 0; i < extensions.size(); ++i) { - // TODO(pbos): Make sure we don't pass unsupported extensions! - webrtc::RtpExtension webrtc_extension(extensions[i].uri.c_str(), - extensions[i].id); - webrtc_extensions.push_back(webrtc_extension); - } - send_rtp_extensions_ = webrtc_extensions; + if (!ValidateRtpHeaderExtensionIds(extensions)) + return false; + + send_rtp_extensions_ = FilterRtpExtensions(extensions); for (std::map::iterator it = send_streams_.begin(); it != send_streams_.end(); diff --git a/talk/media/webrtc/webrtcvideoengine2_unittest.cc b/talk/media/webrtc/webrtcvideoengine2_unittest.cc index 45ea161f0b..0d46e2b641 100644 --- a/talk/media/webrtc/webrtcvideoengine2_unittest.cc +++ b/talk/media/webrtc/webrtcvideoengine2_unittest.cc @@ -49,6 +49,8 @@ static const cricket::VideoCodec kUlpfecCodec(117, "ulpfec", 0, 0, 0, 0); static const uint32 kSsrcs1[] = {1}; static const uint32 kRtxSsrcs1[] = {4}; +static const char kUnsupportedExtensionName[] = + "urn:ietf:params:rtp-hdrext:unsupported"; void VerifyCodecHasDefaultFeedbackParams(const cricket::VideoCodec& codec) { EXPECT_TRUE(codec.HasFeedbackParam(cricket::FeedbackParam( @@ -822,6 +824,110 @@ TEST_F(WebRtcVideoChannel2Test, RecvAbsoluteSendTimeHeaderExtensions) { webrtc::RtpExtension::kAbsSendTime); } +TEST_F(WebRtcVideoChannel2Test, + SetSendRtpHeaderExtensionsExcludeUnsupportedExtensions) { + const int kUnsupportedId = 1; + const int kTOffsetId = 2; + + std::vector extensions; + extensions.push_back(cricket::RtpHeaderExtension( + kUnsupportedExtensionName, kUnsupportedId)); + extensions.push_back(cricket::RtpHeaderExtension( + webrtc::RtpExtension::kTOffset, kTOffsetId)); + EXPECT_TRUE(channel_->SetSendRtpHeaderExtensions(extensions)); + FakeVideoSendStream* send_stream = + AddSendStream(cricket::StreamParams::CreateLegacy(123)); + + // Only timestamp offset extension is set to send stream, + // unsupported rtp extension is ignored. + ASSERT_EQ(1u, send_stream->GetConfig().rtp.extensions.size()); + EXPECT_STREQ(webrtc::RtpExtension::kTOffset, + send_stream->GetConfig().rtp.extensions[0].name.c_str()); +} + +TEST_F(WebRtcVideoChannel2Test, + SetRecvRtpHeaderExtensionsExcludeUnsupportedExtensions) { + const int kUnsupportedId = 1; + const int kTOffsetId = 2; + + std::vector extensions; + extensions.push_back(cricket::RtpHeaderExtension( + kUnsupportedExtensionName, kUnsupportedId)); + extensions.push_back(cricket::RtpHeaderExtension( + webrtc::RtpExtension::kTOffset, kTOffsetId)); + EXPECT_TRUE(channel_->SetRecvRtpHeaderExtensions(extensions)); + FakeVideoReceiveStream* recv_stream = + AddRecvStream(cricket::StreamParams::CreateLegacy(123)); + + // Only timestamp offset extension is set to receive stream, + // unsupported rtp extension is ignored. + ASSERT_EQ(1u, recv_stream->GetConfig().rtp.extensions.size()); + EXPECT_STREQ(webrtc::RtpExtension::kTOffset, + recv_stream->GetConfig().rtp.extensions[0].name.c_str()); +} + +TEST_F(WebRtcVideoChannel2Test, + SetSendRtpHeaderExtensionsRejectsIncorrectIds) { + const size_t kNumIncorrectIds = 4; + const int kIncorrectIds[kNumIncorrectIds] = {-2, -1, 15, 16}; + for (size_t i = 0; i < kNumIncorrectIds; ++i) { + std::vector extensions; + extensions.push_back(cricket::RtpHeaderExtension( + webrtc::RtpExtension::kTOffset, kIncorrectIds[i])); + EXPECT_FALSE(channel_->SetSendRtpHeaderExtensions(extensions)) + << "Bad extension id '" << kIncorrectIds[i] << "' accepted."; + } +} + +TEST_F(WebRtcVideoChannel2Test, + SetRecvRtpHeaderExtensionsRejectsIncorrectIds) { + const size_t kNumIncorrectIds = 4; + const int kIncorrectIds[kNumIncorrectIds] = {-2, -1, 15, 16}; + for (size_t i = 0; i < kNumIncorrectIds; ++i) { + std::vector extensions; + extensions.push_back(cricket::RtpHeaderExtension( + webrtc::RtpExtension::kTOffset, kIncorrectIds[i])); + EXPECT_FALSE(channel_->SetRecvRtpHeaderExtensions(extensions)) + << "Bad extension id '" << kIncorrectIds[i] << "' accepted."; + } +} + +TEST_F(WebRtcVideoChannel2Test, + SetSendRtpHeaderExtensionsRejectsDuplicateIds) { + const int id = 1; + std::vector extensions; + extensions.push_back(cricket::RtpHeaderExtension( + webrtc::RtpExtension::kTOffset, id)); + extensions.push_back(cricket::RtpHeaderExtension( + kRtpAbsoluteSenderTimeHeaderExtension, id)); + EXPECT_FALSE(channel_->SetSendRtpHeaderExtensions(extensions)); + + // Duplicate entries are also not supported. + extensions.clear(); + extensions.push_back(cricket::RtpHeaderExtension( + webrtc::RtpExtension::kTOffset, id)); + extensions.push_back(extensions.back()); + EXPECT_FALSE(channel_->SetSendRtpHeaderExtensions(extensions)); +} + +TEST_F(WebRtcVideoChannel2Test, + SetRecvRtpHeaderExtensionsRejectsDuplicateIds) { + const int id = 1; + std::vector extensions; + extensions.push_back(cricket::RtpHeaderExtension( + webrtc::RtpExtension::kTOffset, id)); + extensions.push_back(cricket::RtpHeaderExtension( + kRtpAbsoluteSenderTimeHeaderExtension, id)); + EXPECT_FALSE(channel_->SetRecvRtpHeaderExtensions(extensions)); + + // Duplicate entries are also not supported. + extensions.clear(); + extensions.push_back(cricket::RtpHeaderExtension( + webrtc::RtpExtension::kTOffset, id)); + extensions.push_back(extensions.back()); + EXPECT_FALSE(channel_->SetRecvRtpHeaderExtensions(extensions)); +} + TEST_F(WebRtcVideoChannel2Test, DISABLED_LeakyBucketTest) { FAIL() << "Not implemented."; // TODO(pbos): Implement. } diff --git a/webrtc/config.h b/webrtc/config.h index 2e96ec1c02..e4bccf90f4 100644 --- a/webrtc/config.h +++ b/webrtc/config.h @@ -10,8 +10,8 @@ // TODO(pbos): Move Config from common.h to here. -#ifndef WEBRTC_VIDEO_ENGINE_NEW_INCLUDE_CONFIG_H_ -#define WEBRTC_VIDEO_ENGINE_NEW_INCLUDE_CONFIG_H_ +#ifndef WEBRTC_CONFIG_H_ +#define WEBRTC_CONFIG_H_ #include #include @@ -73,9 +73,10 @@ struct FecConfig { // RTP header extension to use for the video stream, see RFC 5285. struct RtpExtension { - RtpExtension(const char* name, int id) : name(name), id(id) {} + RtpExtension(const std::string& name, int id) : name(name), id(id) {} std::string ToString() const; - // TODO(mflodman) Add API to query supported extensions. + static bool IsSupported(const std::string& name); + static const char* kTOffset; static const char* kAbsSendTime; std::string name; @@ -109,4 +110,4 @@ struct VideoStream { } // namespace webrtc -#endif // WEBRTC_VIDEO_ENGINE_NEW_INCLUDE_CONFIG_H_ +#endif // WEBRTC_CONFIG_H_ diff --git a/webrtc/video/call.cc b/webrtc/video/call.cc index 95e1c7b7a3..bcce6f04f5 100644 --- a/webrtc/video/call.cc +++ b/webrtc/video/call.cc @@ -33,6 +33,12 @@ namespace webrtc { const char* RtpExtension::kTOffset = "urn:ietf:params:rtp-hdrext:toffset"; const char* RtpExtension::kAbsSendTime = "http://www.webrtc.org/experiments/rtp-hdrext/abs-send-time"; + +bool RtpExtension::IsSupported(const std::string& name) { + return name == webrtc::RtpExtension::kTOffset || + name == webrtc::RtpExtension::kAbsSendTime; +} + namespace internal { class CpuOveruseObserverProxy : public webrtc::CpuOveruseObserver {