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

Potential fix for rtx/red issue where red is removed only from the remote description.

Browse Source 
BUG=5675
R=pbos@webrtc.org

Review URL: https://codereview.webrtc.org/1964473002 .

Cr-Commit-Position: refs/heads/master@{#12776}
This commit is contained in:
Stefan Holmer
2016-05-17 16:33:30 +02:00
parent 9b2228fdfc
commit 2b1f651d15
3 changed files with 80 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@ -263,39 +263,6 @@ inline bool ContainsHeaderExtension(
return false;
}
// Merges two fec configs and logs an error if a conflict arises
// such that merging in different order would trigger a different output.
static void MergeFecConfig(const webrtc::FecConfig& other,
webrtc::FecConfig* output) {
if (other.ulpfec_payload_type != -1) {
if (output->ulpfec_payload_type != -1 &&
output->ulpfec_payload_type != other.ulpfec_payload_type) {
LOG(LS_WARNING) << "Conflict merging ulpfec_payload_type configs: "
<< output->ulpfec_payload_type << " and "
<< other.ulpfec_payload_type;
}
output->ulpfec_payload_type = other.ulpfec_payload_type;
}
if (other.red_payload_type != -1) {
if (output->red_payload_type != -1 &&
output->red_payload_type != other.red_payload_type) {
LOG(LS_WARNING) << "Conflict merging red_payload_type configs: "
<< output->red_payload_type << " and "
<< other.red_payload_type;
}
output->red_payload_type = other.red_payload_type;
}
if (other.red_rtx_payload_type != -1) {
if (output->red_rtx_payload_type != -1 &&
output->red_rtx_payload_type != other.red_rtx_payload_type) {
LOG(LS_WARNING) << "Conflict merging red_rtx_payload_type configs: "
<< output->red_rtx_payload_type << " and "
<< other.red_rtx_payload_type;
}
output->red_rtx_payload_type = other.red_rtx_payload_type;
}
}
// Returns true if the given codec is disallowed from doing simulcast.
bool IsCodecBlacklistedForSimulcast(const std::string& codec_name) {
return CodecNamesEq(codec_name, kH264CodecName) ||
@ -682,7 +649,8 @@ WebRtcVideoChannel2::WebRtcVideoChannel2(
video_config_(config.video),
external_encoder_factory_(external_encoder_factory),
external_decoder_factory_(external_decoder_factory),
default_send_options_(options) {
default_send_options_(options),
red_disabled_by_remote_side_(false) {
RTC_DCHECK(thread_checker_.CalledOnValidThread());
rtcp_receiver_report_ssrc_ = kDefaultRtcpReceiverReportSsrc;
@ -872,6 +840,19 @@ bool WebRtcVideoChannel2::SetSendParameters(const VideoSendParameters& params) {
: webrtc::RtcpMode::kCompound);
}
}
if (changed_params.codec) {
bool red_was_disabled = red_disabled_by_remote_side_;
red_disabled_by_remote_side_ =
changed_params.codec->fec.red_payload_type == -1;
if (red_was_disabled != red_disabled_by_remote_side_) {
for (auto& kv : receive_streams_) {
// In practice VideoChannel::SetRemoteContent appears to most of the
// time also call UpdateRemoteStreams, which recreates the receive
// streams. If that's always true this call isn't needed.
kv.second->SetFecDisabledRemotely(red_disabled_by_remote_side_);
}
}
}
}
send_params_ = params;
return true;
@ -1237,7 +1218,7 @@ bool WebRtcVideoChannel2::AddRecvStream(const StreamParams& sp,
receive_streams_[ssrc] = new WebRtcVideoReceiveStream(
call_, sp, config, external_decoder_factory_, default_stream,
recv_codecs_);
recv_codecs_, red_disabled_by_remote_side_);
return true;
}
@ -1272,10 +1253,6 @@ void WebRtcVideoChannel2::ConfigureReceiverRtp(
}
}
for (size_t i = 0; i < recv_codecs_.size(); ++i) {
MergeFecConfig(recv_codecs_[i].fec, &config->rtp.fec);
}
for (size_t i = 0; i < recv_codecs_.size(); ++i) {
uint32_t rtx_ssrc;
if (recv_codecs_[i].rtx_payload_type != -1 &&
@ -2239,13 +2216,15 @@ WebRtcVideoChannel2::WebRtcVideoReceiveStream::WebRtcVideoReceiveStream(
const webrtc::VideoReceiveStream::Config& config,
WebRtcVideoDecoderFactory* external_decoder_factory,
bool default_stream,
const std::vector<VideoCodecSettings>& recv_codecs)
const std::vector<VideoCodecSettings>& recv_codecs,
bool red_disabled_by_remote_side)
: call_(call),
ssrcs_(sp.ssrcs),
ssrc_groups_(sp.ssrc_groups),
stream_(NULL),
default_stream_(default_stream),
config_(config),
red_disabled_by_remote_side_(red_disabled_by_remote_side),
external_decoder_factory_(external_decoder_factory),
sink_(NULL),
last_width_(-1),
@ -2421,7 +2400,13 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::RecreateWebRtcStream() {
if (stream_ != NULL) {
call_->DestroyVideoReceiveStream(stream_);
}
stream_ = call_->CreateVideoReceiveStream(config_);
webrtc::VideoReceiveStream::Config config = config_;
if (red_disabled_by_remote_side_) {
config.rtp.fec.red_payload_type = -1;
config.rtp.fec.ulpfec_payload_type = -1;
config.rtp.fec.red_rtx_payload_type = -1;
}
stream_ = call_->CreateVideoReceiveStream(config);
stream_->Start();
}
@ -2529,6 +2514,12 @@ WebRtcVideoChannel2::WebRtcVideoReceiveStream::GetVideoReceiverInfo() {
return info;
}
void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetFecDisabledRemotely(
bool disable) {
red_disabled_by_remote_side_ = disable;
RecreateWebRtcStream();
}
WebRtcVideoChannel2::VideoCodecSettings::VideoCodecSettings()
: rtx_payload_type(-1) {}

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

@ -422,7 +422,8 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport {
const webrtc::VideoReceiveStream::Config& config,
WebRtcVideoDecoderFactory* external_decoder_factory,
bool default_stream,
const std::vector<VideoCodecSettings>& recv_codecs);
const std::vector<VideoCodecSettings>& recv_codecs,
bool red_disabled_by_remote_side);
~WebRtcVideoReceiveStream();
const std::vector<uint32_t>& GetSsrcs() const;
@ -442,6 +443,14 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport {
VideoReceiverInfo GetVideoReceiverInfo();
// Used to disable RED/FEC when the remote description doesn't contain those
// codecs. This is needed to be able to work around an RTX bug which is only
// happening if the remote side doesn't send RED, but the local side is
// configured to receive RED.
// TODO(holmer): Remove this after a couple of Chrome versions, M53-54
// time frame.
void SetFecDisabledRemotely(bool disable);
private:
struct AllocatedDecoder {
AllocatedDecoder(webrtc::VideoDecoder* decoder,
@ -472,6 +481,7 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport {
webrtc::VideoReceiveStream* stream_;
const bool default_stream_;
webrtc::VideoReceiveStream::Config config_;
bool red_disabled_by_remote_side_;
WebRtcVideoDecoderFactory* const external_decoder_factory_;
std::vector<AllocatedDecoder> allocated_decoders_;
@ -542,6 +552,7 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport {
VideoSendParameters send_params_;
VideoOptions default_send_options_;
VideoRecvParameters recv_params_;
bool red_disabled_by_remote_side_;
};
} // namespace cricket

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

@ -2678,6 +2678,7 @@ TEST_F(WebRtcVideoChannel2Test,
TEST_F(WebRtcVideoChannel2Test, SetRecvCodecsWithoutFecDisablesFec) {
cricket::VideoSendParameters send_parameters;
send_parameters.codecs.push_back(kVp8Codec);
send_parameters.codecs.push_back(kRedCodec);
send_parameters.codecs.push_back(kUlpfecCodec);
ASSERT_TRUE(channel_->SetSendParameters(send_parameters));
@ -2696,6 +2697,41 @@ TEST_F(WebRtcVideoChannel2Test, SetRecvCodecsWithoutFecDisablesFec) {
<< "SetSendCodec without FEC should disable current FEC.";
}
TEST_F(WebRtcVideoChannel2Test, SetSendParamsWithoutFecDisablesReceivingFec) {
FakeVideoReceiveStream* stream = AddRecvStream();
webrtc::VideoReceiveStream::Config config = stream->GetConfig();
EXPECT_EQ(kUlpfecCodec.id, config.rtp.fec.ulpfec_payload_type);
cricket::VideoRecvParameters recv_parameters;
recv_parameters.codecs.push_back(kVp8Codec);
recv_parameters.codecs.push_back(kRedCodec);
recv_parameters.codecs.push_back(kUlpfecCodec);
ASSERT_TRUE(channel_->SetRecvParameters(recv_parameters));
stream = fake_call_->GetVideoReceiveStreams()[0];
ASSERT_TRUE(stream != NULL);
config = stream->GetConfig();
EXPECT_EQ(kUlpfecCodec.id, config.rtp.fec.ulpfec_payload_type)
<< "FEC should be enabled on the recieve stream.";
cricket::VideoSendParameters send_parameters;
send_parameters.codecs.push_back(kVp8Codec);
ASSERT_TRUE(channel_->SetSendParameters(send_parameters));
stream = fake_call_->GetVideoReceiveStreams()[0];
config = stream->GetConfig();
EXPECT_EQ(-1, config.rtp.fec.ulpfec_payload_type)
<< "FEC should have been disabled when we know the other side won't do "
"FEC.";
send_parameters.codecs.push_back(kRedCodec);
send_parameters.codecs.push_back(kUlpfecCodec);
ASSERT_TRUE(channel_->SetSendParameters(send_parameters));
stream = fake_call_->GetVideoReceiveStreams()[0];
config = stream->GetConfig();
EXPECT_EQ(kUlpfecCodec.id, config.rtp.fec.ulpfec_payload_type)
<< "FEC should be enabled on the recieve stream.";
}
TEST_F(WebRtcVideoChannel2Test, SetSendCodecsRejectDuplicateFecPayloads) {
cricket::VideoRecvParameters parameters;
parameters.codecs.push_back(kVp8Codec);