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

Recreate FlexfecReceiveStream separately from VideoReceiveStream.

Browse Source 
After this CL, reconfiguring the FlexFEC payload type at the
WebRtcVideoChannel2 level will no longer lead to the recreation of
the VideoReceiveStream. This means that the jitter buffer will
not be destroyed and a smoother video playback is achieved during
SDP renegotiation.

BUG=webrtc:5654

Review-Url: https://codereview.webrtc.org/2911913002
Cr-Commit-Position: refs/heads/master@{#18318}
This commit is contained in:
brandtr 2017-05-30 01:31:37 -07:00 committed by Commit Bot
parent eaf4a1e103
commit 11fb472ae4
3 changed files with 142 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

100
webrtc/media/engine/webrtcvideoengine2.cc
View File

@ -654,6 +654,7 @@ WebRtcVideoChannel2::WebRtcVideoChannel2(
rtcp_receiver_report_ssrc_ = kDefaultRtcpReceiverReportSsrc;
sending_ = false;
recv_codecs_ = MapCodecs(GetSupportedCodecs(external_encoder_factory));
recv_flexfec_payload_type_ = recv_codecs_.front().flexfec_payload_type;
}
WebRtcVideoChannel2::~WebRtcVideoChannel2() {
@ -682,12 +683,13 @@ WebRtcVideoChannel2::SelectSendVideoCodec(
return rtc::Optional<VideoCodecSettings>();
}
bool WebRtcVideoChannel2::ReceiveCodecsHaveChanged(
bool WebRtcVideoChannel2::NonFlexfecReceiveCodecsHaveChanged(
std::vector<VideoCodecSettings> before,
std::vector<VideoCodecSettings> after) {
if (before.size() != after.size()) {
return true;
}
// The receive codec order doesn't matter, so we sort the codecs before
// comparing. This is necessary because currently the
// only way to change the send codec is to munge SDP, which causes
@ -702,7 +704,12 @@ bool WebRtcVideoChannel2::ReceiveCodecsHaveChanged(
};
std::sort(before.begin(), before.end(), comparison);
std::sort(after.begin(), after.end(), comparison);
return before != after;
// Changes in FlexFEC payload type are handled separately in
// WebRtcVideoChannel2::GetChangedRecvParameters, so disregard FlexFEC in the
// comparison here.
return !std::equal(before.begin(), before.end(), after.begin(),
VideoCodecSettings::EqualsDisregardingFlexfec);
}
bool WebRtcVideoChannel2::GetChangedSendParameters(
@ -982,7 +989,7 @@ bool WebRtcVideoChannel2::GetChangedRecvParameters(
}
}
if (ReceiveCodecsHaveChanged(recv_codecs_, mapped_codecs)) {
if (NonFlexfecReceiveCodecsHaveChanged(recv_codecs_, mapped_codecs)) {
changed_params->codec_settings =
rtc::Optional<std::vector<VideoCodecSettings>>(mapped_codecs);
}
@ -995,6 +1002,12 @@ bool WebRtcVideoChannel2::GetChangedRecvParameters(
rtc::Optional<std::vector<webrtc::RtpExtension>>(filtered_extensions);
}
int flexfec_payload_type = mapped_codecs.front().flexfec_payload_type;
if (flexfec_payload_type != recv_flexfec_payload_type_) {
changed_params->flexfec_payload_type =
rtc::Optional<int>(flexfec_payload_type);
}
return true;
}
@ -1005,6 +1018,12 @@ bool WebRtcVideoChannel2::SetRecvParameters(const VideoRecvParameters& params) {
if (!GetChangedRecvParameters(params, &changed_params)) {
return false;
}
if (changed_params.flexfec_payload_type) {
LOG(LS_INFO) << "Changing FlexFEC payload type (recv) from "
<< recv_flexfec_payload_type_ << " to "
<< *changed_params.flexfec_payload_type;
recv_flexfec_payload_type_ = *changed_params.flexfec_payload_type;
}
if (changed_params.rtp_header_extensions) {
recv_rtp_extensions_ = *changed_params.rtp_header_extensions;
}
@ -1288,6 +1307,7 @@ void WebRtcVideoChannel2::ConfigureReceiverRtp(
config->rtp.extensions = recv_rtp_extensions_;
// TODO(brandtr): Generalize when we add support for multistream protection.
flexfec_config->payload_type = recv_flexfec_payload_type_;
if (IsFlexfecAdvertisedFieldTrialEnabled() &&
sp.GetFecFrSsrc(ssrc, &flexfec_config->remote_ssrc)) {
flexfec_config->protected_media_ssrcs = {ssrc};
@ -1464,11 +1484,13 @@ void WebRtcVideoChannel2::OnPacketReceived(
for (auto& codec : recv_codecs_) {
if (payload_type == codec.rtx_payload_type ||
payload_type == codec.ulpfec.red_rtx_payload_type ||
payload_type == codec.ulpfec.ulpfec_payload_type ||
payload_type == codec.flexfec_payload_type) {
payload_type == codec.ulpfec.ulpfec_payload_type) {
return;
}
}
if (payload_type == recv_flexfec_payload_type_) {
return;
}
switch (unsignalled_ssrc_handler_->OnUnsignalledSsrc(this, ssrc)) {
case UnsignalledSsrcHandler::kDropPacket:
@ -2199,7 +2221,9 @@ WebRtcVideoChannel2::WebRtcVideoReceiveStream::WebRtcVideoReceiveStream(
config_.renderer = this;
std::vector<AllocatedDecoder> old_decoders;
ConfigureCodecs(recv_codecs, &old_decoders);
RecreateWebRtcStream();
ConfigureFlexfecCodec(flexfec_config.payload_type);
MaybeRecreateWebRtcFlexfecStream();
RecreateWebRtcVideoStream();
RTC_DCHECK(old_decoders.empty());
}
@ -2301,12 +2325,16 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::ConfigureCodecs(
}
config_.rtp.ulpfec = recv_codecs.front().ulpfec;
flexfec_config_.payload_type = recv_codecs.front().flexfec_payload_type;
config_.rtp.nack.rtp_history_ms =
HasNack(recv_codecs.begin()->codec) ? kNackHistoryMs : 0;
}
void WebRtcVideoChannel2::WebRtcVideoReceiveStream::ConfigureFlexfecCodec(
int flexfec_payload_type) {
flexfec_config_.payload_type = flexfec_payload_type;
}
void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetLocalSsrc(
uint32_t local_ssrc) {
// TODO(pbos): Consider turning this sanity check into a RTC_DCHECK. You
@ -2324,7 +2352,8 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetLocalSsrc(
LOG(LS_INFO)
<< "RecreateWebRtcStream (recv) because of SetLocalSsrc; local_ssrc="
<< local_ssrc;
RecreateWebRtcStream();
MaybeRecreateWebRtcFlexfecStream();
RecreateWebRtcVideoStream();
}
void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetFeedbackParameters(
@ -2356,47 +2385,61 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetFeedbackParameters(
<< "RecreateWebRtcStream (recv) because of SetFeedbackParameters; nack="
<< nack_enabled << ", remb=" << remb_enabled
<< ", transport_cc=" << transport_cc_enabled;
RecreateWebRtcStream();
MaybeRecreateWebRtcFlexfecStream();
RecreateWebRtcVideoStream();
}
void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetRecvParameters(
const ChangedRecvParameters& params) {
bool needs_recreation = false;
bool video_needs_recreation = false;
bool flexfec_needs_recreation = false;
std::vector<AllocatedDecoder> old_decoders;
if (params.codec_settings) {
ConfigureCodecs(*params.codec_settings, &old_decoders);
needs_recreation = true;
video_needs_recreation = true;
}
if (params.rtp_header_extensions) {
config_.rtp.extensions = *params.rtp_header_extensions;
flexfec_config_.rtp_header_extensions = *params.rtp_header_extensions;
needs_recreation = true;
video_needs_recreation = true;
flexfec_needs_recreation = true;
}
if (needs_recreation) {
LOG(LS_INFO) << "RecreateWebRtcStream (recv) because of SetRecvParameters";
RecreateWebRtcStream();
if (params.flexfec_payload_type) {
ConfigureFlexfecCodec(*params.flexfec_payload_type);
flexfec_needs_recreation = true;
}
if (flexfec_needs_recreation) {
LOG(LS_INFO) << "MaybeRecreateWebRtcFlexfecStream (recv) because of "
"SetRecvParameters";
MaybeRecreateWebRtcFlexfecStream();
}
if (video_needs_recreation) {
LOG(LS_INFO)
<< "RecreateWebRtcVideoStream (recv) because of SetRecvParameters";
RecreateWebRtcVideoStream();
ClearDecoders(&old_decoders);
}
}
void WebRtcVideoChannel2::WebRtcVideoReceiveStream::RecreateWebRtcStream() {
void WebRtcVideoChannel2::WebRtcVideoReceiveStream::
RecreateWebRtcVideoStream() {
if (stream_) {
call_->DestroyVideoReceiveStream(stream_);
stream_ = nullptr;
}
webrtc::VideoReceiveStream::Config config = config_.Copy();
config.rtp.protected_by_flexfec = (flexfec_stream_ != nullptr);
stream_ = call_->CreateVideoReceiveStream(std::move(config));
stream_->Start();
}
void WebRtcVideoChannel2::WebRtcVideoReceiveStream::
MaybeRecreateWebRtcFlexfecStream() {
if (flexfec_stream_) {
call_->DestroyFlexfecReceiveStream(flexfec_stream_);
flexfec_stream_ = nullptr;
}
const bool use_flexfec = flexfec_config_.IsCompleteAndEnabled();
// TODO(nisse): There are way too many copies here. And why isn't
// the argument to CreateVideoReceiveStream a const ref?
webrtc::VideoReceiveStream::Config config = config_.Copy();
config.rtp.protected_by_flexfec = use_flexfec;
stream_ = call_->CreateVideoReceiveStream(config.Copy());
stream_->Start();
if (use_flexfec) {
if (flexfec_config_.IsCompleteAndEnabled()) {
flexfec_stream_ = call_->CreateFlexfecReceiveStream(flexfec_config_);
flexfec_stream_->Start();
}
@ -2523,6 +2566,13 @@ bool WebRtcVideoChannel2::VideoCodecSettings::operator==(
rtx_payload_type == other.rtx_payload_type;
}
bool WebRtcVideoChannel2::VideoCodecSettings::EqualsDisregardingFlexfec(
const WebRtcVideoChannel2::VideoCodecSettings& a,
const WebRtcVideoChannel2::VideoCodecSettings& b) {
return a.codec == b.codec && a.ulpfec == b.ulpfec &&
a.rtx_payload_type == b.rtx_payload_type;
}
bool WebRtcVideoChannel2::VideoCodecSettings::operator!=(
const WebRtcVideoChannel2::VideoCodecSettings& other) const {
return !(*this == other);

23
webrtc/media/engine/webrtcvideoengine2.h
View File

@ -191,9 +191,16 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport {
struct VideoCodecSettings {
VideoCodecSettings();
// Checks if all members of |*this| are equal to the corresponding members
// of |other|.
bool operator==(const VideoCodecSettings& other) const;
bool operator!=(const VideoCodecSettings& other) const;
// Checks if all members of |a|, except |flexfec_payload_type|, are equal
// to the corresponding members of |b|.
static bool EqualsDisregardingFlexfec(const VideoCodecSettings& a,
const VideoCodecSettings& b);
VideoCodec codec;
webrtc::UlpfecConfig ulpfec;
int flexfec_payload_type;
@ -213,6 +220,10 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport {
// These optionals are unset if not changed.
rtc::Optional<std::vector<VideoCodecSettings>> codec_settings;
rtc::Optional<std::vector<webrtc::RtpExtension>> rtp_header_extensions;
// Keep track of the FlexFEC payload type separately from |codec_settings|.
// This allows us to recreate the FlexfecReceiveStream separately from the
// VideoReceiveStream when the FlexFEC payload type is changed.
rtc::Optional<int> flexfec_payload_type;
};
bool GetChangedSendParameters(const VideoSendParameters& params,
@ -407,10 +418,12 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport {
bool external;
};
void RecreateWebRtcStream();
void RecreateWebRtcVideoStream();
void MaybeRecreateWebRtcFlexfecStream();
void ConfigureCodecs(const std::vector<VideoCodecSettings>& recv_codecs,
std::vector<AllocatedDecoder>* old_codecs);
void ConfigureFlexfecCodec(int flexfec_payload_type);
AllocatedDecoder CreateOrReuseVideoDecoder(
std::vector<AllocatedDecoder>* old_decoder,
const VideoCodec& codec);
@ -460,8 +473,9 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport {
rtc::Optional<VideoCodecSettings> SelectSendVideoCodec(
const std::vector<VideoCodecSettings>& remote_mapped_codecs) const;
static bool ReceiveCodecsHaveChanged(std::vector<VideoCodecSettings> before,
std::vector<VideoCodecSettings> after);
static bool NonFlexfecReceiveCodecsHaveChanged(
std::vector<VideoCodecSettings> before,
std::vector<VideoCodecSettings> after);
void FillSenderStats(VideoMediaInfo* info, bool log_stats);
void FillReceiverStats(VideoMediaInfo* info, bool log_stats);
@ -496,6 +510,9 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport {
WebRtcVideoDecoderFactory* const external_decoder_factory_;
std::vector<VideoCodecSettings> recv_codecs_;
std::vector<webrtc::RtpExtension> recv_rtp_extensions_;
// See reason for keeping track of the FlexFEC payload type separately in
// comment in WebRtcVideoChannel2::ChangedRecvParameters.
int recv_flexfec_payload_type_;
webrtc::Call::Config::BitrateConfig bitrate_config_;
// TODO(deadbeef): Don't duplicate information between
// send_params/recv_params, rtp_extensions, options, etc.

47
webrtc/media/engine/webrtcvideoengine2_unittest.cc
View File

@ -2510,6 +2510,53 @@ TEST_F(WebRtcVideoChannel2FlexfecRecvTest, SetDefaultRecvCodecsWithSsrc) {
EXPECT_EQ(kSsrcs1[0], config.protected_media_ssrcs[0]);
}
TEST_F(WebRtcVideoChannel2FlexfecRecvTest,
EnablingFlexfecDoesNotRecreateVideoReceiveStream) {
cricket::VideoRecvParameters recv_parameters;
recv_parameters.codecs.push_back(GetEngineCodec("VP8"));
ASSERT_TRUE(channel_->SetRecvParameters(recv_parameters));
AddRecvStream(
CreatePrimaryWithFecFrStreamParams("cname", kSsrcs1[0], kFlexfecSsrc));
EXPECT_EQ(1, fake_call_->GetNumCreatedReceiveStreams());
EXPECT_EQ(1U, fake_call_->GetVideoReceiveStreams().size());
// Enable FlexFEC.
recv_parameters.codecs.push_back(GetEngineCodec("flexfec-03"));
ASSERT_TRUE(channel_->SetRecvParameters(recv_parameters));
EXPECT_EQ(2, fake_call_->GetNumCreatedReceiveStreams())
<< "Enabling FlexFEC should create FlexfecReceiveStream.";
EXPECT_EQ(1U, fake_call_->GetVideoReceiveStreams().size())
<< "Enabling FlexFEC should not create VideoReceiveStream.";
EXPECT_EQ(1U, fake_call_->GetFlexfecReceiveStreams().size())
<< "Enabling FlexFEC should create a single FlexfecReceiveStream.";
}
TEST_F(WebRtcVideoChannel2FlexfecRecvTest,
DisablingFlexfecDoesNotRecreateVideoReceiveStream) {
cricket::VideoRecvParameters recv_parameters;
recv_parameters.codecs.push_back(GetEngineCodec("VP8"));
recv_parameters.codecs.push_back(GetEngineCodec("flexfec-03"));
ASSERT_TRUE(channel_->SetRecvParameters(recv_parameters));
AddRecvStream(
CreatePrimaryWithFecFrStreamParams("cname", kSsrcs1[0], kFlexfecSsrc));
EXPECT_EQ(2, fake_call_->GetNumCreatedReceiveStreams());
EXPECT_EQ(1U, fake_call_->GetVideoReceiveStreams().size());
EXPECT_EQ(1U, fake_call_->GetFlexfecReceiveStreams().size());
// Disable FlexFEC.
recv_parameters.codecs.clear();
recv_parameters.codecs.push_back(GetEngineCodec("VP8"));
ASSERT_TRUE(channel_->SetRecvParameters(recv_parameters));
EXPECT_EQ(2, fake_call_->GetNumCreatedReceiveStreams())
<< "Disabling FlexFEC should not recreate VideoReceiveStream.";
EXPECT_EQ(1U, fake_call_->GetVideoReceiveStreams().size())
<< "Disabling FlexFEC should not destroy VideoReceiveStream.";
EXPECT_TRUE(fake_call_->GetFlexfecReceiveStreams().empty())
<< "Disabling FlexFEC should destroy FlexfecReceiveStream.";
}
// TODO(brandtr): When FlexFEC is no longer behind a field trial, merge all
// tests that use this test fixture into the corresponding "non-field trial"
// tests.