New method RtpReceiver::GetLatestTimestamps.
The two timestamps, rtp time and corresponding system time, are always
used together, for audio/video sync. The new method reads both
timestamps, without releasing a lock in between. Ensures that the
caller gets values corresponding to the same packet.
Bug: webrtc:7135
Change-Id: I25bdcbe9ad620016bfad39841b339c266efade14
Reviewed-on: https://webrtc-review.googlesource.com/4062
Commit-Queue: Niels Moller <nisse@webrtc.org>
Commit-Queue: Stefan Holmer <stefan@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Reviewed-by: Fredrik Solenberg <solenberg@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20120}diff --git a/audio/audio_receive_stream.cc b/audio/audio_receive_stream.cc
index 2a57551..730248e 100644
--- a/audio/audio_receive_stream.cc
+++ b/audio/audio_receive_stream.cc
@@ -268,10 +268,9 @@
RTC_DCHECK(rtp_rtcp);
RTC_DCHECK(rtp_receiver);
- if (!rtp_receiver->Timestamp(&info.latest_received_capture_timestamp)) {
- return rtc::Optional<Syncable::Info>();
- }
- if (!rtp_receiver->LastReceivedTimeMs(&info.latest_receive_time_ms)) {
+ if (!rtp_receiver->GetLatestTimestamps(
+ &info.latest_received_capture_timestamp,
+ &info.latest_receive_time_ms)) {
return rtc::Optional<Syncable::Info>();
}
if (rtp_rtcp->RemoteNTP(&info.capture_time_ntp_secs,
diff --git a/modules/rtp_rtcp/include/rtp_receiver.h b/modules/rtp_rtcp/include/rtp_receiver.h
index f213a2b..b99ff3d 100644
--- a/modules/rtp_rtcp/include/rtp_receiver.h
+++ b/modules/rtp_rtcp/include/rtp_receiver.h
@@ -77,12 +77,11 @@
PayloadUnion payload_specific,
bool in_order) = 0;
- // Gets the last received timestamp. Returns true if a packet has been
- // received, false otherwise.
- virtual bool Timestamp(uint32_t* timestamp) const = 0;
- // Gets the time in milliseconds when the last timestamp was received.
- // Returns true if a packet has been received, false otherwise.
- virtual bool LastReceivedTimeMs(int64_t* receive_time_ms) const = 0;
+ // Gets the RTP timestamp and the corresponding monotonic system
+ // time for the most recent in-order packet. Returns true on
+ // success, false if no packet has been received.
+ virtual bool GetLatestTimestamps(uint32_t* timestamp,
+ int64_t* receive_time_ms) const = 0;
// Returns the remote SSRC of the currently received RTP stream.
virtual uint32_t SSRC() const = 0;
diff --git a/modules/rtp_rtcp/source/rtp_receiver_impl.cc b/modules/rtp_rtcp/source/rtp_receiver_impl.cc
index 8a8669e..45faee2 100644
--- a/modules/rtp_rtcp/source/rtp_receiver_impl.cc
+++ b/modules/rtp_rtcp/source/rtp_receiver_impl.cc
@@ -225,24 +225,16 @@
return sources;
}
-bool RtpReceiverImpl::Timestamp(uint32_t* timestamp) const {
+bool RtpReceiverImpl::GetLatestTimestamps(uint32_t* timestamp,
+ int64_t* receive_time_ms) const {
rtc::CritScope lock(&critical_section_rtp_receiver_);
- if (!HaveReceivedFrame())
+ if (last_received_frame_time_ms_ < 0)
return false;
+
*timestamp = last_received_timestamp_;
- return true;
-}
-
-bool RtpReceiverImpl::LastReceivedTimeMs(int64_t* receive_time_ms) const {
- rtc::CritScope lock(&critical_section_rtp_receiver_);
- if (!HaveReceivedFrame())
- return false;
*receive_time_ms = last_received_frame_time_ms_;
- return true;
-}
-bool RtpReceiverImpl::HaveReceivedFrame() const {
- return last_received_frame_time_ms_ >= 0;
+ return true;
}
// Implementation note: must not hold critsect when called.
diff --git a/modules/rtp_rtcp/source/rtp_receiver_impl.h b/modules/rtp_rtcp/source/rtp_receiver_impl.h
index 0ce23dd..e8adc3b 100644
--- a/modules/rtp_rtcp/source/rtp_receiver_impl.h
+++ b/modules/rtp_rtcp/source/rtp_receiver_impl.h
@@ -47,9 +47,8 @@
PayloadUnion payload_specific,
bool in_order) override;
- // Returns the last received timestamp.
- bool Timestamp(uint32_t* timestamp) const override;
- bool LastReceivedTimeMs(int64_t* receive_time_ms) const override;
+ bool GetLatestTimestamps(uint32_t* timestamp,
+ int64_t* receive_time_ms) const override;
uint32_t SSRC() const override;
@@ -70,9 +69,6 @@
}
private:
- bool HaveReceivedFrame() const
- RTC_EXCLUSIVE_LOCKS_REQUIRED(critical_section_rtp_receiver_);
-
void CheckSSRCChanged(const RTPHeader& rtp_header);
void CheckCSRC(const WebRtcRTPHeader& rtp_header);
int32_t CheckPayloadChanged(const RTPHeader& rtp_header,
diff --git a/modules/rtp_rtcp/test/testAPI/test_api_audio.cc b/modules/rtp_rtcp/test/testAPI/test_api_audio.cc
index f0416c4..c3bf4b9 100644
--- a/modules/rtp_rtcp/test/testAPI/test_api_audio.cc
+++ b/modules/rtp_rtcp/test/testAPI/test_api_audio.cc
@@ -185,8 +185,11 @@
EXPECT_EQ(test_ssrc, rtp_receiver2_->SSRC());
uint32_t timestamp;
- EXPECT_TRUE(rtp_receiver2_->Timestamp(×tamp));
+ int64_t receive_time_ms;
+ EXPECT_TRUE(
+ rtp_receiver2_->GetLatestTimestamps(×tamp, &receive_time_ms));
EXPECT_EQ(test_timestamp, timestamp);
+ EXPECT_EQ(fake_clock.TimeInMilliseconds(), receive_time_ms);
}
TEST_F(RtpRtcpAudioTest, DTMF) {
@@ -264,23 +267,30 @@
uint32_t in_timestamp = 0;
for (const auto& c : kCngCodecs) {
uint32_t timestamp;
+ int64_t receive_time_ms;
EXPECT_TRUE(module1->SendOutgoingData(
webrtc::kAudioFrameSpeech, kPcmuPayloadType, in_timestamp, -1,
kTestPayload, 4, nullptr, nullptr, nullptr));
EXPECT_EQ(test_ssrc, rtp_receiver2_->SSRC());
- EXPECT_TRUE(rtp_receiver2_->Timestamp(×tamp));
+ EXPECT_TRUE(
+ rtp_receiver2_->GetLatestTimestamps(×tamp, &receive_time_ms));
EXPECT_EQ(test_timestamp + in_timestamp, timestamp);
+ EXPECT_EQ(fake_clock.TimeInMilliseconds(), receive_time_ms);
in_timestamp += 10;
+ fake_clock.AdvanceTimeMilliseconds(20);
EXPECT_TRUE(module1->SendOutgoingData(webrtc::kAudioFrameCN, c.payload_type,
in_timestamp, -1, kTestPayload, 1,
nullptr, nullptr, nullptr));
EXPECT_EQ(test_ssrc, rtp_receiver2_->SSRC());
- EXPECT_TRUE(rtp_receiver2_->Timestamp(×tamp));
+ EXPECT_TRUE(
+ rtp_receiver2_->GetLatestTimestamps(×tamp, &receive_time_ms));
EXPECT_EQ(test_timestamp + in_timestamp, timestamp);
+ EXPECT_EQ(fake_clock.TimeInMilliseconds(), receive_time_ms);
in_timestamp += 10;
+ fake_clock.AdvanceTimeMilliseconds(20);
}
}
diff --git a/video/video_receive_stream.cc b/video/video_receive_stream.cc
index c0a7514..cca0511 100644
--- a/video/video_receive_stream.cc
+++ b/video/video_receive_stream.cc
@@ -364,9 +364,9 @@
RtpReceiver* rtp_receiver = rtp_video_stream_receiver_.GetRtpReceiver();
RTC_DCHECK(rtp_receiver);
- if (!rtp_receiver->Timestamp(&info.latest_received_capture_timestamp))
- return rtc::Optional<Syncable::Info>();
- if (!rtp_receiver->LastReceivedTimeMs(&info.latest_receive_time_ms))
+ if (!rtp_receiver->GetLatestTimestamps(
+ &info.latest_received_capture_timestamp,
+ &info.latest_receive_time_ms))
return rtc::Optional<Syncable::Info>();
RtpRtcp* rtp_rtcp = rtp_video_stream_receiver_.rtp_rtcp();
