Enable continuous audio polling from ADM after StopPlay in VoIP API
Current VoIP Engine logic stops ADM from polling registered audio channel when caller invokes StopPlay which can leads to incoming RTP to be flushed and undesirable statistics report. Instead, VoipBase::StopPlay should silence the decoded audio sample from NetEq as muted to avoid mixing while allowing it go through prior process for correct ingress statistic values. The ADM stop playing logic will be triggered when all audio channels are released by VoipBase::ReleaseChannel API. Bug: webrtc:12121 Change-Id: I410eea4ea13f93acb465ab162a3c14c9819e2b92 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/191140 Commit-Queue: Tim Na <natim@webrtc.org> Reviewed-by: Per Åhgren <peah@webrtc.org> Cr-Commit-Position: refs/heads/master@{#32553}
This commit is contained in:
@ -73,6 +73,12 @@ AudioMixer::Source::AudioFrameInfo AudioIngress::GetAudioFrameWithInfo(
|
||||
constexpr double kAudioSampleDurationSeconds = 0.01;
|
||||
output_audio_level_.ComputeLevel(*audio_frame, kAudioSampleDurationSeconds);
|
||||
|
||||
// If caller invoked StopPlay(), then mute the frame.
|
||||
if (!playing_) {
|
||||
AudioFrameOperations::Mute(audio_frame);
|
||||
muted = true;
|
||||
}
|
||||
|
||||
// Set first rtp timestamp with first audio frame with valid timestamp.
|
||||
if (first_rtp_timestamp_ < 0 && audio_frame->timestamp_ != 0) {
|
||||
first_rtp_timestamp_ = audio_frame->timestamp_;
|
||||
@ -127,10 +133,6 @@ void AudioIngress::SetReceiveCodecs(
|
||||
}
|
||||
|
||||
void AudioIngress::ReceivedRTPPacket(rtc::ArrayView<const uint8_t> rtp_packet) {
|
||||
if (!IsPlaying()) {
|
||||
return;
|
||||
}
|
||||
|
||||
RtpPacketReceived rtp_packet_received;
|
||||
rtp_packet_received.Parse(rtp_packet.data(), rtp_packet.size());
|
||||
|
||||
|
@ -181,5 +181,48 @@ TEST_F(AudioIngressTest, PreferredSampleRate) {
|
||||
EXPECT_EQ(ingress_->PreferredSampleRate(), kPcmuFormat.clockrate_hz);
|
||||
}
|
||||
|
||||
// This test highlights the case where caller invokes StopPlay() which then
|
||||
// AudioIngress should play silence frame afterwards.
|
||||
TEST_F(AudioIngressTest, GetMutedAudioFrameAfterRtpReceivedAndStopPlay) {
|
||||
// StopPlay before we start sending RTP packet with sine wave.
|
||||
ingress_->StopPlay();
|
||||
|
||||
// Send 6 RTP packets to generate more than 100 ms audio sample to get
|
||||
// valid speech level.
|
||||
constexpr int kNumRtp = 6;
|
||||
int rtp_count = 0;
|
||||
rtc::Event event;
|
||||
auto handle_rtp = [&](const uint8_t* packet, size_t length, Unused) {
|
||||
ingress_->ReceivedRTPPacket(rtc::ArrayView<const uint8_t>(packet, length));
|
||||
if (++rtp_count == kNumRtp) {
|
||||
event.Set();
|
||||
}
|
||||
return true;
|
||||
};
|
||||
EXPECT_CALL(transport_, SendRtp).WillRepeatedly(Invoke(handle_rtp));
|
||||
for (int i = 0; i < kNumRtp * 2; i++) {
|
||||
egress_->SendAudioData(GetAudioFrame(i));
|
||||
fake_clock_.AdvanceTimeMilliseconds(10);
|
||||
}
|
||||
event.Wait(/*give_up_after_ms=*/1000);
|
||||
|
||||
for (int i = 0; i < kNumRtp * 2; ++i) {
|
||||
AudioFrame audio_frame;
|
||||
EXPECT_EQ(
|
||||
ingress_->GetAudioFrameWithInfo(kPcmuFormat.clockrate_hz, &audio_frame),
|
||||
AudioMixer::Source::AudioFrameInfo::kMuted);
|
||||
const int16_t* audio_data = audio_frame.data();
|
||||
size_t length =
|
||||
audio_frame.samples_per_channel_ * audio_frame.num_channels_;
|
||||
for (size_t j = 0; j < length; ++j) {
|
||||
EXPECT_EQ(audio_data[j], 0);
|
||||
}
|
||||
}
|
||||
|
||||
// Now we should still see valid speech output level as StopPlay won't affect
|
||||
// the measurement.
|
||||
EXPECT_EQ(ingress_->GetSpeechOutputLevelFullRange(), kAudioLevel);
|
||||
}
|
||||
|
||||
} // namespace
|
||||
} // namespace webrtc
|
||||
|
@ -161,8 +161,7 @@ void VoipCore::ReleaseChannel(ChannelId channel) {
|
||||
// Destroy channel outside of the lock.
|
||||
rtc::scoped_refptr<AudioChannel> audio_channel;
|
||||
|
||||
// Check if process thread is no longer needed.
|
||||
bool stop_process_thread = false;
|
||||
bool no_channels_after_release = false;
|
||||
|
||||
{
|
||||
MutexLock lock(&lock_);
|
||||
@ -173,18 +172,24 @@ void VoipCore::ReleaseChannel(ChannelId channel) {
|
||||
channels_.erase(iter);
|
||||
}
|
||||
|
||||
// Check if this is the last channel we have.
|
||||
stop_process_thread = channels_.empty();
|
||||
no_channels_after_release = channels_.empty();
|
||||
}
|
||||
|
||||
if (!audio_channel) {
|
||||
RTC_LOG(LS_WARNING) << "Channel " << channel << " not found";
|
||||
}
|
||||
|
||||
if (stop_process_thread) {
|
||||
if (no_channels_after_release) {
|
||||
// Release audio channel first to have it DeRegisterModule first.
|
||||
audio_channel = nullptr;
|
||||
process_thread_->Stop();
|
||||
|
||||
// Make sure to stop playout on ADM if it is playing.
|
||||
if (audio_device_module_->Playing()) {
|
||||
if (audio_device_module_->StopPlayout() != 0) {
|
||||
RTC_LOG(LS_WARNING) << "StopPlayout failed";
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -280,7 +285,15 @@ bool VoipCore::StopSend(ChannelId channel) {
|
||||
|
||||
bool VoipCore::StartPlayout(ChannelId channel) {
|
||||
auto audio_channel = GetChannel(channel);
|
||||
if (!audio_channel || !audio_channel->StartPlay()) {
|
||||
if (!audio_channel) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (audio_channel->IsPlaying()) {
|
||||
return true;
|
||||
}
|
||||
|
||||
if (!audio_channel->StartPlay()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
@ -305,24 +318,6 @@ bool VoipCore::StopPlayout(ChannelId channel) {
|
||||
|
||||
audio_channel->StopPlay();
|
||||
|
||||
bool stop_device = true;
|
||||
{
|
||||
MutexLock lock(&lock_);
|
||||
for (auto kv : channels_) {
|
||||
rtc::scoped_refptr<AudioChannel>& channel = kv.second;
|
||||
if (channel->IsPlaying()) {
|
||||
stop_device = false;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (stop_device && audio_device_module_->Playing()) {
|
||||
if (audio_device_module_->StopPlayout() != 0) {
|
||||
RTC_LOG(LS_ERROR) << "StopPlayout failed";
|
||||
return false;
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
|
Reference in New Issue
Block a user