From 725e484e33ec92daf4d3c722a0487f8cb244b637 Mon Sep 17 00:00:00 2001 From: magjed Date: Wed, 16 Nov 2016 00:48:13 -0800 Subject: [PATCH] Use different RTX payload types for different H264 profiles This CL is a quick fix to unblock H264 High Profile. This CL is supposed to be superseded by a proper fix of https://bugs.chromium.org/p/webrtc/issues/detail?id=6705 like https://codereview.webrtc.org/2493133002/. BUG=webrtc:6677 Review-Url: https://codereview.webrtc.org/2497773003 Cr-Commit-Position: refs/heads/master@{#15099} --- webrtc/media/base/mediaconstants.cc | 3 +- webrtc/media/base/mediaconstants.h | 3 +- webrtc/media/engine/fakewebrtcvideoengine.h | 4 ++ webrtc/media/engine/payload_type_mapper.cc | 2 +- .../engine/payload_type_mapper_unittest.cc | 3 +- webrtc/media/engine/webrtcvideoengine2.cc | 17 +++++- .../engine/webrtcvideoengine2_unittest.cc | 61 ++++++++++++++----- 7 files changed, 72 insertions(+), 21 deletions(-) diff --git a/webrtc/media/base/mediaconstants.cc b/webrtc/media/base/mediaconstants.cc index 27fdd2ab1e..5acd8cdb01 100644 --- a/webrtc/media/base/mediaconstants.cc +++ b/webrtc/media/base/mediaconstants.cc @@ -117,7 +117,8 @@ const int kDefaultFlexfecPlType = 118; const int kDefaultRtxVp8PlType = 96; const int kDefaultRtxVp9PlType = 97; const int kDefaultRtxRedPlType = 98; -const int kDefaultRtxH264PlType = 99; +const int kDefaultRtxH264ConstrainedBaselinePlType = 99; +const int kDefaultRtxH264ConstrainedHighPlType = 102; const int kDefaultVideoMaxFramerate = 60; } // namespace cricket diff --git a/webrtc/media/base/mediaconstants.h b/webrtc/media/base/mediaconstants.h index c5dd5e3673..02a7ba239e 100644 --- a/webrtc/media/base/mediaconstants.h +++ b/webrtc/media/base/mediaconstants.h @@ -139,7 +139,8 @@ extern const int kDefaultFlexfecPlType; extern const int kDefaultRtxVp8PlType; extern const int kDefaultRtxVp9PlType; extern const int kDefaultRtxRedPlType; -extern const int kDefaultRtxH264PlType; +extern const int kDefaultRtxH264ConstrainedBaselinePlType; +extern const int kDefaultRtxH264ConstrainedHighPlType; extern const int kDefaultVideoMaxFramerate; } // namespace cricket diff --git a/webrtc/media/engine/fakewebrtcvideoengine.h b/webrtc/media/engine/fakewebrtcvideoengine.h index 008f7a12d2..a3fb30ce16 100644 --- a/webrtc/media/engine/fakewebrtcvideoengine.h +++ b/webrtc/media/engine/fakewebrtcvideoengine.h @@ -233,6 +233,10 @@ class FakeWebRtcVideoEncoderFactory : public WebRtcVideoEncoderFactory { encoders_have_internal_sources_ = internal_source; } + void AddSupportedVideoCodec(const cricket::VideoCodec& codec) { + codecs_.push_back(codec); + } + void AddSupportedVideoCodecType(const std::string& name) { codecs_.push_back(cricket::VideoCodec(name)); } diff --git a/webrtc/media/engine/payload_type_mapper.cc b/webrtc/media/engine/payload_type_mapper.cc index 58eab16ead..a0d4e0cde7 100644 --- a/webrtc/media/engine/payload_type_mapper.cc +++ b/webrtc/media/engine/payload_type_mapper.cc @@ -70,7 +70,7 @@ PayloadTypeMapper::PayloadTypeMapper() {{kRtxCodecName, 90000, 0, {{kCodecParamAssociatedPayloadType, std::to_string(kDefaultH264PlType)}}}, - kDefaultRtxH264PlType}, + kDefaultRtxH264ConstrainedBaselinePlType}, // Other codecs {{kVp8CodecName, 90000, 0}, kDefaultVp8PlType}, {{kVp9CodecName, 90000, 0}, kDefaultVp9PlType}, diff --git a/webrtc/media/engine/payload_type_mapper_unittest.cc b/webrtc/media/engine/payload_type_mapper_unittest.cc index 8e00dbc2b7..5d6976e666 100644 --- a/webrtc/media/engine/payload_type_mapper_unittest.cc +++ b/webrtc/media/engine/payload_type_mapper_unittest.cc @@ -78,7 +78,8 @@ TEST_F(PayloadTypeMapperTest, WebRTCPayloadTypes) { }; EXPECT_EQ(kDefaultRtxVp8PlType, rtx_mapping(kDefaultVp8PlType)); EXPECT_EQ(kDefaultRtxVp9PlType, rtx_mapping(kDefaultVp9PlType)); - EXPECT_EQ(kDefaultRtxH264PlType, rtx_mapping(kDefaultH264PlType)); + EXPECT_EQ(kDefaultRtxH264ConstrainedBaselinePlType, + rtx_mapping(kDefaultH264PlType)); EXPECT_EQ(kDefaultRtxRedPlType, rtx_mapping(kDefaultRedPlType)); EXPECT_EQ(102, FindMapping({kIlbcCodecName, 8000, 1})); diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc index 7e06ea5fdc..c43dff80d3 100644 --- a/webrtc/media/engine/webrtcvideoengine2.cc +++ b/webrtc/media/engine/webrtcvideoengine2.cc @@ -22,6 +22,7 @@ #include "webrtc/base/timeutils.h" #include "webrtc/base/trace_event.h" #include "webrtc/call.h" +#include "webrtc/common_video/h264/profile_level_id.h" #include "webrtc/media/engine/constants.h" #include "webrtc/media/engine/simulcast.h" #include "webrtc/media/engine/videoencodersoftwarefallbackwrapper.h" @@ -404,7 +405,21 @@ void AddCodecAndMaybeRtxCodec(const VideoCodec& codec, } else if (CodecNamesEq(codec.name, kVp9CodecName)) { rtx_payload_type = kDefaultRtxVp9PlType; } else if (CodecNamesEq(codec.name, kH264CodecName)) { - rtx_payload_type = kDefaultRtxH264PlType; + // Parse H264 profile. + const rtc::Optional profile_level_id = + webrtc::H264::ParseSdpProfileLevelId(codec.params); + if (!profile_level_id) + return; + const webrtc::H264::Profile profile = profile_level_id->profile; + // In H.264, we only support rtx for constrained baseline and constrained + // high profile. + if (profile == webrtc::H264::kProfileConstrainedBaseline) { + rtx_payload_type = kDefaultRtxH264ConstrainedBaselinePlType; + } else if (profile == webrtc::H264::kProfileConstrainedHigh) { + rtx_payload_type = kDefaultRtxH264ConstrainedHighPlType; + } else { + return; + } } else if (CodecNamesEq(codec.name, kRedCodecName)) { rtx_payload_type = kDefaultRtxRedPlType; } else { diff --git a/webrtc/media/engine/webrtcvideoengine2_unittest.cc b/webrtc/media/engine/webrtcvideoengine2_unittest.cc index 7fab75df29..19b7d644cd 100644 --- a/webrtc/media/engine/webrtcvideoengine2_unittest.cc +++ b/webrtc/media/engine/webrtcvideoengine2_unittest.cc @@ -16,6 +16,7 @@ #include "webrtc/base/arraysize.h" #include "webrtc/base/gunit.h" #include "webrtc/base/stringutils.h" +#include "webrtc/common_video/h264/profile_level_id.h" #include "webrtc/logging/rtc_event_log/rtc_event_log.h" #include "webrtc/media/base/testutils.h" #include "webrtc/media/base/videoengine_unittest.h" @@ -61,6 +62,22 @@ void VerifyCodecHasDefaultFeedbackParams(const cricket::VideoCodec& codec) { cricket::kRtcpFbParamCcm, cricket::kRtcpFbCcmParamFir))); } +// Return true if any codec in |codecs| is an RTX codec with associated payload +// type |payload_type|. +bool HasRtxCodec(const std::vector& codecs, + int payload_type) { + for (const cricket::VideoCodec& codec : codecs) { + int associated_payload_type; + if (cricket::CodecNamesEq(codec.name.c_str(), "rtx") && + codec.GetParam(cricket::kCodecParamAssociatedPayloadType, + &associated_payload_type) && + associated_payload_type == payload_type) { + return true; + } + } + return false; +} + static rtc::scoped_refptr CreateBlackFrameBuffer( int width, int height) { @@ -375,26 +392,38 @@ TEST_F(WebRtcVideoEngine2Test, UseExternalFactoryForVp8WhenSupported) { // built with no internal H264 support. This test should be updated // if/when we start adding RTX codecs for unrecognized codec names. TEST_F(WebRtcVideoEngine2Test, RtxCodecAddedForExternalCodec) { + using webrtc::H264::ProfileLevelIdToString; + using webrtc::H264::ProfileLevelId; + using webrtc::H264::kLevel1; + cricket::VideoCodec h264_constrained_baseline("H264"); + h264_constrained_baseline.params[kH264FmtpProfileLevelId] = + *ProfileLevelIdToString( + ProfileLevelId(webrtc::H264::kProfileConstrainedBaseline, kLevel1)); + cricket::VideoCodec h264_constrained_high("H264"); + h264_constrained_high.params[kH264FmtpProfileLevelId] = + *ProfileLevelIdToString( + ProfileLevelId(webrtc::H264::kProfileConstrainedHigh, kLevel1)); + cricket::VideoCodec h264_high("H264"); + h264_high.params[kH264FmtpProfileLevelId] = *ProfileLevelIdToString( + ProfileLevelId(webrtc::H264::kProfileHigh, kLevel1)); + cricket::FakeWebRtcVideoEncoderFactory encoder_factory; - encoder_factory.AddSupportedVideoCodecType("H264"); + encoder_factory.AddSupportedVideoCodec(h264_constrained_baseline); + encoder_factory.AddSupportedVideoCodec(h264_constrained_high); + encoder_factory.AddSupportedVideoCodec(h264_high); engine_.SetExternalEncoderFactory(&encoder_factory); engine_.Init(); - auto codecs = engine_.codecs(); - // First figure out what payload type the test codec got assigned. - auto test_codec_it = - std::find_if(codecs.begin(), codecs.end(), - [](const VideoCodec& c) { return c.name == "H264"; }); - ASSERT_NE(codecs.end(), test_codec_it); - // Now search for an RTX codec for it. - EXPECT_TRUE(std::any_of(codecs.begin(), codecs.end(), - [&test_codec_it](const VideoCodec& c) { - int associated_payload_type; - return c.name == "rtx" && - c.GetParam(kCodecParamAssociatedPayloadType, - &associated_payload_type) && - associated_payload_type == test_codec_it->id; - })); + // First figure out what payload types the test codecs got assigned. + const std::vector codecs = engine_.codecs(); + // Now search for RTX codecs for them. Expect that Constrained Baseline and + // Constrained High got an associated RTX codec, but not High. + EXPECT_TRUE(HasRtxCodec( + codecs, FindMatchingCodec(codecs, h264_constrained_baseline)->id)); + EXPECT_TRUE(HasRtxCodec( + codecs, FindMatchingCodec(codecs, h264_constrained_high)->id)); + EXPECT_FALSE(HasRtxCodec( + codecs, FindMatchingCodec(codecs, h264_high)->id)); } void WebRtcVideoEngine2Test::TestExtendedEncoderOveruse(