Change NetEq::GetPlayoutTimestamp to return an rtc::Optional<uint32_t>

webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc

Index: webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc
diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc b/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc
index e1eb403022cecac01aec1273864568f3208d3438..2fd52982469832900762178d2067f1d5157752ac 100644
--- a/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc
+++ b/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc
@@ -478,11 +478,9 @@ TEST_F(NetEqImplTest, VerifyTimestampPropagation) {
   // The value of the last of the output samples is the same as the number of
   // samples played from the decoded packet. Thus, this number + the RTP
   // timestamp should match the playout timestamp.
-  uint32_t timestamp = 0;
-  EXPECT_TRUE(neteq_->GetPlayoutTimestamp(&timestamp));
   EXPECT_EQ(rtp_header.header.timestamp +
                 output.data_[output.samples_per_channel_ - 1],
-            timestamp);
+            *neteq_->GetPlayoutTimestamp());

[kwiberg-webrtc, 2016/04/05 14:04:02]

IIRC, Optional::operator* does a DCHECK, not a CHECK. So consider writing this
instead:

EXPECT_EQ(rtc::Optional<uint32_t>(...), neteq_->GetPlayoutTimestamp());

[hlundin-webrtc, 2016/04/05 14:19:44]

Done.

 
   // Check the timestamp for the last value in the sync buffer. This should
   // be one full frame length ahead of the RTP timestamp.
@@ -714,8 +712,6 @@ TEST_F(NetEqImplTest, CodecInternalCng) {
 
   const size_t kMaxOutputSize = static_cast<size_t>(10 * kSampleRateKhz);
   AudioFrame output;
-  uint32_t timestamp;
-  uint32_t last_timestamp;
   AudioFrame::SpeechType expected_type[8] = {
       AudioFrame::kNormalSpeech, AudioFrame::kNormalSpeech,
       AudioFrame::kCNG, AudioFrame::kCNG,
@@ -731,16 +727,19 @@ TEST_F(NetEqImplTest, CodecInternalCng) {
   };
 
   EXPECT_EQ(NetEq::kOK, neteq_->GetAudio(&output));
-  EXPECT_TRUE(neteq_->GetPlayoutTimestamp(&last_timestamp));
+  rtc::Optional<uint32_t> last_timestamp = neteq_->GetPlayoutTimestamp();
+  EXPECT_TRUE(last_timestamp);
 
   for (size_t i = 1; i < 6; ++i) {
     ASSERT_EQ(kMaxOutputSize, output.samples_per_channel_);
     EXPECT_EQ(1u, output.num_channels_);
     EXPECT_EQ(expected_type[i - 1], output.speech_type_);
-    EXPECT_TRUE(neteq_->GetPlayoutTimestamp(&timestamp));
+    rtc::Optional<uint32_t> timestamp = neteq_->GetPlayoutTimestamp();
+    EXPECT_TRUE(timestamp);
     EXPECT_EQ(NetEq::kOK, neteq_->GetAudio(&output));
-    EXPECT_TRUE(neteq_->GetPlayoutTimestamp(&timestamp));
-    EXPECT_EQ(timestamp, last_timestamp + expected_timestamp_increment[i]);
+    timestamp = neteq_->GetPlayoutTimestamp();
+    EXPECT_TRUE(timestamp);
+    EXPECT_EQ(*timestamp, *last_timestamp + expected_timestamp_increment[i]);

[kwiberg-webrtc, 2016/04/05 14:04:02]

To avoid dereferencing invalid Optionals (which will crash if DCHECKS are
enabled), consider using ASSERT_TRUE when checking that they are valid.

[hlundin-webrtc, 2016/04/05 14:19:45]

Yes, I learned that the hard way in the next CL after this (where I already
corrected it). Now I will get merge conflicts...

     last_timestamp = timestamp;
   }
 
@@ -756,8 +755,9 @@ TEST_F(NetEqImplTest, CodecInternalCng) {
     EXPECT_EQ(1u, output.num_channels_);
     EXPECT_EQ(expected_type[i - 1], output.speech_type_);
     EXPECT_EQ(NetEq::kOK, neteq_->GetAudio(&output));
-    EXPECT_TRUE(neteq_->GetPlayoutTimestamp(&timestamp));
-    EXPECT_EQ(timestamp, last_timestamp + expected_timestamp_increment[i]);
+    rtc::Optional<uint32_t> timestamp = neteq_->GetPlayoutTimestamp();
+    EXPECT_TRUE(timestamp);
+    EXPECT_EQ(*timestamp, *last_timestamp + expected_timestamp_increment[i]);
     last_timestamp = timestamp;
   }