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

webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc

 /*
  *  Copyright (c) 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 460 matching lines @@
   EXPECT_EQ(1u, output.num_channels_);
   EXPECT_EQ(AudioFrame::kNormalSpeech, output.speech_type_);
 
   // Start with a simple check that the fake decoder is behaving as expected.
   EXPECT_EQ(kPayloadLengthSamples,
             static_cast<size_t>(decoder_.next_value() - 1));
 
   // 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.
   const SyncBuffer* sync_buffer = neteq_->sync_buffer_for_test();
   ASSERT_TRUE(sync_buffer != NULL);
   EXPECT_EQ(rtp_header.header.timestamp + kPayloadLengthSamples,
             sync_buffer->end_timestamp());
 
   // Check that the number of samples still to play from the sync buffer add
   // up with what was already played out.
@@ skipping 211 matching lines @@
 
   // Insert second packet (decoder will return CNG).
   payload[0] = 1;
   rtp_header.header.sequenceNumber++;
   rtp_header.header.timestamp += kPayloadLengthSamples;
   EXPECT_EQ(NetEq::kOK,
             neteq_->InsertPacket(rtp_header, payload, kReceiveTime));
 
   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,
       AudioFrame::kCNG, AudioFrame::kCNG,
       AudioFrame::kNormalSpeech, AudioFrame::kNormalSpeech
   };
   int expected_timestamp_increment[8] = {
       -1,  // will not be used.
       10 * kSampleRateKhz,
       0, 0,  // timestamp does not increase during CNG mode.
       0, 0,
       50 * kSampleRateKhz, 10 * kSampleRateKhz
   };
 
   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;
   }
 
   // Insert third packet, which leaves a gap from last packet.
   payload[0] = 2;
   rtp_header.header.sequenceNumber += 2;
   rtp_header.header.timestamp += 2 * kPayloadLengthSamples;
   EXPECT_EQ(NetEq::kOK,
             neteq_->InsertPacket(rtp_header, payload, kReceiveTime));
 
   for (size_t i = 6; i < 8; ++i) {
     ASSERT_EQ(kMaxOutputSize, output.samples_per_channel_);
     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;
   }
 
   // Now check the packet buffer, and make sure it is empty.
   EXPECT_TRUE(packet_buffer_->Empty());
 
   EXPECT_CALL(mock_decoder, Die());
 }
 
 TEST_F(NetEqImplTest, UnsupportedDecoder) {
@@ skipping 405 matching lines @@
 // Tests that the return value from last_output_sample_rate_hz() is equal to the
 // configured inital sample rate.
 TEST_F(NetEqImplTest, InitialLastOutputSampleRate) {
   UseNoMocks();
   config_.sample_rate_hz = 48000;
   CreateInstance();
   EXPECT_EQ(48000, neteq_->last_output_sample_rate_hz());
 }
 
 }// namespace webrtc