Negotiate H264 profiles in SDP
This CL will start to distinguish H264 profiles during SDP negotiation.
We currently don't look at the H264 profile at all and assume they are
all Constrained Baseline Level 3.1. This CL will start to check profiles
for equality when matching, and will generate the correct answer H264
level.
Each local supported H264 profile needs to be listed explicitly in the
list of local supported codecs, even if they are redundant. For example,
Baseline profile should be listed explicitly even though another profile
that is a superset of Baseline is also listed. The reason for this is to
simplify the code and avoid profile intersection during matching. So
VideoCodec::Matches will check for profile equality, and not check if
one codec is a subset of the other. This also leads to the nice property
that VideoCodec::Matches is symmetric, i.e. iif a.Matches(b) then
b.Matches(a).
BUG=webrtc:6337
TBR=tkchin@webrtc.org
Review-Url: https://codereview.webrtc.org/2483173002
Cr-Commit-Position: refs/heads/master@{#15051}
diff --git a/webrtc/api/android/jni/androidmediaencoder_jni.cc b/webrtc/api/android/jni/androidmediaencoder_jni.cc
index 92b5aae..a4daa7a 100644
--- a/webrtc/api/android/jni/androidmediaencoder_jni.cc
+++ b/webrtc/api/android/jni/androidmediaencoder_jni.cc
@@ -1355,7 +1355,7 @@
ALOGW << "No HW video encoder for codec " << codec.name;
return nullptr;
}
- if (IsCodecSupported(supported_codecs_, codec)) {
+ if (FindMatchingCodec(supported_codecs_, codec)) {
ALOGD << "Create HW video encoder for " << codec.name;
const VideoCodecType type = cricket::CodecTypeFromName(codec.name);
return new MediaCodecVideoEncoder(AttachCurrentThreadIfNeeded(), type,
diff --git a/webrtc/common_video/h264/profile_level_id.cc b/webrtc/common_video/h264/profile_level_id.cc
index c7e933a..f682862 100644
--- a/webrtc/common_video/h264/profile_level_id.cc
+++ b/webrtc/common_video/h264/profile_level_id.cc
@@ -263,6 +263,14 @@
const CodecParameterMap& local_supported_params,
const CodecParameterMap& remote_offered_params,
CodecParameterMap* answer_params) {
+ // If both local and remote haven't set profile-level-id, they are both using
+ // the default profile. In this case, don't set profile-level-id in answer
+ // either.
+ if (!local_supported_params.count(kProfileLevelId) &&
+ !remote_offered_params.count(kProfileLevelId)) {
+ return;
+ }
+
// Parse profile-level-ids.
const rtc::Optional<ProfileLevelId> local_profile_level_id =
ParseSdpProfileLevelId(local_supported_params);
diff --git a/webrtc/common_video/h264/profile_level_id_unittest.cc b/webrtc/common_video/h264/profile_level_id_unittest.cc
index ff4b5ba..38a84d5 100644
--- a/webrtc/common_video/h264/profile_level_id_unittest.cc
+++ b/webrtc/common_video/h264/profile_level_id_unittest.cc
@@ -152,7 +152,7 @@
CodecParameterMap answer_params;
GenerateProfileLevelIdForAnswer(CodecParameterMap(), CodecParameterMap(),
&answer_params);
- EXPECT_EQ("42e01f", answer_params["profile-level-id"]);
+ EXPECT_TRUE(answer_params.empty());
}
TEST(H264ProfileLevelId,
diff --git a/webrtc/media/base/codec.cc b/webrtc/media/base/codec.cc
index 66bc9ff..0320e58 100644
--- a/webrtc/media/base/codec.cc
+++ b/webrtc/media/base/codec.cc
@@ -17,10 +17,20 @@
#include "webrtc/base/logging.h"
#include "webrtc/base/stringencode.h"
#include "webrtc/base/stringutils.h"
+#include "webrtc/common_video/h264/profile_level_id.h"
namespace cricket {
-const int kMaxPayloadId = 127;
+static bool IsSameH264Profile(const CodecParameterMap& params1,
+ const CodecParameterMap& params2) {
+ const rtc::Optional<webrtc::H264::ProfileLevelId> profile_level_id =
+ webrtc::H264::ParseSdpProfileLevelId(params1);
+ const rtc::Optional<webrtc::H264::ProfileLevelId> other_profile_level_id =
+ webrtc::H264::ParseSdpProfileLevelId(params2);
+ // Compare H264 profiles, but not levels.
+ return profile_level_id && other_profile_level_id &&
+ profile_level_id->profile == other_profile_level_id->profile;
+}
bool FeedbackParam::operator==(const FeedbackParam& other) const {
return _stricmp(other.id().c_str(), id().c_str()) == 0 &&
@@ -218,6 +228,14 @@
return Codec::operator==(c);
}
+bool VideoCodec::Matches(const VideoCodec& other) const {
+ if (!Codec::Matches(other))
+ return false;
+ if (CodecNamesEq(name.c_str(), kH264CodecName))
+ return IsSameH264Profile(params, other.params);
+ return true;
+}
+
VideoCodec VideoCodec::CreateRtxCodec(int rtx_payload_type,
int associated_payload_type) {
VideoCodec rtx_codec(rtx_payload_type, kRtxCodecName);
@@ -317,12 +335,19 @@
return webrtc::kVideoCodecUnknown;
}
-bool IsCodecSupported(const std::vector<VideoCodec>& supported_codecs,
- const VideoCodec& codec) {
- return std::any_of(supported_codecs.begin(), supported_codecs.end(),
- [&codec](const VideoCodec& supported_codec) -> bool {
- return CodecNamesEq(codec.name, supported_codec.name);
- });
+const VideoCodec* FindMatchingCodec(
+ const std::vector<VideoCodec>& supported_codecs,
+ const VideoCodec& codec) {
+ for (const VideoCodec& supported_codec : supported_codecs) {
+ if (!CodecNamesEq(codec.name, supported_codec.name))
+ continue;
+ if (CodecNamesEq(codec.name.c_str(), kH264CodecName) &&
+ !IsSameH264Profile(codec.params, supported_codec.params)) {
+ continue;
+ }
+ return &supported_codec;
+ }
+ return nullptr;
}
} // namespace cricket
diff --git a/webrtc/media/base/codec.h b/webrtc/media/base/codec.h
index 3bd93a9..ca439a6 100644
--- a/webrtc/media/base/codec.h
+++ b/webrtc/media/base/codec.h
@@ -24,8 +24,6 @@
typedef std::map<std::string, std::string> CodecParameterMap;
-extern const int kMaxPayloadId;
-
class FeedbackParam {
public:
FeedbackParam(const std::string& id, const std::string& param)
@@ -154,6 +152,11 @@
VideoCodec(VideoCodec&& c);
virtual ~VideoCodec() = default;
+ // Indicates if this video codec is the same as the other video codec, e.g. if
+ // they are both VP8 or VP9, or if they are both H264 with the same H264
+ // profile. H264 levels however are not compared.
+ bool Matches(const VideoCodec& codec) const;
+
std::string ToString() const;
VideoCodec& operator=(const VideoCodec& c);
@@ -213,8 +216,11 @@
bool HasNack(const Codec& codec);
bool HasRemb(const Codec& codec);
bool HasTransportCc(const Codec& codec);
-bool IsCodecSupported(const std::vector<VideoCodec>& supported_codecs,
- const VideoCodec& codec);
+// Returns the first codec in |supported_codecs| that matches |codec|, or
+// nullptr if no codec matches.
+const VideoCodec* FindMatchingCodec(
+ const std::vector<VideoCodec>& supported_codecs,
+ const VideoCodec& codec);
} // namespace cricket
diff --git a/webrtc/media/engine/fakewebrtcvideoengine.h b/webrtc/media/engine/fakewebrtcvideoengine.h
index a6cfeb3..008f7a1 100644
--- a/webrtc/media/engine/fakewebrtcvideoengine.h
+++ b/webrtc/media/engine/fakewebrtcvideoengine.h
@@ -195,7 +195,7 @@
webrtc::VideoEncoder* CreateVideoEncoder(
const cricket::VideoCodec& codec) override {
rtc::CritScope lock(&crit_);
- if (!IsCodecSupported(codecs_, codec))
+ if (!FindMatchingCodec(codecs_, codec))
return nullptr;
FakeWebRtcVideoEncoder* encoder = new FakeWebRtcVideoEncoder();
encoders_.push_back(encoder);
diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc
index efec0b5..2ebbd78 100644
--- a/webrtc/media/engine/webrtcvideoengine2.cc
+++ b/webrtc/media/engine/webrtcvideoengine2.cc
@@ -638,7 +638,7 @@
out << ", ";
}
// Don't add internally-supported codecs twice.
- if (IsCodecSupported(supported_codecs, codec))
+ if (FindMatchingCodec(supported_codecs, codec))
continue;
// External video encoders are given payloads 120-127. This also means that
@@ -699,7 +699,12 @@
GetSupportedCodecs(external_encoder_factory_);
// Select the first remote codec that is supported locally.
for (const VideoCodecSettings& remote_mapped_codec : remote_mapped_codecs) {
- if (IsCodecSupported(local_supported_codecs, remote_mapped_codec.codec))
+ // For H264, we will limit the encode level to the remote offered level
+ // regardless if level asymmetry is allowed or not. This is strictly not
+ // following the spec in https://tools.ietf.org/html/rfc6184#section-8.2.2
+ // since we should limit the encode level to the lower of local and remote
+ // level when level asymmetry is not allowed.
+ if (FindMatchingCodec(local_supported_codecs, remote_mapped_codec.codec))
return rtc::Optional<VideoCodecSettings>(remote_mapped_codec);
}
// No remote codec was supported.
@@ -955,7 +960,7 @@
const std::vector<VideoCodec> local_supported_codecs =
GetSupportedCodecs(external_encoder_factory_);
for (const VideoCodecSettings& mapped_codec : mapped_codecs) {
- if (!IsCodecSupported(local_supported_codecs, mapped_codec.codec)) {
+ if (!FindMatchingCodec(local_supported_codecs, mapped_codec.codec)) {
LOG(LS_ERROR) << "SetRecvParameters called with unsupported video codec: "
<< mapped_codec.codec.ToString();
return false;
diff --git a/webrtc/media/engine/webrtcvideoengine2_unittest.cc b/webrtc/media/engine/webrtcvideoengine2_unittest.cc
index d42afce..7fab75d 100644
--- a/webrtc/media/engine/webrtcvideoengine2_unittest.cc
+++ b/webrtc/media/engine/webrtcvideoengine2_unittest.cc
@@ -2519,7 +2519,8 @@
TEST_F(WebRtcVideoChannel2Test, SetSendCodecsAcceptAllValidPayloadTypes) {
cricket::VideoSendParameters parameters;
parameters.codecs.push_back(kVp8Codec);
- for (int payload_type = 0; payload_type <= 127; ++payload_type) {
+ // Only the dynamic payload types are valid for video codecs.
+ for (int payload_type = 96; payload_type <= 127; ++payload_type) {
parameters.codecs[0].id = payload_type;
EXPECT_TRUE(channel_->SetSendParameters(parameters))
<< "Payload type '" << payload_type << "' rejected.";
diff --git a/webrtc/pc/DEPS b/webrtc/pc/DEPS
index e1c9e39..2bb6253 100644
--- a/webrtc/pc/DEPS
+++ b/webrtc/pc/DEPS
@@ -1,6 +1,7 @@
include_rules = [
"+webrtc/api",
"+webrtc/base",
+ "+webrtc/common_video/h264",
"+webrtc/logging/rtc_event_log",
"+webrtc/media",
"+webrtc/p2p",
diff --git a/webrtc/pc/mediasession.cc b/webrtc/pc/mediasession.cc
index cc93a8b..f91a729 100644
--- a/webrtc/pc/mediasession.cc
+++ b/webrtc/pc/mediasession.cc
@@ -22,6 +22,7 @@
#include "webrtc/base/helpers.h"
#include "webrtc/base/logging.h"
#include "webrtc/base/stringutils.h"
+#include "webrtc/common_video/h264/profile_level_id.h"
#include "webrtc/media/base/cryptoparams.h"
#include "webrtc/media/base/mediaconstants.h"
#include "webrtc/p2p/base/p2pconstants.h"
@@ -778,6 +779,10 @@
RTC_DCHECK(apt_it != theirs.params.end());
negotiated.SetParam(kCodecParamAssociatedPayloadType, apt_it->second);
}
+ if (CodecNamesEq(ours.name.c_str(), kH264CodecName)) {
+ webrtc::H264::GenerateProfileLevelIdForAnswer(
+ ours.params, theirs.params, &negotiated.params);
+ }
negotiated.id = theirs.id;
negotiated.name = theirs.name;
negotiated_codecs->push_back(std::move(negotiated));
diff --git a/webrtc/sdk/objc/Framework/Classes/videotoolboxvideocodecfactory.cc b/webrtc/sdk/objc/Framework/Classes/videotoolboxvideocodecfactory.cc
index b7a5141..0772530 100644
--- a/webrtc/sdk/objc/Framework/Classes/videotoolboxvideocodecfactory.cc
+++ b/webrtc/sdk/objc/Framework/Classes/videotoolboxvideocodecfactory.cc
@@ -47,7 +47,7 @@
VideoEncoder* VideoToolboxVideoEncoderFactory::CreateVideoEncoder(
const cricket::VideoCodec& codec) {
#if defined(WEBRTC_IOS)
- if (IsCodecSupported(supported_codecs_, codec)) {
+ if (FindMatchingCodec(supported_codecs_, codec)) {
LOG(LS_INFO) << "Creating HW encoder for " << codec.name;
return new H264VideoToolboxEncoder();
}
@@ -83,7 +83,7 @@
VideoCodecType type) {
const auto codec = cricket::VideoCodec(NameFromCodecType(type));
#if defined(WEBRTC_IOS)
- if (IsCodecSupported(supported_codecs_, codec)) {
+ if (FindMatchingCodec(supported_codecs_, codec)) {
LOG(LS_INFO) << "Creating HW decoder for " << codec.name;
return new H264VideoToolboxDecoder();
}
