admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Reland "Move ulpfec, red, and flexfec codec to video engine"

Browse Source 
This is a reland of 154ee1fd8547768a49b7d67ce586ef5d3c5d9ebc
Original change's description:
> Move ulpfec, red, and flexfec codec to video engine
>
> These codecs are currently being added in the internal encoder factory.
> This means that the new injectable video codec factories will miss them.
> This CL moves adding them into the video engine so that both factory
> types will get them.
>
> This CL makes a functional change in that RED, ULPFEC, and FlexFec will
> be placed after both the internal and external codecs. Previously,
> it was placed between the internal and external codecs.
>
> Bug: webrtc:8527
> Change-Id: I5aa7a3ca674f621b17cf3aa095a225c753488e09
> Reviewed-on: https://webrtc-review.googlesource.com/22964
> Commit-Queue: Magnus Jedvert <magjed@webrtc.org>
> Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#20700}
TBR=brandt@webrtc.org

Bug: webrtc:8527
Change-Id: I79ced9a909fd424f1308d62e449268dcc9289538
Reviewed-on: https://webrtc-review.googlesource.com/24060
Reviewed-by: Magnus Jedvert <magjed@webrtc.org>
Commit-Queue: Magnus Jedvert <magjed@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20749}
This commit is contained in:
Magnus Jedvert 2017-11-18 12:08:55 +01:00 committed by Commit Bot
parent c040daec11
commit 9b16e2d354
3 changed files with 130 additions and 112 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
media/engine/internalencoderfactory.cc
View File

