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

Fix and improve FlexFEC configuration for RTP/RTCP.

Browse Source 
Fix: Order of assignments is now correct, after being incorrect
due to an incorrect merge between
https://codereview.webrtc.org/2617373002/ and
https://codereview.webrtc.org/2589713003.

Improvement: Set parameters in more places, allowing for
correct reconfiguration. Add TODOs to point of minor issues
with current configuration.

TESTED=By locally patching an application using this code.
BUG=webrtc:5654

Review-Url: https://codereview.webrtc.org/2660403004
Cr-Commit-Position: refs/heads/master@{#16431}
This commit is contained in:
brandtr 2017-02-03 04:43:41 -08:00 committed by Commit bot
parent 4cb1b7546b
commit 9d58d94585
4 changed files with 43 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
webrtc/media/engine/fakewebrtccall.cc
View File

@ -270,7 +270,8 @@ FakeVideoReceiveStream::FakeVideoReceiveStream(
webrtc::VideoReceiveStream::Config config)
: config_(std::move(config)), receiving_(false) {}
const webrtc::VideoReceiveStream::Config& FakeVideoReceiveStream::GetConfig() {
const webrtc::VideoReceiveStream::Config& FakeVideoReceiveStream::GetConfig()
const {
return config_;
}

2
webrtc/media/engine/fakewebrtccall.h
View File

@ -174,7 +174,7 @@ class FakeVideoReceiveStream final : public webrtc::VideoReceiveStream {
public:
explicit FakeVideoReceiveStream(webrtc::VideoReceiveStream::Config config);
const webrtc::VideoReceiveStream::Config& GetConfig();
const webrtc::VideoReceiveStream::Config& GetConfig() const;
bool IsReceiving() const;

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

@ -1224,18 +1224,20 @@ void WebRtcVideoChannel2::ConfigureReceiverRtp(
config->rtp.transport_cc =
send_codec_ ? HasTransportCc(send_codec_->codec) : false;
sp.GetFidSsrc(ssrc, &config->rtp.rtx_ssrc);
config->rtp.extensions = recv_rtp_extensions_;
// TODO(brandtr): Generalize when we add support for multistream protection.
if (sp.GetFecFrSsrc(ssrc, &flexfec_config->remote_ssrc)) {
flexfec_config->protected_media_ssrcs = {ssrc};
flexfec_config->local_ssrc = config->rtp.local_ssrc;
flexfec_config->rtcp_mode = config->rtp.rtcp_mode;
// TODO(brandtr): We should be spec-compliant and set |transport_cc| here
// based on the rtcp-fb for the FlexFEC codec, not the media codec.
flexfec_config->transport_cc = config->rtp.transport_cc;
flexfec_config->rtp_header_extensions = config->rtp.extensions;
}
sp.GetFidSsrc(ssrc, &config->rtp.rtx_ssrc);
config->rtp.extensions = recv_rtp_extensions_;
}
bool WebRtcVideoChannel2::RemoveRecvStream(uint32_t ssrc) {
@ -2250,7 +2252,10 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetFeedbackParameters(
config_.rtp.nack.rtp_history_ms = nack_history_ms;
config_.rtp.transport_cc = transport_cc_enabled;
config_.rtp.rtcp_mode = rtcp_mode;
flexfec_config_.rtcp_mode = rtcp_mode;
// TODO(brandtr): We should be spec-compliant and set |transport_cc| here
// based on the rtcp-fb for the FlexFEC codec, not the media codec.
flexfec_config_.transport_cc = config_.rtp.transport_cc;
flexfec_config_.rtcp_mode = config_.rtp.rtcp_mode;
LOG(LS_INFO)
<< "RecreateWebRtcStream (recv) because of SetFeedbackParameters; nack="
<< nack_enabled << ", remb=" << remb_enabled
@ -2268,6 +2273,7 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetRecvParameters(
}
if (params.rtp_header_extensions) {
config_.rtp.extensions = *params.rtp_header_extensions;
flexfec_config_.rtp_header_extensions = *params.rtp_header_extensions;
needs_recreation = true;
}
if (needs_recreation) {

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

@ -16,6 +16,7 @@
#include "webrtc/base/arraysize.h"
#include "webrtc/base/gunit.h"
#include "webrtc/base/stringutils.h"
#include "webrtc/call/flexfec_receive_stream.h"
#include "webrtc/common_video/h264/profile_level_id.h"
#include "webrtc/logging/rtc_event_log/rtc_event_log.h"
#include "webrtc/media/base/mediaconstants.h"
@ -2424,22 +2425,40 @@ TEST_F(WebRtcVideoChannel2FlexfecTest, SetSendCodecsWithoutFec) {
TEST_F(WebRtcVideoChannel2FlexfecTest, SetRecvCodecsWithFec) {
AddRecvStream(
CreatePrimaryWithFecFrStreamParams("cname", kSsrcs1[0], kFlexfecSsrc));
const std::vector<FakeFlexfecReceiveStream*>& streams =
fake_call_->GetFlexfecReceiveStreams();
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));
ASSERT_EQ(1U, streams.size());
const FakeFlexfecReceiveStream* stream_with_recv_params = streams.front();
const std::vector<FakeFlexfecReceiveStream*>& flexfec_streams =
fake_call_->GetFlexfecReceiveStreams();
ASSERT_EQ(1U, flexfec_streams.size());
const FakeFlexfecReceiveStream* flexfec_stream = flexfec_streams.front();
const webrtc::FlexfecReceiveStream::Config& flexfec_stream_config =
flexfec_stream->GetConfig();
EXPECT_EQ(GetEngineCodec("flexfec-03").id,
stream_with_recv_params->GetConfig().payload_type);
EXPECT_EQ(kFlexfecSsrc, stream_with_recv_params->GetConfig().remote_ssrc);
EXPECT_EQ(1U,
stream_with_recv_params->GetConfig().protected_media_ssrcs.size());
EXPECT_EQ(kSsrcs1[0],
stream_with_recv_params->GetConfig().protected_media_ssrcs[0]);
flexfec_stream_config.payload_type);
EXPECT_EQ(kFlexfecSsrc, flexfec_stream_config.remote_ssrc);
ASSERT_EQ(1U, flexfec_stream_config.protected_media_ssrcs.size());
EXPECT_EQ(kSsrcs1[0], flexfec_stream_config.protected_media_ssrcs[0]);
const std::vector<FakeVideoReceiveStream*>& video_streams =
fake_call_->GetVideoReceiveStreams();
const FakeVideoReceiveStream* video_stream = video_streams.front();
const webrtc::VideoReceiveStream::Config& video_stream_config =
video_stream->GetConfig();
EXPECT_EQ(video_stream_config.rtp.local_ssrc,
flexfec_stream_config.local_ssrc);
EXPECT_EQ(video_stream_config.rtp.rtcp_mode, flexfec_stream_config.rtcp_mode);
EXPECT_EQ(video_stream_config.rtcp_send_transport,
flexfec_stream_config.rtcp_send_transport);
// TODO(brandtr): Update this EXPECT when we set |transport_cc| in a
// spec-compliant way.
EXPECT_EQ(video_stream_config.rtp.transport_cc,
flexfec_stream_config.transport_cc);
EXPECT_EQ(video_stream_config.rtp.rtcp_mode, flexfec_stream_config.rtcp_mode);
EXPECT_EQ(video_stream_config.rtp.extensions,
flexfec_stream_config.rtp_header_extensions);
}
TEST_F(WebRtcVideoChannel2Test,