From 58cd31665cb4856d1d51ec6c9acd0757133a859e Mon Sep 17 00:00:00 2001 From: "turaj@webrtc.org" Date: Thu, 31 Oct 2013 15:15:55 +0000 Subject: [PATCH] Address Clag Analyzer issues. Following are the issues related to NetEq 4, discovered by Clang Analyzer. https://x20web.corp.google.com/~pbos/scan-build-2013-10-10-1/report-b44b95.html#EndPath Valid; perhaps unlikely, addressed. https://x20web.corp.google.com/~pbos/scan-build-2013-10-10-1/report-6beef6.html#EndPath Valid, addressed. https://x20web.corp.google.com/~pbos/scan-build-2013-10-10-1/report-2e3883.html#EndPath Valid; Addressed https://x20web.corp.google.com/~pbos/scan-build-2013-10-10-1/report-293659.html#EndPath Valid; Addressed. https://x20web.corp.google.com/~pbos/scan-build-2013-10-10-1/report-b875cd.html#EndPath Valid; Addressed. https://x20web.corp.google.com/~pbos/scan-build-2013-10-10-1/index.html Not valid; https://x20web.corp.google.com/~pbos/scan-build-2013-10-10-1/report-86f2ed.html#EndPath Not Valid; the assert statement will be short-circuited, however I also added a check of nullity of |packet|. https://x20web.corp.google.com/~pbos/scan-build-2013-10-10-1/report-3a5669.html#EndPath Not Valid: |energy_input| and |energy_expand| are both non-negative, therefore if-statement condition on line 226 is not satisfied unless |energy_input| >= 1. Therefore |energy_input| cannot be zero after normalization to 14-bits, i.e. operations on lines 228 & 229. https://x20web.corp.google.com/~pbos/scan-build-2013-10-10-1/report-2f914f.html#EndPath Valid; addressed. https://x20web.corp.google.com/~pbos/scan-build-2013-10-10-1/report-2332b1.html#EndPath Valid; addressed. https://x20web.corp.google.com/~pbos/scan-build-2013-10-10-1/report-de8dea.html#EndPath Not valid; |out_len| is set when Process() is called, however, it makes sense to initialize to zero when declaring |out_len|. https://x20web.corp.google.com/~pbos/scan-build-2013-10-10-1/report-b671a3.html#EndPath Valid; addressed. BUG= R=henrik.lundin@webrtc.org Review URL: https://webrtc-codereview.appspot.com/2729005 git-svn-id: http://webrtc.googlecode.com/svn/trunk@5064 4adac7df-926f-26a2-2b94-8c16560cd09d --- webrtc/modules/audio_coding/neteq4/expand.cc | 10 ++++++---- .../modules/audio_coding/neteq4/neteq_impl.cc | 9 +++++---- .../audio_coding/neteq4/neteq_unittest.cc | 5 ++--- .../audio_coding/neteq4/payload_splitter.cc | 1 - .../audio_coding/neteq4/test/RTPencode.cc | 2 +- .../audio_coding/neteq4/test/RTPjitter.cc | 19 +++++++++++++++++-- 6 files changed, 31 insertions(+), 15 deletions(-) diff --git a/webrtc/modules/audio_coding/neteq4/expand.cc b/webrtc/modules/audio_coding/neteq4/expand.cc index b9157c363b..73f2ef85a5 100644 --- a/webrtc/modules/audio_coding/neteq4/expand.cc +++ b/webrtc/modules/audio_coding/neteq4/expand.cc @@ -415,9 +415,11 @@ void Expand::AnalyzeSignal(int16_t* random_vector) { // Calculate correlation in downsampled domain (4 kHz sample rate). int16_t correlation_scale; - int correlation_length = Correlation(audio_history, signal_length, - correlation_vector, &correlation_scale); - correlation_length = 51; // TODO(hlundin): Legacy bit-exactness. + int correlation_length = 51; // TODO(hlundin): Legacy bit-exactness. + // If it is decided to break bit-exactness |correlation_length| should be + // initialized to the return value of Correlation(). + Correlation(audio_history, signal_length, correlation_vector, + &correlation_scale); // Find peaks in correlation vector. DspHelper::PeakDetection(correlation_vector, correlation_length, @@ -449,7 +451,7 @@ void Expand::AnalyzeSignal(int16_t* random_vector) { // Find the maximizing index |i| of the cost function // f[i] = best_correlation[i] / best_distortion[i]. - int32_t best_ratio = -1; + int32_t best_ratio = std::numeric_limits::min(); int best_index = -1; for (int i = 0; i < kNumCorrelationCandidates; ++i) { int32_t ratio; diff --git a/webrtc/modules/audio_coding/neteq4/neteq_impl.cc b/webrtc/modules/audio_coding/neteq4/neteq_impl.cc index a5c45ff7b4..73ca5e4e21 100644 --- a/webrtc/modules/audio_coding/neteq4/neteq_impl.cc +++ b/webrtc/modules/audio_coding/neteq4/neteq_impl.cc @@ -1259,6 +1259,7 @@ int NetEqImpl::DecodeLoop(PacketList* packet_list, Operations* operation, delete[] packet->payload; delete packet; + packet = NULL; if (decode_length > 0) { *decoded_length += decode_length; // Update |decoder_frame_length_| with number of samples per channel. @@ -1287,10 +1288,10 @@ int NetEqImpl::DecodeLoop(PacketList* packet_list, Operations* operation, } } // End of decode loop. - // If the list is not empty at this point, it must hold exactly one CNG - // packet. - assert(packet_list->empty() || - (packet_list->size() == 1 && + // If the list is not empty at this point, either a decoding error terminated + // the while-loop, or list must hold exactly one CNG packet. + assert(packet_list->empty() || *decoded_length < 0 || + (packet_list->size() == 1 && packet && decoder_database_->IsComfortNoise(packet->header.payloadType))); return 0; } diff --git a/webrtc/modules/audio_coding/neteq4/neteq_unittest.cc b/webrtc/modules/audio_coding/neteq4/neteq_unittest.cc index 1b1e950a82..a8cff5ed3b 100644 --- a/webrtc/modules/audio_coding/neteq4/neteq_unittest.cc +++ b/webrtc/modules/audio_coding/neteq4/neteq_unittest.cc @@ -330,7 +330,7 @@ void NetEqDecodingTest::DecodeAndCompare(const std::string &rtp_file, std::ostringstream ss; ss << "Lap number " << i++ << " in DecodeAndCompare while loop"; SCOPED_TRACE(ss.str()); // Print out the parameter values on failure. - int out_len; + int out_len = 0; ASSERT_NO_FATAL_FAILURE(Process(&rtp, &out_len)); ASSERT_NO_FATAL_FAILURE(ref_files.ProcessReference(out_data_, out_len)); } @@ -583,7 +583,6 @@ TEST_F(NetEqDecodingTest, DISABLED_ON_ANDROID(TestFrameWaitingTimeStatistics)) { std::vector waiting_times; neteq_->WaitingTimes(&waiting_times); - int len = waiting_times.size(); EXPECT_EQ(num_frames, waiting_times.size()); // Since all frames are dumped into NetEQ at once, but pulled out with 10 ms // spacing (per definition), we expect the delay to increase with 10 ms for @@ -594,7 +593,7 @@ TEST_F(NetEqDecodingTest, DISABLED_ON_ANDROID(TestFrameWaitingTimeStatistics)) { // Check statistics again and make sure it's been reset. neteq_->WaitingTimes(&waiting_times); - len = waiting_times.size(); + int len = waiting_times.size(); EXPECT_EQ(0, len); // Process > 100 frames, and make sure that that we get statistics diff --git a/webrtc/modules/audio_coding/neteq4/payload_splitter.cc b/webrtc/modules/audio_coding/neteq4/payload_splitter.cc index e07b61f701..56039a57ec 100644 --- a/webrtc/modules/audio_coding/neteq4/payload_splitter.cc +++ b/webrtc/modules/audio_coding/neteq4/payload_splitter.cc @@ -331,7 +331,6 @@ void PayloadSplitter::SplitBySamples(const Packet* packet, new_packet->primary = packet->primary; new_packet->payload = new uint8_t[len]; memcpy(new_packet->payload, payload_ptr, len); - payload_ptr += len; new_packets->push_back(new_packet); } } diff --git a/webrtc/modules/audio_coding/neteq4/test/RTPencode.cc b/webrtc/modules/audio_coding/neteq4/test/RTPencode.cc index 7368b0265d..bc806091b0 100644 --- a/webrtc/modules/audio_coding/neteq4/test/RTPencode.cc +++ b/webrtc/modules/audio_coding/neteq4/test/RTPencode.cc @@ -1618,7 +1618,7 @@ int NetEQTest_encode(int coder, int16_t *indata, int frameLen, unsigned char * e #ifdef CODEC_G722 else if (coder==webrtc::kDecoderG722) { /*g722 */ cdlen=WebRtcG722_Encode(g722EncState[k], indata, frameLen, (int16_t*)encoded); - cdlen=frameLen>>1; + assert(cdlen == frameLen>>1); } #endif #ifdef CODEC_ILBC diff --git a/webrtc/modules/audio_coding/neteq4/test/RTPjitter.cc b/webrtc/modules/audio_coding/neteq4/test/RTPjitter.cc index 79af181fbe..eeb4c90140 100644 --- a/webrtc/modules/audio_coding/neteq4/test/RTPjitter.cc +++ b/webrtc/modules/audio_coding/neteq4/test/RTPjitter.cc @@ -64,7 +64,9 @@ int main(int argc, char* argv[]) unsigned int dat_len, rtp_len, Npack, k; arr_time *time_vec; char firstline[FIRSTLINELEN]; - unsigned char *rtp_vec = NULL, **packet_ptr, *temp_packet; + unsigned char* rtp_vec = NULL; + unsigned char** packet_ptr = NULL; + unsigned char* temp_packet = NULL; const unsigned int kRtpDumpHeaderSize = 4 + 4 + 4 + 2 + 2; uint16_t len; uint32_t *offset; @@ -113,6 +115,11 @@ int main(int argc, char* argv[]) dat_len++; } + if (dat_len == 0) { + fprintf(stderr, "Error: dat_file is empty, no arrival time is given.\n"); + goto closing; + } + qsort(time_vec,dat_len,sizeof(arr_time),compare_arr_time); @@ -146,6 +153,11 @@ int main(int argc, char* argv[]) len=(uint16_t) fread(&rtp_vec[rtp_len], sizeof(unsigned char), 2, in_file); // read length of next packet } + if (Npack == 0) { + fprintf(stderr, "Error: No RTP packet found.\n"); + goto closing; + } + packet_ptr = (unsigned char **) malloc(Npack*sizeof(unsigned char*)); packet_ptr[0]=rtp_vec; @@ -182,7 +194,10 @@ int main(int argc, char* argv[]) closing: free(time_vec); free(rtp_vec); - fclose(in_file); + if (packet_ptr != NULL) { + free(packet_ptr); + } + fclose(in_file); fclose(dat_file); fclose(out_file);