zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Updates RtpVideoSender to populate RtpRtcp::Config.field_trials

Browse Source 
This caused at least one trial in RTPSender not to be properly parsed.

This CL also updates RtpVideoSender and RtpPayloadParams to use
WebRtcKeyValueConfig instead of the static field_trial methods, in
order to facilitate injectable behavior in the future.

Bug: webrtc:11508
Change-Id: I995939bd3e7c2f81e5050383c3e4daf933498520
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/173705
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31108}
This commit is contained in:
Erik Språng
2020-04-18 14:36:59 +02:00
committed by Commit Bot
parent ed005be788
commit cbc0cbaaec
6 changed files with 59 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
call/BUILD.gn
View File

@ -155,6 +155,7 @@ rtc_library("rtp_sender") {
"../api/transport:field_trial_based_config",
"../api/transport:goog_cc",
"../api/transport:network_control",
"../api/transport:webrtc_key_value_config",
"../api/units:data_rate",
"../api/units:time_delta",
"../api/units:timestamp",
@ -179,7 +180,6 @@ rtc_library("rtp_sender") {
"../rtc_base:rtc_base_approved",
"../rtc_base:rtc_task_queue",
"../rtc_base/task_utils:repeating_task",
"../system_wrappers:field_trial",
"//third_party/abseil-cpp/absl/algorithm:container",
"//third_party/abseil-cpp/absl/container:inlined_vector",
"//third_party/abseil-cpp/absl/strings:strings",

11
call/rtp_payload_params.cc
View File

@ -16,6 +16,7 @@
#include "absl/algorithm/container.h"
#include "absl/container/inlined_vector.h"
#include "absl/strings/match.h"
#include "absl/types/variant.h"
#include "api/video/video_timing.h"
#include "modules/video_coding/codecs/h264/include/h264_globals.h"
@ -28,7 +29,6 @@
#include "rtc_base/logging.h"
#include "rtc_base/random.h"
#include "rtc_base/time_utils.h"
#include "system_wrappers/include/field_trial.h"
namespace webrtc {
@ -135,12 +135,15 @@ void SetVideoTiming(const EncodedImage& image, VideoSendTiming* timing) {
} // namespace
RtpPayloadParams::RtpPayloadParams(const uint32_t ssrc,
const RtpPayloadState* state)
const RtpPayloadState* state,
const WebRtcKeyValueConfig& trials)
: ssrc_(ssrc),
generic_picture_id_experiment_(
field_trial::IsEnabled("WebRTC-GenericPictureId")),
absl::StartsWith(trials.Lookup("WebRTC-GenericPictureId"),
"Enabled")),
generic_descriptor_experiment_(
!field_trial::IsDisabled("WebRTC-GenericDescriptor")) {
!absl::StartsWith(trials.Lookup("WebRTC-GenericDescriptor"),
"Disabled")) {
for (auto& spatial_layer : last_shared_frame_id_)
spatial_layer.fill(-1);

5
call/rtp_payload_params.h
View File

@ -14,6 +14,7 @@
#include <array>
#include "absl/types/optional.h"
#include "api/transport/webrtc_key_value_config.h"
#include "api/video_codecs/video_encoder.h"
#include "call/rtp_config.h"
#include "modules/rtp_rtcp/source/rtp_generic_frame_descriptor.h"
@ -29,7 +30,9 @@ class RtpRtcp;
// TODO(nisse): Make these properties not codec specific.
class RtpPayloadParams final {
public:
RtpPayloadParams(const uint32_t ssrc, const RtpPayloadState* state);
RtpPayloadParams(const uint32_t ssrc,
const RtpPayloadState* state,
const WebRtcKeyValueConfig& trials);
RtpPayloadParams(const RtpPayloadParams& other);
~RtpPayloadParams();

27
call/rtp_payload_params_unittest.cc
View File

@ -18,6 +18,7 @@
#include "absl/container/inlined_vector.h"
#include "absl/types/optional.h"
#include "absl/types/variant.h"
#include "api/transport/field_trial_based_config.h"
#include "api/video/video_content_type.h"
#include "api/video/video_rotation.h"
#include "modules/video_coding/codecs/h264/include/h264_globals.h"
@ -50,7 +51,7 @@ TEST(RtpPayloadParamsTest, InfoMappedToRtpVideoHeader_Vp8) {
state2.tl0_pic_idx = kTl0PicIdx;
std::map<uint32_t, RtpPayloadState> states = {{kSsrc2, state2}};
RtpPayloadParams params(kSsrc2, &state2);
RtpPayloadParams params(kSsrc2, &state2, FieldTrialBasedConfig());
EncodedImage encoded_image;
encoded_image.rotation_ = kVideoRotation_90;
encoded_image.content_type_ = VideoContentType::SCREENSHARE;
@ -90,7 +91,7 @@ TEST(RtpPayloadParamsTest, InfoMappedToRtpVideoHeader_Vp9) {
RtpPayloadState state;
state.picture_id = kPictureId;
state.tl0_pic_idx = kTl0PicIdx;
RtpPayloadParams params(kSsrc1, &state);
RtpPayloadParams params(kSsrc1, &state, FieldTrialBasedConfig());
EncodedImage encoded_image;
encoded_image.rotation_ = kVideoRotation_90;
@ -150,7 +151,7 @@ TEST(RtpPayloadParamsTest, InfoMappedToRtpVideoHeader_H264) {
RtpPayloadState state;
state.picture_id = kPictureId;
state.tl0_pic_idx = kInitialTl0PicIdx1;
RtpPayloadParams params(kSsrc1, &state);
RtpPayloadParams params(kSsrc1, &state, FieldTrialBasedConfig());
EncodedImage encoded_image;
CodecSpecificInfo codec_info;
@ -203,7 +204,7 @@ TEST(RtpPayloadParamsTest, PictureIdIsSetForVp8) {
CodecSpecificInfo codec_info;
codec_info.codecType = kVideoCodecVP8;
RtpPayloadParams params(kSsrc1, &state);
RtpPayloadParams params(kSsrc1, &state, FieldTrialBasedConfig());
RTPVideoHeader header =
params.GetRtpVideoHeader(encoded_image, &codec_info, kDontCare);
EXPECT_EQ(kVideoCodecVP8, header.codec);
@ -226,7 +227,7 @@ TEST(RtpPayloadParamsTest, PictureIdWraps) {
codec_info.codecType = kVideoCodecVP8;
codec_info.codecSpecific.VP8.temporalIdx = kNoTemporalIdx;
RtpPayloadParams params(kSsrc1, &state);
RtpPayloadParams params(kSsrc1, &state, FieldTrialBasedConfig());
RTPVideoHeader header =
params.GetRtpVideoHeader(encoded_image, &codec_info, kDontCare);
EXPECT_EQ(kVideoCodecVP8, header.codec);
@ -250,7 +251,7 @@ TEST(RtpPayloadParamsTest, Tl0PicIdxUpdatedForVp8) {
codec_info.codecType = kVideoCodecVP8;
codec_info.codecSpecific.VP8.temporalIdx = 1;
RtpPayloadParams params(kSsrc1, &state);
RtpPayloadParams params(kSsrc1, &state, FieldTrialBasedConfig());
RTPVideoHeader header =
params.GetRtpVideoHeader(encoded_image, &codec_info, kDontCare);
@ -286,7 +287,7 @@ TEST(RtpPayloadParamsTest, Tl0PicIdxUpdatedForVp9) {
codec_info.codecSpecific.VP9.temporal_idx = 1;
codec_info.codecSpecific.VP9.first_frame_in_picture = true;
RtpPayloadParams params(kSsrc1, &state);
RtpPayloadParams params(kSsrc1, &state, FieldTrialBasedConfig());
RTPVideoHeader header =
params.GetRtpVideoHeader(encoded_image, &codec_info, kDontCare);
@ -329,7 +330,7 @@ TEST(RtpPayloadParamsTest, PictureIdForOldGenericFormat) {
codec_info.codecType = kVideoCodecGeneric;
encoded_image._frameType = VideoFrameType::kVideoFrameKey;
RtpPayloadParams params(kSsrc1, &state);
RtpPayloadParams params(kSsrc1, &state, FieldTrialBasedConfig());
RTPVideoHeader header =
params.GetRtpVideoHeader(encoded_image, &codec_info, 10);
@ -357,7 +358,7 @@ TEST(RtpPayloadParamsTest, GenericDescriptorForGenericCodec) {
CodecSpecificInfo codec_info;
codec_info.codecType = kVideoCodecGeneric;
RtpPayloadParams params(kSsrc1, &state);
RtpPayloadParams params(kSsrc1, &state, FieldTrialBasedConfig());
RTPVideoHeader header =
params.GetRtpVideoHeader(encoded_image, &codec_info, 0);
@ -380,7 +381,7 @@ TEST(RtpPayloadParamsTest, SetsGenericFromGenericFrameInfo) {
EncodedImage encoded_image;
CodecSpecificInfo codec_info;
RtpPayloadParams params(kSsrc1, &state);
RtpPayloadParams params(kSsrc1, &state, FieldTrialBasedConfig());
encoded_image._frameType = VideoFrameType::kVideoFrameKey;
codec_info.generic_frame_info =
@ -424,7 +425,7 @@ class RtpPayloadParamsVp8ToGenericTest : public ::testing::Test {
RtpPayloadParamsVp8ToGenericTest()
: generic_descriptor_field_trial_("WebRTC-GenericDescriptor/Enabled/"),
state_(),
params_(123, &state_) {}
params_(123, &state_, trials_config_) {}
void ConvertAndCheck(int temporal_index,
int64_t shared_frame_id,
@ -461,6 +462,7 @@ class RtpPayloadParamsVp8ToGenericTest : public ::testing::Test {
protected:
test::ScopedFieldTrials generic_descriptor_field_trial_;
FieldTrialBasedConfig trials_config_;
RtpPayloadState state_;
RtpPayloadParams params_;
};
@ -520,7 +522,7 @@ class RtpPayloadParamsH264ToGenericTest : public ::testing::Test {
RtpPayloadParamsH264ToGenericTest()
: generic_descriptor_field_trial_("WebRTC-GenericDescriptor/Enabled/"),
state_(),
params_(123, &state_) {}
params_(123, &state_, trials_config_) {}
void ConvertAndCheck(int temporal_index,
int64_t shared_frame_id,
@ -557,6 +559,7 @@ class RtpPayloadParamsH264ToGenericTest : public ::testing::Test {
protected:
test::ScopedFieldTrials generic_descriptor_field_trial_;
FieldTrialBasedConfig trials_config_;
RtpPayloadState state_;
RtpPayloadParams params_;
};

51
call/rtp_video_sender.cc
View File

@ -30,7 +30,6 @@
#include "rtc_base/checks.h"
#include "rtc_base/location.h"
#include "rtc_base/logging.h"
#include "system_wrappers/include/field_trial.h"
namespace webrtc {
@ -55,20 +54,22 @@ static const size_t kPathMTU = 1500;
using webrtc_internal_rtp_video_sender::RtpStreamSender;
bool PayloadTypeSupportsSkippingFecPackets(const std::string& payload_name) {
bool PayloadTypeSupportsSkippingFecPackets(const std::string& payload_name,
const WebRtcKeyValueConfig& trials) {
const VideoCodecType codecType = PayloadStringToCodecType(payload_name);
if (codecType == kVideoCodecVP8 || codecType == kVideoCodecVP9) {
return true;
}
if (codecType == kVideoCodecGeneric &&
field_trial::IsEnabled("WebRTC-GenericPictureId")) {
absl::StartsWith(trials.Lookup("WebRTC-GenericPictureId"), "Enabled")) {
return true;
}
return false;
}
bool ShouldDisableRedAndUlpfec(bool flexfec_enabled,
const RtpConfig& rtp_config) {
const RtpConfig& rtp_config,
const WebRtcKeyValueConfig& trials) {
// Consistency of NACK and RED+ULPFEC parameters is checked in this function.
const bool nack_enabled = rtp_config.nack.rtp_history_ms > 0;
@ -80,7 +81,8 @@ bool ShouldDisableRedAndUlpfec(bool flexfec_enabled,
bool should_disable_red_and_ulpfec = false;
if (webrtc::field_trial::IsEnabled("WebRTC-DisableUlpFecExperiment")) {
if (absl::StartsWith(trials.Lookup("WebRTC-DisableUlpFecExperiment"),
"Enabled")) {
RTC_LOG(LS_INFO) << "Experiment to disable sending ULPFEC is enabled.";
should_disable_red_and_ulpfec = true;
}
@ -99,7 +101,7 @@ bool ShouldDisableRedAndUlpfec(bool flexfec_enabled,
// is a waste of bandwidth since FEC packets still have to be transmitted.
// Note that this is not the case with FlexFEC.
if (nack_enabled && IsUlpfecEnabled() &&
!PayloadTypeSupportsSkippingFecPackets(rtp_config.payload_name)) {
!PayloadTypeSupportsSkippingFecPackets(rtp_config.payload_name, trials)) {
RTC_LOG(LS_WARNING)
<< "Transmitting payload type without picture ID using "
"NACK+ULPFEC is a waste of bandwidth since ULPFEC packets "
@ -122,7 +124,8 @@ std::unique_ptr<VideoFecGenerator> MaybeCreateFecGenerator(
Clock* clock,
const RtpConfig& rtp,
const std::map<uint32_t, RtpState>& suspended_ssrcs,
int simulcast_index) {
int simulcast_index,
const WebRtcKeyValueConfig& trials) {
// If flexfec is configured that takes priority.
if (rtp.flexfec.payload_type >= 0) {
RTC_DCHECK_GE(rtp.flexfec.payload_type, 0);
@ -168,7 +171,8 @@ std::unique_ptr<VideoFecGenerator> MaybeCreateFecGenerator(
RTPSender::FecExtensionSizes(), rtp_state, clock);
} else if (rtp.ulpfec.red_payload_type >= 0 &&
rtp.ulpfec.ulpfec_payload_type >= 0 &&
!ShouldDisableRedAndUlpfec(/*flexfec_enabled=*/false, rtp)) {
!ShouldDisableRedAndUlpfec(/*flexfec_enabled=*/false, rtp,
trials)) {
// Flexfec not configured, but ulpfec is and is not disabled.
return std::make_unique<UlpfecGenerator>(
rtp.ulpfec.red_payload_type, rtp.ulpfec.ulpfec_payload_type, clock);
@ -192,7 +196,8 @@ std::vector<RtpStreamSender> CreateRtpStreamSenders(
OverheadObserver* overhead_observer,
FrameEncryptorInterface* frame_encryptor,
const CryptoOptions& crypto_options,
rtc::scoped_refptr<FrameTransformerInterface> frame_transformer) {
rtc::scoped_refptr<FrameTransformerInterface> frame_transformer,
const WebRtcKeyValueConfig& trials) {
RTC_DCHECK_GT(rtp_config.ssrcs.size(), 0);
RtpRtcp::Configuration configuration;
@ -227,6 +232,7 @@ std::vector<RtpStreamSender> CreateRtpStreamSenders(
crypto_options.sframe.require_frame_encryption;
configuration.extmap_allow_mixed = rtp_config.extmap_allow_mixed;
configuration.rtcp_report_interval_ms = rtcp_report_interval_ms;
configuration.field_trials = &trials;
std::vector<RtpStreamSender> rtp_streams;
@ -237,7 +243,7 @@ std::vector<RtpStreamSender> CreateRtpStreamSenders(
configuration.local_media_ssrc = rtp_config.ssrcs[i];
std::unique_ptr<VideoFecGenerator> fec_generator =
MaybeCreateFecGenerator(clock, rtp_config, suspended_ssrcs, i);
MaybeCreateFecGenerator(clock, rtp_config, suspended_ssrcs, i, trials);
configuration.fec_generator = fec_generator.get();
video_config.fec_generator = fec_generator.get();
@ -255,20 +261,19 @@ std::vector<RtpStreamSender> CreateRtpStreamSenders(
// Set NACK.
rtp_rtcp->SetStorePacketsStatus(true, kMinSendSidePacketHistorySize);
FieldTrialBasedConfig field_trial_config;
video_config.clock = configuration.clock;
video_config.rtp_sender = rtp_rtcp->RtpSender();
video_config.frame_encryptor = frame_encryptor;
video_config.require_frame_encryption =
crypto_options.sframe.require_frame_encryption;
video_config.enable_retransmit_all_layers = false;
video_config.field_trials = &field_trial_config;
video_config.field_trials = &trials;
const bool using_flexfec =
fec_generator &&
fec_generator->GetFecType() == VideoFecGenerator::FecType::kFlexFec;
const bool should_disable_red_and_ulpfec =
ShouldDisableRedAndUlpfec(using_flexfec, rtp_config);
ShouldDisableRedAndUlpfec(using_flexfec, rtp_config, trials);
if (!should_disable_red_and_ulpfec &&
rtp_config.ulpfec.red_payload_type != -1) {
video_config.red_payload_type = rtp_config.ulpfec.red_payload_type;
@ -318,12 +323,15 @@ RtpVideoSender::RtpVideoSender(
FrameEncryptorInterface* frame_encryptor,
const CryptoOptions& crypto_options,
rtc::scoped_refptr<FrameTransformerInterface> frame_transformer)
: send_side_bwe_with_overhead_(
webrtc::field_trial::IsEnabled("WebRTC-SendSideBwe-WithOverhead")),
account_for_packetization_overhead_(!webrtc::field_trial::IsDisabled(
"WebRTC-SubtractPacketizationOverhead")),
use_early_loss_detection_(
!webrtc::field_trial::IsDisabled("WebRTC-UseEarlyLossDetection")),
: send_side_bwe_with_overhead_(absl::StartsWith(
field_trials_.Lookup("WebRTC-SendSideBwe-WithOverhead"),
"Enabled")),
account_for_packetization_overhead_(!absl::StartsWith(
field_trials_.Lookup("WebRTC-SubtractPacketizationOverhead"),
"Disabled")),
use_early_loss_detection_(!absl::StartsWith(
field_trials_.Lookup("WebRTC-UseEarlyLossDetection"),
"Disabled")),
has_packet_feedback_(TransportSeqNumExtensionConfigured(rtp_config)),
active_(false),
module_process_thread_(nullptr),
@ -343,7 +351,8 @@ RtpVideoSender::RtpVideoSender(
this,
frame_encryptor,
crypto_options,
std::move(frame_transformer))),
std::move(frame_transformer),
field_trials_)),
rtp_config_(rtp_config),
codec_type_(GetVideoCodecType(rtp_config)),
transport_(transport),
@ -365,7 +374,7 @@ RtpVideoSender::RtpVideoSender(
state = &it->second;
shared_frame_id_ = std::max(shared_frame_id_, state->shared_frame_id);
}
params_.push_back(RtpPayloadParams(ssrc, state));
params_.push_back(RtpPayloadParams(ssrc, state, field_trials_));
}
// RTP/RTCP initialization.

2
call/rtp_video_sender.h
View File

@ -22,6 +22,7 @@
#include "api/fec_controller.h"
#include "api/fec_controller_override.h"
#include "api/rtc_event_log/rtc_event_log.h"
#include "api/transport/field_trial_based_config.h"
#include "api/video_codecs/video_encoder.h"
#include "call/rtp_config.h"
#include "call/rtp_payload_params.h"
@ -160,6 +161,7 @@ class RtpVideoSender : public RtpVideoSenderInterface,
bool NackEnabled() const;
uint32_t GetPacketizationOverheadRate() const;
const FieldTrialBasedConfig field_trials_;
const bool send_side_bwe_with_overhead_;
const bool account_for_packetization_overhead_;
const bool use_early_loss_detection_;