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

Revert of StartTimestamp generated randomly in RtpSender constructor (patchset #4 id:60001 of https://codereview.webrtc.org/2241193002/ )

Browse Source 
Reason for revert:
Breaks downstream code.

Original issue's description:
> StartTimestamp generated randomly in RtpSender constructor
> instead of not-randomly at SetSendingState(true)
> Renamed to timestamp_offset_ to better match meaning of the variable.
>
> R=asapersson@webrtc.org, terelius@webrtc.org
>
> Committed: https://crrev.com/4466782ae43e1b1125a55ee7e18abd10dd37cede
> Cr-Commit-Position: refs/heads/master@{#13796}

TBR=asapersson@webrtc.org,terelius@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Review-Url: https://codereview.webrtc.org/2248413002
Cr-Commit-Position: refs/heads/master@{#13798}
This commit is contained in:
danilchap 2016-08-17 08:12:21 -07:00 committed by Commit bot
parent 5a25d9504a
commit 86c96948e3
7 changed files with 44 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
webrtc/modules/rtp_rtcp/source/rtcp_sender.cc
View File

@ -161,7 +161,7 @@ RTCPSender::RTCPSender(
sending_(false),
remb_enabled_(false),
next_time_to_send_rtcp_(0),
timestamp_offset_(0),
start_timestamp_(0),
last_rtp_timestamp_(0),
last_frame_capture_time_ms_(-1),
ssrc_(0),
@ -288,9 +288,9 @@ void RTCPSender::SetMaxPayloadLength(size_t max_payload_length) {
max_payload_length_ = max_payload_length;
}
void RTCPSender::SetTimestampOffset(uint32_t timestamp_offset) {
void RTCPSender::SetStartTimestamp(uint32_t start_timestamp) {
rtc::CritScope lock(&critical_section_rtcp_sender_);
timestamp_offset_ = timestamp_offset;
start_timestamp_ = start_timestamp;
}
void RTCPSender::SetLastRtpTime(uint32_t rtp_timestamp,
@ -444,7 +444,7 @@ std::unique_ptr<rtcp::RtcpPacket> RTCPSender::BuildSR(const RtcpContext& ctx) {
// timestamp as the last frame's timestamp + the time since the last frame
// was captured.
uint32_t rtp_timestamp =
timestamp_offset_ + last_rtp_timestamp_ +
start_timestamp_ + last_rtp_timestamp_ +
(clock_->TimeInMilliseconds() - last_frame_capture_time_ms_) *
(ctx.feedback_state_.frequency_hz / 1000);

4
webrtc/modules/rtp_rtcp/source/rtcp_sender.h
View File

@ -93,7 +93,7 @@ class RTCPSender {
int32_t SetNackStatus(bool enable);
void SetTimestampOffset(uint32_t timestamp_offset);
void SetStartTimestamp(uint32_t start_timestamp);
void SetLastRtpTime(uint32_t rtp_timestamp, int64_t capture_time_ms);
@ -214,7 +214,7 @@ class RTCPSender {
int64_t next_time_to_send_rtcp_ GUARDED_BY(critical_section_rtcp_sender_);
uint32_t timestamp_offset_ GUARDED_BY(critical_section_rtcp_sender_);
uint32_t start_timestamp_ GUARDED_BY(critical_section_rtcp_sender_);
uint32_t last_rtp_timestamp_ GUARDED_BY(critical_section_rtcp_sender_);
int64_t last_frame_capture_time_ms_ GUARDED_BY(critical_section_rtcp_sender_);
uint32_t ssrc_ GUARDED_BY(critical_section_rtcp_sender_);

4
webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc
View File

@ -243,7 +243,7 @@ class RtcpSenderTest : public ::testing::Test {
nullptr, nullptr, &test_transport_));
rtcp_sender_->SetSSRC(kSenderSsrc);
rtcp_sender_->SetRemoteSSRC(kRemoteSsrc);
rtcp_sender_->SetTimestampOffset(kStartRtpTimestamp);
rtcp_sender_->SetStartTimestamp(kStartRtpTimestamp);
rtcp_sender_->SetLastRtpTime(kRtpTimestamp, clock_.TimeInMilliseconds());
}
@ -813,7 +813,7 @@ TEST_F(RtcpSenderTest, ByeMustBeLast) {
nullptr, nullptr, &mock_transport));
rtcp_sender_->SetSSRC(kSenderSsrc);
rtcp_sender_->SetRemoteSSRC(kRemoteSsrc);
rtcp_sender_->SetTimestampOffset(kStartRtpTimestamp);
rtcp_sender_->SetStartTimestamp(kStartRtpTimestamp);
rtcp_sender_->SetLastRtpTime(kRtpTimestamp, clock_.TimeInMilliseconds());
// Set up XR VoIP metric to be included with BYE

18
webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
View File

@ -105,11 +105,6 @@ ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration)
uint32_t SSRC = rtp_sender_.SSRC();
rtcp_sender_.SetSSRC(SSRC);
SetRtcpReceiverSsrcs(SSRC);
// Make sure rtcp sender use same timestamp offset as rtp sender.
rtcp_sender_.SetTimestampOffset(rtp_sender_.TimestampOffset());
// Set default packet size limit.
SetMaxTransferUnit(IP_PACKET_SIZE);
}
@ -267,13 +262,13 @@ int8_t ModuleRtpRtcpImpl::SendPayloadType() const {
}
uint32_t ModuleRtpRtcpImpl::StartTimestamp() const {
return rtp_sender_.TimestampOffset();
return rtp_sender_.StartTimestamp();
}
// Configure start timestamp, default is a random number.
void ModuleRtpRtcpImpl::SetStartTimestamp(const uint32_t timestamp) {
rtcp_sender_.SetTimestampOffset(timestamp);
rtp_sender_.SetTimestampOffset(timestamp);
rtcp_sender_.SetStartTimestamp(timestamp);
rtp_sender_.SetStartTimestamp(timestamp, true);
}
uint16_t ModuleRtpRtcpImpl::SequenceNumber() const {
@ -286,8 +281,8 @@ void ModuleRtpRtcpImpl::SetSequenceNumber(const uint16_t seq_num) {
}
void ModuleRtpRtcpImpl::SetRtpState(const RtpState& rtp_state) {
SetStartTimestamp(rtp_state.start_timestamp);
rtp_sender_.SetRtpState(rtp_state);
rtcp_sender_.SetTimestampOffset(rtp_state.start_timestamp);
}
void ModuleRtpRtcpImpl::SetRtxState(const RtpState& rtp_state) {
@ -358,8 +353,13 @@ int32_t ModuleRtpRtcpImpl::SetSendingStatus(const bool sending) {
collision_detected_ = false;
// Generate a new time_stamp if true and not configured via API
// Generate a new SSRC for the next "call" if false
rtp_sender_.SetSendingStatus(sending);
if (sending) {
// Make sure the RTCP sender has the same timestamp offset.
rtcp_sender_.SetStartTimestamp(rtp_sender_.StartTimestamp());
}
// Make sure that RTCP objects are aware of our SSRC (it could have changed
// Due to collision)

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

@ -109,6 +109,8 @@ RTPSender::RTPSender(
send_packet_observer_(send_packet_observer),
bitrate_callback_(bitrate_callback),
// RTP variables
start_timestamp_forced_(false),
start_timestamp_(0),
ssrc_db_(SSRCDatabase::GetSSRCDatabase()),
remote_ssrc_(0),
sequence_number_forced_(false),
@ -126,8 +128,6 @@ RTPSender::RTPSender(
ssrc_rtx_ = ssrc_db_->CreateSSRC();
RTC_DCHECK(ssrc_rtx_ != 0);
// This random initialization is not intended to be cryptographic strong.
timestamp_offset_ = random_.Rand<uint32_t>();
// Random start, 16 bits. Can't be 0.
sequence_number_rtx_ = random_.Rand(1, kMaxInitRtpSeqNumber);
sequence_number_ = random_.Rand(1, kMaxInitRtpSeqNumber);
@ -1089,7 +1089,7 @@ int32_t RTPSender::BuildRtpHeader(uint8_t* data_buffer,
if (!sending_media_)
return -1;
timestamp_ = timestamp_offset_ + capture_timestamp;
timestamp_ = start_timestamp_ + capture_timestamp;
last_timestamp_time_ms_ = clock_->TimeInMilliseconds();
uint32_t sequence_number = sequence_number_++;
capture_time_ms_ = capture_time_ms;
@ -1489,7 +1489,13 @@ bool RTPSender::UpdateTransportSequenceNumber(RtpPacketToSend* packet,
}
void RTPSender::SetSendingStatus(bool enabled) {
if (!enabled) {
if (enabled) {
uint32_t frequency_hz = SendPayloadFrequency();
uint32_t RTPtime = CurrentRtp(*clock_, frequency_hz);
// Will be ignored if it's already configured via API.
SetStartTimestamp(RTPtime, false);
} else {
rtc::CritScope lock(&send_critsect_);
if (!ssrc_forced_) {
// Generate a new SSRC.
@ -1520,14 +1526,21 @@ uint32_t RTPSender::Timestamp() const {
return timestamp_;
}
void RTPSender::SetTimestampOffset(uint32_t timestamp) {
void RTPSender::SetStartTimestamp(uint32_t timestamp, bool force) {
rtc::CritScope lock(&send_critsect_);
timestamp_offset_ = timestamp;
if (force) {
start_timestamp_forced_ = true;
start_timestamp_ = timestamp;
} else {
if (!start_timestamp_forced_) {
start_timestamp_ = timestamp;
}
}
}
uint32_t RTPSender::TimestampOffset() const {
uint32_t RTPSender::StartTimestamp() const {
rtc::CritScope lock(&send_critsect_);
return timestamp_offset_;
return start_timestamp_;
}
uint32_t RTPSender::GenerateNewSSRC() {
@ -1692,7 +1705,6 @@ void RTPSender::SetRtpState(const RtpState& rtp_state) {
rtc::CritScope lock(&send_critsect_);
sequence_number_ = rtp_state.sequence_number;
sequence_number_forced_ = true;
timestamp_offset_ = rtp_state.start_timestamp;
timestamp_ = rtp_state.timestamp;
capture_time_ms_ = rtp_state.capture_time_ms;
last_timestamp_time_ms_ = rtp_state.last_timestamp_time_ms;
@ -1704,7 +1716,7 @@ RtpState RTPSender::GetRtpState() const {
RtpState state;
state.sequence_number = sequence_number_;
state.start_timestamp = timestamp_offset_;
state.start_timestamp = start_timestamp_;
state.timestamp = timestamp_;
state.capture_time_ms = capture_time_ms_;
state.last_timestamp_time_ms = last_timestamp_time_ms_;
@ -1723,7 +1735,7 @@ RtpState RTPSender::GetRtxRtpState() const {
RtpState state;
state.sequence_number = sequence_number_rtx_;
state.start_timestamp = timestamp_offset_;
state.start_timestamp = start_timestamp_;
return state;
}

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

@ -90,8 +90,8 @@ class RTPSender {
void GetDataCounters(StreamDataCounters* rtp_stats,
StreamDataCounters* rtx_stats) const;
uint32_t TimestampOffset() const;
void SetTimestampOffset(uint32_t timestamp);
uint32_t StartTimestamp() const;
void SetStartTimestamp(uint32_t timestamp, bool force);
uint32_t GenerateNewSSRC();
void SetSSRC(uint32_t ssrc);
@ -402,7 +402,8 @@ class RTPSender {
BitrateStatisticsObserver* const bitrate_callback_;
// RTP variables
uint32_t timestamp_offset_ GUARDED_BY(send_critsect_);
bool start_timestamp_forced_ GUARDED_BY(send_critsect_);
uint32_t start_timestamp_ GUARDED_BY(send_critsect_);
SSRCDatabase* const ssrc_db_;
uint32_t remote_ssrc_ GUARDED_BY(send_critsect_);
bool sequence_number_forced_ GUARDED_BY(send_critsect_);

1
webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
View File

@ -150,7 +150,6 @@ class RtpSenderTest : public ::testing::Test {
&retransmission_rate_limiter_));
rtp_sender_->SetSequenceNumber(kSeqNum);
rtp_sender_->SetSendPayloadType(kPayload);
rtp_sender_->SetTimestampOffset(0);
}
SimulatedClock fake_clock_;