Revert of H264 codec: Check profile-level-id when matching (patchset #2 id:60001 of https://codereview.webrtc.org/2347863003/ )
Reason for revert:
Breaks H264 for external encoders in WebRTC as well as breaking H264 interop with e.g. Edge.
Original issue's description:
> H264 codec: Check profile-level-id when matching
>
> For the H264 video codec, comparing the codec name is not enough
> for determining a match. The profile-level-id must also match.
> This CL:
> - Specializes the VideoCodec::Matches function with extra logic for
> matching H264 codecs.
> - Adds unittests for matching H264 video codecs.
> - Removes the unnecessary CodecTest fixture class.
>
> BUG=webrtc:6337,chromium:645599
>
> Committed: https://crrev.com/68979ab7dd971ab6e983b23c83154ba05e183fb8
> Cr-Commit-Position: refs/heads/master@{#14546}
TBR=kthelgason@webrtc.org,hta@webrtc.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=webrtc:6337,chromium:645599,webrtc:6552,webrtc:6402
Review URL: https://codereview.webrtc.org/2440123002 .
Cr-Commit-Position: refs/heads/master@{#14759}
diff --git a/webrtc/media/base/codec.cc b/webrtc/media/base/codec.cc
index 26d1faf..1e06e8e 100644
--- a/webrtc/media/base/codec.cc
+++ b/webrtc/media/base/codec.cc
@@ -18,19 +18,6 @@
#include "webrtc/base/stringencode.h"
#include "webrtc/base/stringutils.h"
-namespace {
-
-// Return the contained value for |key| if available, and |default_value|
-// otherwise.
-std::string GetParamOrDefault(const cricket::Codec& codec,
- const std::string& key,
- const std::string& default_value) {
- cricket::CodecParameterMap::const_iterator iter = codec.params.find(key);
- return (iter == codec.params.end()) ? default_value : iter->second;
-}
-
-} // anonymous namespace
-
namespace cricket {
const int kMaxPayloadId = 127;
@@ -240,31 +227,6 @@
return Codec::operator==(c);
}
-bool VideoCodec::Matches(const VideoCodec& codec) const {
- if (!Codec::Matches(codec))
- return false;
- // TODO(magjed): It would be better to have this logic in a H264 subclass. See
- // http://crbug/webrtc/6385 for more info.
- if (!CodecNamesEq(name, kH264CodecName))
- return true;
- // H264 codecs need to have matching profile-level-id.
- const std::string our_profile_level_id = GetParamOrDefault(
- *this, kH264FmtpProfileLevelId, kH264FmtpDefaultProfileLevelId);
- const std::string their_profile_level_id = GetParamOrDefault(
- codec, kH264FmtpProfileLevelId, kH264FmtpDefaultProfileLevelId);
- if (our_profile_level_id == their_profile_level_id)
- return true;
- // At this point, profile-level-id is not an exact match, but that is still ok
- // if only level_idc differs and level asymmetry is allowed.
- const bool level_asymmetry_allowed =
- GetParamOrDefault(*this, kH264FmtpLevelAsymmetryAllowed, "0") == "1" &&
- GetParamOrDefault(codec, kH264FmtpLevelAsymmetryAllowed, "0") == "1";
- // Ignore level_idc and compare only profile_idc and profile_iop.
- const bool is_profile_match = (our_profile_level_id.substr(0, 4) ==
- their_profile_level_id.substr(0, 4));
- return level_asymmetry_allowed && is_profile_match;
-}
-
VideoCodec VideoCodec::CreateRtxCodec(int rtx_payload_type,
int associated_payload_type) {
VideoCodec rtx_codec(rtx_payload_type, kRtxCodecName);
diff --git a/webrtc/media/base/codec.h b/webrtc/media/base/codec.h
index 807e4b6..facdfef 100644
--- a/webrtc/media/base/codec.h
+++ b/webrtc/media/base/codec.h
@@ -146,9 +146,6 @@
VideoCodec(const VideoCodec& c);
virtual ~VideoCodec() = default;
- // Indicates if this codec is compatible with the specified codec.
- bool Matches(const VideoCodec& codec) const;
-
std::string ToString() const;
VideoCodec& operator=(const VideoCodec& c);
diff --git a/webrtc/media/base/codec_unittest.cc b/webrtc/media/base/codec_unittest.cc
index 230cf08..7ad9733 100644
--- a/webrtc/media/base/codec_unittest.cc
+++ b/webrtc/media/base/codec_unittest.cc
@@ -20,7 +20,12 @@
using cricket::kCodecParamMaxBitrate;
using cricket::kCodecParamMinBitrate;
-TEST(CodecTest, TestCodecOperators) {
+class CodecTest : public testing::Test {
+ public:
+ CodecTest() {}
+};
+
+TEST_F(CodecTest, TestCodecOperators) {
Codec c0(96, "D", 1000);
c0.SetParam("a", 1);
@@ -53,7 +58,7 @@
EXPECT_TRUE(c5 == c6);
}
-TEST(CodecTest, TestAudioCodecOperators) {
+TEST_F(CodecTest, TestAudioCodecOperators) {
AudioCodec c0(96, "A", 44100, 20000, 2);
AudioCodec c1(95, "A", 44100, 20000, 2);
AudioCodec c2(96, "x", 44100, 20000, 2);
@@ -90,7 +95,7 @@
EXPECT_TRUE(c13 == c10);
}
-TEST(CodecTest, TestAudioCodecMatches) {
+TEST_F(CodecTest, TestAudioCodecMatches) {
// Test a codec with a static payload type.
AudioCodec c0(95, "A", 44100, 20000, 1);
EXPECT_TRUE(c0.Matches(AudioCodec(95, "", 44100, 20000, 1)));
@@ -130,7 +135,7 @@
EXPECT_FALSE(c3.Matches(AudioCodec(96, "A", 44100, 20000, 0)));
}
-TEST(CodecTest, TestVideoCodecOperators) {
+TEST_F(CodecTest, TestVideoCodecOperators) {
VideoCodec c0(96, "V");
VideoCodec c1(95, "V");
VideoCodec c2(96, "x");
@@ -162,7 +167,7 @@
EXPECT_TRUE(c13 == c10);
}
-TEST(CodecTest, TestVideoCodecMatches) {
+TEST_F(CodecTest, TestVideoCodecMatches) {
// Test a codec with a static payload type.
VideoCodec c0(95, "V");
EXPECT_TRUE(c0.Matches(VideoCodec(95, "")));
@@ -178,71 +183,6 @@
EXPECT_FALSE(c1.Matches(VideoCodec(95, "V")));
}
-TEST(CodecTest, TestVideoCodecMatchesH264Baseline) {
- const VideoCodec no_params(96, cricket::kH264CodecName);
-
- VideoCodec baseline(96, cricket::kH264CodecName);
- baseline.SetParam(cricket::kH264FmtpProfileLevelId,
- cricket::kH264FmtpDefaultProfileLevelId);
-
- EXPECT_TRUE(baseline.Matches(baseline));
- EXPECT_TRUE(baseline.Matches(no_params));
- EXPECT_TRUE(no_params.Matches(baseline));
- EXPECT_TRUE(no_params.Matches(no_params));
-}
-
-TEST(CodecTest, TestVideoCodecMatchesH264Profiles) {
- VideoCodec baseline(96, cricket::kH264CodecName);
- baseline.SetParam(cricket::kH264FmtpProfileLevelId,
- cricket::kH264FmtpDefaultProfileLevelId);
- baseline.SetParam(cricket::kH264FmtpLevelAsymmetryAllowed, "1");
-
- VideoCodec constrained_baseline(96, cricket::kH264CodecName);
- constrained_baseline.SetParam(cricket::kH264FmtpProfileLevelId,
- cricket::kH264ProfileLevelConstrainedBaseline);
- constrained_baseline.SetParam(cricket::kH264FmtpLevelAsymmetryAllowed, "1");
-
- EXPECT_TRUE(baseline.Matches(baseline));
- EXPECT_FALSE(baseline.Matches(constrained_baseline));
- EXPECT_FALSE(constrained_baseline.Matches(baseline));
- EXPECT_TRUE(constrained_baseline.Matches(constrained_baseline));
-}
-
-TEST(CodecTest, TestVideoCodecMatchesH264LevelAsymmetry) {
- // Constrained Baseline Profile Level 1.0.
- VideoCodec cbp_1_0(96, cricket::kH264CodecName);
- cbp_1_0.SetParam(cricket::kH264FmtpProfileLevelId,
- "42e00a");
-
- VideoCodec cbp_1_0_asymmetry_allowed = cbp_1_0;
- cbp_1_0_asymmetry_allowed.SetParam(cricket::kH264FmtpLevelAsymmetryAllowed,
- "1");
-
- // Constrained Baseline Profile Level 3.1.
- VideoCodec cbp_3_1(96, cricket::kH264CodecName);
- cbp_3_1.SetParam(cricket::kH264FmtpProfileLevelId, "42e01f");
-
- VideoCodec cbp_3_1_asymmetry_allowed = cbp_3_1;
- cbp_3_1_asymmetry_allowed.SetParam(cricket::kH264FmtpLevelAsymmetryAllowed,
- "1");
-
- // It's ok to differ in level-asymmetry-allowed param as long as the level is
- // the same.
- EXPECT_TRUE(cbp_1_0.Matches(cbp_1_0_asymmetry_allowed));
- EXPECT_TRUE(cbp_3_1.Matches(cbp_3_1_asymmetry_allowed));
-
- // Both codecs need to accept level asymmetry if levels differ.
- EXPECT_FALSE(cbp_1_0.Matches(cbp_3_1_asymmetry_allowed));
- EXPECT_FALSE(cbp_1_0_asymmetry_allowed.Matches(cbp_3_1));
- EXPECT_TRUE(cbp_1_0_asymmetry_allowed.Matches(cbp_3_1_asymmetry_allowed));
-
- // Test explicitly disabling level asymmetry. It should have the same behavior
- // as missing the param.
- cbp_1_0.SetParam(cricket::kH264FmtpLevelAsymmetryAllowed, "0");
- EXPECT_TRUE(cbp_1_0.Matches(cbp_1_0_asymmetry_allowed));
- EXPECT_FALSE(cbp_1_0.Matches(cbp_3_1_asymmetry_allowed));
-}
-
TEST(CodecTest, TestDataCodecMatches) {
// Test a codec with a static payload type.
DataCodec c0(95, "D");
@@ -259,7 +199,7 @@
EXPECT_FALSE(c1.Matches(DataCodec(95, "D")));
}
-TEST(CodecTest, TestSetParamGetParamAndRemoveParam) {
+TEST_F(CodecTest, TestSetParamGetParamAndRemoveParam) {
AudioCodec codec;
codec.SetParam("a", "1");
codec.SetParam("b", "x");
@@ -280,7 +220,7 @@
EXPECT_FALSE(codec.RemoveParam("c"));
}
-TEST(CodecTest, TestIntersectFeedbackParams) {
+TEST_F(CodecTest, TestIntersectFeedbackParams) {
const FeedbackParam a1("a", "1");
const FeedbackParam b2("b", "2");
const FeedbackParam b3("b", "3");
@@ -299,7 +239,7 @@
EXPECT_FALSE(c1.HasFeedbackParam(c3));
}
-TEST(CodecTest, TestGetCodecType) {
+TEST_F(CodecTest, TestGetCodecType) {
// Codec type comparison should be case insenstive on names.
const VideoCodec codec(96, "V");
const VideoCodec rtx_codec(96, "rTx");
@@ -311,7 +251,7 @@
EXPECT_EQ(VideoCodec::CODEC_RED, red_codec.GetCodecType());
}
-TEST(CodecTest, TestCreateRtxCodec) {
+TEST_F(CodecTest, TestCreateRtxCodec) {
VideoCodec rtx_codec = VideoCodec::CreateRtxCodec(96, 120);
EXPECT_EQ(96, rtx_codec.id);
EXPECT_EQ(VideoCodec::CODEC_RTX, rtx_codec.GetCodecType());
@@ -321,7 +261,7 @@
EXPECT_EQ(120, associated_payload_type);
}
-TEST(CodecTest, TestValidateCodecFormat) {
+TEST_F(CodecTest, TestValidateCodecFormat) {
const VideoCodec codec(96, "V");
ASSERT_TRUE(codec.ValidateCodecFormat());
@@ -362,7 +302,7 @@
EXPECT_TRUE(different_bitrates.ValidateCodecFormat());
}
-TEST(CodecTest, TestToCodecParameters) {
+TEST_F(CodecTest, TestToCodecParameters) {
const VideoCodec v(96, "V");
webrtc::RtpCodecParameters codec_params_1 = v.ToCodecParameters();
EXPECT_EQ(96, codec_params_1.payload_type);
diff --git a/webrtc/media/base/mediaconstants.cc b/webrtc/media/base/mediaconstants.cc
index 0bde3f9..2bbca7c 100644
--- a/webrtc/media/base/mediaconstants.cc
+++ b/webrtc/media/base/mediaconstants.cc
@@ -97,9 +97,6 @@
// RFC 6184 RTP Payload Format for H.264 video
const char kH264FmtpProfileLevelId[] = "profile-level-id";
-// If no profile-level-id is present as a parameter, the Baseline Profile
-// without additional constraints at Level 1 is implied.
-const char kH264FmtpDefaultProfileLevelId[] = "42000a";
const char kH264FmtpLevelAsymmetryAllowed[] = "level-asymmetry-allowed";
const char kH264FmtpPacketizationMode[] = "packetization-mode";
const char kH264ProfileLevelConstrainedBaseline[] = "42e01f";
diff --git a/webrtc/media/base/mediaconstants.h b/webrtc/media/base/mediaconstants.h
index 03a19de..57ddcbd 100644
--- a/webrtc/media/base/mediaconstants.h
+++ b/webrtc/media/base/mediaconstants.h
@@ -123,7 +123,6 @@
// RFC 6184 RTP Payload Format for H.264 video
extern const char kH264FmtpProfileLevelId[];
-extern const char kH264FmtpDefaultProfileLevelId[];
extern const char kH264FmtpLevelAsymmetryAllowed[];
extern const char kH264FmtpPacketizationMode[];
extern const char kH264ProfileLevelConstrainedBaseline[];
