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}

audio/audio_receive_stream.cc

@@ -268,10 +268,9 @@ rtc::Optional<Syncable::Info> AudioReceiveStream::GetInfo() const {
   RTC_DCHECK(rtp_rtcp);
   RTC_DCHECK(rtp_receiver);
 
-  if (!rtp_receiver->Timestamp(&info.latest_received_capture_timestamp)) {
+  if (!rtp_receiver->GetLatestTimestamps(
-    return rtc::Optional<Syncable::Info>();
+          &info.latest_received_capture_timestamp,
-  }
+          &info.latest_receive_time_ms)) {
-  if (!rtp_receiver->LastReceivedTimeMs(&info.latest_receive_time_ms)) {
     return rtc::Optional<Syncable::Info>();
   }
   if (rtp_rtcp->RemoteNTP(&info.capture_time_ntp_secs,

modules/rtp_rtcp/include/rtp_receiver.h

@@ -77,12 +77,11 @@ class RtpReceiver {
                                  PayloadUnion payload_specific,
                                  bool in_order) = 0;
 
-  // Gets the last received timestamp. Returns true if a packet has been
+  // Gets the RTP timestamp and the corresponding monotonic system
-  // received, false otherwise.
+  // time for the most recent in-order packet. Returns true on
-  virtual bool Timestamp(uint32_t* timestamp) const = 0;
+  // success, false if no packet has been received.
-  // Gets the time in milliseconds when the last timestamp was received.
+  virtual bool GetLatestTimestamps(uint32_t* timestamp,
-  // Returns true if a packet has been received, false otherwise.
+                                   int64_t* receive_time_ms) const = 0;
-  virtual bool LastReceivedTimeMs(int64_t* receive_time_ms) const = 0;
 
   // Returns the remote SSRC of the currently received RTP stream.
   virtual uint32_t SSRC() const = 0;

modules/rtp_rtcp/source/rtp_receiver_impl.cc

@@ -225,24 +225,16 @@ std::vector<RtpSource> RtpReceiverImpl::GetSources() const {
   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 true;
-  return last_received_frame_time_ms_ >= 0;
 }
 
 // Implementation note: must not hold critsect when called.

modules/rtp_rtcp/source/rtp_receiver_impl.h

@@ -47,9 +47,8 @@ class RtpReceiverImpl : public RtpReceiver {
                          PayloadUnion payload_specific,
                          bool in_order) override;
 
-  // Returns the last received timestamp.
+  bool GetLatestTimestamps(uint32_t* timestamp,
-  bool Timestamp(uint32_t* timestamp) const override;
+                           int64_t* receive_time_ms) const override;
-  bool LastReceivedTimeMs(int64_t* receive_time_ms) const override;
 
   uint32_t SSRC() const override;
 
@@ -70,9 +69,6 @@ class RtpReceiverImpl : public RtpReceiver {
   }
 
  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,

modules/rtp_rtcp/test/testAPI/test_api_audio.cc

@@ -185,8 +185,11 @@ TEST_F(RtpRtcpAudioTest, Basic) {
 
   EXPECT_EQ(test_ssrc, rtp_receiver2_->SSRC());
   uint32_t timestamp;
-  EXPECT_TRUE(rtp_receiver2_->Timestamp(&timestamp));
+  int64_t receive_time_ms;
+  EXPECT_TRUE(
+      rtp_receiver2_->GetLatestTimestamps(&timestamp, &receive_time_ms));
   EXPECT_EQ(test_timestamp, timestamp);
+  EXPECT_EQ(fake_clock.TimeInMilliseconds(), receive_time_ms);
 }
 
 TEST_F(RtpRtcpAudioTest, DTMF) {
@@ -264,23 +267,30 @@ TEST_F(RtpRtcpAudioTest, ComfortNoise) {
   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(&timestamp));
+    EXPECT_TRUE(
+        rtp_receiver2_->GetLatestTimestamps(&timestamp, &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(&timestamp));
+    EXPECT_TRUE(
+        rtp_receiver2_->GetLatestTimestamps(&timestamp, &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);
   }
 }
 

video/video_receive_stream.cc

@@ -364,9 +364,9 @@ rtc::Optional<Syncable::Info> VideoReceiveStream::GetInfo() const {
 
   RtpReceiver* rtp_receiver = rtp_video_stream_receiver_.GetRtpReceiver();
   RTC_DCHECK(rtp_receiver);
-  if (!rtp_receiver->Timestamp(&info.latest_received_capture_timestamp))
+  if (!rtp_receiver->GetLatestTimestamps(
-    return rtc::Optional<Syncable::Info>();
+          &info.latest_received_capture_timestamp,
-  if (!rtp_receiver->LastReceivedTimeMs(&info.latest_receive_time_ms))
+          &info.latest_receive_time_ms))
     return rtc::Optional<Syncable::Info>();
 
   RtpRtcp* rtp_rtcp = rtp_video_stream_receiver_.rtp_rtcp();