diff --git a/webrtc/api/android/jni/androidmediaencoder_jni.cc b/webrtc/api/android/jni/androidmediaencoder_jni.cc index 92b5aae65c..a4daa7a539 100644 --- a/webrtc/api/android/jni/androidmediaencoder_jni.cc +++ b/webrtc/api/android/jni/androidmediaencoder_jni.cc @@ -1355,7 +1355,7 @@ webrtc::VideoEncoder* MediaCodecVideoEncoderFactory::CreateVideoEncoder( 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 c7e933ac32..f682862ca1 100644 --- a/webrtc/common_video/h264/profile_level_id.cc +++ b/webrtc/common_video/h264/profile_level_id.cc @@ -263,6 +263,14 @@ void GenerateProfileLevelIdForAnswer( 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 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 ff4b5ba7f7..38a84d576e 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 @@ TEST(H264ProfileLevelId, TestGenerateProfileLevelIdForAnswerEmpty) { 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 66bc9ff7d9..0320e58c72 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 profile_level_id = + webrtc::H264::ParseSdpProfileLevelId(params1); + const rtc::Optional 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 @@ bool VideoCodec::operator==(const VideoCodec& c) const { 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 @@ webrtc::VideoCodecType CodecTypeFromName(const std::string& name) { return webrtc::kVideoCodecUnknown; } -bool IsCodecSupported(const std::vector& 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& 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 3bd93a9247..ca439a6e2f 100644 --- a/webrtc/media/base/codec.h +++ b/webrtc/media/base/codec.h @@ -24,8 +24,6 @@ namespace cricket { typedef std::map CodecParameterMap; -extern const int kMaxPayloadId; - class FeedbackParam { public: FeedbackParam(const std::string& id, const std::string& param) @@ -154,6 +152,11 @@ struct VideoCodec : public Codec { 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 @@ webrtc::VideoCodecType CodecTypeFromName(const std::string& name); bool HasNack(const Codec& codec); bool HasRemb(const Codec& codec); bool HasTransportCc(const Codec& codec); -bool IsCodecSupported(const std::vector& 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& supported_codecs, + const VideoCodec& codec); } // namespace cricket diff --git a/webrtc/media/engine/fakewebrtcvideoengine.h b/webrtc/media/engine/fakewebrtcvideoengine.h index a6cfeb3edf..008f7a12d2 100644 --- a/webrtc/media/engine/fakewebrtcvideoengine.h +++ b/webrtc/media/engine/fakewebrtcvideoengine.h @@ -195,7 +195,7 @@ class FakeWebRtcVideoEncoderFactory : public WebRtcVideoEncoderFactory { 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 efec0b5210..2ebbd78e98 100644 --- a/webrtc/media/engine/webrtcvideoengine2.cc +++ b/webrtc/media/engine/webrtcvideoengine2.cc @@ -638,7 +638,7 @@ static std::vector GetSupportedCodecs( 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 @@ WebRtcVideoChannel2::SelectSendVideoCodec( 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(remote_mapped_codec); } // No remote codec was supported. @@ -955,7 +960,7 @@ bool WebRtcVideoChannel2::GetChangedRecvParameters( const std::vector 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 d42afcefb3..7fab75df29 100644 --- a/webrtc/media/engine/webrtcvideoengine2_unittest.cc +++ b/webrtc/media/engine/webrtcvideoengine2_unittest.cc @@ -2519,7 +2519,8 @@ TEST_F(WebRtcVideoChannel2Test, SetSendCodecsRejectBadPayloadTypes) { 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 e1c9e39ab2..2bb6253519 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 cc93a8b170..f91a729720 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 @@ static void NegotiateCodecs(const std::vector& local_codecs, 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 b7a5141dfd..07725309bf 100644 --- a/webrtc/sdk/objc/Framework/Classes/videotoolboxvideocodecfactory.cc +++ b/webrtc/sdk/objc/Framework/Classes/videotoolboxvideocodecfactory.cc @@ -47,7 +47,7 @@ VideoToolboxVideoEncoderFactory::~VideoToolboxVideoEncoderFactory() {} 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 @@ VideoDecoder* VideoToolboxVideoDecoderFactory::CreateVideoDecoder( 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(); }