From d35964a1ceb35dd9ab1c8aca1198b2b87221e4ff Mon Sep 17 00:00:00 2001 From: "pwestin@webrtc.org" Date: Tue, 30 Apr 2013 16:06:10 +0000 Subject: [PATCH] Fixing AV sync. Increased 2 const to allow for a bigger difference in AV sync. BUG=1711 Re-wrote the ComputeDelays to be readable and remove the possibilities of returning values lower than base_target_delay_ms R=mflodman@webrtc.org, mikhal@webrtc.org, niklas.enbom@webrtc.org Review URL: https://webrtc-codereview.appspot.com/1367004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@3922 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../main/source/jitter_buffer_common.h | 2 +- .../video_coding/main/source/timing.cc | 3 +- webrtc/video_engine/stream_synchronization.cc | 144 ++++++++---------- 3 files changed, 63 insertions(+), 86 deletions(-) diff --git a/webrtc/modules/video_coding/main/source/jitter_buffer_common.h b/webrtc/modules/video_coding/main/source/jitter_buffer_common.h index 0b62d2044b..e935c32f0a 100644 --- a/webrtc/modules/video_coding/main/source/jitter_buffer_common.h +++ b/webrtc/modules/video_coding/main/source/jitter_buffer_common.h @@ -17,7 +17,7 @@ namespace webrtc { enum { kMaxNumberOfFrames = 300 }; enum { kStartNumberOfFrames = 6 }; -enum { kMaxVideoDelayMs = 2000 }; +enum { kMaxVideoDelayMs = 10000 }; enum VCMJitterBufferEnum { kMaxConsecutiveOldFrames = 60, diff --git a/webrtc/modules/video_coding/main/source/timing.cc b/webrtc/modules/video_coding/main/source/timing.cc index 1e0475b607..6330913eb8 100644 --- a/webrtc/modules/video_coding/main/source/timing.cc +++ b/webrtc/modules/video_coding/main/source/timing.cc @@ -331,7 +331,8 @@ VCMTiming::TargetVideoDelay() const uint32_t VCMTiming::TargetDelayInternal() const { - return _requiredDelayMs + MaxDecodeTimeMs() + _renderDelayMs; + return std::max(_minTotalDelayMs, + _requiredDelayMs + MaxDecodeTimeMs() + _renderDelayMs); } } diff --git a/webrtc/video_engine/stream_synchronization.cc b/webrtc/video_engine/stream_synchronization.cc index 3e8d0eaafa..00154f8c19 100644 --- a/webrtc/video_engine/stream_synchronization.cc +++ b/webrtc/video_engine/stream_synchronization.cc @@ -29,14 +29,12 @@ struct ViESyncDelay { extra_video_delay_ms = 0; last_video_delay_ms = 0; extra_audio_delay_ms = 0; - last_sync_delay = 0; network_delay = 120; } int extra_video_delay_ms; int last_video_delay_ms; int extra_audio_delay_ms; - int last_sync_delay; int network_delay; }; @@ -92,6 +90,8 @@ bool StreamSynchronization::ComputeDelays(int relative_delay_ms, int* extra_audio_delay_ms, int* total_video_delay_target_ms) { assert(extra_audio_delay_ms && total_video_delay_target_ms); + + int current_video_delay_ms = *total_video_delay_target_ms; WEBRTC_TRACE(webrtc::kTraceInfo, webrtc::kTraceVideo, video_channel_id_, "Audio delay is: %d for voice channel: %d", current_audio_delay_ms, audio_channel_id_); @@ -104,7 +104,7 @@ bool StreamSynchronization::ComputeDelays(int relative_delay_ms, "Current diff is: %d for audio channel: %d", relative_delay_ms, audio_channel_id_); - int current_diff_ms = *total_video_delay_target_ms - current_audio_delay_ms + + int current_diff_ms = current_video_delay_ms - current_audio_delay_ms + relative_delay_ms; avg_diff_ms_ = ((kFilterLength - 1) * avg_diff_ms_ + @@ -119,110 +119,81 @@ bool StreamSynchronization::ComputeDelays(int relative_delay_ms, diff_ms = std::min(diff_ms, kMaxChangeMs); diff_ms = std::max(diff_ms, -kMaxChangeMs); - int video_delay_ms = base_target_delay_ms_; + // Reset the average after a move to prevent overshooting reaction. + avg_diff_ms_ = 0; + if (diff_ms > 0) { // The minimum video delay is longer than the current audio delay. - // We need to decrease extra video delay, if we have added extra delay - // earlier, or add extra audio delay. - if (channel_delay_->extra_video_delay_ms > 0) { + // We need to decrease extra video delay, or add extra audio delay. + if (channel_delay_->extra_video_delay_ms > base_target_delay_ms_) { // We have extra delay added to ViE. Reduce this delay before adding // extra delay to VoE. - - // This is the desired delay, we can't reduce more than this. - video_delay_ms = *total_video_delay_target_ms; - - // Check that we don't reduce the delay more than what is allowed. - if (video_delay_ms < channel_delay_->last_video_delay_ms - diff_ms) { - video_delay_ms = channel_delay_->last_video_delay_ms - diff_ms; - channel_delay_->extra_video_delay_ms = - video_delay_ms - *total_video_delay_target_ms; - } else { - channel_delay_->extra_video_delay_ms = 0; - } - channel_delay_->last_video_delay_ms = video_delay_ms; - channel_delay_->last_sync_delay = -1; + channel_delay_->extra_video_delay_ms -= diff_ms; channel_delay_->extra_audio_delay_ms = base_target_delay_ms_; } else { // channel_delay_->extra_video_delay_ms > 0 // We have no extra video delay to remove, increase the audio delay. - if (channel_delay_->last_sync_delay >= 0) { - // We have increased the audio delay earlier, increase it even more. - // Increase the audio delay. - channel_delay_->extra_audio_delay_ms += diff_ms; - - // Don't set a too high delay. - channel_delay_->extra_audio_delay_ms = std::min( - channel_delay_->extra_audio_delay_ms, - base_target_delay_ms_ + kMaxDeltaDelayMs); - - // Don't add any extra video delay. - video_delay_ms = *total_video_delay_target_ms; - channel_delay_->extra_video_delay_ms = 0; - channel_delay_->last_video_delay_ms = video_delay_ms; - channel_delay_->last_sync_delay = 1; - } else { // channel_delay_->last_sync_delay >= 0 - // First time after a delay change, don't add any extra delay. - // This is to not toggle back and forth too much. - channel_delay_->extra_audio_delay_ms = base_target_delay_ms_; - // Set minimum video delay - video_delay_ms = *total_video_delay_target_ms; - channel_delay_->extra_video_delay_ms = 0; - channel_delay_->last_video_delay_ms = video_delay_ms; - channel_delay_->last_sync_delay = 0; - } + channel_delay_->extra_audio_delay_ms += diff_ms; + channel_delay_->extra_video_delay_ms = base_target_delay_ms_; } } else { // if (diff_ms > 0) - // The minimum video delay is lower than the current audio delay. - // We need to decrease possible extra audio delay, or - // add extra video delay. + // The video delay is lower than the current audio delay. + // We need to decrease extra audio delay, or add extra video delay. if (channel_delay_->extra_audio_delay_ms > base_target_delay_ms_) { // We have extra delay in VoiceEngine. // Start with decreasing the voice delay. // Note: diff_ms is negative; add the negative difference. channel_delay_->extra_audio_delay_ms += diff_ms; - - if (channel_delay_->extra_audio_delay_ms < 0) { - // Negative values not allowed. - channel_delay_->extra_audio_delay_ms = base_target_delay_ms_; - channel_delay_->last_sync_delay = 0; - } else { - // There is more audio delay to use for the next round. - channel_delay_->last_sync_delay = 1; - } - - // Keep the video delay at the minimum values. - video_delay_ms = *total_video_delay_target_ms; - channel_delay_->extra_video_delay_ms = 0; - channel_delay_->last_video_delay_ms = video_delay_ms; - } else { // channel_delay_->extra_audio_delay_ms > 0 + channel_delay_->extra_video_delay_ms = base_target_delay_ms_; + } else { // channel_delay_->extra_audio_delay_ms > base_target_delay_ms_ // We have no extra delay in VoiceEngine, increase the video delay. + // Note: diff_ms is negative; subtract the negative difference. + channel_delay_->extra_video_delay_ms -= diff_ms; // X - (-Y) = X + Y. channel_delay_->extra_audio_delay_ms = base_target_delay_ms_; - - // This is the desired delay. - // Note: diff_ms is negative. - video_delay_ms = std::max(*total_video_delay_target_ms, - channel_delay_->last_video_delay_ms) - diff_ms; - - // Verify we don't go above the maximum allowed delay. - video_delay_ms = std::min(video_delay_ms, - base_target_delay_ms_ + kMaxDeltaDelayMs); - - // Store the values. - channel_delay_->extra_video_delay_ms = - video_delay_ms - *total_video_delay_target_ms; - channel_delay_->last_video_delay_ms = video_delay_ms; - channel_delay_->last_sync_delay = -1; } } - avg_diff_ms_ = 0; + + // Make sure that video is never below our target. + channel_delay_->extra_video_delay_ms = std::max( + channel_delay_->extra_video_delay_ms, base_target_delay_ms_); + + int new_video_delay_ms; + if (channel_delay_->extra_video_delay_ms > base_target_delay_ms_) { + new_video_delay_ms = channel_delay_->extra_video_delay_ms; + } else { + // No change to the extra video delay. We are changing audio and we only + // allow to change one at the time. + new_video_delay_ms = channel_delay_->last_video_delay_ms; + } + + // Make sure that we don't go below the extra video delay. + new_video_delay_ms = std::max( + new_video_delay_ms, channel_delay_->extra_video_delay_ms); + + // Verify we don't go above the maximum allowed video delay. + new_video_delay_ms = + std::min(new_video_delay_ms, base_target_delay_ms_ + kMaxDeltaDelayMs); + + // Make sure that audio is never below our target. + channel_delay_->extra_audio_delay_ms = + std::max(base_target_delay_ms_, channel_delay_->extra_audio_delay_ms); + + // Verify we don't go above the maximum allowed audio delay. + channel_delay_->extra_audio_delay_ms = std::min( + channel_delay_->extra_audio_delay_ms, + base_target_delay_ms_ + kMaxDeltaDelayMs); + + // Remember our last video delay. + channel_delay_->last_video_delay_ms = new_video_delay_ms; + WEBRTC_TRACE(webrtc::kTraceInfo, webrtc::kTraceVideo, video_channel_id_, "Sync video delay %d ms for video channel and audio delay %d for audio " "channel %d", - video_delay_ms, channel_delay_->extra_audio_delay_ms, audio_channel_id_); + new_video_delay_ms, channel_delay_->extra_audio_delay_ms, + audio_channel_id_); + // Return values. *extra_audio_delay_ms = channel_delay_->extra_audio_delay_ms; - video_delay_ms = std::max(video_delay_ms, 0); - *total_video_delay_target_ms = std::max(*total_video_delay_target_ms, - video_delay_ms); + *total_video_delay_target_ms = new_video_delay_ms; return true; } @@ -230,10 +201,15 @@ void StreamSynchronization::SetTargetBufferingDelay(int target_delay_ms) { // Initial extra delay for audio (accounting for existing extra delay). channel_delay_->extra_audio_delay_ms += target_delay_ms - base_target_delay_ms_; + // The video delay is compared to the last value (and how much we can update // is limited by that as well). channel_delay_->last_video_delay_ms += target_delay_ms - base_target_delay_ms_; + + channel_delay_->extra_video_delay_ms += + target_delay_ms - base_target_delay_ms_; + // Video is already delayed by the desired amount. base_target_delay_ms_ = target_delay_ms; }