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

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

Browse Source 
This reverts commit 154ee1fd8547768a49b7d67ce586ef5d3c5d9ebc.

Reason for revert: Breaks AppRTCMobileTest on Android64 (M Nexus5X) at https://build.chromium.org/p/client.webrtc/console

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=brandtr@webrtc.org,magjed@webrtc.org

Change-Id: I20569ae5aa4e5d794c8f7605ff5d2dd708442ae1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:8527
Reviewed-on: https://webrtc-review.googlesource.com/23640
Reviewed-by: Oleh Prypin <oprypin@webrtc.org>
Commit-Queue: Oleh Prypin <oprypin@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20707}
This commit is contained in:
Oleh Prypin 2017-11-16 11:59:29 +00:00 committed by Commit Bot
parent 84c1a15d3c
commit da850ef88b
3 changed files with 110 additions and 129 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
media/engine/internalencoderfactory.cc
View File

@ -15,9 +15,24 @@
#include "modules/video_coding/codecs/h264/include/h264.h"
#include "modules/video_coding/codecs/vp8/include/vp8.h"
#include "modules/video_coding/codecs/vp9/include/vp9.h"
#include "system_wrappers/include/field_trial.h"
namespace cricket {
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
InternalEncoderFactory::InternalEncoderFactory() {
supported_codecs_.push_back(VideoCodec(kVp8CodecName));
if (webrtc::VP9Encoder::IsSupported())
@ -25,6 +40,19 @@ InternalEncoderFactory::InternalEncoderFactory() {
for (const webrtc::SdpVideoFormat& format : webrtc::SupportedH264Codecs())
supported_codecs_.push_back(VideoCodec(format));
supported_codecs_.push_back(VideoCodec(kRedCodecName));
supported_codecs_.push_back(VideoCodec(kUlpfecCodecName));
if (IsFlexfecAdvertisedFieldTrialEnabled()) {
VideoCodec flexfec_codec(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_codec.SetParam(kFlexfecFmtpRepairWindow, "10000000");
supported_codecs_.push_back(flexfec_codec);
}
}
InternalEncoderFactory::~InternalEncoderFactory() {}

133
media/engine/webrtcvideoengine.cc
View File

@ -155,18 +155,25 @@ 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 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.
// 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.
bool IsFlexfecAdvertisedFieldTrialEnabled() {
return webrtc::field_trial::IsEnabled("WebRTC-FlexFEC-03-Advertised");
}
@ -186,70 +193,6 @@ 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 << '{';
@ -554,7 +497,7 @@ WebRtcVideoChannel* WebRtcVideoEngine::CreateChannel(
}
std::vector<VideoCodec> WebRtcVideoEngine::codecs() const {
return AssignPayloadTypesAndDefaultCodecs(encoder_factory_.get());
return AssignPayloadTypesAndAddAssociatedRtxCodecs(encoder_factory_.get());
}
RtpCapabilities WebRtcVideoEngine::GetCapabilities() const {
@ -583,6 +526,48 @@ 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,
@ -602,7 +587,7 @@ WebRtcVideoChannel::WebRtcVideoChannel(
rtcp_receiver_report_ssrc_ = kDefaultRtcpReceiverReportSsrc;
sending_ = false;
recv_codecs_ =
MapCodecs(AssignPayloadTypesAndDefaultCodecs(encoder_factory_));
MapCodecs(AssignPayloadTypesAndAddAssociatedRtxCodecs(encoder_factory_));
recv_flexfec_payload_type_ = recv_codecs_.front().flexfec_payload_type;
}
@ -617,7 +602,7 @@ rtc::Optional<WebRtcVideoChannel::VideoCodecSettings>
WebRtcVideoChannel::SelectSendVideoCodec(
const std::vector<VideoCodecSettings>& remote_mapped_codecs) const {
const std::vector<VideoCodec> local_supported_codecs =
AssignPayloadTypesAndDefaultCodecs(encoder_factory_);
AssignPayloadTypesAndAddAssociatedRtxCodecs(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
@ -934,7 +919,7 @@ bool WebRtcVideoChannel::GetChangedRecvParameters(
// Verify that every mapped codec is supported locally.
const std::vector<VideoCodec> local_supported_codecs =
AssignPayloadTypesAndDefaultCodecs(encoder_factory_);
AssignPayloadTypesAndAddAssociatedRtxCodecs(encoder_factory_);
for (const VideoCodecSettings& mapped_codec : mapped_codecs) {
if (!FindMatchingCodec(local_supported_codecs, mapped_codec.codec)) {
RTC_LOG(LS_ERROR)

78
media/engine/webrtcvideoengine_unittest.cc
View File

@ -181,13 +181,9 @@ 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) const;
cricket::VideoCodec GetEngineCodec(const std::string& name);
VideoMediaChannel* SetUpForExternalEncoderFactory();
@ -569,10 +565,9 @@ TEST_F(WebRtcVideoEngineTest, PropagatesInputFrameTimestamp) {
EXPECT_TRUE(channel->RemoveSendStream(kSsrc));
}
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];
cricket::VideoCodec WebRtcVideoEngineTest::GetEngineCodec(
const std::string& name) {
for (const cricket::VideoCodec& engine_codec : engine_.codecs()) {
if (!CodecNamesEq(name, engine_codec.name))
continue;
// The tests only use H264 Constrained Baseline. Make sure we don't return
@ -585,16 +580,11 @@ int WebRtcVideoEngineTest::GetEngineCodecIndex(const std::string& name) const {
continue;
}
}
return i;
return engine_codec;
}
// This point should never be reached.
ADD_FAILURE() << "Unrecognized codec name: " << name;
return -1;
}
cricket::VideoCodec WebRtcVideoEngineTest::GetEngineCodec(
const std::string& name) const {
return engine_.codecs()[GetEngineCodecIndex(name)];
return cricket::VideoCodec();
}
VideoMediaChannel* WebRtcVideoEngineTest::SetUpForExternalEncoderFactory() {
@ -806,39 +796,35 @@ TEST_F(WebRtcVideoEngineTest,
std::find_if(codecs_after.begin(), codecs_after.end(), is_flexfec));
}
// Test that external codecs are added after internal SW codecs.
// Test that external codecs are added to the end of the supported codec list.
TEST_F(WebRtcVideoEngineTest, ReportSupportedExternalCodecs) {
const char* kFakeExternalCodecName = "FakeExternalCodec";
encoder_factory_->AddSupportedVideoCodecType(kFakeExternalCodecName);
encoder_factory_->AddSupportedVideoCodecType("FakeExternalCodec");
// 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);
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);
}
// 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) {
const char* kFakeExternalCodecName1 = "FakeExternalCodec1";
const char* kFakeExternalCodecName2 = "FakeExternalCodec2";
// Set up external encoder factory with first codec, and initialize engine.
encoder_factory_->AddSupportedVideoCodecType(kFakeExternalCodecName1);
encoder_factory_->AddSupportedVideoCodecType("FakeExternalCodec1");
// 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(kFakeExternalCodecName2);
encoder_factory_->AddSupportedVideoCodecType("FakeExternalCodec2");
std::vector<cricket::VideoCodec> codecs_after(engine_.codecs());
EXPECT_EQ(codecs_before.size() + 1, codecs_after.size());
// 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);
EXPECT_EQ("FakeExternalCodec2", codecs_after.back().name);
}
TEST_F(WebRtcVideoEngineTest, RegisterExternalDecodersIfSupported) {
@ -924,28 +910,10 @@ TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, Vp8) {
// Verify the codecs from the engine.
const std::vector<VideoCodec> engine_codecs = engine.codecs();
// Verify default codecs has been added correctly.
EXPECT_EQ(5u, engine_codecs.size());
// Verify an RTX codec has been added correctly.
EXPECT_EQ(2u, 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));