Add helper functions for negotiating H264 profile level id

webrtc/common_video/h264/profile_level_id_unittest.cc

Index: webrtc/common_video/h264/profile_level_id_unittest.cc
diff --git a/webrtc/common_video/h264/profile_level_id_unittest.cc b/webrtc/common_video/h264/profile_level_id_unittest.cc
index 491fda14aaf89fb80be630dcfef5d357832948df..db4c8a9870466d0152d6343866f9b67907f1982c 100644
--- a/webrtc/common_video/h264/profile_level_id_unittest.cc
+++ b/webrtc/common_video/h264/profile_level_id_unittest.cc
@@ -109,5 +109,89 @@ TEST(H264ProfileLevelId, TestToStringInvalid) {
       ProfileLevelId(static_cast<Profile>(255), kLevel3_1)));
 }
 
+TEST(H264ProfileLevelId, TestParseSdpProfileLevelIdEmpty) {
+  const rtc::Optional<ProfileLevelId> profile_level_id =
+      ParseSdpProfileLevelId(CodecParameterMap());
+  EXPECT_TRUE(profile_level_id);
+  EXPECT_EQ(kProfileConstrainedBaseline, profile_level_id->profile);
+  EXPECT_EQ(kLevel3_1, profile_level_id->level);
+}
+
+TEST(H264ProfileLevelId, TestParseSdpProfileLevelIdConstrainedHigh) {
+  CodecParameterMap params;
+  params["profile-level-id"] = "640c2a";
+  const rtc::Optional<ProfileLevelId> profile_level_id =
+      ParseSdpProfileLevelId(params);
+  EXPECT_TRUE(profile_level_id);
+  EXPECT_EQ(kProfileConstrainedHigh, profile_level_id->profile);
+  EXPECT_EQ(kLevel4_2, profile_level_id->level);
+}
+
+TEST(H264ProfileLevelId, TestParseSdpProfileLevelIdInvalid) {
+  CodecParameterMap params;
+  params["profile-level-id"] = "foobar";
+  EXPECT_FALSE(ParseSdpProfileLevelId(params));
+}
+
+TEST(H264ProfileLevelId, TestNegotiateProfileLevelIdEmpty) {
+  const rtc::Optional<ProfileLevelId> profile_level_id =
+      NegotiateProfileLevelId(CodecParameterMap(), CodecParameterMap());
+  EXPECT_TRUE(profile_level_id);
+  EXPECT_EQ(kProfileConstrainedBaseline, profile_level_id->profile);
+  EXPECT_EQ(kLevel3_1, profile_level_id->level);
+}
+
+TEST(H264ProfileLevelId, TestNegotiateProfileLevelIdInvalid) {
+  CodecParameterMap local_params;
+  local_params["profile-level-id"] = "foobar";
+  EXPECT_FALSE(NegotiateProfileLevelId(local_params, CodecParameterMap()));
+}
+
+TEST(H264ProfileLevelId, TestNegotiateProfileLevelIdDifferentProfiles) {
+  CodecParameterMap local_params;
+  local_params["profile-level-id"] = "42e01f";
+  CodecParameterMap remote_params;
+  remote_params["profile-level-id"] = "640c2a";
+  EXPECT_FALSE(NegotiateProfileLevelId(local_params, remote_params));

[hta-webrtc, 2016/11/08 09:39:06]

Given that profiles form subsets too, I'm not sure whether this is correct - if
the remote accepts high, we are OK with sending constrained baseline; they'll be
able to handle it.

[magjed_webrtc, 2016/11/08 15:48:40]

This test does not make sense anymore since the profiles must match before
calling NegotiateProfileLevelId so I removed it.

Regarding your question; we can't answer a different profile from the offered
profile. We must either answer the same profile or reject the payload type
completely. We will send different H264 profiles in different payload types.

+}
+
+TEST(H264ProfileLevelId, TestNegotiateProfileLevelIdLevelSymmetryRejected) {
+  CodecParameterMap local_params;
+  local_params["profile-level-id"] = "42e015";
+  CodecParameterMap remote_params;
+  remote_params["profile-level-id"] = "42e01f";
+  // The codec should be rejected since local can't handle the remote level.
+  EXPECT_FALSE(NegotiateProfileLevelId(local_params, remote_params));

[hta-webrtc, 2016/11/08 09:39:06]

This test is wrong.

[magjed_webrtc, 2016/11/08 15:48:40]

Yes, it's removed now.

+}
+
+TEST(H264ProfileLevelId, TestNegotiateProfileLevelIdLevelSymmetryCapped) {
+  CodecParameterMap local_params;
+  local_params["profile-level-id"] = "42e01f";
+  CodecParameterMap remote_params;
+  remote_params["profile-level-id"] = "42e015";
+  const rtc::Optional<ProfileLevelId> profile_level_id =
+      NegotiateProfileLevelId(local_params, remote_params);
+  EXPECT_TRUE(profile_level_id);
+  EXPECT_EQ(kProfileConstrainedBaseline, profile_level_id->profile);
+  // The level should be capped by the remote capabilities.
+  EXPECT_EQ(kLevel2_1, profile_level_id->level);

[hta-webrtc, 2016/11/08 09:39:06]

It's not very readable to set the parameters in terms of strings and then make
the check in terms of kLevel.
Would it be more readable to say

EXPECT_EQ("42e015", profile_level_id->ToString());

?

[magjed_webrtc, 2016/11/08 15:48:40]

Done, strings are used everywhere now.

+}
+
+TEST(H264ProfileLevelId,
+     TestNegotiateProfileLevelIdConstrainedBaselineLevelAsymmetry) {
+  CodecParameterMap local_params;
+  local_params["profile-level-id"] = "42e01f";
+  local_params["level-asymmetry-allowed"] = "1";
+  CodecParameterMap remote_params;
+  remote_params["profile-level-id"] = "42e015";
+  remote_params["level-asymmetry-allowed"] = "1";
+  const rtc::Optional<ProfileLevelId> profile_level_id =
+      NegotiateProfileLevelId(local_params, remote_params);
+  EXPECT_TRUE(profile_level_id);
+  EXPECT_EQ(kProfileConstrainedBaseline, profile_level_id->profile);
+  // We should be able to send a level different from the remote capabilities.
+  EXPECT_EQ(kLevel3_1, profile_level_id->level);

[hta-webrtc, 2016/11/08 09:39:06]

Actually, we should send with the remote capabilities. The remote capabilities
are what they can accept, and we should never send more than that.

[magjed_webrtc, 2016/11/08 15:48:40]

Yeah, the 'send level' in the comment is actually the answer level. We can
answer a higher level than what was offered and the remote side is allowed to
use that level for encoding.

+}
+
 }  // namespace H264
 }  // namespace webrtc