From ed865b5d460ae0046e5a56a22a7041d533cbdb02 Mon Sep 17 00:00:00 2001 From: "henrik.lundin@webrtc.org" Date: Thu, 6 Mar 2014 10:28:07 +0000 Subject: [PATCH] NetEq4: Changing the behavior of playout_timestamp_ update The variable playout_timestamp_ was not updated to the latest decoded timestamp while comfort noise was played. Instead, it was upadted using dead reckoning, which caused it to drift away from the timestamps of the incoming CNG packets. Now it is updated also during comfort noise playout. Since the change is only in NetEq4, this change also makes the test PlaysOutAudioAndVideoInSync use both ACM1/NetEq3 and ACM2/NetEq4. Re-enabling one NetEq unit test that is no longer failing thanks to this CL. BUG=2932 R=stefan@webrtc.org, turaj@webrtc.org Review URL: https://webrtc-codereview.appspot.com/8799004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@5649 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../modules/audio_coding/neteq4/neteq_impl.cc | 8 +-- .../audio_coding/neteq4/neteq_unittest.cc | 5 +- webrtc/video/call_perf_tests.cc | 54 +++++++++++++++---- 3 files changed, 51 insertions(+), 16 deletions(-) diff --git a/webrtc/modules/audio_coding/neteq4/neteq_impl.cc b/webrtc/modules/audio_coding/neteq4/neteq_impl.cc index d6fce18cc..4ffe3e79e 100644 --- a/webrtc/modules/audio_coding/neteq4/neteq_impl.cc +++ b/webrtc/modules/audio_coding/neteq4/neteq_impl.cc @@ -833,10 +833,10 @@ int NetEqImpl::GetAudioInternal(size_t max_length, int16_t* output, sync_buffer_->set_dtmf_index(sync_buffer_->Size()); } - if ((last_mode_ != kModeExpand) && (last_mode_ != kModeRfc3389Cng)) { - // If last operation was neither expand, nor comfort noise, calculate the - // |playout_timestamp_| from the |sync_buffer_|. However, do not update the - // |playout_timestamp_| if it would be moved "backwards". + if (last_mode_ != kModeExpand) { + // If last operation was not expand, calculate the |playout_timestamp_| from + // the |sync_buffer_|. However, do not update the |playout_timestamp_| if it + // would be moved "backwards". uint32_t temp_timestamp = sync_buffer_->end_timestamp() - static_cast(sync_buffer_->FutureLength()); if (static_cast(temp_timestamp - playout_timestamp_) > 0) { diff --git a/webrtc/modules/audio_coding/neteq4/neteq_unittest.cc b/webrtc/modules/audio_coding/neteq4/neteq_unittest.cc index 47e9e855e..b3372766b 100644 --- a/webrtc/modules/audio_coding/neteq4/neteq_unittest.cc +++ b/webrtc/modules/audio_coding/neteq4/neteq_unittest.cc @@ -788,15 +788,14 @@ void NetEqDecodingTest::LongCngWithClockDrift(double drift_factor) { EXPECT_GE(delay_after, delay_before - 20 * 16); } -TEST_F(NetEqDecodingTest, DISABLED_ON_ANDROID(LongCngWithClockNegativeDrift)) { +TEST_F(NetEqDecodingTest, DISABLED_ON_ANDROID(LongCngWithNegativeClockDrift)) { // Apply a clock drift of -25 ms / s (sender faster than receiver). const double kDriftFactor = 1000.0 / (1000.0 + 25.0); LongCngWithClockDrift(kDriftFactor); } // TODO(hlundin): Re-enable this test and fix the issues to make it pass. -TEST_F(NetEqDecodingTest, - DISABLED_ON_ANDROID(DISABLED_LongCngWithClockPositiveDrift)) { +TEST_F(NetEqDecodingTest, DISABLED_ON_ANDROID(LongCngWithPositiveClockDrift)) { // Apply a clock drift of +25 ms / s (sender slower than receiver). const double kDriftFactor = 1000.0 / (1000.0 - 25.0); LongCngWithClockDrift(kDriftFactor); diff --git a/webrtc/video/call_perf_tests.cc b/webrtc/video/call_perf_tests.cc index e4de87775..59f119a85 100644 --- a/webrtc/video/call_perf_tests.cc +++ b/webrtc/video/call_perf_tests.cc @@ -16,6 +16,8 @@ #include "testing/gtest/include/gtest/gtest.h" #include "webrtc/call.h" +#include "webrtc/common.h" +#include "webrtc/modules/audio_coding/main/interface/audio_coding_module.h" #include "webrtc/modules/remote_bitrate_estimator/include/rtp_to_ntp.h" #include "webrtc/modules/rtp_rtcp/interface/rtp_header_parser.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_utility.h" @@ -151,14 +153,16 @@ class VideoRtcpAndSyncObserver : public SyncRtcpObserver, public VideoRenderer { VideoRtcpAndSyncObserver(Clock* clock, int voe_channel, VoEVideoSync* voe_sync, - SyncRtcpObserver* audio_observer) + SyncRtcpObserver* audio_observer, + bool using_new_acm) : SyncRtcpObserver(FakeNetworkPipe::Config()), clock_(clock), voe_channel_(voe_channel), voe_sync_(voe_sync), audio_observer_(audio_observer), creation_time_ms_(clock_->TimeInMilliseconds()), - first_time_in_sync_(-1) {} + first_time_in_sync_(-1), + using_new_acm_(using_new_acm) {} virtual void RenderFrame(const I420VideoFrame& video_frame, int time_to_render_ms) OVERRIDE { @@ -177,8 +181,16 @@ class VideoRtcpAndSyncObserver : public SyncRtcpObserver, public VideoRenderer { int64_t stream_offset = latest_audio_ntp - latest_video_ntp; std::stringstream ss; ss << stream_offset; - webrtc::test::PrintResult( - "stream_offset", "", "synchronization", ss.str(), "ms", false); + std::stringstream acm_type; + if (using_new_acm_) { + acm_type << "_acm2"; + } + webrtc::test::PrintResult("stream_offset", + acm_type.str(), + "synchronization", + ss.str(), + "ms", + false); int64_t time_since_creation = now_ms - creation_time_ms_; // During the first couple of seconds audio and video can falsely be // estimated as being synchronized. We don't want to trigger on those. @@ -188,7 +200,7 @@ class VideoRtcpAndSyncObserver : public SyncRtcpObserver, public VideoRenderer { if (first_time_in_sync_ == -1) { first_time_in_sync_ = now_ms; webrtc::test::PrintResult("sync_convergence_time", - "", + acm_type.str(), "synchronization", time_since_creation, "ms", @@ -206,9 +218,19 @@ class VideoRtcpAndSyncObserver : public SyncRtcpObserver, public VideoRenderer { SyncRtcpObserver* audio_observer_; int64_t creation_time_ms_; int64_t first_time_in_sync_; + bool using_new_acm_; }; -TEST_F(CallPerfTest, PlaysOutAudioAndVideoInSync) { +class ParamCallPerfTest : public CallPerfTest, + public ::testing::WithParamInterface { + public: + ParamCallPerfTest() : CallPerfTest(), use_new_acm_(GetParam()) {} + + protected: + bool use_new_acm_; +}; + +TEST_P(ParamCallPerfTest, PlaysOutAudioAndVideoInSync) { VoiceEngine* voice_engine = VoiceEngine::Create(); VoEBase* voe_base = VoEBase::GetInterface(voice_engine); VoECodec* voe_codec = VoECodec::GetInterface(voice_engine); @@ -220,13 +242,24 @@ TEST_F(CallPerfTest, PlaysOutAudioAndVideoInSync) { test::FakeAudioDevice fake_audio_device(Clock::GetRealTimeClock(), audio_filename); EXPECT_EQ(0, voe_base->Init(&fake_audio_device, NULL)); - int channel = voe_base->CreateChannel(); + Config config; + if (use_new_acm_) { + config.Set( + new webrtc::NewAudioCodingModuleFactory()); + } else { + config.Set( + new webrtc::AudioCodingModuleFactory()); + } + int channel = voe_base->CreateChannel(config); FakeNetworkPipe::Config net_config; net_config.queue_delay_ms = 500; SyncRtcpObserver audio_observer(net_config); - VideoRtcpAndSyncObserver observer( - Clock::GetRealTimeClock(), channel, voe_sync, &audio_observer); + VideoRtcpAndSyncObserver observer(Clock::GetRealTimeClock(), + channel, + voe_sync, + &audio_observer, + use_new_acm_); Call::Config receiver_config(observer.ReceiveTransport()); receiver_config.voice_engine = voice_engine; @@ -329,6 +362,9 @@ TEST_F(CallPerfTest, PlaysOutAudioAndVideoInSync) { VoiceEngine::Delete(voice_engine); } +// Test with both ACM1 and ACM2. +INSTANTIATE_TEST_CASE_P(SwitchAcm, ParamCallPerfTest, ::testing::Bool()); + TEST_F(CallPerfTest, RegisterCpuOveruseObserver) { // Verifies that either a normal or overuse callback is triggered. class OveruseCallbackObserver : public test::RtpRtcpObserver,