Fix for unbounded increase in audio delay when no audio packets are flowing in
WebRTC’s Audio Video sync can go in unbounded loop and keep on increasing audio delay if audio packets stop coming in. The issue happens, if StreamSynchronization::ComputeDelays has: 1. relative_delay_ms = some positive value which causes avg_diff_ms_ > 30ms 2. current_audio_delay_ms < current_video_delay_ms 3. audio_delay_.extra_ms > 0 and video_delay_.extra_ms = 0 To compensate for relative delay, audio_delay_.extra_ms gets incremented every time StreamSynchronization::ComputeDelays is called by RtpStreamsSynchronizer::Process(), which happens every 1sec RtpStreamsSynchronizer::Process() will try to set the new delay to audio stream by calling syncable_audio_->SetMinimumPlayoutDelay(target_audio_delay_ms); This ends up calling DelayManager::SetMinimumDelay and update minimum_delay_ms_ But this update has no impact on the value returned by NetEqImpl::FilteredCurrentDelayMs (as there are no audio packets flowing in, hence neteq is not running) which is called next time RtpStreamsSynchronizer::Process(), runs and tried to compute the new audio delay (audio_info→current_delay_ms) This causes audio delay to be increased in every iteration and it grows unbounded. I guess it will stop growing above 10sec as that is hardcoded max delay in NetEQ. To avoid this added a check to not adjust delays when no new audio stream has come in. Bug: webrtc:11894 Change-Id: If648f9227e43c351f887d054876cb119cc1a917e Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/183340 Reviewed-by: Åsa Persson <asapersson@webrtc.org> Reviewed-by: Ivo Creusen <ivoc@webrtc.org> Commit-Queue: Shyam Sadhwani <shyamsadhwani@fb.com> Cr-Commit-Position: refs/heads/master@{#32106}
This commit is contained in:
parent
c6dbc5ee80
commit
986e745106
@ -89,11 +89,18 @@ void RtpStreamsSynchronizer::Process() {
|
||||
log_stats = true;
|
||||
}
|
||||
|
||||
int64_t last_audio_receive_time_ms =
|
||||
audio_measurement_.latest_receive_time_ms;
|
||||
absl::optional<Syncable::Info> audio_info = syncable_audio_->GetInfo();
|
||||
if (!audio_info || !UpdateMeasurements(&audio_measurement_, *audio_info)) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (last_audio_receive_time_ms == audio_measurement_.latest_receive_time_ms) {
|
||||
// No new audio packet has been received since last update.
|
||||
return;
|
||||
}
|
||||
|
||||
int64_t last_video_receive_ms = video_measurement_.latest_receive_time_ms;
|
||||
absl::optional<Syncable::Info> video_info = syncable_video_->GetInfo();
|
||||
if (!video_info || !UpdateMeasurements(&video_measurement_, *video_info)) {
|
||||
|
||||
@ -92,11 +92,18 @@ void RtpStreamsSynchronizer::UpdateDelay() {
|
||||
log_stats = true;
|
||||
}
|
||||
|
||||
int64_t last_audio_receive_time_ms =
|
||||
audio_measurement_.latest_receive_time_ms;
|
||||
absl::optional<Syncable::Info> audio_info = syncable_audio_->GetInfo();
|
||||
if (!audio_info || !UpdateMeasurements(&audio_measurement_, *audio_info)) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (last_audio_receive_time_ms == audio_measurement_.latest_receive_time_ms) {
|
||||
// No new audio packet has been received since last update.
|
||||
return;
|
||||
}
|
||||
|
||||
int64_t last_video_receive_ms = video_measurement_.latest_receive_time_ms;
|
||||
absl::optional<Syncable::Info> video_info = syncable_video_->GetInfo();
|
||||
if (!video_info || !UpdateMeasurements(&video_measurement_, *video_info)) {
|
||||
|
||||
@ -383,6 +383,63 @@ TEST_F(StreamSynchronizationTest, AudioDelayed) {
|
||||
total_audio_delay_ms);
|
||||
}
|
||||
|
||||
TEST_F(StreamSynchronizationTest, NoAudioIncomingUnboundedIncrease) {
|
||||
// Test how audio delay can grow unbounded when audio stops coming in.
|
||||
// This is handled in caller of RtpStreamsSynchronizer, for example in
|
||||
// RtpStreamsSynchronizer by not updating delays when audio samples stop
|
||||
// coming in.
|
||||
const int kVideoDelayMs = 300;
|
||||
const int kAudioDelayMs = 100;
|
||||
int current_audio_delay_ms = kAudioDelayMs;
|
||||
int total_audio_delay_ms = 0;
|
||||
int total_video_delay_ms = 0;
|
||||
|
||||
EXPECT_TRUE(DelayedStreams(/*audio_delay_ms=*/0, kVideoDelayMs,
|
||||
current_audio_delay_ms, &total_audio_delay_ms,
|
||||
&total_video_delay_ms));
|
||||
EXPECT_EQ(0, total_video_delay_ms);
|
||||
// The delay is not allowed to change more than this.
|
||||
EXPECT_EQ((kVideoDelayMs - kAudioDelayMs) / kSmoothingFilter,
|
||||
total_audio_delay_ms);
|
||||
int last_total_audio_delay_ms = total_audio_delay_ms;
|
||||
|
||||
// Set new current audio delay: simulate audio samples are flowing in.
|
||||
current_audio_delay_ms = total_audio_delay_ms;
|
||||
|
||||
clock_sender_.AdvanceTimeMilliseconds(1000);
|
||||
clock_receiver_.AdvanceTimeMilliseconds(1000);
|
||||
EXPECT_TRUE(DelayedStreams(/*audio_delay_ms=*/0, kVideoDelayMs,
|
||||
current_audio_delay_ms, &total_audio_delay_ms,
|
||||
&total_video_delay_ms));
|
||||
EXPECT_EQ(0, total_video_delay_ms);
|
||||
EXPECT_EQ(last_total_audio_delay_ms +
|
||||
MaxAudioDelayChangeMs(current_audio_delay_ms, kVideoDelayMs),
|
||||
total_audio_delay_ms);
|
||||
last_total_audio_delay_ms = total_audio_delay_ms;
|
||||
|
||||
// Simulate no incoming audio by not update audio delay.
|
||||
const int kSimulationSecs = 300; // 5min
|
||||
const int kMaxDeltaDelayMs = 10000; // max delay for audio in webrtc
|
||||
for (auto time_secs = 0; time_secs < kSimulationSecs; time_secs++) {
|
||||
clock_sender_.AdvanceTimeMilliseconds(1000);
|
||||
clock_receiver_.AdvanceTimeMilliseconds(1000);
|
||||
EXPECT_TRUE(DelayedStreams(/*audio_delay_ms=*/0, kVideoDelayMs,
|
||||
current_audio_delay_ms, &total_audio_delay_ms,
|
||||
&total_video_delay_ms));
|
||||
EXPECT_EQ(0, total_video_delay_ms);
|
||||
|
||||
// Audio delay does not go above kMaxDeltaDelayMs.
|
||||
EXPECT_EQ(std::min(kMaxDeltaDelayMs,
|
||||
last_total_audio_delay_ms +
|
||||
MaxAudioDelayChangeMs(current_audio_delay_ms,
|
||||
kVideoDelayMs)),
|
||||
total_audio_delay_ms);
|
||||
last_total_audio_delay_ms = total_audio_delay_ms;
|
||||
}
|
||||
// By now the audio delay has grown unbounded to kMaxDeltaDelayMs.
|
||||
EXPECT_EQ(kMaxDeltaDelayMs, last_total_audio_delay_ms);
|
||||
}
|
||||
|
||||
TEST_F(StreamSynchronizationTest, BothDelayedVideoLater) {
|
||||
BothDelayedVideoLaterTest(0);
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user