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

Make configuration logic harsher in FlexfecReceiveStream.

Browse Source 
Before this change, the configuration logic in FlexfecReceiveStream
tried to make unsupported configurations work, e.g., by dropping the
protection of some media streams when multiple media streams were
protected by a single FlexFEC stream. This CL makes the configuration logic
return more errors on such unsupported configurations.
This harmonizes the logic with the new configuration logic in
VideoSendStream, for the FlexfecSender.

BUG=webrtc:5654

Review-Url: https://codereview.webrtc.org/2499963002
Cr-Commit-Position: refs/heads/master@{#15083}
This commit is contained in:
brandtr 2016-11-15 05:26:45 -08:00 committed by Commit bot
parent e950cadba5
commit 43c31e7afe
3 changed files with 30 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
webrtc/call/flexfec_receive_stream.cc
View File

@ -25,26 +25,39 @@ std::string FlexfecReceiveStream::Stats::ToString(int64_t time_ms) const {
namespace {
// TODO(brandtr): Update this function when we support multistream protection.
std::unique_ptr<FlexfecReceiver> MaybeUpdateConfigAndCreateFlexfecReceiver(
FlexfecReceiveStream::Config* config,
std::unique_ptr<FlexfecReceiver> MaybeCreateFlexfecReceiver(
const FlexfecReceiveStream::Config& config,
RecoveredPacketReceiver* recovered_packet_callback) {
if (config->protected_media_ssrcs.empty()) {
LOG(LS_ERROR) << "No protected media SSRC supplied. "
<< "This FlexfecReceiveStream will therefore be useless.";
if (config.flexfec_payload_type < 0) {
LOG(LS_WARNING) << "Invalid FlexFEC payload type given. "
<< "This FlexfecReceiveStream will therefore be useless.";
return nullptr;
} else if (config->protected_media_ssrcs.size() > 1) {
}
RTC_DCHECK_GE(config.flexfec_payload_type, 0);
RTC_DCHECK_LE(config.flexfec_payload_type, 127);
if (config.flexfec_ssrc == 0) {
LOG(LS_WARNING) << "Invalid FlexFEC SSRC given. "
<< "This FlexfecReceiveStream will therefore be useless.";
return nullptr;
}
if (config.protected_media_ssrcs.empty()) {
LOG(LS_WARNING) << "No protected media SSRC supplied. "
<< "This FlexfecReceiveStream will therefore be useless.";
return nullptr;
}
if (config.protected_media_ssrcs.size() > 1) {
LOG(LS_WARNING)
<< "The supplied FlexfecConfig contained multiple protected "
"media streams, but our implementation currently only "
"supports protecting a single media stream. This "
"FlexfecReceiveStream will therefore only accept media "
"packets from the first supplied media stream, with SSRC "
<< config->protected_media_ssrcs[0] << ".";
config->protected_media_ssrcs.resize(1);
"supports protecting a single media stream. "
"To avoid confusion, disabling FlexFEC completely.";
return nullptr;
}
return std::unique_ptr<FlexfecReceiver>(new FlexfecReceiver(
config->flexfec_ssrc, config->protected_media_ssrcs[0],
recovered_packet_callback));
RTC_DCHECK_EQ(1U, config.protected_media_ssrcs.size());
return std::unique_ptr<FlexfecReceiver>(
new FlexfecReceiver(config.flexfec_ssrc, config.protected_media_ssrcs[0],
recovered_packet_callback));
}
} // namespace
@ -56,9 +69,8 @@ FlexfecReceiveStream::FlexfecReceiveStream(
RecoveredPacketReceiver* recovered_packet_callback)
: started_(false),
config_(configuration),
receiver_(MaybeUpdateConfigAndCreateFlexfecReceiver(
&config_,
recovered_packet_callback)) {
receiver_(
MaybeCreateFlexfecReceiver(config_, recovered_packet_callback)) {
LOG(LS_INFO) << "FlexfecReceiveStream: " << config_.ToString();
}

2
webrtc/call/flexfec_receive_stream.h
View File

@ -42,7 +42,7 @@ class FlexfecReceiveStream : public webrtc::FlexfecReceiveStream {
rtc::CriticalSection crit_;
bool started_ GUARDED_BY(crit_);
Config config_;
const Config config_;
const std::unique_ptr<FlexfecReceiver> receiver_;
};

14
webrtc/call/flexfec_receive_stream_unittest.cc
View File

@ -54,20 +54,6 @@ TEST(FlexfecReceiveStreamTest, DoesNotProcessPacketWhenNoMediaSsrcGiven) {
receive_stream.AddAndProcessReceivedPacket(packet, packet_length));
}
// TODO(brandtr): Remove when we support multistream protection.
TEST(FlexfecReceiveStreamTest, CannotProtectMultipleMediaStreams) {
FlexfecReceiveStream::Config config;
config.flexfec_payload_type = 118;
config.flexfec_ssrc = 424223;
config.protected_media_ssrcs = {123, 456};
MockRecoveredPacketReceiver callback;
internal::FlexfecReceiveStream receive_stream(config, &callback);
ASSERT_EQ(1U, receive_stream.config().protected_media_ssrcs.size());
EXPECT_EQ(config.protected_media_ssrcs[0],
receive_stream.config().protected_media_ssrcs[0]);
}
// Create a FlexFEC packet that protects a single media packet and ensure
// that the callback is called. Correctness of recovery is checked in the
// FlexfecReceiver unit tests.