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

Reland of StartTimestamp generated randomly in RtpSender constructor (patchset #1 id:1 of https://codereview.webrtc.org/2248413002/ )

Browse Source 
Reason for revert:
Reland: downstream code expectation about rtp_sender timestamp adjusted.

Original issue's description:
> Revert of StartTimestamp generated randomly in RtpSender constructor (patchset #4 id:60001 of https://codereview.webrtc.org/2241193002/ )
>
> 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
>
> Committed: https://crrev.com/86c96948e340cf8b879bddb0c7293f3b5ad4dad4
> Cr-Commit-Position: refs/heads/master@{#13798}

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/2257083002
Cr-Commit-Position: refs/heads/master@{#13811}
This commit is contained in:
danilchap 2016-08-18 02:01:49 -07:00 committed by Commit bot
parent d4e9f62ea7
commit 71fead2146
7 changed files with 32 additions and 44 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),
start_timestamp_(0),
timestamp_offset_(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::SetStartTimestamp(uint32_t start_timestamp) {
void RTCPSender::SetTimestampOffset(uint32_t timestamp_offset) {
rtc::CritScope lock(&critical_section_rtcp_sender_);
start_timestamp_ = start_timestamp;
timestamp_offset_ = timestamp_offset;
}
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 =
start_timestamp_ + last_rtp_timestamp_ +
timestamp_offset_ + 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 SetStartTimestamp(uint32_t start_timestamp);
void SetTimestampOffset(uint32_t timestamp_offset);
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 start_timestamp_ GUARDED_BY(critical_section_rtcp_sender_);
uint32_t timestamp_offset_ 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_->SetStartTimestamp(kStartRtpTimestamp);
rtcp_sender_->SetTimestampOffset(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_->SetStartTimestamp(kStartRtpTimestamp);
rtcp_sender_->SetTimestampOffset(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,6 +105,11 @@ 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);
}
@ -262,13 +267,13 @@ int8_t ModuleRtpRtcpImpl::SendPayloadType() const {
}
uint32_t ModuleRtpRtcpImpl::StartTimestamp() const {
return rtp_sender_.StartTimestamp();
return rtp_sender_.TimestampOffset();
}
// Configure start timestamp, default is a random number.
void ModuleRtpRtcpImpl::SetStartTimestamp(const uint32_t timestamp) {
rtcp_sender_.SetStartTimestamp(timestamp);
rtp_sender_.SetStartTimestamp(timestamp, true);
rtcp_sender_.SetTimestampOffset(timestamp);
rtp_sender_.SetTimestampOffset(timestamp);
}
uint16_t ModuleRtpRtcpImpl::SequenceNumber() const {
@ -281,8 +286,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) {
@ -353,13 +358,8 @@ 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,8 +109,6 @@ 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),
@ -128,6 +126,8 @@ 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_ = start_timestamp_ + capture_timestamp;
timestamp_ = timestamp_offset_ + capture_timestamp;
last_timestamp_time_ms_ = clock_->TimeInMilliseconds();
uint32_t sequence_number = sequence_number_++;
capture_time_ms_ = capture_time_ms;
@ -1489,13 +1489,7 @@ bool RTPSender::UpdateTransportSequenceNumber(RtpPacketToSend* packet,
}
void RTPSender::SetSendingStatus(bool 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 {
if (!enabled) {
rtc::CritScope lock(&send_critsect_);
if (!ssrc_forced_) {
// Generate a new SSRC.
@ -1526,21 +1520,14 @@ uint32_t RTPSender::Timestamp() const {
return timestamp_;
}
void RTPSender::SetStartTimestamp(uint32_t timestamp, bool force) {
void RTPSender::SetTimestampOffset(uint32_t timestamp) {
rtc::CritScope lock(&send_critsect_);
if (force) {
start_timestamp_forced_ = true;
start_timestamp_ = timestamp;
} else {
if (!start_timestamp_forced_) {
start_timestamp_ = timestamp;
}
}
timestamp_offset_ = timestamp;
}
uint32_t RTPSender::StartTimestamp() const {
uint32_t RTPSender::TimestampOffset() const {
rtc::CritScope lock(&send_critsect_);
return start_timestamp_;
return timestamp_offset_;
}
uint32_t RTPSender::GenerateNewSSRC() {
@ -1705,6 +1692,7 @@ 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;
@ -1716,7 +1704,7 @@ RtpState RTPSender::GetRtpState() const {
RtpState state;
state.sequence_number = sequence_number_;
state.start_timestamp = start_timestamp_;
state.start_timestamp = timestamp_offset_;
state.timestamp = timestamp_;
state.capture_time_ms = capture_time_ms_;
state.last_timestamp_time_ms = last_timestamp_time_ms_;
@ -1735,7 +1723,7 @@ RtpState RTPSender::GetRtxRtpState() const {
RtpState state;
state.sequence_number = sequence_number_rtx_;
state.start_timestamp = start_timestamp_;
state.start_timestamp = timestamp_offset_;
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 StartTimestamp() const;
void SetStartTimestamp(uint32_t timestamp, bool force);
uint32_t TimestampOffset() const;
void SetTimestampOffset(uint32_t timestamp);
uint32_t GenerateNewSSRC();
void SetSSRC(uint32_t ssrc);
@ -402,8 +402,7 @@ class RTPSender {
BitrateStatisticsObserver* const bitrate_callback_;
// RTP variables
bool start_timestamp_forced_ GUARDED_BY(send_critsect_);
uint32_t start_timestamp_ GUARDED_BY(send_critsect_);
uint32_t timestamp_offset_ 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,6 +150,7 @@ class RtpSenderTest : public ::testing::Test {
&retransmission_rate_limiter_));
rtp_sender_->SetSequenceNumber(kSeqNum);
rtp_sender_->SetSendPayloadType(kPayload);
rtp_sender_->SetTimestampOffset(0);
}
SimulatedClock fake_clock_;