From 90cf20d761d2218cb0338b7966696209634b712d Mon Sep 17 00:00:00 2001 From: bodamnam Date: Mon, 20 Feb 2023 08:37:19 +0000 Subject: Fix jitter/loss calculation failure 1) Fix the incorrect lost packet count 2) Modify to run jitter/loss notification checking routine before checking the other notificaiton condition 3) Add jitter reset operation when the ssrc changed Bug: 268602328 Test: atest ImsMediaNativeTests, Verified with the test case LTE_BTR_5_7160-4. Verified voice call in live network. Change-Id: I5eb5c2a10be5722c988d05b3828eca6c08d47f58 --- .../core/audio/MediaQualityAnalyzer.cpp | 82 ++++++++++------- .../lib/libimsmedia/core/nodes/RtpEncoderNode.cpp | 2 - .../core/audio/MediaQualityAnalyzerTest.cpp | 101 ++++++++++++++------- 3 files changed, 115 insertions(+), 70 deletions(-) diff --git a/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/audio/MediaQualityAnalyzer.cpp b/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/audio/MediaQualityAnalyzer.cpp index 55c2568b..491d9172 100644 --- a/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/audio/MediaQualityAnalyzer.cpp +++ b/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/audio/MediaQualityAnalyzer.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #define DEFAULT_PARAM (-1) #define DEFAULT_INACTIVITY_TIME_FOR_CALL_QUALITY (4) @@ -155,12 +156,7 @@ void MediaQualityAnalyzer::collectInfo(const int32_t streamType, RtpPacket* pack } else if (streamType == kStreamRtpRx && packet != nullptr) { - if (mSSRC != DEFAULT_PARAM && mSSRC != packet->ssrc) - { - IMLOGW0("[collectInfo] ssrc changed"); - } - - // for call qualty report + // for call quality report mCallQuality.setNumRtpPacketsReceived(mCallQuality.getNumRtpPacketsReceived() + 1); mCallQualitySumRelativeJitter += packet->jitter; @@ -186,8 +182,8 @@ void MediaQualityAnalyzer::collectInfo(const int32_t streamType, RtpPacket* pack break; } - // for loss rate, jitter check - if (mSSRC == DEFAULT_PARAM) // stream is reset + // for jitter check + if (mSSRC != packet->ssrc) // stream is reset { mJitterRxPacket = std::abs(packet->jitter); // update rtcp-xr params @@ -424,7 +420,7 @@ void MediaQualityAnalyzer::processMediaQuality() if (mPacketLossDuration != 0 && !mListLostPacket.empty()) { - // calculate loss in duration + // counts received packets for the duration int32_t numReceivedPacketsInDuration = std::count_if(mListRxPacket.begin(), mListRxPacket.end(), [=](RtpPacket* packet) @@ -433,12 +429,21 @@ void MediaQualityAnalyzer::processMediaQuality() mPacketLossDuration); }); + // cumulates the number of lost packets for the duration + std::list listLostPacketInDuration; + std::copy_if(mListLostPacket.begin(), mListLostPacket.end(), + std::back_inserter(listLostPacketInDuration), + [=](LostPacket* packet) + { + return (ImsMediaTimer::GetTimeInMilliSeconds() - packet->markedTime <= + mPacketLossDuration); + }); + int32_t numLostPacketsInDuration = - std::count_if(mListLostPacket.begin(), mListLostPacket.end(), - [=](LostPacket* packet) + std::accumulate(begin(listLostPacketInDuration), end(listLostPacketInDuration), 0, + [=](int i, const LostPacket* packet) { - return (ImsMediaTimer::GetTimeInMilliSeconds() - packet->markedTime <= - mPacketLossDuration); + return packet->numLoss + i; }); if (numLostPacketsInDuration == 0 || numReceivedPacketsInDuration == 0) @@ -450,8 +455,8 @@ void MediaQualityAnalyzer::processMediaQuality() int32_t lossRate = numLostPacketsInDuration * 100 / (numReceivedPacketsInDuration + numLostPacketsInDuration); - IMLOGD3("[processData] mediaQualtyStatus lossRate[%d], received[%d], lost[%d]", - lossRate, numReceivedPacketsInDuration, numLostPacketsInDuration); + IMLOGD3("[processMediaQuality] lossRate[%d], received[%d], lost[%d]", lossRate, + numReceivedPacketsInDuration, numLostPacketsInDuration); mQualityStatus.setRtpPacketLossRate(lossRate); } } @@ -460,6 +465,27 @@ void MediaQualityAnalyzer::processMediaQuality() mQualityStatus.setRtpPacketLossRate(0); } + bool shouldNotify = false; + + // check jitter notification + if (!mJitterThreshold.empty()) + { + if (mJitterChecker.checkNotifiable(mJitterThreshold, mQualityStatus.getRtpJitterMillis())) + { + shouldNotify = true; + } + } + + // check packet loss notification + if (!mPacketLossThreshold.empty()) + { + if (mPacketLossChecker.checkNotifiable( + mPacketLossThreshold, mQualityStatus.getRtpPacketLossRate())) + { + shouldNotify = true; + } + } + IMLOGD_PACKET4(IM_PACKET_LOG_RTP, "[processMediaQuality] rtpInactivity[%d], rtcpInactivity[%d], lossRate[%d], " "jitter[%d]", @@ -499,25 +525,9 @@ void MediaQualityAnalyzer::processMediaQuality() return; } - // check jitter notification - if (!mJitterThreshold.empty()) - { - if (mJitterChecker.checkNotifiable(mJitterThreshold, mQualityStatus.getRtpJitterMillis())) - { - notifyMediaQualityStatus(); - return; - } - } - - // check packet loss notification - if (!mPacketLossThreshold.empty()) + if (shouldNotify) { - if (mPacketLossChecker.checkNotifiable( - mPacketLossThreshold, mQualityStatus.getRtpPacketLossRate())) - { - notifyMediaQualityStatus(); - return; - } + notifyMediaQualityStatus(); } } @@ -585,7 +595,11 @@ uint32_t MediaQualityAnalyzer::getTxPacketSize() uint32_t MediaQualityAnalyzer::getLostPacketSize() { - return mListLostPacket.size(); + return std::accumulate(begin(mListLostPacket), end(mListLostPacket), 0, + [](int i, const LostPacket* packet) + { + return packet->numLoss + i; + }); } void MediaQualityAnalyzer::SendEvent(uint32_t event, uint64_t paramA, uint64_t paramB) diff --git a/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/nodes/RtpEncoderNode.cpp b/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/nodes/RtpEncoderNode.cpp index bcd35a6d..63436f91 100644 --- a/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/nodes/RtpEncoderNode.cpp +++ b/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/nodes/RtpEncoderNode.cpp @@ -480,8 +480,6 @@ bool RtpEncoderNode::ProcessAudioData(ImsMediaSubType subtype, uint8_t* data, ui } else if (timeDiff == 0) { - IMLOGD_PACKET2(IM_PACKET_LOG_RTP, "[ProcessAudioData] skip, prev[%u] curr[%u]", - mPrevTimestamp, currentTimestamp); return false; } else diff --git a/tests/native/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/audio/MediaQualityAnalyzerTest.cpp b/tests/native/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/audio/MediaQualityAnalyzerTest.cpp index a94b6654..bedb63bb 100644 --- a/tests/native/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/audio/MediaQualityAnalyzerTest.cpp +++ b/tests/native/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/audio/MediaQualityAnalyzerTest.cpp @@ -19,7 +19,6 @@ #include #include #include -#include #include #include @@ -100,13 +99,7 @@ public: } } - virtual void onEvent(int32_t type, uint64_t param1, uint64_t param2) - { - (void)type; - (void)param1; - (void)param2; - } - + virtual void onEvent(int32_t /* type */, uint64_t /* param1 */, uint64_t /* param2 */) {} CallQuality getCallQuality() { return mCallQuality; } MediaQualityStatus getMediaQualityStatus() { return mStatus; } @@ -115,6 +108,48 @@ private: MediaQualityStatus mStatus; }; +class FakeMediaQualityAnalyzer : public MediaQualityAnalyzer +{ +public: + FakeMediaQualityAnalyzer() : + MediaQualityAnalyzer() + { + counter = 0; + } + virtual ~FakeMediaQualityAnalyzer() {} + + virtual void start() + { + mCallQuality.setCodecType(convertAudioCodecType( + mCodecType, ImsMediaAudioUtil::FindMaxEvsBandwidthFromRange(mCodecAttribute))); + } + + virtual void stop() + { + notifyCallQuality(); + reset(); + } + + void testProcessCycle(const int32_t numCycle) + { + for (int i = 0; i < numCycle; i++) + { + while (!mListevent.empty()) + { + processEvent(mListevent.front(), mListParamA.front(), mListParamB.front()); + mListevent.pop_front(); + mListParamA.pop_front(); + mListParamB.pop_front(); + } + + processData(++counter); + } + } + +private: + int32_t counter; +}; + class MediaQualityAnalyzerTest : public ::testing::Test { public: @@ -122,21 +157,20 @@ public: virtual ~MediaQualityAnalyzerTest() {} protected: - MediaQualityAnalyzer* mAnalyzer; + FakeMediaQualityAnalyzer* mAnalyzer; AudioConfig mConfig; RtcpConfig mRtcpConfig; AmrParams mAmrParam; EvsParams mEvsParam; FakeMediaQualityCallback mFakeCallback; MockBaseSessionCallback mCallback; - ImsMediaCondition mCondition; virtual void SetUp() override { mCallback.SetDelegate(&mFakeCallback); mCallback.DelegateToFake(); - mAnalyzer = new MediaQualityAnalyzer(); + mAnalyzer = new FakeMediaQualityAnalyzer(); mRtcpConfig.setCanonicalName(kCanonicalName); mRtcpConfig.setTransmitPort(kTransmitPort); mRtcpConfig.setIntervalSec(kIntervalSec); @@ -172,7 +206,6 @@ protected: mAnalyzer->setCallback(&mCallback); mAnalyzer->setConfig(&mConfig); - mCondition.reset(); } virtual void TearDown() override @@ -243,7 +276,8 @@ TEST_F(MediaQualityAnalyzerTest, TestCollectTxPackets) mAnalyzer->SendEvent(kCollectPacketInfo, kStreamRtpTx, reinterpret_cast(packet)); } - mCondition.wait_timeout(1100); // 1.1 sec + mAnalyzer->testProcessCycle(1); + EXPECT_EQ(mAnalyzer->getTxPacketSize(), numPackets); EXPECT_EQ(mAnalyzer->getRxPacketSize(), 0); EXPECT_EQ(mAnalyzer->getLostPacketSize(), 0); @@ -265,7 +299,7 @@ TEST_F(MediaQualityAnalyzerTest, TestRtpInactivityNotRunning) threshold.setRtpInactivityTimerMillis(std::vector{0}); mAnalyzer->setMediaQualityThreshold(threshold); mAnalyzer->start(); - mCondition.wait_timeout(2100); // 2.1 sec + mAnalyzer->testProcessCycle(2); mAnalyzer->stop(); } @@ -277,13 +311,13 @@ TEST_F(MediaQualityAnalyzerTest, TestRtpInactivityRunning) threshold.setRtpInactivityTimerMillis(kRtpInactivityTimerMillis); mAnalyzer->setMediaQualityThreshold(threshold); mAnalyzer->start(); - mCondition.wait_timeout(2100); // 2.1 sec + mAnalyzer->testProcessCycle(2); // Check MediaQualityStatus value MediaQualityStatus quality1 = mFakeCallback.getMediaQualityStatus(); EXPECT_EQ(quality1.getRtpInactivityTimeMillis(), 2000); - mCondition.wait_timeout(2100); // 2.1 sec + mAnalyzer->testProcessCycle(2); // Check MediaQualityStatus value MediaQualityStatus quality2 = mFakeCallback.getMediaQualityStatus(); @@ -293,7 +327,7 @@ TEST_F(MediaQualityAnalyzerTest, TestRtpInactivityRunning) packet->seqNum = 0; mAnalyzer->SendEvent(kCollectPacketInfo, kStreamRtpRx, reinterpret_cast(packet)); - mCondition.wait_timeout(3100); // 3.1 sec + mAnalyzer->testProcessCycle(3); MediaQualityStatus quality3 = mFakeCallback.getMediaQualityStatus(); EXPECT_EQ(quality3.getRtpInactivityTimeMillis(), 2000); @@ -308,21 +342,20 @@ TEST_F(MediaQualityAnalyzerTest, TestRtcpInactivity) threshold.setRtcpInactivityTimerMillis(kRtcpInactivityTimerMillis); mAnalyzer->setMediaQualityThreshold(threshold); mAnalyzer->start(); - mCondition.wait_timeout(2100); // 2.1 sec + mAnalyzer->testProcessCycle(2); // Check MediaQualityStatus value MediaQualityStatus quality1 = mFakeCallback.getMediaQualityStatus(); EXPECT_EQ(quality1.getRtcpInactivityTimeMillis(), 2000); - mCondition.wait_timeout(2100); // 2.1 sec + mAnalyzer->testProcessCycle(2); // Check MediaQualityStatus value MediaQualityStatus quality2 = mFakeCallback.getMediaQualityStatus(); EXPECT_EQ(quality2.getRtcpInactivityTimeMillis(), 2000); mAnalyzer->SendEvent(kCollectPacketInfo, kStreamRtcp); - - mCondition.wait_timeout(3100); // 3.1 sec + mAnalyzer->testProcessCycle(3); MediaQualityStatus quality3 = mFakeCallback.getMediaQualityStatus(); EXPECT_EQ(quality3.getRtcpInactivityTimeMillis(), 2000); @@ -333,7 +366,7 @@ TEST_F(MediaQualityAnalyzerTest, TestCallQualityInactivity) { EXPECT_CALL(mCallback, onEvent(kAudioCallQualityChangedInd, _, _)).Times(2); mAnalyzer->start(); - mCondition.wait_timeout(4100); // 4.1 sec + mAnalyzer->testProcessCycle(4); mAnalyzer->stop(); // Check CallQuality value @@ -369,7 +402,7 @@ TEST_F(MediaQualityAnalyzerTest, TestCallQualityLevelChanged) SessionCallbackParameter* param = new SessionCallbackParameter(kReportPacketLossGap, 5, 1); mAnalyzer->SendEvent(kCollectOptionalInfo, reinterpret_cast(param), 0); - mCondition.wait_timeout(5100); // 5.1 sec + mAnalyzer->testProcessCycle(5); EXPECT_EQ(mAnalyzer->getTxPacketSize(), 0); EXPECT_EQ(mAnalyzer->getRxPacketSize(), numPackets - 1); @@ -409,7 +442,7 @@ TEST_F(MediaQualityAnalyzerTest, TestJitterInd) mAnalyzer->SendEvent(kCollectPacketInfo, kStreamRtpRx, reinterpret_cast(packet)); } - mCondition.wait_timeout(1100); // 1.1 sec + mAnalyzer->testProcessCycle(1); EXPECT_EQ(mAnalyzer->getTxPacketSize(), 0); EXPECT_EQ(mAnalyzer->getRxPacketSize(), numPackets); @@ -452,7 +485,8 @@ TEST_F(MediaQualityAnalyzerTest, TestSsrcChange) mAnalyzer->SendEvent(kCollectPacketInfo, kStreamRtpRx, reinterpret_cast(packet)); } - mCondition.wait_timeout(1100); // 1.1 sec + mAnalyzer->testProcessCycle(1); + EXPECT_EQ(mAnalyzer->getTxPacketSize(), 0); EXPECT_EQ(mAnalyzer->getRxPacketSize(), numPackets); EXPECT_EQ(mAnalyzer->getLostPacketSize(), 0); @@ -481,7 +515,7 @@ TEST_F(MediaQualityAnalyzerTest, TestPacketLossInd) { RtpPacket* packet = new RtpPacket(); - if (i == 5) // make 10% loss rate + if (i == 5 || i == 6) // make 20% loss rate { continue; } @@ -492,14 +526,14 @@ TEST_F(MediaQualityAnalyzerTest, TestPacketLossInd) mAnalyzer->SendEvent(kCollectPacketInfo, kStreamRtpRx, reinterpret_cast(packet)); } - SessionCallbackParameter* param = new SessionCallbackParameter(kReportPacketLossGap, 5, 1); + SessionCallbackParameter* param = new SessionCallbackParameter(kReportPacketLossGap, 5, 2); mAnalyzer->SendEvent(kCollectOptionalInfo, reinterpret_cast(param), 0); - mCondition.wait_timeout(1100); // 1.1 sec + mAnalyzer->testProcessCycle(1); EXPECT_EQ(mAnalyzer->getTxPacketSize(), 0); - EXPECT_EQ(mAnalyzer->getRxPacketSize(), numPackets - 1); - EXPECT_EQ(mAnalyzer->getLostPacketSize(), 1); + EXPECT_EQ(mAnalyzer->getRxPacketSize(), numPackets - 2); + EXPECT_EQ(mAnalyzer->getLostPacketSize(), 2); mAnalyzer->stop(); @@ -507,10 +541,10 @@ TEST_F(MediaQualityAnalyzerTest, TestPacketLossInd) EXPECT_EQ(mAnalyzer->getRxPacketSize(), 0); EXPECT_EQ(mAnalyzer->getLostPacketSize(), 0); - EXPECT_EQ(mFakeCallback.getCallQuality().getNumRtpPacketsNotReceived(), 1); + EXPECT_EQ(mFakeCallback.getCallQuality().getNumRtpPacketsNotReceived(), 2); MediaQualityStatus status = mFakeCallback.getMediaQualityStatus(); - EXPECT_EQ(status.getRtpPacketLossRate(), 10); + EXPECT_EQ(status.getRtpPacketLossRate(), 20); } TEST_F(MediaQualityAnalyzerTest, TestNotifyMediaQualityStatus) @@ -521,7 +555,6 @@ TEST_F(MediaQualityAnalyzerTest, TestNotifyMediaQualityStatus) threshold.setNotifyCurrentStatus(true); mAnalyzer->setMediaQualityThreshold(threshold); mAnalyzer->start(); - - mCondition.wait_timeout(2100); // 2.1 sec + mAnalyzer->testProcessCycle(2); mAnalyzer->stop(); } \ No newline at end of file -- cgit v1.2.3