From 09f1560f813a36f4176c1fb1068f4d7105c26175 Mon Sep 17 00:00:00 2001 From: ossu Date: Mon, 29 Aug 2016 03:59:05 -0700 Subject: [PATCH] TimestampScaler no longer depends on NetEqDecoder to figure out scaling. This is still a tiny lie, since it checks for kCodecArbitrary to avoid scaling a codec with an external decoder, because of missing information in that case. The main point is still true, though. Once the next CL is in, removing NetEqDecoder usage completely in DecoderDatabase, another workaround will be in place for external decoders until we can fully deprecate them. BUG=webrtc:5805 Review-Url: https://codereview.webrtc.org/2270063006 Cr-Commit-Position: refs/heads/master@{#13952} --- .../audio_coding/neteq/decoder_database.h | 12 ++++-- .../audio_coding/neteq/timestamp_scaler.cc | 41 ++++++------------- .../neteq/timestamp_scaler_unittest.cc | 40 +++++++++++++----- 3 files changed, 51 insertions(+), 42 deletions(-) diff --git a/webrtc/modules/audio_coding/neteq/decoder_database.h b/webrtc/modules/audio_coding/neteq/decoder_database.h index 14548749bf..6d277c95c9 100644 --- a/webrtc/modules/audio_coding/neteq/decoder_database.h +++ b/webrtc/modules/audio_coding/neteq/decoder_database.h @@ -59,10 +59,14 @@ class DecoderDatabase { void DropDecoder() const { decoder_.reset(); } int SampleRateHz() const { - RTC_DCHECK_EQ(1, !!decoder_ + !!external_decoder_ + !!cng_decoder_); - return decoder_ ? decoder_->SampleRateHz() - : external_decoder_ ? external_decoder_->SampleRateHz() - : cng_decoder_->sample_rate_hz; + const AudioDecoder* decoder = GetDecoder(); + RTC_DCHECK_EQ(1, !!decoder + !!cng_decoder_); + return decoder ? decoder->SampleRateHz() : cng_decoder_->sample_rate_hz; + } + + const SdpAudioFormat& GetFormat() const { + RTC_DCHECK(audio_format_); + return *audio_format_; } // Returns true if |codec_type| is comfort noise. diff --git a/webrtc/modules/audio_coding/neteq/timestamp_scaler.cc b/webrtc/modules/audio_coding/neteq/timestamp_scaler.cc index b228e017b2..1f28639e43 100644 --- a/webrtc/modules/audio_coding/neteq/timestamp_scaler.cc +++ b/webrtc/modules/audio_coding/neteq/timestamp_scaler.cc @@ -11,7 +11,6 @@ #include "webrtc/modules/audio_coding/neteq/timestamp_scaler.h" #include "webrtc/modules/audio_coding/neteq/decoder_database.h" -#include "webrtc/modules/audio_coding/neteq/defines.h" #include "webrtc/system_wrappers/include/logging.h" namespace webrtc { @@ -43,39 +42,25 @@ uint32_t TimestampScaler::ToInternal(uint32_t external_timestamp, // Payload type is unknown. Do not scale. return external_timestamp; } - switch (info->codec_type) { - case NetEqDecoder::kDecoderG722: - case NetEqDecoder::kDecoderG722_2ch: { - // Use timestamp scaling with factor 2 (two output samples per RTP - // timestamp). - numerator_ = 2; - denominator_ = 1; - break; - } - case NetEqDecoder::kDecoderAVT: - case NetEqDecoder::kDecoderCNGnb: - case NetEqDecoder::kDecoderCNGwb: - case NetEqDecoder::kDecoderCNGswb32kHz: - case NetEqDecoder::kDecoderCNGswb48kHz: { - // Do not change the timestamp scaling settings for DTMF or CNG. - break; - } - default: { - // Do not use timestamp scaling for any other codec. - numerator_ = 1; - denominator_ = 1; - break; + if (!(info->IsComfortNoise() || info->IsDtmf())) { + // Do not change the timestamp scaling settings for DTMF or CNG. + numerator_ = info->SampleRateHz(); + if (info->codec_type == NetEqDecoder::kDecoderArbitrary) { + // We have no format mapping for "arbitrary" external codecs, so we cannot + // support timestamp scaling of them. + denominator_ = numerator_; + } else { + denominator_ = info->GetFormat().clockrate_hz; } } - - if (!(numerator_ == 1 && denominator_ == 1)) { + if (numerator_ != denominator_) { // We have a scale factor != 1. if (!first_packet_received_) { external_ref_ = external_timestamp; internal_ref_ = external_timestamp; first_packet_received_ = true; } - int64_t external_diff = external_timestamp - external_ref_; + const int64_t external_diff = int64_t{external_timestamp} - external_ref_; assert(denominator_ > 0); // Should not be possible. external_ref_ = external_timestamp; internal_ref_ += (external_diff * numerator_) / denominator_; @@ -88,11 +73,11 @@ uint32_t TimestampScaler::ToInternal(uint32_t external_timestamp, uint32_t TimestampScaler::ToExternal(uint32_t internal_timestamp) const { - if (!first_packet_received_ || (numerator_ == 1 && denominator_ == 1)) { + if (!first_packet_received_ || (numerator_ == denominator_)) { // Not initialized, or scale factor is 1. return internal_timestamp; } else { - int64_t internal_diff = internal_timestamp - internal_ref_; + const int64_t internal_diff = int64_t{internal_timestamp} - internal_ref_; assert(numerator_ > 0); // Should not be possible. // Do not update references in this method. // Switch |denominator_| and |numerator_| to convert the other way. diff --git a/webrtc/modules/audio_coding/neteq/timestamp_scaler_unittest.cc b/webrtc/modules/audio_coding/neteq/timestamp_scaler_unittest.cc index 91e1342c70..1b8910ef64 100644 --- a/webrtc/modules/audio_coding/neteq/timestamp_scaler_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/timestamp_scaler_unittest.cc @@ -12,6 +12,7 @@ #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" +#include "webrtc/modules/audio_coding/codecs/builtin_audio_decoder_factory.h" #include "webrtc/modules/audio_coding/neteq/mock/mock_decoder_database.h" #include "webrtc/modules/audio_coding/neteq/packet.h" @@ -23,8 +24,10 @@ namespace webrtc { TEST(TimestampScaler, TestNoScaling) { MockDecoderDatabase db; + auto factory = CreateBuiltinAudioDecoderFactory(); // Use PCMu, because it doesn't use scaled timestamps. - const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderPCMu, ""); + const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderPCMu, "", + factory); static const uint8_t kRtpPayloadType = 0; EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadType)) .WillRepeatedly(Return(&info)); @@ -43,8 +46,10 @@ TEST(TimestampScaler, TestNoScaling) { TEST(TimestampScaler, TestNoScalingLargeStep) { MockDecoderDatabase db; + auto factory = CreateBuiltinAudioDecoderFactory(); // Use PCMu, because it doesn't use scaled timestamps. - const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderPCMu, ""); + const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderPCMu, "", + factory); static const uint8_t kRtpPayloadType = 0; EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadType)) .WillRepeatedly(Return(&info)); @@ -68,8 +73,10 @@ TEST(TimestampScaler, TestNoScalingLargeStep) { TEST(TimestampScaler, TestG722) { MockDecoderDatabase db; + auto factory = CreateBuiltinAudioDecoderFactory(); // Use G722, which has a factor 2 scaling. - const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderG722, ""); + const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderG722, "", + factory); static const uint8_t kRtpPayloadType = 17; EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadType)) .WillRepeatedly(Return(&info)); @@ -92,8 +99,10 @@ TEST(TimestampScaler, TestG722) { TEST(TimestampScaler, TestG722LargeStep) { MockDecoderDatabase db; + auto factory = CreateBuiltinAudioDecoderFactory(); // Use G722, which has a factor 2 scaling. - const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderG722, ""); + const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderG722, "", + factory); static const uint8_t kRtpPayloadType = 17; EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadType)) .WillRepeatedly(Return(&info)); @@ -120,9 +129,12 @@ TEST(TimestampScaler, TestG722LargeStep) { TEST(TimestampScaler, TestG722WithCng) { MockDecoderDatabase db; + auto factory = CreateBuiltinAudioDecoderFactory(); // Use G722, which has a factor 2 scaling. - const DecoderDatabase::DecoderInfo info_g722(NetEqDecoder::kDecoderG722, ""); - const DecoderDatabase::DecoderInfo info_cng(NetEqDecoder::kDecoderCNGwb, ""); + const DecoderDatabase::DecoderInfo info_g722(NetEqDecoder::kDecoderG722, "", + factory); + const DecoderDatabase::DecoderInfo info_cng(NetEqDecoder::kDecoderCNGwb, "", + factory); static const uint8_t kRtpPayloadTypeG722 = 17; static const uint8_t kRtpPayloadTypeCng = 13; EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadTypeG722)) @@ -161,8 +173,10 @@ TEST(TimestampScaler, TestG722WithCng) { // as many tests here. TEST(TimestampScaler, TestG722Packet) { MockDecoderDatabase db; + auto factory = CreateBuiltinAudioDecoderFactory(); // Use G722, which has a factor 2 scaling. - const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderG722, ""); + const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderG722, "", + factory); static const uint8_t kRtpPayloadType = 17; EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadType)) .WillRepeatedly(Return(&info)); @@ -189,8 +203,10 @@ TEST(TimestampScaler, TestG722Packet) { // we are not doing as many tests here. TEST(TimestampScaler, TestG722PacketList) { MockDecoderDatabase db; + auto factory = CreateBuiltinAudioDecoderFactory(); // Use G722, which has a factor 2 scaling. - const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderG722, ""); + const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderG722, "", + factory); static const uint8_t kRtpPayloadType = 17; EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadType)) .WillRepeatedly(Return(&info)); @@ -218,8 +234,10 @@ TEST(TimestampScaler, TestG722PacketList) { TEST(TimestampScaler, TestG722Reset) { MockDecoderDatabase db; + auto factory = CreateBuiltinAudioDecoderFactory(); // Use G722, which has a factor 2 scaling. - const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderG722, ""); + const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderG722, "", + factory); static const uint8_t kRtpPayloadType = 17; EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadType)) .WillRepeatedly(Return(&info)); @@ -258,7 +276,9 @@ TEST(TimestampScaler, TestG722Reset) { // timestamp scaler. TEST(TimestampScaler, TestOpusLargeStep) { MockDecoderDatabase db; - const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderOpus, ""); + auto factory = CreateBuiltinAudioDecoderFactory(); + const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderOpus, "", + factory); static const uint8_t kRtpPayloadType = 17; EXPECT_CALL(db, GetDecoderInfo(kRtpPayloadType)) .WillRepeatedly(Return(&info));