From 45a4c41eda9066a62e7e324b008a35c0295f15c7 Mon Sep 17 00:00:00 2001 From: Mirko Bonadei Date: Tue, 31 Jul 2018 15:07:28 +0200 Subject: [PATCH] Never invoke rtc::LogMessage::SetLogToStderr outside of main. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rtc::LogMessage::SetLogToStderr should only be invoked by the main function in order to enable or disable logging in a consistent way [1]. Usage of rtc::LogMessage::SetLogToStderr in other parts of the codebase creates complex behaviors and confusion. [1] - https://cs.chromium.org/chromium/src/third_party/webrtc/test/test_main.cc?l=88&rcl=665174fdbb4e0540eccb27cf7412348f1b65534c Bug: None Change-Id: Iae86fb14d7ca40af6d78d0f0cd81c5a39f65068d Reviewed-on: https://webrtc-review.googlesource.com/91442 Reviewed-by: Erik Språng Reviewed-by: Jonas Olsson Reviewed-by: Ilya Nikolaevskiy Reviewed-by: Steve Anton Commit-Queue: Steve Anton Cr-Commit-Position: refs/heads/master@{#24154} --- api/test/video_quality_test_fixture.h | 1 - video/BUILD.gn | 3 +++ video/full_stack_tests.cc | 4 ++-- video/screenshare_loopback.cc | 7 +++++-- video/sv_loopback.cc | 7 +++++-- video/video_loopback.cc | 7 +++++-- video/video_quality_test.cc | 4 +--- 7 files changed, 21 insertions(+), 12 deletions(-) diff --git a/api/test/video_quality_test_fixture.h b/api/test/video_quality_test_fixture.h index 5228f62a71..c8734ca8a6 100644 --- a/api/test/video_quality_test_fixture.h +++ b/api/test/video_quality_test_fixture.h @@ -92,7 +92,6 @@ class VideoQualityTestFixtureInterface { bool infer_streams; } ss[2]; struct Logging { - bool logs; std::string rtc_event_log_name; std::string rtp_dump_name; std::string encoded_frame_base_path; diff --git a/video/BUILD.gn b/video/BUILD.gn index 274104e886..96941f409d 100644 --- a/video/BUILD.gn +++ b/video/BUILD.gn @@ -270,6 +270,7 @@ if (rtc_include_tests) { ] deps = [ ":video_quality_test", + "../rtc_base:logging", "../rtc_base:rtc_base_approved", "../system_wrappers:field_trial_default", "../system_wrappers:metrics_default", @@ -296,6 +297,7 @@ if (rtc_include_tests) { deps = [ ":video_quality_test", + "../rtc_base:logging", "../rtc_base:rtc_base_approved", "../system_wrappers:field_trial_default", "../system_wrappers:metrics_default", @@ -321,6 +323,7 @@ if (rtc_include_tests) { ] deps = [ ":video_quality_test", + "../rtc_base:logging", "../rtc_base:rtc_base_approved", "../system_wrappers:field_trial_default", "../system_wrappers:metrics_default", diff --git a/video/full_stack_tests.cc b/video/full_stack_tests.cc index 441c15b156..0b7bfb37d6 100644 --- a/video/full_stack_tests.cc +++ b/video/full_stack_tests.cc @@ -56,8 +56,8 @@ struct ParamsWithLogging : public VideoQualityTest::Params { public: ParamsWithLogging() { // Use these logging flags by default, for everything. - logging = {flags::FLAG_logs, flags::RtcEventLogName(), - flags::RtpDumpName(), flags::EncodedFramePath()}; + logging = {flags::RtcEventLogName(), flags::RtpDumpName(), + flags::EncodedFramePath()}; } }; diff --git a/video/screenshare_loopback.cc b/video/screenshare_loopback.cc index 0fc89f5c38..ddc190d7ea 100644 --- a/video/screenshare_loopback.cc +++ b/video/screenshare_loopback.cc @@ -11,6 +11,7 @@ #include #include "rtc_base/flags.h" +#include "rtc_base/logging.h" #include "rtc_base/stringencode.h" #include "system_wrappers/include/field_trial_default.h" #include "test/field_trial.h" @@ -314,8 +315,8 @@ void Loopback() { flags::OutputFilename(), flags::GraphTitle()}; params.pipe = pipe_config; - params.logging = {flags::FLAG_logs, flags::RtcEventLogName(), - flags::RtpDumpName(), flags::EncodedFramePath()}; + params.logging = {flags::RtcEventLogName(), flags::RtpDumpName(), + flags::EncodedFramePath()}; if (flags::NumStreams() > 1 && flags::Stream0().empty() && flags::Stream1().empty()) { @@ -350,6 +351,8 @@ int main(int argc, char* argv[]) { return 0; } + rtc::LogMessage::SetLogToStderr(webrtc::flags::FLAG_logs); + webrtc::test::ValidateFieldTrialsStringOrDie( webrtc::flags::FLAG_force_fieldtrials); // InitFieldTrialsFromString stores the char*, so the char array must outlive diff --git a/video/sv_loopback.cc b/video/sv_loopback.cc index cb77f70cec..b2a1e2e260 100644 --- a/video/sv_loopback.cc +++ b/video/sv_loopback.cc @@ -11,6 +11,7 @@ #include #include "rtc_base/flags.h" +#include "rtc_base/logging.h" #include "rtc_base/stringencode.h" #include "system_wrappers/include/field_trial_default.h" #include "test/field_trial.h" @@ -529,8 +530,8 @@ void Loopback() { flags::GetCaptureDevice()}; params.audio = {flags::FLAG_audio, flags::FLAG_audio_video_sync, flags::FLAG_audio_dtx}; - params.logging = {flags::FLAG_logs, flags::FLAG_rtc_event_log_name, - flags::FLAG_rtp_dump_name, flags::FLAG_encoded_frame_path}; + params.logging = {flags::FLAG_rtc_event_log_name, flags::FLAG_rtp_dump_name, + flags::FLAG_encoded_frame_path}; params.analyzer = {"dual_streams", 0.0, 0.0, @@ -598,6 +599,8 @@ int main(int argc, char* argv[]) { return 0; } + rtc::LogMessage::SetLogToStderr(webrtc::flags::FLAG_logs); + webrtc::test::ValidateFieldTrialsStringOrDie( webrtc::flags::FLAG_force_fieldtrials); // InitFieldTrialsFromString stores the char*, so the char array must outlive diff --git a/video/video_loopback.cc b/video/video_loopback.cc index 6d1faaa7a4..a4d4547dfc 100644 --- a/video/video_loopback.cc +++ b/video/video_loopback.cc @@ -11,6 +11,7 @@ #include #include "rtc_base/flags.h" +#include "rtc_base/logging.h" #include "system_wrappers/include/field_trial_default.h" #include "test/field_trial.h" #include "test/gtest.h" @@ -307,8 +308,8 @@ void Loopback() { flags::GetCaptureDevice()}; params.audio = {flags::FLAG_audio, flags::FLAG_audio_video_sync, flags::FLAG_audio_dtx}; - params.logging = {flags::FLAG_logs, flags::FLAG_rtc_event_log_name, - flags::FLAG_rtp_dump_name, flags::FLAG_encoded_frame_path}; + params.logging = {flags::FLAG_rtc_event_log_name, flags::FLAG_rtp_dump_name, + flags::FLAG_encoded_frame_path}; params.screenshare[0].enabled = false; params.analyzer = {"video", 0.0, @@ -351,6 +352,8 @@ int main(int argc, char* argv[]) { return 0; } + rtc::LogMessage::SetLogToStderr(webrtc::flags::FLAG_logs); + webrtc::test::ValidateFieldTrialsStringOrDie( webrtc::flags::FLAG_force_fieldtrials); // InitFieldTrialsFromString stores the char*, so the char array must outlive diff --git a/video/video_quality_test.cc b/video/video_quality_test.cc index abc8376704..cad86aed0e 100644 --- a/video/video_quality_test.cc +++ b/video/video_quality_test.cc @@ -115,7 +115,7 @@ VideoQualityTest::Params::Params() std::vector()}, {std::vector(), 0, 0, -1, InterLayerPredMode::kOn, std::vector()}}, - logging({false, "", "", ""}) {} + logging({"", "", ""}) {} VideoQualityTest::Params::~Params() = default; @@ -774,7 +774,6 @@ VideoQualityTest::CreateReceiveTransport() { } void VideoQualityTest::RunWithAnalyzer(const Params& params) { - rtc::LogMessage::SetLogToStderr(params.logging.logs); num_video_streams_ = params.call.dual_video ? 2 : 1; std::unique_ptr send_transport; std::unique_ptr recv_transport; @@ -969,7 +968,6 @@ void VideoQualityTest::SetupAudio(Transport* transport) { } void VideoQualityTest::RunWithRenderers(const Params& params) { - rtc::LogMessage::SetLogToStderr(params.logging.logs); num_video_streams_ = params.call.dual_video ? 2 : 1; std::unique_ptr send_transport; std::unique_ptr recv_transport;