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

Fix issues with RestartingSendStreamPreservesRtpStatesWithRtx

Browse Source 
double check rtp_sender in sending mode when altering sequence_number
adjust test to skip validating timestamp on rtx streams
fix test by waiting for all 3 media streams instead of 3 out 6 media and rtx streams.

BUG=webrtc:4332

Review-Url: https://codereview.webrtc.org/2177523002
Cr-Commit-Position: refs/heads/master@{#13587}
This commit is contained in:
danilchap 2016-08-01 06:58:34 -07:00 committed by Commit bot
parent 95e756035e
commit 32cd2c4103
5 changed files with 34 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
webrtc/modules/rtp_rtcp/source/rtp_sender.cc
View File

@ -850,7 +850,8 @@ bool RTPSender::PrepareAndSendPacket(uint8_t* buffer,
uint8_t data_buffer_rtx[IP_PACKET_SIZE];
if (send_over_rtx) {
BuildRtxPacket(buffer, &length, data_buffer_rtx);
if (!BuildRtxPacket(buffer, &length, data_buffer_rtx))
return false;
buffer_to_send_ptr = data_buffer_rtx;
}
@ -1154,6 +1155,8 @@ int32_t RTPSender::BuildRtpHeader(uint8_t* data_buffer,
int64_t capture_time_ms) {
assert(payload_type >= 0);
rtc::CritScope lock(&send_critsect_);
if (!sending_media_)
return -1;
timestamp_ = start_timestamp_ + capture_timestamp;
last_timestamp_time_ms_ = clock_->TimeInMilliseconds();
@ -1793,9 +1796,12 @@ int32_t RTPSender::SetFecParameters(
return 0;
}
void RTPSender::BuildRtxPacket(uint8_t* buffer, size_t* length,
bool RTPSender::BuildRtxPacket(uint8_t* buffer,
size_t* length,
uint8_t* buffer_rtx) {
rtc::CritScope lock(&send_critsect_);
if (!sending_media_)
return false;
uint8_t* data_buffer_rtx = buffer_rtx;
// Add RTX header.
RtpUtility::RtpHeaderParser rtp_parser(
@ -1836,6 +1842,7 @@ void RTPSender::BuildRtxPacket(uint8_t* buffer, size_t* length,
memcpy(ptr, buffer + rtp_header.headerLength,
*length - rtp_header.headerLength);
*length += 2;
return true;
}
void RTPSender::RegisterRtpStatisticsCallback(

3
webrtc/modules/rtp_rtcp/source/rtp_sender.h
View File

@ -373,8 +373,7 @@ class RTPSender : public RTPSenderInterface {
size_t header_length,
size_t padding_length);
void BuildRtxPacket(uint8_t* buffer, size_t* length,
uint8_t* buffer_rtx);
bool BuildRtxPacket(uint8_t* buffer, size_t* length, uint8_t* buffer_rtx);
bool SendPacketToNetwork(const uint8_t* packet,
size_t size,

7
webrtc/modules/rtp_rtcp/source/rtp_sender_audio.cc
View File

@ -418,8 +418,11 @@ int32_t RTPSenderAudio::SendTelephoneEventPacket(bool ended,
}
do {
// Send DTMF data
rtp_sender_->BuildRtpHeader(dtmfbuffer, dtmf_payload_type, marker_bit,
dtmf_timestamp, clock_->TimeInMilliseconds());
int32_t header_length = rtp_sender_->BuildRtpHeader(
dtmfbuffer, dtmf_payload_type, marker_bit, dtmf_timestamp,
clock_->TimeInMilliseconds());
if (header_length <= 0)
return -1;
// reset CSRC and X bit
dtmfbuffer[0] &= 0xe0;

6
webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc
View File

@ -266,8 +266,10 @@ int32_t RTPSenderVideo::SendVideo(RtpVideoCodecTypes video_type,
}
// Write RTP header.
rtp_sender_->BuildRtpHeader(dataBuffer, payload_type, last,
capture_timestamp, capture_time_ms);
int32_t header_length = rtp_sender_->BuildRtpHeader(
dataBuffer, payload_type, last, capture_timestamp, capture_time_ms);
if (header_length <= 0)
return -1;
// According to
// http://www.etsi.org/deliver/etsi_ts/126100_126199/126114/12.07.00_60/

31
webrtc/video/end_to_end_tests.cc
View File

@ -3062,9 +3062,9 @@ void EndToEndTest::TestRtpStatePreservation(bool use_rtx,
: test::RtpRtcpObserver(kDefaultTimeoutMs),
ssrcs_to_observe_(kNumSsrcs) {
for (size_t i = 0; i < kNumSsrcs; ++i) {
configured_ssrcs_[kVideoSendSsrcs[i]] = true;
ssrc_is_rtx_[kVideoSendSsrcs[i]] = false;
if (use_rtx)
configured_ssrcs_[kSendRtxSsrcs[i]] = true;
ssrc_is_rtx_[kSendRtxSsrcs[i]] = true;
}
}
@ -3107,7 +3107,7 @@ void EndToEndTest::TestRtpStatePreservation(bool use_rtx,
const bool only_padding =
header.headerLength + header.paddingLength == length;
EXPECT_TRUE(configured_ssrcs_[ssrc])
EXPECT_TRUE(ssrc_is_rtx_.find(ssrc) != ssrc_is_rtx_.end())
<< "Received SSRC that wasn't configured: " << ssrc;
static const int64_t kMaxSequenceNumberGap = 100;
@ -3133,14 +3133,16 @@ void EndToEndTest::TestRtpStatePreservation(bool use_rtx,
}
}
rtc::CritScope lock(&crit_);
ValidateTimestampGap(ssrc, timestamp, only_padding);
if (!ssrc_is_rtx_[ssrc]) {
rtc::CritScope lock(&crit_);
ValidateTimestampGap(ssrc, timestamp, only_padding);
// Wait for media packets on all ssrcs.
if (!ssrc_observed_[ssrc] && !only_padding) {
ssrc_observed_[ssrc] = true;
if (--ssrcs_to_observe_ == 0)
observation_complete_.Set();
// Wait for media packets on all ssrcs.
if (!ssrc_observed_[ssrc] && !only_padding) {
ssrc_observed_[ssrc] = true;
if (--ssrcs_to_observe_ == 0)
observation_complete_.Set();
}
}
return SEND_PACKET;
@ -3162,7 +3164,7 @@ void EndToEndTest::TestRtpStatePreservation(bool use_rtx,
SequenceNumberUnwrapper seq_numbers_unwrapper_;
std::map<uint32_t, std::list<int64_t>> last_observed_seq_numbers_;
std::map<uint32_t, uint32_t> last_observed_timestamp_;
std::map<uint32_t, bool> configured_ssrcs_;
std::map<uint32_t, bool> ssrc_is_rtx_;
rtc::CriticalSection crit_;
size_t ssrcs_to_observe_ GUARDED_BY(crit_);
@ -3272,14 +3274,11 @@ TEST_F(EndToEndTest, RestartingSendStreamPreservesRtpState) {
TestRtpStatePreservation(false, false);
}
// These tests are flaky. See:
// https://bugs.chromium.org/p/webrtc/issues/detail?id=4332
TEST_F(EndToEndTest, DISABLED_RestartingSendStreamPreservesRtpStatesWithRtx) {
TEST_F(EndToEndTest, RestartingSendStreamPreservesRtpStatesWithRtx) {
TestRtpStatePreservation(true, false);
}
TEST_F(EndToEndTest,
DISABLED_RestartingSendStreamKeepsRtpAndRtcpTimestampsSynced) {
TEST_F(EndToEndTest, RestartingSendStreamKeepsRtpAndRtcpTimestampsSynced) {
TestRtpStatePreservation(true, true);
}