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

Revert of Use RtxReceiveStream. (patchset #5 id:80001 of https://codereview.webrtc.org/3008773002/ )

Browse Source 
Reason for revert:
A few perf tests broken, including

RampUpTest.UpDownUpAbsSendTimeSimulcastRedRtx
RampUpTest.UpDownUpTransportSequenceNumberRtx
RampUpTest.UpDownUpTransportSequenceNumberPacketLoss

Original issue's description:
> Use RtxReceiveStream.
>
> This also has the beneficial side-effect that when a media stream
> which is protected by FlexFEC receives an RTX retransmission, the
> retransmitted media packet is passed into the FlexFEC machinery,
> which should improve its ability to recover packets via FEC.
>
> BUG=webrtc:7135
>
> Review-Url: https://codereview.webrtc.org/3008773002
> Cr-Commit-Position: refs/heads/master@{#19649}
> Committed: 5c0f6c62ea

TBR=brandtr@webrtc.org,danilchap@webrtc.org,stefan@webrtc.org,magjed@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:7135

Review-Url: https://codereview.webrtc.org/3010983002
Cr-Commit-Position: refs/heads/master@{#19653}
This commit is contained in:
nisse 2017-09-04 04:22:15 -07:00 committed by Commit Bot
parent 9af8f948a7
commit 3c39c0137a
10 changed files with 57 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
webrtc/call/rampup_tests.cc
View File

@ -207,9 +207,8 @@ void RampUpTester::ModifyVideoConfigs(
recv_config.rtp.ulpfec.ulpfec_payload_type =
send_config->rtp.ulpfec.ulpfec_payload_type;
if (rtx_) {
recv_config.rtp.rtx_associated_payload_types
[send_config->rtp.ulpfec.red_rtx_payload_type] =
send_config->rtp.ulpfec.red_payload_type;
recv_config.rtp.ulpfec.red_rtx_payload_type =
send_config->rtp.ulpfec.red_rtx_payload_type;
}
}

5
webrtc/call/video_receive_stream.h
View File

@ -165,11 +165,6 @@ class VideoReceiveStream {
NackConfig nack;
// See UlpfecConfig for description.
// TODO(nisse): UlpfecConfig includes the field red_rtx_payload_type,
// which duplicates info in the rtx_associated_payload_types mapping. So
// delete the use of UlpfecConfig here, and replace by the values which
// make sense in this context, likely those are ulpfec_payload_type_ and
// red_payload_type_.
UlpfecConfig ulpfec;
// SSRC for retransmissions.

5
webrtc/media/engine/webrtcvideoengine.cc
View File

@ -2188,11 +2188,6 @@ void WebRtcVideoChannel::WebRtcVideoReceiveStream::ConfigureCodecs(
config_.rtp.nack.rtp_history_ms =
HasNack(recv_codecs.begin()->codec) ? kNackHistoryMs : 0;
if (config_.rtp.ulpfec.red_rtx_payload_type != -1) {
config_.rtp
.rtx_associated_payload_types[config_.rtp.ulpfec.red_rtx_payload_type] =
config_.rtp.ulpfec.red_payload_type;
}
}
void WebRtcVideoChannel::WebRtcVideoReceiveStream::ConfigureFlexfecCodec(

54
webrtc/media/engine/webrtcvideoengine_unittest.cc
View File

@ -83,32 +83,6 @@ bool HasRtxCodec(const std::vector<cricket::VideoCodec>& codecs,
return false;
}
// TODO(nisse): Duplicated in call.cc.
const int* FindKeyByValue(const std::map<int, int>& m, int v) {
for (const auto& kv : m) {
if (kv.second == v)
return &kv.first;
}
return nullptr;
}
bool HasRtxReceiveAssociation(
const webrtc::VideoReceiveStream::Config& config,
int payload_type) {
return FindKeyByValue(config.rtp.rtx_associated_payload_types,
payload_type) != nullptr;
}
// Check that there's an Rtx payload type for each decoder.
bool VerifyRtxReceiveAssociations(
const webrtc::VideoReceiveStream::Config& config) {
for (const auto& decoder : config.decoders) {
if (!HasRtxReceiveAssociation(config, decoder.payload_type))
return false;
}
return true;
}
rtc::scoped_refptr<webrtc::VideoFrameBuffer> CreateBlackFrameBuffer(
int width,
int height) {
@ -138,6 +112,15 @@ cricket::MediaConfig GetMediaConfig() {
return media_config;
}
// TODO(nisse): Duplicated in call.cc.
const int* FindKeyByValue(const std::map<int, int>& m, int v) {
for (const auto& kv : m) {
if (kv.second == v)
return &kv.first;
}
return nullptr;
}
} // namespace
namespace cricket {
@ -1333,12 +1316,9 @@ TEST_F(WebRtcVideoChannelTest, RecvStreamWithSimAndRtx) {
cricket::CreateSimWithRtxStreamParams("cname", ssrcs, rtx_ssrcs));
EXPECT_FALSE(
recv_stream->GetConfig().rtp.rtx_associated_payload_types.empty());
EXPECT_TRUE(VerifyRtxReceiveAssociations(recv_stream->GetConfig()))
EXPECT_EQ(recv_stream->GetConfig().decoders.size(),
recv_stream->GetConfig().rtp.rtx_associated_payload_types.size())
<< "RTX should be mapped for all decoders/payload types.";
EXPECT_TRUE(HasRtxReceiveAssociation(recv_stream->GetConfig(),
GetEngineCodec("red").id))
<< "RTX should be mapped for the RED payload type";
EXPECT_EQ(rtx_ssrcs[0], recv_stream->GetConfig().rtp.rtx_ssrc);
}
@ -1349,12 +1329,6 @@ TEST_F(WebRtcVideoChannelTest, RecvStreamWithRtx) {
params.AddFidSsrc(kSsrcs1[0], kRtxSsrcs1[0]);
FakeVideoReceiveStream* recv_stream = AddRecvStream(params);
EXPECT_EQ(kRtxSsrcs1[0], recv_stream->GetConfig().rtp.rtx_ssrc);
EXPECT_TRUE(VerifyRtxReceiveAssociations(recv_stream->GetConfig()))
<< "RTX should be mapped for all decoders/payload types.";
EXPECT_TRUE(HasRtxReceiveAssociation(recv_stream->GetConfig(),
GetEngineCodec("red").id))
<< "RTX should be mapped for the RED payload type";
}
TEST_F(WebRtcVideoChannelTest, RecvStreamNoRtx) {
@ -3822,11 +3796,9 @@ TEST_F(WebRtcVideoChannelTest, DefaultReceiveStreamReconfiguresToUseRtx) {
recv_stream = fake_call_->GetVideoReceiveStreams()[0];
EXPECT_FALSE(
recv_stream->GetConfig().rtp.rtx_associated_payload_types.empty());
EXPECT_TRUE(VerifyRtxReceiveAssociations(recv_stream->GetConfig()))
EXPECT_EQ(recv_stream->GetConfig().decoders.size(),
recv_stream->GetConfig().rtp.rtx_associated_payload_types.size())
<< "RTX should be mapped for all decoders/payload types.";
EXPECT_TRUE(HasRtxReceiveAssociation(recv_stream->GetConfig(),
GetEngineCodec("red").id))
<< "RTX should be mapped also for the RED payload type";
EXPECT_EQ(rtx_ssrcs[0], recv_stream->GetConfig().rtp.rtx_ssrc);
}

3
webrtc/video/BUILD.gn
View File

@ -60,9 +60,6 @@ rtc_static_library("video") {
"../call:call_interfaces",
"../call:rtp_interfaces",
"../call:video_stream_api",
# For RtxReceiveStream.
"../call:rtp_receiver",
"../common_video",
"../logging:rtc_event_log_api",
"../media:rtc_media_base",

4
webrtc/video/end_to_end_tests.cc
View File

@ -1184,9 +1184,7 @@ void EndToEndTest::DecodesRetransmittedFrame(bool enable_rtx, bool enable_red) {
send_config->rtp.rtx.payload_type = kSendRtxPayloadType;
(*receive_configs)[0].rtp.rtx_ssrc = kSendRtxSsrcs[0];
(*receive_configs)[0]
.rtp.rtx_associated_payload_types[(payload_type_ == kRedPayloadType)
? kRtxRedPayloadType
: kSendRtxPayloadType] =
.rtp.rtx_associated_payload_types[kSendRtxPayloadType] =
payload_type_;
}
// Configure encoding and decoding with VP8, since generic packetization

38
webrtc/video/rtp_video_stream_receiver.cc
View File

@ -105,6 +105,7 @@ RtpVideoStreamReceiver::RtpVideoStreamReceiver(
rtp_receive_statistics_(ReceiveStatistics::Create(clock_)),
ulpfec_receiver_(UlpfecReceiver::Create(config->rtp.remote_ssrc, this)),
receiving_(false),
restored_packet_in_use_(false),
last_packet_log_ms_(-1),
rtp_rtcp_(CreateRtpRtcpModule(rtp_receive_statistics_.get(),
transport,
@ -145,8 +146,12 @@ RtpVideoStreamReceiver::RtpVideoStreamReceiver(
rtp_receive_statistics_->SetMaxReorderingThreshold(max_reordering_threshold);
if (config_.rtp.rtx_ssrc) {
// Needed for rtp_payload_registry_.RtxEnabled().
rtp_payload_registry_.SetRtxSsrc(config_.rtp.rtx_ssrc);
for (const auto& kv : config_.rtp.rtx_associated_payload_types) {
RTC_DCHECK_NE(kv.first, 0);
rtp_payload_registry_.SetRtxPayloadType(kv.first, kv.second);
}
}
if (IsUlpfecEnabled()) {
@ -163,6 +168,11 @@ RtpVideoStreamReceiver::RtpVideoStreamReceiver(
strncpy(red_codec.plName, "red", sizeof(red_codec.plName));
red_codec.plType = config_.rtp.ulpfec.red_payload_type;
RTC_CHECK(AddReceiveCodec(red_codec));
if (config_.rtp.ulpfec.red_rtx_payload_type != -1) {
rtp_payload_registry_.SetRtxPayloadType(
config_.rtp.ulpfec.red_rtx_payload_type,
config_.rtp.ulpfec.red_payload_type);
}
}
if (config_.rtp.rtcp_xr.receiver_reference_time_report)
@ -485,7 +495,31 @@ void RtpVideoStreamReceiver::ParseAndHandleEncapsulatingHeader(
}
ulpfec_receiver_->ProcessReceivedFec();
} else if (rtp_payload_registry_.IsRtx(header)) {
LOG(LS_WARNING) << "Unexpected RTX packet on media ssrc";
if (header.headerLength + header.paddingLength == packet_length) {
// This is an empty packet and should be silently dropped before trying to
// parse the RTX header.
return;
}
// Remove the RTX header and parse the original RTP header.
if (packet_length < header.headerLength)
return;
if (packet_length > sizeof(restored_packet_))
return;
if (restored_packet_in_use_) {
LOG(LS_WARNING) << "Multiple RTX headers detected, dropping packet.";
return;
}
if (!rtp_payload_registry_.RestoreOriginalPacket(
restored_packet_, packet, &packet_length, config_.rtp.remote_ssrc,
header)) {
LOG(LS_WARNING) << "Incoming RTX packet: Invalid RTP header ssrc: "
<< header.ssrc << " payload type: "
<< static_cast<int>(header.payloadType);
return;
}
restored_packet_in_use_ = true;
OnRecoveredPacket(restored_packet_, packet_length);
restored_packet_in_use_ = false;
}
}

2
webrtc/video/rtp_video_stream_receiver.h
View File

@ -185,6 +185,8 @@ class RtpVideoStreamReceiver : public RtpData,
rtc::SequencedTaskChecker worker_task_checker_;
bool receiving_ GUARDED_BY(worker_task_checker_);
uint8_t restored_packet_[IP_PACKET_SIZE] GUARDED_BY(worker_task_checker_);
bool restored_packet_in_use_ GUARDED_BY(worker_task_checker_);
int64_t last_packet_log_ms_ GUARDED_BY(worker_task_checker_);
const std::unique_ptr<RtpRtcp> rtp_rtcp_;

11
webrtc/video/video_receive_stream.cc
View File

@ -17,7 +17,6 @@
#include <utility>
#include "webrtc/call/rtp_stream_receiver_controller_interface.h"
#include "webrtc/call/rtx_receive_stream.h"
#include "webrtc/common_types.h"
#include "webrtc/common_video/h264/profile_level_id.h"
#include "webrtc/common_video/libyuv/include/webrtc_libyuv.h"
@ -33,7 +32,6 @@
#include "webrtc/rtc_base/location.h"
#include "webrtc/rtc_base/logging.h"
#include "webrtc/rtc_base/optional.h"
#include "webrtc/rtc_base/ptr_util.h"
#include "webrtc/rtc_base/trace_event.h"
#include "webrtc/system_wrappers/include/clock.h"
#include "webrtc/system_wrappers/include/field_trial.h"
@ -122,7 +120,7 @@ VideoReceiveStream::VideoReceiveStream(
decoder_payload_types.insert(decoder.payload_type);
}
video_receiver_.SetRenderDelay(config_.render_delay_ms);
video_receiver_.SetRenderDelay(config.render_delay_ms);
jitter_estimator_.reset(new VCMJitterEstimator(clock_));
frame_buffer_.reset(new video_coding::FrameBuffer(
@ -133,12 +131,9 @@ VideoReceiveStream::VideoReceiveStream(
// Register with RtpStreamReceiverController.
media_receiver_ = receiver_controller->CreateReceiver(
config_.rtp.remote_ssrc, &rtp_video_stream_receiver_);
if (config_.rtp.rtx_ssrc) {
rtx_receive_stream_ = rtc::MakeUnique<RtxReceiveStream>(
&rtp_video_stream_receiver_, config.rtp.rtx_associated_payload_types,
config_.rtp.remote_ssrc);
if (config.rtp.rtx_ssrc) {
rtx_receiver_ = receiver_controller->CreateReceiver(
config_.rtp.rtx_ssrc, rtx_receive_stream_.get());
config_.rtp.rtx_ssrc, &rtp_video_stream_receiver_);
}
}

2
webrtc/video/video_receive_stream.h
View File

@ -38,7 +38,6 @@ class ProcessThread;
class RTPFragmentationHeader;
class RtpStreamReceiverInterface;
class RtpStreamReceiverControllerInterface;
class RtxReceiveStream;
class VCMTiming;
class VCMJitterEstimator;
@ -142,7 +141,6 @@ class VideoReceiveStream : public webrtc::VideoReceiveStream,
std::unique_ptr<video_coding::FrameBuffer> frame_buffer_;
std::unique_ptr<RtpStreamReceiverInterface> media_receiver_;
std::unique_ptr<RtxReceiveStream> rtx_receive_stream_;
std::unique_ptr<RtpStreamReceiverInterface> rtx_receiver_;
// Whenever we are in an undecodable state (stream has just started or due to