Add new method AcmReceiver::last_packet_sample_rate_hz()

webrtc/modules/audio_coding/main/acm2/acm_receiver_unittest_oldapi.cc

Index: webrtc/modules/audio_coding/main/acm2/acm_receiver_unittest_oldapi.cc
diff --git a/webrtc/modules/audio_coding/main/acm2/acm_receiver_unittest_oldapi.cc b/webrtc/modules/audio_coding/main/acm2/acm_receiver_unittest_oldapi.cc
index 3bda1168d42afdcfc3e58d88207cf072d6e053be..75953a9c8d3302da8e647da4371c1255b09d38f0 100644
--- a/webrtc/modules/audio_coding/main/acm2/acm_receiver_unittest_oldapi.cc
+++ b/webrtc/modules/audio_coding/main/acm2/acm_receiver_unittest_oldapi.cc
@@ -330,7 +330,7 @@ TEST_F(AcmReceiverTestOldApi,
 
   // Has received, only, DTX. Last Audio codec is undefined.
   EXPECT_EQ(-1, receiver_->LastAudioCodec(&codec));
-  EXPECT_EQ(-1, receiver_->last_audio_codec_id());
+  EXPECT_FALSE(receiver_->last_packet_sample_rate_hz());
 
   for (auto id : kCodecId) {
     const CodecIdInst c(id);
@@ -344,7 +344,8 @@ TEST_F(AcmReceiverTestOldApi,
     // of type "speech."
     ASSERT_TRUE(packet_sent_);
     ASSERT_EQ(kAudioFrameSpeech, last_frame_type_);
-    EXPECT_EQ(c.id, receiver_->last_audio_codec_id());
+    ASSERT_TRUE(receiver_->last_packet_sample_rate_hz());
+    EXPECT_EQ(c.inst.plfreq, *receiver_->last_packet_sample_rate_hz());

[kwiberg-webrtc, 2015/11/23 13:03:50]

Optional has an operator== function, so you should be able to just assert
equality to rtc::Optional<int>(c.inst.plfreq).

[hlundin-webrtc, 2015/11/23 13:50:45]

Acknowledged, but that won't give the same behavior as my code. I'm ASSERTing
that I get a value, but only EXPECTing it to be identical to plfreq. So, yes, I
could write the second line as 
EXPECT_EQ(rtc::Optional<int>(c.inst.plfreq),
receiver_->last_packet_sample_rate_hz());
but I don't think it makes the code any better.

[kwiberg-webrtc, 2015/11/23 14:03:35]

Well, no, what I suggested won't do exactly what you currently do, so if you
want exactly that, don't change it.

[hlundin-webrtc, 2015/11/23 14:13:34]

Right. I activated my brain now and realized that I don't need the ASSERT_TRUE
anymore. Your suggestion beats mine.

 
     // Set VAD on to send DTX. Then check if the "Last Audio codec" returns
     // the expected codec.
@@ -356,7 +357,7 @@ TEST_F(AcmReceiverTestOldApi,
       InsertOnePacketOfSilence(c.id);
       ASSERT_TRUE(packet_sent_);
     }
-    EXPECT_EQ(c.id, receiver_->last_audio_codec_id());
+    EXPECT_EQ(c.inst.plfreq, *receiver_->last_packet_sample_rate_hz());

[kwiberg-webrtc, 2015/11/23 13:03:50]

Same.

[hlundin-webrtc, 2015/11/23 13:50:45]

Will it make the code better?
EXPECT_EQ(rtc::Optional<int>(c.inst.plfreq),
receiver_->last_packet_sample_rate_hz());
vs. what I have now.

[kwiberg-webrtc, 2015/11/23 14:03:35]

In this case, arguably yes, since Optional's operator* will behave badly if the
Optional is unset. Alternatively, you could test separately if it's empty like
you did above.

[hlundin-webrtc, 2015/11/23 14:13:35]

Done.

     EXPECT_EQ(0, receiver_->LastAudioCodec(&codec));
     EXPECT_TRUE(CodecsEqual(c.inst, codec));
   }