@ -16,24 +16,9 @@
#include "modules/video_coding/codecs/vp8/include/vp8.h"
#include "modules/video_coding/codecs/vp9/include/vp9.h"
#include "rtc_base/logging.h"
#include "system_wrappers/include/field_trial.h"
namespace webrtc {
namespace {
// If this field trial is enabled, the "flexfec-03" codec will be advertised
// as being supported by the InternalEncoderFactory. This means that
// "flexfec-03" will appear in the default SDP offer, and we therefore need to
// be ready to receive FlexFEC packets from the remote. It also means that
// FlexFEC SSRCs will be generated by MediaSession and added as "a=ssrc:" and
// "a=ssrc-group:" lines in the local SDP.
bool IsFlexfecAdvertisedFieldTrialEnabled() {
return webrtc::field_trial::IsEnabled("WebRTC-FlexFEC-03-Advertised");
}
} // namespace
std::vector<SdpVideoFormat> InternalEncoderFactory::GetSupportedFormats()
const {
std::vector<SdpVideoFormat> supported_codecs;
@ -44,20 +29,6 @@ std::vector<SdpVideoFormat> InternalEncoderFactory::GetSupportedFormats()
for (const webrtc::SdpVideoFormat& format : webrtc::SupportedH264Codecs())
supported_codecs.push_back(format);
supported_codecs.push_back(SdpVideoFormat(cricket::kRedCodecName));
supported_codecs.push_back(SdpVideoFormat(cricket::kUlpfecCodecName));
if (IsFlexfecAdvertisedFieldTrialEnabled()) {
// This value is currently arbitrarily set to 10 seconds. (The unit
// is microseconds.) This parameter MUST be present in the SDP, but
// we never use the actual value anywhere in our code however.
// TODO(brandtr): Consider honouring this value in the sender and receiver.
SdpVideoFormat::Parameters params = {
{cricket::kFlexfecFmtpRepairWindow, "10000000"}};
supported_codecs.push_back(
SdpVideoFormat(cricket::kFlexfecCodecName, params));
}
return supported_codecs;
}

133
media/engine/webrtcvideoengine.cc
View File

@ -155,25 +155,18 @@ class NullVideoDecoder : public webrtc::VideoDecoder {
const char* ImplementationName() const override { return "NullVideoDecoder"; }
};
std::vector<VideoCodec> AssignPayloadTypesAndAddAssociatedRtxCodecs(
const std::vector<webrtc::SdpVideoFormat>& input_formats);
std::vector<VideoCodec> AssignPayloadTypesAndAddAssociatedRtxCodecs(
const webrtc::VideoEncoderFactory* encoder_factory) {
return encoder_factory ? AssignPayloadTypesAndAddAssociatedRtxCodecs(
encoder_factory->GetSupportedFormats())
: std::vector<VideoCodec>();
}
// If this field trial is enabled, we will enable sending FlexFEC and disable
// sending ULPFEC whenever the former has been negotiated in the SDPs.
bool IsFlexfecFieldTrialEnabled() {
return webrtc::field_trial::IsEnabled("WebRTC-FlexFEC-03");
}
// If this field trial is enabled, the "flexfec-03" codec may have been
// advertised as being supported in the local SDP. That means that we must be
// ready to receive FlexFEC packets. See internalencoderfactory.cc.
// If this field trial is enabled, the "flexfec-03" codec will be advertised
// as being supported. This means that "flexfec-03" will appear in the default
// SDP offer, and we therefore need to be ready to receive FlexFEC packets from
// the remote. It also means that FlexFEC SSRCs will be generated by
// MediaSession and added as "a=ssrc:" and "a=ssrc-group:" lines in the local
// SDP.
bool IsFlexfecAdvertisedFieldTrialEnabled() {
return webrtc::field_trial::IsEnabled("WebRTC-FlexFEC-03-Advertised");
}
@ -193,6 +186,70 @@ void AddDefaultFeedbackParams(VideoCodec* codec) {
codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamNack, kRtcpFbNackParamPli));
}
// This function will assign dynamic payload types (in the range [96, 127]) to
// the input codecs, and also add ULPFEC, RED, FlexFEC, and associated RTX
// codecs for recognized codecs (VP8, VP9, H264, and RED). It will also add
// default feedback params to the codecs.
std::vector<VideoCodec> AssignPayloadTypesAndDefaultCodecs(
std::vector<webrtc::SdpVideoFormat> input_formats) {
if (input_formats.empty())
return std::vector<VideoCodec>();
static const int kFirstDynamicPayloadType = 96;
static const int kLastDynamicPayloadType = 127;
int payload_type = kFirstDynamicPayloadType;
input_formats.push_back(webrtc::SdpVideoFormat(kRedCodecName));
input_formats.push_back(webrtc::SdpVideoFormat(kUlpfecCodecName));
if (IsFlexfecAdvertisedFieldTrialEnabled()) {
webrtc::SdpVideoFormat flexfec_format(kFlexfecCodecName);
// This value is currently arbitrarily set to 10 seconds. (The unit
// is microseconds.) This parameter MUST be present in the SDP, but
// we never use the actual value anywhere in our code however.
// TODO(brandtr): Consider honouring this value in the sender and receiver.
flexfec_format.parameters = {{kFlexfecFmtpRepairWindow, "10000000"}};
input_formats.push_back(flexfec_format);
}
std::vector<VideoCodec> output_codecs;
for (const webrtc::SdpVideoFormat& format : input_formats) {
VideoCodec codec(format);
codec.id = payload_type;
AddDefaultFeedbackParams(&codec);
output_codecs.push_back(codec);
// Increment payload type.
++payload_type;
if (payload_type > kLastDynamicPayloadType)
break;
// Add associated RTX codec for recognized codecs.
// TODO(deadbeef): Should we add RTX codecs for external codecs whose names
// we don't recognize?
if (CodecNamesEq(codec.name, kVp8CodecName) ||
CodecNamesEq(codec.name, kVp9CodecName) ||
CodecNamesEq(codec.name, kH264CodecName) ||
CodecNamesEq(codec.name, kRedCodecName)) {
output_codecs.push_back(
VideoCodec::CreateRtxCodec(payload_type, codec.id));
// Increment payload type.
++payload_type;
if (payload_type > kLastDynamicPayloadType)
break;
}
}
return output_codecs;
}
std::vector<VideoCodec> AssignPayloadTypesAndDefaultCodecs(
const webrtc::VideoEncoderFactory* encoder_factory) {
return encoder_factory ? AssignPayloadTypesAndDefaultCodecs(
encoder_factory->GetSupportedFormats())
: std::vector<VideoCodec>();
}
static std::string CodecVectorToString(const std::vector<VideoCodec>& codecs) {
std::stringstream out;
out << '{';
@ -497,7 +554,7 @@ WebRtcVideoChannel* WebRtcVideoEngine::CreateChannel(
}
std::vector<VideoCodec> WebRtcVideoEngine::codecs() const {
return AssignPayloadTypesAndAddAssociatedRtxCodecs(encoder_factory_.get());
return AssignPayloadTypesAndDefaultCodecs(encoder_factory_.get());
}
RtpCapabilities WebRtcVideoEngine::GetCapabilities() const {
@ -526,48 +583,6 @@ RtpCapabilities WebRtcVideoEngine::GetCapabilities() const {
return capabilities;
}
namespace {
// This function will assign dynamic payload types (in the range [96, 127]) to
// the input codecs, and also add associated RTX codecs for recognized codecs
// (VP8, VP9, H264, and RED). It will also add default feedback params to the
// codecs.
std::vector<VideoCodec> AssignPayloadTypesAndAddAssociatedRtxCodecs(
const std::vector<webrtc::SdpVideoFormat>& input_formats) {
static const int kFirstDynamicPayloadType = 96;
static const int kLastDynamicPayloadType = 127;
int payload_type = kFirstDynamicPayloadType;
std::vector<VideoCodec> output_codecs;
for (const webrtc::SdpVideoFormat& format : input_formats) {
VideoCodec codec(format);
codec.id = payload_type;
AddDefaultFeedbackParams(&codec);
output_codecs.push_back(codec);
// Increment payload type.
++payload_type;
if (payload_type > kLastDynamicPayloadType)
break;
// Add associated RTX codec for recognized codecs.
// TODO(deadbeef): Should we add RTX codecs for external codecs whose names
// we don't recognize?
if (CodecNamesEq(codec.name, kVp8CodecName) ||
CodecNamesEq(codec.name, kVp9CodecName) ||
CodecNamesEq(codec.name, kH264CodecName) ||
CodecNamesEq(codec.name, kRedCodecName)) {
output_codecs.push_back(
VideoCodec::CreateRtxCodec(payload_type, codec.id));
// Increment payload type.
++payload_type;
if (payload_type > kLastDynamicPayloadType)
break;
}
}
return output_codecs;
}
} // namespace
WebRtcVideoChannel::WebRtcVideoChannel(
webrtc::Call* call,
const MediaConfig& config,
@ -587,7 +602,7 @@ WebRtcVideoChannel::WebRtcVideoChannel(
rtcp_receiver_report_ssrc_ = kDefaultRtcpReceiverReportSsrc;
sending_ = false;
recv_codecs_ =
MapCodecs(AssignPayloadTypesAndAddAssociatedRtxCodecs(encoder_factory_));
MapCodecs(AssignPayloadTypesAndDefaultCodecs(encoder_factory_));
recv_flexfec_payload_type_ = recv_codecs_.front().flexfec_payload_type;
}
@ -602,7 +617,7 @@ rtc::Optional<WebRtcVideoChannel::VideoCodecSettings>
WebRtcVideoChannel::SelectSendVideoCodec(
const std::vector<VideoCodecSettings>& remote_mapped_codecs) const {
const std::vector<VideoCodec> local_supported_codecs =
AssignPayloadTypesAndAddAssociatedRtxCodecs(encoder_factory_);
AssignPayloadTypesAndDefaultCodecs(encoder_factory_);
// Select the first remote codec that is supported locally.
for (const VideoCodecSettings& remote_mapped_codec : remote_mapped_codecs) {
// For H264, we will limit the encode level to the remote offered level
@ -919,7 +934,7 @@ bool WebRtcVideoChannel::GetChangedRecvParameters(
// Verify that every mapped codec is supported locally.
const std::vector<VideoCodec> local_supported_codecs =
AssignPayloadTypesAndAddAssociatedRtxCodecs(encoder_factory_);
AssignPayloadTypesAndDefaultCodecs(encoder_factory_);
for (const VideoCodecSettings& mapped_codec : mapped_codecs) {
if (!FindMatchingCodec(local_supported_codecs, mapped_codec.codec)) {
RTC_LOG(LS_ERROR)

80
media/engine/webrtcvideoengine_unittest.cc
View File

@ -181,9 +181,13 @@ class WebRtcVideoEngineTest : public ::testing::Test {
}
protected:
// Find the index of the codec in the engine with the given name. The codec
// must be present.
int GetEngineCodecIndex(const std::string& name) const;
// Find the codec in the engine with the given name. The codec must be
// present.
cricket::VideoCodec GetEngineCodec(const std::string& name);
cricket::VideoCodec GetEngineCodec(const std::string& name) const;
VideoMediaChannel* SetUpForExternalEncoderFactory();
@ -565,9 +569,10 @@ TEST_F(WebRtcVideoEngineTest, PropagatesInputFrameTimestamp) {
EXPECT_TRUE(channel->RemoveSendStream(kSsrc));
}
cricket::VideoCodec WebRtcVideoEngineTest::GetEngineCodec(
const std::string& name) {
for (const cricket::VideoCodec& engine_codec : engine_.codecs()) {
int WebRtcVideoEngineTest::GetEngineCodecIndex(const std::string& name) const {
const std::vector<cricket::VideoCodec> codecs = engine_.codecs();
for (size_t i = 0; i < codecs.size(); ++i) {
const cricket::VideoCodec engine_codec = codecs[i];
if (!CodecNamesEq(name, engine_codec.name))
continue;
// The tests only use H264 Constrained Baseline. Make sure we don't return
@ -580,11 +585,16 @@ cricket::VideoCodec WebRtcVideoEngineTest::GetEngineCodec(
continue;
}
}
return engine_codec;
return i;
}
// This point should never be reached.
ADD_FAILURE() << "Unrecognized codec name: " << name;
return cricket::VideoCodec();
return -1;
}
cricket::VideoCodec WebRtcVideoEngineTest::GetEngineCodec(
const std::string& name) const {
return engine_.codecs()[GetEngineCodecIndex(name)];
}
VideoMediaChannel* WebRtcVideoEngineTest::SetUpForExternalEncoderFactory() {
@ -796,35 +806,39 @@ TEST_F(WebRtcVideoEngineTest,
std::find_if(codecs_after.begin(), codecs_after.end(), is_flexfec));
}
// Test that external codecs are added to the end of the supported codec list.
// Test that external codecs are added after internal SW codecs.
TEST_F(WebRtcVideoEngineTest, ReportSupportedExternalCodecs) {
encoder_factory_->AddSupportedVideoCodecType("FakeExternalCodec");
const char* kFakeExternalCodecName = "FakeExternalCodec";
encoder_factory_->AddSupportedVideoCodecType(kFakeExternalCodecName);
std::vector<cricket::VideoCodec> codecs(engine_.codecs());
ASSERT_GE(codecs.size(), 2u);
cricket::VideoCodec internal_codec = codecs.front();
cricket::VideoCodec external_codec = codecs.back();
// The external codec will appear last in the vector.
EXPECT_EQ("VP8", internal_codec.name);
EXPECT_EQ("FakeExternalCodec", external_codec.name);
// The external codec should appear after the internal codec in the vector.
const int vp8_index = GetEngineCodecIndex("VP8");
const int fake_external_codec_index =
GetEngineCodecIndex(kFakeExternalCodecName);
EXPECT_LT(vp8_index, fake_external_codec_index);
}
// Test that an external codec that was added after the engine was initialized
// does show up in the codec list after it was added.
TEST_F(WebRtcVideoEngineTest, ReportSupportedExternalCodecsWithAddedCodec) {
// Set up external encoder factory with first codec, and initialize engine.
encoder_factory_->AddSupportedVideoCodecType("FakeExternalCodec1");
const char* kFakeExternalCodecName1 = "FakeExternalCodec1";
const char* kFakeExternalCodecName2 = "FakeExternalCodec2";
// Set up external encoder factory with first codec, and initialize engine.
encoder_factory_->AddSupportedVideoCodecType(kFakeExternalCodecName1);
// The first external codec will appear last in the vector.
std::vector<cricket::VideoCodec> codecs_before(engine_.codecs());
EXPECT_EQ("FakeExternalCodec1", codecs_before.back().name);
// Add second codec.
encoder_factory_->AddSupportedVideoCodecType("FakeExternalCodec2");
encoder_factory_->AddSupportedVideoCodecType(kFakeExternalCodecName2);
std::vector<cricket::VideoCodec> codecs_after(engine_.codecs());
EXPECT_EQ(codecs_before.size() + 1, codecs_after.size());
EXPECT_EQ("FakeExternalCodec2", codecs_after.back().name);
// Check that both fake codecs are present and that the second fake codec
// appears after the first fake codec.
const int fake_codec_index1 = GetEngineCodecIndex(kFakeExternalCodecName1);
const int fake_codec_index2 = GetEngineCodecIndex(kFakeExternalCodecName2);
EXPECT_LT(fake_codec_index1, fake_codec_index2);
}
TEST_F(WebRtcVideoEngineTest, RegisterExternalDecodersIfSupported) {
@ -910,10 +924,28 @@ TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, Vp8) {
// Verify the codecs from the engine.
const std::vector<VideoCodec> engine_codecs = engine.codecs();
// Verify an RTX codec has been added correctly.
EXPECT_EQ(2u, engine_codecs.size());
// Verify default codecs has been added correctly.
EXPECT_EQ(5u, engine_codecs.size());
EXPECT_EQ("VP8", engine_codecs.at(0).name);
// RTX codec for VP8.
EXPECT_EQ("rtx", engine_codecs.at(1).name);
int vp8_associated_payload;
EXPECT_TRUE(engine_codecs.at(1).GetParam(kCodecParamAssociatedPayloadType,
&vp8_associated_payload));
EXPECT_EQ(vp8_associated_payload, engine_codecs.at(0).id);
EXPECT_EQ(kRedCodecName, engine_codecs.at(2).name);
// RTX codec for RED.
EXPECT_EQ("rtx", engine_codecs.at(3).name);
int red_associated_payload;
EXPECT_TRUE(engine_codecs.at(3).GetParam(kCodecParamAssociatedPayloadType,
&red_associated_payload));
EXPECT_EQ(red_associated_payload, engine_codecs.at(2).id);
EXPECT_EQ(kUlpfecCodecName, engine_codecs.at(4).name);
int associated_payload_type;
EXPECT_TRUE(engine_codecs.at(1).GetParam(
cricket::kCodecParamAssociatedPayloadType, &associated_payload_type));