From 7e5dfdbca3a2d2580abed0540952da978cc75566 Mon Sep 17 00:00:00 2001 From: Ali Tofigh Date: Tue, 24 Mar 2020 16:00:51 +0100 Subject: [PATCH] Implement AudioEncoder::GetFrameLengthRange() for all audio encoders. The WebRTC-SendSideBwe-WithOverhead field trial requires audio encoders to properly implement the AudioEncoder::GetFrameLengthRange() function. Thic CL implements the function for all audio encoders in WebRTC in preparation for making that function pure virtual in the interface. Bug: webrtc:11427 Change-Id: Ieab6b6c72c62af6ac9525a20fcb39bd477079551 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/171503 Reviewed-by: Sebastian Jansson Reviewed-by: Minyue Li Commit-Queue: Minyue Li Cr-Commit-Position: refs/heads/master@{#30890} --- modules/audio_coding/BUILD.gn | 11 +++++++++++ modules/audio_coding/codecs/cng/audio_encoder_cng.cc | 9 +++++++++ .../codecs/cng/audio_encoder_cng_unittest.cc | 12 ++++++++++++ .../audio_coding/codecs/g711/audio_encoder_pcm.cc | 6 ++++++ modules/audio_coding/codecs/g711/audio_encoder_pcm.h | 5 +++++ .../audio_coding/codecs/g722/audio_encoder_g722.cc | 6 ++++++ .../audio_coding/codecs/g722/audio_encoder_g722.h | 5 +++++ .../audio_coding/codecs/ilbc/audio_encoder_ilbc.cc | 6 ++++++ .../audio_coding/codecs/ilbc/audio_encoder_ilbc.h | 6 ++++++ .../audio_coding/codecs/isac/audio_encoder_isac_t.h | 5 +++++ .../codecs/isac/audio_encoder_isac_t_impl.h | 7 +++++++ .../opus/audio_encoder_multi_channel_opus_impl.cc | 6 ++++++ .../opus/audio_encoder_multi_channel_opus_impl.h | 4 ++++ .../codecs/red/audio_encoder_copy_red.cc | 5 +++++ .../audio_coding/codecs/red/audio_encoder_copy_red.h | 4 ++++ .../codecs/red/audio_encoder_copy_red_unittest.cc | 11 +++++++++++ 16 files changed, 108 insertions(+) diff --git a/modules/audio_coding/BUILD.gn b/modules/audio_coding/BUILD.gn index 8efc22107d..ceee0c0f07 100644 --- a/modules/audio_coding/BUILD.gn +++ b/modules/audio_coding/BUILD.gn @@ -101,8 +101,10 @@ rtc_library("audio_encoder_cng") { deps = [ ":webrtc_cng", "../../api/audio_codecs:audio_codecs_api", + "../../api/units:time_delta", "../../common_audio", "../../rtc_base:checks", + "//third_party/abseil-cpp/absl/types:optional", ] } @@ -116,6 +118,7 @@ rtc_library("red") { deps = [ "../../api:array_view", "../../api/audio_codecs:audio_codecs_api", + "../../api/units:time_delta", "../../common_audio", "../../rtc_base:checks", "../../rtc_base:rtc_base_approved", @@ -137,8 +140,10 @@ rtc_library("g711") { ":legacy_encoded_audio_frame", "../../api:array_view", "../../api/audio_codecs:audio_codecs_api", + "../../api/units:time_delta", "../../rtc_base:checks", "../../rtc_base:rtc_base_approved", + "//third_party/abseil-cpp/absl/types:optional", ] public_deps = [ ":g711_c" ] # no-presubmit-check TODO(webrtc:8603) } @@ -167,8 +172,10 @@ rtc_library("g722") { "../../api:array_view", "../../api/audio_codecs:audio_codecs_api", "../../api/audio_codecs/g722:audio_encoder_g722_config", + "../../api/units:time_delta", "../../rtc_base:checks", "../../rtc_base:rtc_base_approved", + "//third_party/abseil-cpp/absl/types:optional", ] public_deps = [ ":g722_c" ] # no-presubmit-check TODO(webrtc:8603) } @@ -197,9 +204,11 @@ rtc_library("ilbc") { "../../api:array_view", "../../api/audio_codecs:audio_codecs_api", "../../api/audio_codecs/ilbc:audio_encoder_ilbc_config", + "../../api/units:time_delta", "../../common_audio", "../../rtc_base:checks", "../../rtc_base:rtc_base_approved", + "//third_party/abseil-cpp/absl/types:optional", ] public_deps = [ ":ilbc_c" ] # no-presubmit-check TODO(webrtc:8603) } @@ -372,6 +381,7 @@ rtc_source_set("isac_common") { ":isac_bwinfo", "../../api:scoped_refptr", "../../api/audio_codecs:audio_codecs_api", + "../../api/units:time_delta", "../../rtc_base:checks", "../../rtc_base:rtc_base_approved", "//third_party/abseil-cpp/absl/types:optional", @@ -771,6 +781,7 @@ rtc_library("webrtc_multiopus") { "../../api/audio_codecs:audio_codecs_api", "../../api/audio_codecs/opus:audio_decoder_opus_config", "../../api/audio_codecs/opus:audio_encoder_opus_config", + "../../api/units:time_delta", "../../rtc_base:checks", "../../rtc_base:logging", "../../rtc_base:macromagic", diff --git a/modules/audio_coding/codecs/cng/audio_encoder_cng.cc b/modules/audio_coding/codecs/cng/audio_encoder_cng.cc index 86d3f38b49..600cb0c06a 100644 --- a/modules/audio_coding/codecs/cng/audio_encoder_cng.cc +++ b/modules/audio_coding/codecs/cng/audio_encoder_cng.cc @@ -14,6 +14,8 @@ #include #include +#include "absl/types/optional.h" +#include "api/units/time_delta.h" #include "modules/audio_coding/codecs/cng/webrtc_cng.h" #include "rtc_base/checks.h" @@ -55,6 +57,8 @@ class AudioEncoderCng final : public AudioEncoder { void OnReceivedUplinkBandwidth( int target_audio_bitrate_bps, absl::optional bwe_period_ms) override; + absl::optional> GetFrameLengthRange() + const override; private: EncodedInfo EncodePassive(size_t frames_to_encode, rtc::Buffer* encoded); @@ -225,6 +229,11 @@ void AudioEncoderCng::OnReceivedUplinkBandwidth( bwe_period_ms); } +absl::optional> +AudioEncoderCng::GetFrameLengthRange() const { + return speech_encoder_->GetFrameLengthRange(); +} + AudioEncoder::EncodedInfo AudioEncoderCng::EncodePassive( size_t frames_to_encode, rtc::Buffer* encoded) { diff --git a/modules/audio_coding/codecs/cng/audio_encoder_cng_unittest.cc b/modules/audio_coding/codecs/cng/audio_encoder_cng_unittest.cc index 085deb1609..547feddbf9 100644 --- a/modules/audio_coding/codecs/cng/audio_encoder_cng_unittest.cc +++ b/modules/audio_coding/codecs/cng/audio_encoder_cng_unittest.cc @@ -21,8 +21,11 @@ #include "test/testsupport/rtc_expect_death.h" using ::testing::_; +using ::testing::Eq; using ::testing::InSequence; using ::testing::Invoke; +using ::testing::Not; +using ::testing::Optional; using ::testing::Return; using ::testing::SetArgPointee; @@ -233,6 +236,15 @@ TEST_F(AudioEncoderCngTest, CheckPacketLossFractionPropagation) { cng_->OnReceivedUplinkPacketLossFraction(0.5); } +TEST_F(AudioEncoderCngTest, CheckGetFrameLengthRangePropagation) { + CreateCng(MakeCngConfig()); + auto expected_range = + std::make_pair(TimeDelta::Millis(20), TimeDelta::Millis(20)); + EXPECT_CALL(*mock_encoder_, GetFrameLengthRange()) + .WillRepeatedly(Return(absl::make_optional(expected_range))); + EXPECT_THAT(cng_->GetFrameLengthRange(), Optional(Eq(expected_range))); +} + TEST_F(AudioEncoderCngTest, EncodeCallsVad) { EXPECT_CALL(*mock_encoder_, Num10MsFramesInNextPacket()) .WillRepeatedly(Return(1U)); diff --git a/modules/audio_coding/codecs/g711/audio_encoder_pcm.cc b/modules/audio_coding/codecs/g711/audio_encoder_pcm.cc index fd560e4df0..65e2da479d 100644 --- a/modules/audio_coding/codecs/g711/audio_encoder_pcm.cc +++ b/modules/audio_coding/codecs/g711/audio_encoder_pcm.cc @@ -89,6 +89,12 @@ void AudioEncoderPcm::Reset() { speech_buffer_.clear(); } +absl::optional> +AudioEncoderPcm::GetFrameLengthRange() const { + return {{TimeDelta::Millis(num_10ms_frames_per_packet_ * 10), + TimeDelta::Millis(num_10ms_frames_per_packet_ * 10)}}; +} + size_t AudioEncoderPcmA::EncodeCall(const int16_t* audio, size_t input_len, uint8_t* encoded) { diff --git a/modules/audio_coding/codecs/g711/audio_encoder_pcm.h b/modules/audio_coding/codecs/g711/audio_encoder_pcm.h index e41c2a3cc6..c4413f50a4 100644 --- a/modules/audio_coding/codecs/g711/audio_encoder_pcm.h +++ b/modules/audio_coding/codecs/g711/audio_encoder_pcm.h @@ -11,9 +11,12 @@ #ifndef MODULES_AUDIO_CODING_CODECS_G711_AUDIO_ENCODER_PCM_H_ #define MODULES_AUDIO_CODING_CODECS_G711_AUDIO_ENCODER_PCM_H_ +#include #include +#include "absl/types/optional.h" #include "api/audio_codecs/audio_encoder.h" +#include "api/units/time_delta.h" #include "rtc_base/constructor_magic.h" namespace webrtc { @@ -41,6 +44,8 @@ class AudioEncoderPcm : public AudioEncoder { size_t Max10MsFramesInAPacket() const override; int GetTargetBitrate() const override; void Reset() override; + absl::optional> GetFrameLengthRange() + const override; protected: AudioEncoderPcm(const Config& config, int sample_rate_hz); diff --git a/modules/audio_coding/codecs/g722/audio_encoder_g722.cc b/modules/audio_coding/codecs/g722/audio_encoder_g722.cc index d293163c53..b7d34ba581 100644 --- a/modules/audio_coding/codecs/g722/audio_encoder_g722.cc +++ b/modules/audio_coding/codecs/g722/audio_encoder_g722.cc @@ -79,6 +79,12 @@ void AudioEncoderG722Impl::Reset() { RTC_CHECK_EQ(0, WebRtcG722_EncoderInit(encoders_[i].encoder)); } +absl::optional> +AudioEncoderG722Impl::GetFrameLengthRange() const { + return {{TimeDelta::Millis(num_10ms_frames_per_packet_ * 10), + TimeDelta::Millis(num_10ms_frames_per_packet_ * 10)}}; +} + AudioEncoder::EncodedInfo AudioEncoderG722Impl::EncodeImpl( uint32_t rtp_timestamp, rtc::ArrayView audio, diff --git a/modules/audio_coding/codecs/g722/audio_encoder_g722.h b/modules/audio_coding/codecs/g722/audio_encoder_g722.h index cf45fb54be..c836503f2b 100644 --- a/modules/audio_coding/codecs/g722/audio_encoder_g722.h +++ b/modules/audio_coding/codecs/g722/audio_encoder_g722.h @@ -12,9 +12,12 @@ #define MODULES_AUDIO_CODING_CODECS_G722_AUDIO_ENCODER_G722_H_ #include +#include +#include "absl/types/optional.h" #include "api/audio_codecs/audio_encoder.h" #include "api/audio_codecs/g722/audio_encoder_g722_config.h" +#include "api/units/time_delta.h" #include "modules/audio_coding/codecs/g722/g722_interface.h" #include "rtc_base/buffer.h" #include "rtc_base/constructor_magic.h" @@ -33,6 +36,8 @@ class AudioEncoderG722Impl final : public AudioEncoder { size_t Max10MsFramesInAPacket() const override; int GetTargetBitrate() const override; void Reset() override; + absl::optional> GetFrameLengthRange() + const override; protected: EncodedInfo EncodeImpl(uint32_t rtp_timestamp, diff --git a/modules/audio_coding/codecs/ilbc/audio_encoder_ilbc.cc b/modules/audio_coding/codecs/ilbc/audio_encoder_ilbc.cc index ac9214b658..032de20246 100644 --- a/modules/audio_coding/codecs/ilbc/audio_encoder_ilbc.cc +++ b/modules/audio_coding/codecs/ilbc/audio_encoder_ilbc.cc @@ -127,6 +127,12 @@ void AudioEncoderIlbcImpl::Reset() { num_10ms_frames_buffered_ = 0; } +absl::optional> +AudioEncoderIlbcImpl::GetFrameLengthRange() const { + return {{TimeDelta::Millis(num_10ms_frames_per_packet_ * 10), + TimeDelta::Millis(num_10ms_frames_per_packet_ * 10)}}; +} + size_t AudioEncoderIlbcImpl::RequiredOutputSizeBytes() const { switch (num_10ms_frames_per_packet_) { case 2: diff --git a/modules/audio_coding/codecs/ilbc/audio_encoder_ilbc.h b/modules/audio_coding/codecs/ilbc/audio_encoder_ilbc.h index ada613bb64..fe3e32980e 100644 --- a/modules/audio_coding/codecs/ilbc/audio_encoder_ilbc.h +++ b/modules/audio_coding/codecs/ilbc/audio_encoder_ilbc.h @@ -11,8 +11,12 @@ #ifndef MODULES_AUDIO_CODING_CODECS_ILBC_AUDIO_ENCODER_ILBC_H_ #define MODULES_AUDIO_CODING_CODECS_ILBC_AUDIO_ENCODER_ILBC_H_ +#include + +#include "absl/types/optional.h" #include "api/audio_codecs/audio_encoder.h" #include "api/audio_codecs/ilbc/audio_encoder_ilbc_config.h" +#include "api/units/time_delta.h" #include "modules/audio_coding/codecs/ilbc/ilbc.h" #include "rtc_base/constructor_magic.h" @@ -32,6 +36,8 @@ class AudioEncoderIlbcImpl final : public AudioEncoder { rtc::ArrayView audio, rtc::Buffer* encoded) override; void Reset() override; + absl::optional> GetFrameLengthRange() + const override; private: size_t RequiredOutputSizeBytes() const; diff --git a/modules/audio_coding/codecs/isac/audio_encoder_isac_t.h b/modules/audio_coding/codecs/isac/audio_encoder_isac_t.h index c7b595107f..a3b8e76a30 100644 --- a/modules/audio_coding/codecs/isac/audio_encoder_isac_t.h +++ b/modules/audio_coding/codecs/isac/audio_encoder_isac_t.h @@ -11,10 +11,13 @@ #ifndef MODULES_AUDIO_CODING_CODECS_ISAC_AUDIO_ENCODER_ISAC_T_H_ #define MODULES_AUDIO_CODING_CODECS_ISAC_AUDIO_ENCODER_ISAC_T_H_ +#include #include +#include "absl/types/optional.h" #include "api/audio_codecs/audio_encoder.h" #include "api/scoped_refptr.h" +#include "api/units/time_delta.h" #include "rtc_base/constructor_magic.h" namespace webrtc { @@ -49,6 +52,8 @@ class AudioEncoderIsacT final : public AudioEncoder { rtc::ArrayView audio, rtc::Buffer* encoded) override; void Reset() override; + absl::optional> GetFrameLengthRange() + const override; private: // This value is taken from STREAM_SIZE_MAX_60 for iSAC float (60 ms) and diff --git a/modules/audio_coding/codecs/isac/audio_encoder_isac_t_impl.h b/modules/audio_coding/codecs/isac/audio_encoder_isac_t_impl.h index 27a02b5006..9ddb94326d 100644 --- a/modules/audio_coding/codecs/isac/audio_encoder_isac_t_impl.h +++ b/modules/audio_coding/codecs/isac/audio_encoder_isac_t_impl.h @@ -119,6 +119,13 @@ void AudioEncoderIsacT::Reset() { RecreateEncoderInstance(config_); } +template +absl::optional> +AudioEncoderIsacT::GetFrameLengthRange() const { + return {{TimeDelta::Millis(config_.frame_size_ms), + TimeDelta::Millis(config_.frame_size_ms)}}; +} + template void AudioEncoderIsacT::RecreateEncoderInstance(const Config& config) { RTC_CHECK(config.IsOk()); diff --git a/modules/audio_coding/codecs/opus/audio_encoder_multi_channel_opus_impl.cc b/modules/audio_coding/codecs/opus/audio_encoder_multi_channel_opus_impl.cc index 0614a0b48d..1feef3d359 100644 --- a/modules/audio_coding/codecs/opus/audio_encoder_multi_channel_opus_impl.cc +++ b/modules/audio_coding/codecs/opus/audio_encoder_multi_channel_opus_impl.cc @@ -164,6 +164,12 @@ void AudioEncoderMultiChannelOpusImpl::Reset() { RTC_CHECK(RecreateEncoderInstance(config_)); } +absl::optional> +AudioEncoderMultiChannelOpusImpl::GetFrameLengthRange() const { + return {{TimeDelta::Millis(config_.frame_size_ms), + TimeDelta::Millis(config_.frame_size_ms)}}; +} + // If the given config is OK, recreate the Opus encoder instance with those // settings, save the config, and return true. Otherwise, do nothing and return // false. diff --git a/modules/audio_coding/codecs/opus/audio_encoder_multi_channel_opus_impl.h b/modules/audio_coding/codecs/opus/audio_encoder_multi_channel_opus_impl.h index 593068c645..eadb4a6eb9 100644 --- a/modules/audio_coding/codecs/opus/audio_encoder_multi_channel_opus_impl.h +++ b/modules/audio_coding/codecs/opus/audio_encoder_multi_channel_opus_impl.h @@ -12,12 +12,14 @@ #define MODULES_AUDIO_CODING_CODECS_OPUS_AUDIO_ENCODER_MULTI_CHANNEL_OPUS_IMPL_H_ #include +#include #include #include "absl/types/optional.h" #include "api/audio_codecs/audio_encoder.h" #include "api/audio_codecs/audio_format.h" #include "api/audio_codecs/opus/audio_encoder_multi_channel_opus_config.h" +#include "api/units/time_delta.h" #include "modules/audio_coding/codecs/opus/opus_interface.h" #include "rtc_base/constructor_magic.h" @@ -44,6 +46,8 @@ class AudioEncoderMultiChannelOpusImpl final : public AudioEncoder { int GetTargetBitrate() const override; void Reset() override; + absl::optional> GetFrameLengthRange() + const override; protected: EncodedInfo EncodeImpl(uint32_t rtp_timestamp, diff --git a/modules/audio_coding/codecs/red/audio_encoder_copy_red.cc b/modules/audio_coding/codecs/red/audio_encoder_copy_red.cc index 05aaca17b9..e75806af10 100644 --- a/modules/audio_coding/codecs/red/audio_encoder_copy_red.cc +++ b/modules/audio_coding/codecs/red/audio_encoder_copy_red.cc @@ -132,4 +132,9 @@ void AudioEncoderCopyRed::OnReceivedUplinkBandwidth( bwe_period_ms); } +absl::optional> +AudioEncoderCopyRed::GetFrameLengthRange() const { + return speech_encoder_->GetFrameLengthRange(); +} + } // namespace webrtc diff --git a/modules/audio_coding/codecs/red/audio_encoder_copy_red.h b/modules/audio_coding/codecs/red/audio_encoder_copy_red.h index 2dc13dd405..c6e829eeb6 100644 --- a/modules/audio_coding/codecs/red/audio_encoder_copy_red.h +++ b/modules/audio_coding/codecs/red/audio_encoder_copy_red.h @@ -15,10 +15,12 @@ #include #include +#include #include "absl/types/optional.h" #include "api/array_view.h" #include "api/audio_codecs/audio_encoder.h" +#include "api/units/time_delta.h" #include "rtc_base/buffer.h" #include "rtc_base/constructor_magic.h" @@ -60,6 +62,8 @@ class AudioEncoderCopyRed final : public AudioEncoder { void OnReceivedUplinkBandwidth( int target_audio_bitrate_bps, absl::optional bwe_period_ms) override; + absl::optional> GetFrameLengthRange() + const override; protected: EncodedInfo EncodeImpl(uint32_t rtp_timestamp, diff --git a/modules/audio_coding/codecs/red/audio_encoder_copy_red_unittest.cc b/modules/audio_coding/codecs/red/audio_encoder_copy_red_unittest.cc index 648a88d021..e20515a165 100644 --- a/modules/audio_coding/codecs/red/audio_encoder_copy_red_unittest.cc +++ b/modules/audio_coding/codecs/red/audio_encoder_copy_red_unittest.cc @@ -20,9 +20,12 @@ #include "test/testsupport/rtc_expect_death.h" using ::testing::_; +using ::testing::Eq; using ::testing::InSequence; using ::testing::Invoke; using ::testing::MockFunction; +using ::testing::Not; +using ::testing::Optional; using ::testing::Return; using ::testing::SetArgPointee; @@ -107,6 +110,14 @@ TEST_F(AudioEncoderCopyRedTest, CheckPacketLossFractionPropagation) { red_->OnReceivedUplinkPacketLossFraction(0.5); } +TEST_F(AudioEncoderCopyRedTest, CheckGetFrameLengthRangePropagation) { + auto expected_range = + std::make_pair(TimeDelta::Millis(20), TimeDelta::Millis(20)); + EXPECT_CALL(*mock_encoder_, GetFrameLengthRange()) + .WillRepeatedly(Return(absl::make_optional(expected_range))); + EXPECT_THAT(red_->GetFrameLengthRange(), Optional(Eq(expected_range))); +} + // Checks that the an Encode() call is immediately propagated to the speech // encoder. TEST_F(AudioEncoderCopyRedTest, CheckImmediateEncode) {