Support encrypted RTP extensions (RFC 6904)

webrtc/media/engine/webrtcmediaengine_unittest.cc

 /*
  *  Copyright (c) 2015 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 126 matching lines @@
 
 TEST(WebRtcMediaEngineTest, FilterRtpExtensions_RemoveRedundant) {
   std::vector<RtpExtension> extensions = MakeRedundantExtensions();
   std::vector<webrtc::RtpExtension> filtered =
       FilterRtpExtensions(extensions, SupportedExtensions2, true);
   EXPECT_EQ(6, filtered.size());
   EXPECT_TRUE(IsSorted(filtered));
   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(
       RtpExtension(RtpExtension::kTransportSequenceNumberUri, 3));
   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::kTimestampOffsetUri, 14));
   std::vector<webrtc::RtpExtension> filtered =
       FilterRtpExtensions(extensions, SupportedExtensions2, true);
   EXPECT_EQ(1, filtered.size());
   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));
   extensions.push_back(RtpExtension(RtpExtension::kAbsSendTimeUri, 14));
   extensions.push_back(RtpExtension(RtpExtension::kTimestampOffsetUri, 7));
   std::vector<webrtc::RtpExtension> filtered =
       FilterRtpExtensions(extensions, SupportedExtensions2, true);
   EXPECT_EQ(1, filtered.size());
   EXPECT_EQ(RtpExtension::kAbsSendTimeUri, filtered[0].uri);
 }
@@ skipping 14 matching lines @@
   EXPECT_TRUE(engine);
 }
 
 TEST(WebRtcMediaEngineFactoryTest, CreateWithBuiltinDecoders) {
   std::unique_ptr<MediaEngineInterface> engine(WebRtcMediaEngineFactory::Create(
       nullptr, webrtc::CreateBuiltinAudioDecoderFactory(), nullptr, nullptr));
   EXPECT_TRUE(engine);
 }
 
 }  // namespace cricket