Support encrypted RTP extensions (RFC 6904)

webrtc/media/engine/webrtcmediaengine_unittest.cc

Index: webrtc/media/engine/webrtcmediaengine_unittest.cc
diff --git a/webrtc/media/engine/webrtcmediaengine_unittest.cc b/webrtc/media/engine/webrtcmediaengine_unittest.cc
index c58c75b0c3a81a15e3089b3b29748565f44758e2..1cd362878a2933aeb436376f93aecd1b7bcf3856 100644
--- a/webrtc/media/engine/webrtcmediaengine_unittest.cc
+++ b/webrtc/media/engine/webrtcmediaengine_unittest.cc
@@ -144,6 +144,20 @@ TEST(WebRtcMediaEngineTest, FilterRtpExtensions_RemoveRedundant) {
   EXPECT_NE(filtered[0].uri, filtered[1].uri);
 }
 
+TEST(WebRtcMediaEngineTest, FilterRtpExtensions_RemoveRedundantEncrypted) {
+  std::vector<RtpExtension> extensions = MakeRedundantExtensions();
+  extensions[3] = extensions[2];
+  extensions[3].encrypt = true;

[Taylor Brandstetter, 2017/04/01 00:28:59]

nit: It's a little hard to read this test, since you need to look at what
MakeRedundantExtensions does and figure out what the extensions at indices 2 and
3 are.

Also: unless I'm mistaken, this would result in two extensions having the same
ID, which isn't legal (would be feasible for it to be DCHECKed).

I'd prefer if the test explicitly constructed the list, like the test below
does. The list doesn't need 7 entries; I think just three would be sufficient to
cover the specific functionality being tested in this test.

[joachim, 2017/04/17 10:46:09]

The DCHECK is in "ValidateRtpExtensions" (which is called by
"FilterRtpExtensions"), so it would have been caught if I had run the test
locally with DCHECKS enabled. Changed the test (and my local test setup).

+  std::vector<webrtc::RtpExtension> filtered =
+      FilterRtpExtensions(extensions, SupportedExtensions2, true);
+  EXPECT_EQ(7, filtered.size());
+  EXPECT_TRUE(IsSorted(filtered));
+  EXPECT_EQ(filtered[0].uri, filtered[1].uri);
+  EXPECT_NE(filtered[0].encrypt, filtered[1].encrypt);
+  EXPECT_NE(filtered[0].uri, filtered[2].uri);
+  EXPECT_NE(filtered[1].uri, filtered[2].uri);
+}
+
 TEST(WebRtcMediaEngineTest, FilterRtpExtensions_RemoveRedundantBwe_1) {
   std::vector<RtpExtension> extensions;
   extensions.push_back(
@@ -159,6 +173,27 @@ TEST(WebRtcMediaEngineTest, FilterRtpExtensions_RemoveRedundantBwe_1) {
   EXPECT_EQ(RtpExtension::kTransportSequenceNumberUri, filtered[0].uri);
 }
 
+TEST(WebRtcMediaEngineTest, FilterRtpExtensions_RemoveRedundantBweEncrypted_1) {
+  std::vector<RtpExtension> extensions;
+  extensions.push_back(
+      RtpExtension(RtpExtension::kTransportSequenceNumberUri, 3));
+  extensions.push_back(
+      RtpExtension(RtpExtension::kTransportSequenceNumberUri, 3, true));

[Taylor Brandstetter, 2017/04/01 00:28:59]

They both have the same ID; is that intentional?

[joachim, 2017/04/17 10:46:09]

No, see above.

+  extensions.push_back(RtpExtension(RtpExtension::kTimestampOffsetUri, 9));
+  extensions.push_back(RtpExtension(RtpExtension::kAbsSendTimeUri, 6));
+  extensions.push_back(
+      RtpExtension(RtpExtension::kTransportSequenceNumberUri, 1));
+  extensions.push_back(
+      RtpExtension(RtpExtension::kTransportSequenceNumberUri, 1, true));
+  extensions.push_back(RtpExtension(RtpExtension::kTimestampOffsetUri, 14));
+  std::vector<webrtc::RtpExtension> filtered =
+      FilterRtpExtensions(extensions, SupportedExtensions2, true);
+  EXPECT_EQ(2, filtered.size());
+  EXPECT_EQ(RtpExtension::kTransportSequenceNumberUri, filtered[0].uri);
+  EXPECT_EQ(RtpExtension::kTransportSequenceNumberUri, filtered[1].uri);
+  EXPECT_NE(filtered[0].encrypt, filtered[1].encrypt);
+}
+
 TEST(WebRtcMediaEngineTest, FilterRtpExtensions_RemoveRedundantBwe_2) {
   std::vector<RtpExtension> extensions;
   extensions.push_back(RtpExtension(RtpExtension::kTimestampOffsetUri, 1));