Remove CNG state tracking from NetEq decision logic.

It seems like this is legacy and not useful. A comment mentions
transitioning between CNG and DTMF modes, but there is no way this can
happen currently.

Bug: webrtc:13322
Change-Id: I9e4706cb6ee145ee37a9e11e7cab6ea4ff697dc4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297980
Reviewed-by: Henrik Lundin <henrik.lundin@webrtc.org>
Commit-Queue: Jakob Ivarsson‎ <jakobi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39590}
This commit is contained in:
Jakob Ivarsson 2023-03-16 22:09:39 +01:00 committed by WebRTC LUCI CQ
parent 022d4ec34a
commit 63643357b4
6 changed files with 15 additions and 153 deletions

View File

@ -144,13 +144,6 @@ class NetEqController {
virtual bool SetBaseMinimumDelay(int delay_ms) = 0;
virtual int GetBaseMinimumDelay() const = 0;
// These methods test the `cng_state_` for different conditions.
virtual bool CngRfc3389On() const = 0;
virtual bool CngOff() const = 0;
// Resets the `cng_state_` to kCngOff.
virtual void SetCngOff() = 0;
// Reports back to DecisionLogic whether the decision to do expand remains or
// not. Note that this is necessary, since an expand decision can be changed
// to kNormal in NetEqImpl::GetDecision if there is still enough data in the

View File

@ -129,15 +129,6 @@ void DecisionLogic::SetSampleRate(int fs_hz, size_t output_size_samples) {
NetEq::Operation DecisionLogic::GetDecision(const NetEqStatus& status,
bool* reset_decoder) {
// If last mode was CNG (or Expand, since this could be covering up for
// a lost CNG packet), remember that CNG is on. This is needed if comfort
// noise is interrupted by DTMF.
if (status.last_mode == NetEq::Mode::kRfc3389Cng) {
cng_state_ = kCngRfc3389On;
} else if (status.last_mode == NetEq::Mode::kCodecInternalCng) {
cng_state_ = kCngInternalOn;
}
if (!IsExpand(status.last_mode) && !IsCng(status.last_mode)) {
last_playout_delay_ms_ = GetPlayoutDelayMs(status);
}
@ -311,22 +302,21 @@ NetEq::Operation DecisionLogic::CngOperation(
}
NetEq::Operation DecisionLogic::NoPacket(NetEqController::NetEqStatus status) {
if (cng_state_ == kCngRfc3389On) {
// Keep on playing comfort noise.
return NetEq::Operation::kRfc3389CngNoPacket;
} else if (cng_state_ == kCngInternalOn) {
// Stop CNG after a timeout.
if (config_.cng_timeout_ms &&
status.generated_noise_samples >
static_cast<size_t>(*config_.cng_timeout_ms * sample_rate_khz_)) {
return NetEq::Operation::kExpand;
switch (status.last_mode) {
case NetEq::Mode::kRfc3389Cng:
return NetEq::Operation::kRfc3389CngNoPacket;
case NetEq::Mode::kCodecInternalCng: {
// Stop CNG after a timeout.
if (config_.cng_timeout_ms &&
status.generated_noise_samples >
static_cast<size_t>(*config_.cng_timeout_ms * sample_rate_khz_)) {
return NetEq::Operation::kExpand;
}
return NetEq::Operation::kCodecInternalCng;
}
return NetEq::Operation::kCodecInternalCng;
} else if (status.play_dtmf) {
return NetEq::Operation::kDtmf;
} else {
// Nothing to play, do expand.
return NetEq::Operation::kExpand;
default:
return status.play_dtmf ? NetEq::Operation::kDtmf
: NetEq::Operation::kExpand;
}
}

View File

@ -58,13 +58,6 @@ class DecisionLogic : public NetEqController {
NetEq::Operation GetDecision(const NetEqController::NetEqStatus& status,
bool* reset_decoder) override;
// These methods test the `cng_state_` for different conditions.
bool CngRfc3389On() const override { return cng_state_ == kCngRfc3389On; }
bool CngOff() const override { return cng_state_ == kCngOff; }
// Resets the `cng_state_` to kCngOff.
void SetCngOff() override { cng_state_ = kCngOff; }
void ExpandDecision(NetEq::Operation operation) override {}
// Adds `value` to `sample_memory_`.
@ -111,8 +104,6 @@ class DecisionLogic : public NetEqController {
// The value 5 sets maximum time-stretch rate to about 100 ms/s.
static const int kMinTimescaleInterval = 5;
enum CngState { kCngOff, kCngRfc3389On, kCngInternalOn };
// Updates the `buffer_level_filter_` with the current buffer level
// `buffer_size_samples`.
void FilterBufferLevel(size_t buffer_size_samples);
@ -178,8 +169,6 @@ class DecisionLogic : public NetEqController {
const TickTimer* tick_timer_;
int sample_rate_khz_;
size_t output_size_samples_;
CngState cng_state_ = kCngOff; // Remember if comfort noise is interrupted by
// other event (e.g., DTMF).
size_t noise_fast_forward_ = 0;
size_t packet_length_samples_ = 0;
int sample_memory_ = 0;

View File

@ -36,9 +36,6 @@ class MockNetEqController : public NetEqController {
MOCK_METHOD(bool, SetMinimumDelay, (int delay_ms), (override));
MOCK_METHOD(bool, SetBaseMinimumDelay, (int delay_ms), (override));
MOCK_METHOD(int, GetBaseMinimumDelay, (), (const, override));
MOCK_METHOD(bool, CngRfc3389On, (), (const, override));
MOCK_METHOD(bool, CngOff, (), (const, override));
MOCK_METHOD(void, SetCngOff, (), (override));
MOCK_METHOD(void, ExpandDecision, (NetEq::Operation operation), (override));
MOCK_METHOD(void, AddSampleMemory, (int32_t value), (override));
MOCK_METHOD(int, TargetLevelMs, (), (const, override));

View File

@ -1046,7 +1046,7 @@ int NetEqImpl::GetDecision(Operation* operation,
controller_->noise_fast_forward()
: 0;
if (controller_->CngRfc3389On() || last_mode_ == Mode::kRfc3389Cng) {
if (last_mode_ == Mode::kRfc3389Cng) {
// Because of timestamp peculiarities, we have to "manually" disallow using
// a CNG packet with the same timestamp as the one that was last played.
// This can happen when using redundancy and will cause the timing to shift.
@ -1280,12 +1280,6 @@ int NetEqImpl::GetDecision(Operation* operation,
int extracted_samples = 0;
if (packet) {
sync_buffer_->IncreaseEndTimestamp(packet->timestamp - end_timestamp);
if (*operation != Operation::kRfc3389Cng) {
// We are about to decode and use a non-CNG packet.
controller_->SetCngOff();
}
extracted_samples = ExtractPackets(required_samples, packet_list);
if (extracted_samples < 0) {
return kPacketBufferCorruption;

View File

@ -1377,106 +1377,6 @@ TEST_F(NetEqImplTest, DecodingError) {
EXPECT_CALL(mock_decoder, Die());
}
// This test checks the behavior of NetEq when audio decoder fails during CNG.
TEST_F(NetEqImplTest, DecodingErrorDuringInternalCng) {
UseNoMocks();
// Create a mock decoder object.
MockAudioDecoder mock_decoder;
CreateInstance(
rtc::make_ref_counted<test::AudioDecoderProxyFactory>(&mock_decoder));
const uint8_t kPayloadType = 17; // Just an arbitrary number.
const int kSampleRateHz = 8000;
const int kDecoderErrorCode = -97; // Any negative number.
// We let decoder return 5 ms each time, and therefore, 2 packets make 10 ms.
const size_t kFrameLengthSamples =
static_cast<size_t>(5 * kSampleRateHz / 1000);
const size_t kPayloadLengthBytes = 1; // This can be arbitrary.
uint8_t payload[kPayloadLengthBytes] = {0};
RTPHeader rtp_header;
rtp_header.payloadType = kPayloadType;
rtp_header.sequenceNumber = 0x1234;
rtp_header.timestamp = 0x12345678;
rtp_header.ssrc = 0x87654321;
EXPECT_CALL(mock_decoder, Reset()).WillRepeatedly(Return());
EXPECT_CALL(mock_decoder, SampleRateHz())
.WillRepeatedly(Return(kSampleRateHz));
EXPECT_CALL(mock_decoder, Channels()).WillRepeatedly(Return(1));
EXPECT_CALL(mock_decoder, PacketDuration(_, _))
.WillRepeatedly(Return(rtc::checked_cast<int>(kFrameLengthSamples)));
EXPECT_CALL(mock_decoder, ErrorCode()).WillOnce(Return(kDecoderErrorCode));
int16_t dummy_output[kFrameLengthSamples] = {0};
{
InSequence sequence; // Dummy variable.
// Mock decoder works normally the first 2 times.
EXPECT_CALL(mock_decoder,
DecodeInternal(_, kPayloadLengthBytes, kSampleRateHz, _, _))
.Times(2)
.WillRepeatedly(
DoAll(SetArrayArgument<3>(dummy_output,
dummy_output + kFrameLengthSamples),
SetArgPointee<4>(AudioDecoder::kComfortNoise),
Return(rtc::checked_cast<int>(kFrameLengthSamples))))
.RetiresOnSaturation();
// Then mock decoder fails. A common reason for failure can be buffer being
// too short
EXPECT_CALL(mock_decoder, DecodeInternal(nullptr, 0, kSampleRateHz, _, _))
.WillOnce(Return(-1))
.RetiresOnSaturation();
// Mock decoder finally returns to normal.
EXPECT_CALL(mock_decoder, DecodeInternal(nullptr, 0, kSampleRateHz, _, _))
.Times(2)
.WillRepeatedly(
DoAll(SetArrayArgument<3>(dummy_output,
dummy_output + kFrameLengthSamples),
SetArgPointee<4>(AudioDecoder::kComfortNoise),
Return(rtc::checked_cast<int>(kFrameLengthSamples))));
}
EXPECT_TRUE(neteq_->RegisterPayloadType(kPayloadType,
SdpAudioFormat("l16", 8000, 1)));
// Insert 2 packets. This will make netEq into codec internal CNG mode.
for (int i = 0; i < 2; ++i) {
rtp_header.sequenceNumber += 1;
rtp_header.timestamp += kFrameLengthSamples;
EXPECT_EQ(NetEq::kOK, neteq_->InsertPacket(rtp_header, payload));
}
// Pull audio.
const size_t kMaxOutputSize = static_cast<size_t>(10 * kSampleRateHz / 1000);
AudioFrame output;
bool muted;
EXPECT_EQ(NetEq::kOK, neteq_->GetAudio(&output, &muted));
EXPECT_EQ(kMaxOutputSize, output.samples_per_channel_);
EXPECT_EQ(1u, output.num_channels_);
EXPECT_EQ(AudioFrame::kCNG, output.speech_type_);
// Pull audio again. Decoder fails.
EXPECT_EQ(NetEq::kFail, neteq_->GetAudio(&output, &muted));
EXPECT_EQ(kMaxOutputSize, output.samples_per_channel_);
EXPECT_EQ(1u, output.num_channels_);
// We are not expecting anything for output.speech_type_, since an error was
// returned.
// Pull audio again, should resume codec CNG.
EXPECT_EQ(NetEq::kOK, neteq_->GetAudio(&output, &muted));
EXPECT_EQ(kMaxOutputSize, output.samples_per_channel_);
EXPECT_EQ(1u, output.num_channels_);
EXPECT_EQ(AudioFrame::kCNG, output.speech_type_);
EXPECT_CALL(mock_decoder, Die());
}
// Tests that the return value from last_output_sample_rate_hz() is equal to the
// configured inital sample rate.
TEST_F(NetEqImplTest, InitialLastOutputSampleRate) {
@ -1783,7 +1683,6 @@ TEST_F(NetEqImplTest120ms, Merge) {
Register120msCodec(AudioDecoder::kSpeech);
CreateInstanceWithDelayManagerMock();
EXPECT_CALL(*mock_neteq_controller_, CngOff()).WillRepeatedly(Return(true));
InsertPacket(first_timestamp());
GetFirstPacket();