From 0fcaf99b716c71e721db25f37f62dabf5dcbe69a Mon Sep 17 00:00:00 2001 From: "kjellander@webrtc.org" Date: Thu, 26 Nov 2015 15:24:52 +0100 Subject: [PATCH] Enable cpplint for webrtc/video_engine Enable cpplint and have it use a whitelist that also checks in subdirectories. Move the cpplint check so it runs before the pylint check since that one always run and increases the time to errors for cpplint. Fix all cpplint errors in webrtc/video_engine. BUG=webrtc:5149 TESTED=Fixed issues reported by: find webrtc/video_engine -type f -name *.cc -o -name *.h | xargs cpplint.py followed by 'git cl presubmit'. R=pbos@chromium.org, phoglund@chromium.org TBR=pbos@webrtc.org, phoglund@webrtc.org Review URL: https://codereview.webrtc.org/1481723003 . Cr-Commit-Position: refs/heads/master@{#10808} --- PRESUBMIT.py | 22 +++++++++++++++++-- webrtc/video_engine/overuse_frame_detector.h | 14 ++++++------ .../overuse_frame_detector_unittest.cc | 2 +- .../video_engine/payload_router_unittest.cc | 2 +- .../stream_synchronization_unittest.cc | 3 ++- webrtc/video_engine/vie_channel.cc | 3 ++- webrtc/video_engine/vie_channel.h | 2 ++ webrtc/video_engine/vie_receiver.h | 4 +++- webrtc/video_engine/vie_remb_unittest.cc | 5 ++--- 9 files changed, 40 insertions(+), 17 deletions(-) diff --git a/PRESUBMIT.py b/PRESUBMIT.py index a832761c89..3fdfe7fc11 100755 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -14,6 +14,12 @@ import subprocess import sys +# Directories that will be scanned by cpplint by the presubmit script. +CPPLINT_DIRS = [ + 'webrtc/video_engine', +] + + def _CheckNoIOStreamInHeaders(input_api, output_api): """Checks to make sure no .h files include .""" files = [] @@ -54,6 +60,14 @@ def _CheckNoFRIEND_TEST(input_api, output_api): 'use FRIEND_TEST_ALL_PREFIXES() instead.\n' + '\n'.join(problems))] +def _IsLintWhitelisted(whitelist_dirs, file_path): + """ Checks if a file is whitelisted for lint check.""" + for path in whitelist_dirs: + if os.path.dirname(file_path).startswith(path): + return True + return False + + def _CheckApprovedFilesLintClean(input_api, output_api, source_file_filter=None): """Checks that all new or whitelisted .cc and .h files pass cpplint.py. @@ -68,6 +82,10 @@ def _CheckApprovedFilesLintClean(input_api, output_api, # pylint: disable=W0212 cpplint._cpplint_state.ResetErrorCounts() + # Create a platform independent whitelist for the CPPLINT_DIRS. + whitelist_dirs = [input_api.os_path.join(*path.split('/')) + for path in CPPLINT_DIRS] + # Use the strictest verbosity level for cpplint.py (level 1) which is the # default when running cpplint.py from command line. # To make it possible to work with not-yet-converted code, we're only applying @@ -76,7 +94,7 @@ def _CheckApprovedFilesLintClean(input_api, output_api, files = [] for f in input_api.AffectedSourceFiles(source_file_filter): # Note that moved/renamed files also count as added. - if f.Action() == 'A': + if f.Action() == 'A' or _IsLintWhitelisted(whitelist_dirs, f.LocalPath()): files.append(f.AbsoluteLocalPath()) for file_name in files: @@ -249,6 +267,7 @@ def _RunPythonTests(input_api, output_api): def _CommonChecks(input_api, output_api): """Checks common to both upload and commit.""" results = [] + results.extend(_CheckApprovedFilesLintClean(input_api, output_api)) results.extend(input_api.canned_checks.RunPylint(input_api, output_api, black_list=(r'^.*gviz_api\.py$', r'^.*gaeunit\.py$', @@ -298,7 +317,6 @@ def _CommonChecks(input_api, output_api): input_api, output_api)) results.extend(input_api.canned_checks.CheckChangeTodoHasOwner( input_api, output_api)) - results.extend(_CheckApprovedFilesLintClean(input_api, output_api)) results.extend(_CheckNoIOStreamInHeaders(input_api, output_api)) results.extend(_CheckNoFRIEND_TEST(input_api, output_api)) results.extend(_CheckGypChanges(input_api, output_api)) diff --git a/webrtc/video_engine/overuse_frame_detector.h b/webrtc/video_engine/overuse_frame_detector.h index 0deb5ba83c..050b14fd00 100644 --- a/webrtc/video_engine/overuse_frame_detector.h +++ b/webrtc/video_engine/overuse_frame_detector.h @@ -50,7 +50,7 @@ struct CpuOveruseOptions { // Method based on encode time of frames. bool enable_encode_usage_method; int low_encode_usage_threshold_percent; // Threshold for triggering underuse. - int high_encode_usage_threshold_percent; // Threshold for triggering overuse. + int high_encode_usage_threshold_percent; // Threshold for triggering overuse. bool enable_extended_processing_usage; // Include a larger time span (in // addition to encode time) for // measuring the processing time of a @@ -61,9 +61,9 @@ struct CpuOveruseOptions { int min_frame_samples; // The minimum number of frames required. int min_process_count; // The number of initial process times required before // triggering an overuse/underuse. - int high_threshold_consecutive_count; // The number of consecutive checks - // above the high threshold before - // triggering an overuse. + int high_threshold_consecutive_count; // The number of consecutive checks + // above the high threshold before + // triggering an overuse. }; struct CpuOveruseMetrics { @@ -71,9 +71,9 @@ struct CpuOveruseMetrics { : avg_encode_time_ms(-1), encode_usage_percent(-1) {} - int avg_encode_time_ms; // The average encode time in ms. - int encode_usage_percent; // The average encode time divided by the average - // time difference between incoming captured frames. + int avg_encode_time_ms; // Average encode time in ms. + int encode_usage_percent; // Average encode time divided by the average time + // difference between incoming captured frames. }; class CpuOveruseMetricsObserver { diff --git a/webrtc/video_engine/overuse_frame_detector_unittest.cc b/webrtc/video_engine/overuse_frame_detector_unittest.cc index d502f02204..5524ba5a0f 100644 --- a/webrtc/video_engine/overuse_frame_detector_unittest.cc +++ b/webrtc/video_engine/overuse_frame_detector_unittest.cc @@ -188,7 +188,7 @@ TEST_F(OveruseFrameDetectorTest, ConstantOveruseGivesNoNormalUsage) { ReinitializeOveruseDetector(); EXPECT_CALL(*(observer_.get()), NormalUsage()).Times(0); EXPECT_CALL(*(observer_.get()), OveruseDetected()).Times(64); - for(size_t i = 0; i < 64; ++i) { + for (size_t i = 0; i < 64; ++i) { TriggerOveruse(options_.high_threshold_consecutive_count); } } diff --git a/webrtc/video_engine/payload_router_unittest.cc b/webrtc/video_engine/payload_router_unittest.cc index acaa4006c4..11c664b4a1 100644 --- a/webrtc/video_engine/payload_router_unittest.cc +++ b/webrtc/video_engine/payload_router_unittest.cc @@ -182,7 +182,7 @@ TEST_F(PayloadRouterTest, SetTargetSendBitrates) { const uint32_t bitrate_1 = 10000; const uint32_t bitrate_2 = 76543; - std::vector bitrates (2, bitrate_1); + std::vector bitrates(2, bitrate_1); bitrates[1] = bitrate_2; EXPECT_CALL(rtp_1, SetTargetSendBitrate(bitrate_1)) .Times(1); diff --git a/webrtc/video_engine/stream_synchronization_unittest.cc b/webrtc/video_engine/stream_synchronization_unittest.cc index 7136f1e1c7..1c55c2ba5d 100644 --- a/webrtc/video_engine/stream_synchronization_unittest.cc +++ b/webrtc/video_engine/stream_synchronization_unittest.cc @@ -8,9 +8,10 @@ * be found in the AUTHORS file in the root of the source tree. */ -#include #include +#include + #include "testing/gtest/include/gtest/gtest.h" #include "webrtc/video_engine/stream_synchronization.h" diff --git a/webrtc/video_engine/vie_channel.cc b/webrtc/video_engine/vie_channel.cc index 2e56056b5f..91c862b642 100644 --- a/webrtc/video_engine/vie_channel.cc +++ b/webrtc/video_engine/vie_channel.cc @@ -11,6 +11,7 @@ #include "webrtc/video_engine/vie_channel.h" #include +#include #include #include "webrtc/base/checks.h" @@ -60,7 +61,7 @@ class ChannelStatsObserver : public CallStatsObserver { class ViEChannelProtectionCallback : public VCMProtectionCallback { public: - ViEChannelProtectionCallback(ViEChannel* owner) : owner_(owner) {} + explicit ViEChannelProtectionCallback(ViEChannel* owner) : owner_(owner) {} ~ViEChannelProtectionCallback() {} diff --git a/webrtc/video_engine/vie_channel.h b/webrtc/video_engine/vie_channel.h index 32f0301707..d5030de017 100644 --- a/webrtc/video_engine/vie_channel.h +++ b/webrtc/video_engine/vie_channel.h @@ -12,6 +12,8 @@ #define WEBRTC_VIDEO_ENGINE_VIE_CHANNEL_H_ #include +#include +#include #include "webrtc/base/scoped_ptr.h" #include "webrtc/base/scoped_ref_ptr.h" diff --git a/webrtc/video_engine/vie_receiver.h b/webrtc/video_engine/vie_receiver.h index d75622f656..b0c5749f6a 100644 --- a/webrtc/video_engine/vie_receiver.h +++ b/webrtc/video_engine/vie_receiver.h @@ -12,6 +12,7 @@ #define WEBRTC_VIDEO_ENGINE_VIE_RECEIVER_H_ #include +#include #include "webrtc/base/scoped_ptr.h" #include "webrtc/engine_configurations.h" @@ -84,6 +85,7 @@ class ViEReceiver : public RtpData { bool OnRecoveredPacket(const uint8_t* packet, size_t packet_length) override; ReceiveStatistics* GetReceiveStatistics() const; + private: int InsertRTPPacket(const uint8_t* rtp_packet, size_t rtp_packet_length, const PacketTime& packet_time); @@ -125,6 +127,6 @@ class ViEReceiver : public RtpData { int64_t last_packet_log_ms_; }; -} // namespace webrt +} // namespace webrtc #endif // WEBRTC_VIDEO_ENGINE_VIE_RECEIVER_H_ diff --git a/webrtc/video_engine/vie_remb_unittest.cc b/webrtc/video_engine/vie_remb_unittest.cc index b6fbf29bf2..f319d0d363 100644 --- a/webrtc/video_engine/vie_remb_unittest.cc +++ b/webrtc/video_engine/vie_remb_unittest.cc @@ -11,11 +11,10 @@ // This file includes unit tests for ViERemb. -#include "testing/gmock/include/gmock/gmock.h" -#include "testing/gtest/include/gtest/gtest.h" - #include +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" #include "webrtc/base/scoped_ptr.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp.h" #include "webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h"