From 8e126fca476a2264e65f3c311795fe1748d32a0f Mon Sep 17 00:00:00 2001 From: Bjorn Terelius Date: Mon, 18 Dec 2017 14:21:53 +0100 Subject: [PATCH] Remove RtcEventLoggingStarted and RtcEventLoggingStopped events. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This prevents the programmer from accidentally adding LOG_START and LOG_END events to the log without actually starting the log. This also makes it easier to ensure that the LOG_START event always ends up first and the LOG_END event always ends up last in the log file. Bug: webrtc:8111 Change-Id: I4e6c9306f8559ff184b5185f8728409f8dcebfa0 Reviewed-on: https://webrtc-review.googlesource.com/34400 Reviewed-by: Elad Alon Commit-Queue: Björn Terelius Cr-Commit-Position: refs/heads/master@{#21486} --- logging/BUILD.gn | 4 -- .../encoder/rtc_event_log_encoder.h | 3 ++ .../encoder/rtc_event_log_encoder_legacy.cc | 43 +++++++------------ .../encoder/rtc_event_log_encoder_legacy.h | 5 ++- .../encoder/rtc_event_log_encoder_unittest.cc | 12 ++---- logging/rtc_event_log/events/rtc_event.h | 2 - .../events/rtc_event_logging_started.cc | 23 ---------- .../events/rtc_event_logging_started.h | 29 ------------- .../events/rtc_event_logging_stopped.cc | 23 ---------- .../events/rtc_event_logging_stopped.h | 29 ------------- logging/rtc_event_log/rtc_event_log.cc | 17 +++----- .../rtc_event_log/rtc_event_log_unittest.cc | 2 - 12 files changed, 30 insertions(+), 162 deletions(-) delete mode 100644 logging/rtc_event_log/events/rtc_event_logging_started.cc delete mode 100644 logging/rtc_event_log/events/rtc_event_logging_started.h delete mode 100644 logging/rtc_event_log/events/rtc_event_logging_stopped.cc delete mode 100644 logging/rtc_event_log/events/rtc_event_logging_stopped.h diff --git a/logging/BUILD.gn b/logging/BUILD.gn index 39442d41f5..e755b16345 100644 --- a/logging/BUILD.gn +++ b/logging/BUILD.gn @@ -39,10 +39,6 @@ rtc_source_set("rtc_event_log_api") { "rtc_event_log/events/rtc_event_bwe_update_delay_based.h", "rtc_event_log/events/rtc_event_bwe_update_loss_based.cc", "rtc_event_log/events/rtc_event_bwe_update_loss_based.h", - "rtc_event_log/events/rtc_event_logging_started.cc", - "rtc_event_log/events/rtc_event_logging_started.h", - "rtc_event_log/events/rtc_event_logging_stopped.cc", - "rtc_event_log/events/rtc_event_logging_stopped.h", "rtc_event_log/events/rtc_event_probe_cluster_created.cc", "rtc_event_log/events/rtc_event_probe_cluster_created.h", "rtc_event_log/events/rtc_event_probe_result_failure.cc", diff --git a/logging/rtc_event_log/encoder/rtc_event_log_encoder.h b/logging/rtc_event_log/encoder/rtc_event_log_encoder.h index dfa59ed10e..8f509ce896 100644 --- a/logging/rtc_event_log/encoder/rtc_event_log_encoder.h +++ b/logging/rtc_event_log/encoder/rtc_event_log_encoder.h @@ -22,6 +22,9 @@ class RtcEventLogEncoder { public: virtual ~RtcEventLogEncoder() = default; + virtual std::string EncodeLogStart(int64_t timestamp_us) = 0; + virtual std::string EncodeLogEnd(int64_t timestamp_us) = 0; + virtual std::string Encode(const RtcEvent& event) = 0; virtual std::string EncodeBatch( diff --git a/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.cc b/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.cc index 0f3f3be201..c05f7717c8 100644 --- a/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.cc +++ b/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.cc @@ -17,8 +17,6 @@ #include "logging/rtc_event_log/events/rtc_event_audio_send_stream_config.h" #include "logging/rtc_event_log/events/rtc_event_bwe_update_delay_based.h" #include "logging/rtc_event_log/events/rtc_event_bwe_update_loss_based.h" -#include "logging/rtc_event_log/events/rtc_event_logging_started.h" -#include "logging/rtc_event_log/events/rtc_event_logging_stopped.h" #include "logging/rtc_event_log/events/rtc_event_probe_cluster_created.h" #include "logging/rtc_event_log/events/rtc_event_probe_result_failure.h" #include "logging/rtc_event_log/events/rtc_event_probe_result_success.h" @@ -107,6 +105,21 @@ rtclog::VideoReceiveConfig_RtcpMode ConvertRtcpMode(RtcpMode rtcp_mode) { } } // namespace + +std::string RtcEventLogEncoderLegacy::EncodeLogStart(int64_t timestamp_us) { + rtclog::Event rtclog_event; + rtclog_event.set_timestamp_us(timestamp_us); + rtclog_event.set_type(rtclog::Event::LOG_START); + return Serialize(&rtclog_event); +} + +std::string RtcEventLogEncoderLegacy::EncodeLogEnd(int64_t timestamp_us) { + rtclog::Event rtclog_event; + rtclog_event.set_timestamp_us(timestamp_us); + rtclog_event.set_type(rtclog::Event::LOG_END); + return Serialize(&rtclog_event); +} + std::string RtcEventLogEncoderLegacy::EncodeBatch( std::deque>::const_iterator begin, std::deque>::const_iterator end) { @@ -159,16 +172,6 @@ std::string RtcEventLogEncoderLegacy::Encode(const RtcEvent& event) { return EncodeBweUpdateLossBased(rtc_event); } - case RtcEvent::Type::LoggingStarted: { - auto& rtc_event = static_cast(event); - return EncodeLoggingStarted(rtc_event); - } - - case RtcEvent::Type::LoggingStopped: { - auto& rtc_event = static_cast(event); - return EncodeLoggingStopped(rtc_event); - } - case RtcEvent::Type::ProbeClusterCreated: { auto& rtc_event = static_cast(event); return EncodeProbeClusterCreated(rtc_event); @@ -341,22 +344,6 @@ std::string RtcEventLogEncoderLegacy::EncodeBweUpdateLossBased( return Serialize(&rtclog_event); } -std::string RtcEventLogEncoderLegacy::EncodeLoggingStarted( - const RtcEventLoggingStarted& event) { - rtclog::Event rtclog_event; - rtclog_event.set_timestamp_us(event.timestamp_us_); - rtclog_event.set_type(rtclog::Event::LOG_START); - return Serialize(&rtclog_event); -} - -std::string RtcEventLogEncoderLegacy::EncodeLoggingStopped( - const RtcEventLoggingStopped& event) { - rtclog::Event rtclog_event; - rtclog_event.set_timestamp_us(event.timestamp_us_); - rtclog_event.set_type(rtclog::Event::LOG_END); - return Serialize(&rtclog_event); -} - std::string RtcEventLogEncoderLegacy::EncodeProbeClusterCreated( const RtcEventProbeClusterCreated& event) { rtclog::Event rtclog_event; diff --git a/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.h b/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.h index 9f795a6501..a8173135ae 100644 --- a/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.h +++ b/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.h @@ -52,6 +52,9 @@ class RtcEventLogEncoderLegacy final : public RtcEventLogEncoder { std::string Encode(const RtcEvent& event) override; + std::string EncodeLogStart(int64_t timestamp_us) override; + std::string EncodeLogEnd(int64_t timestamp_us) override; + std::string EncodeBatch( std::deque>::const_iterator begin, std::deque>::const_iterator end) override; @@ -69,8 +72,6 @@ class RtcEventLogEncoderLegacy final : public RtcEventLogEncoder { std::string EncodeBweUpdateDelayBased( const RtcEventBweUpdateDelayBased& event); std::string EncodeBweUpdateLossBased(const RtcEventBweUpdateLossBased& event); - std::string EncodeLoggingStarted(const RtcEventLoggingStarted& event); - std::string EncodeLoggingStopped(const RtcEventLoggingStopped& event); std::string EncodeProbeClusterCreated( const RtcEventProbeClusterCreated& event); std::string EncodeProbeResultFailure(const RtcEventProbeResultFailure& event); diff --git a/logging/rtc_event_log/encoder/rtc_event_log_encoder_unittest.cc b/logging/rtc_event_log/encoder/rtc_event_log_encoder_unittest.cc index ce4297cf3b..00dad29d0f 100644 --- a/logging/rtc_event_log/encoder/rtc_event_log_encoder_unittest.cc +++ b/logging/rtc_event_log/encoder/rtc_event_log_encoder_unittest.cc @@ -22,8 +22,6 @@ #include "logging/rtc_event_log/events/rtc_event_audio_send_stream_config.h" #include "logging/rtc_event_log/events/rtc_event_bwe_update_delay_based.h" #include "logging/rtc_event_log/events/rtc_event_bwe_update_loss_based.h" -#include "logging/rtc_event_log/events/rtc_event_logging_started.h" -#include "logging/rtc_event_log/events/rtc_event_logging_stopped.h" #include "logging/rtc_event_log/events/rtc_event_probe_cluster_created.h" #include "logging/rtc_event_log/events/rtc_event_probe_result_failure.h" #include "logging/rtc_event_log/events/rtc_event_probe_result_success.h" @@ -332,10 +330,9 @@ TEST_P(RtcEventLogEncoderTest, RtcEventBweUpdateLossBased) { } TEST_P(RtcEventLogEncoderTest, RtcEventLoggingStarted) { - auto event = rtc::MakeUnique(); - const int64_t timestamp_us = event->timestamp_us_; + const int64_t timestamp_us = rtc::TimeMicros(); - ASSERT_TRUE(parsed_log_.ParseString(encoder_->Encode(*event))); + ASSERT_TRUE(parsed_log_.ParseString(encoder_->EncodeLogStart(timestamp_us))); ASSERT_EQ(parsed_log_.GetNumberOfEvents(), 1u); ASSERT_EQ(parsed_log_.GetEventType(0), ParsedRtcEventLog::LOG_START); @@ -343,10 +340,9 @@ TEST_P(RtcEventLogEncoderTest, RtcEventLoggingStarted) { } TEST_P(RtcEventLogEncoderTest, RtcEventLoggingStopped) { - auto event = rtc::MakeUnique(); - const int64_t timestamp_us = event->timestamp_us_; + const int64_t timestamp_us = rtc::TimeMicros(); - ASSERT_TRUE(parsed_log_.ParseString(encoder_->Encode(*event))); + ASSERT_TRUE(parsed_log_.ParseString(encoder_->EncodeLogEnd(timestamp_us))); ASSERT_EQ(parsed_log_.GetNumberOfEvents(), 1u); ASSERT_EQ(parsed_log_.GetEventType(0), ParsedRtcEventLog::LOG_END); diff --git a/logging/rtc_event_log/events/rtc_event.h b/logging/rtc_event_log/events/rtc_event.h index 186e0658cf..6410698673 100644 --- a/logging/rtc_event_log/events/rtc_event.h +++ b/logging/rtc_event_log/events/rtc_event.h @@ -37,8 +37,6 @@ class RtcEvent { AudioSendStreamConfig, BweUpdateDelayBased, BweUpdateLossBased, - LoggingStarted, - LoggingStopped, ProbeClusterCreated, ProbeResultFailure, ProbeResultSuccess, diff --git a/logging/rtc_event_log/events/rtc_event_logging_started.cc b/logging/rtc_event_log/events/rtc_event_logging_started.cc deleted file mode 100644 index 47780b8110..0000000000 --- a/logging/rtc_event_log/events/rtc_event_logging_started.cc +++ /dev/null @@ -1,23 +0,0 @@ -/* - * Copyright (c) 2017 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include "logging/rtc_event_log/events/rtc_event_logging_started.h" - -namespace webrtc { - -RtcEvent::Type RtcEventLoggingStarted::GetType() const { - return RtcEvent::Type::LoggingStarted; -} - -bool RtcEventLoggingStarted::IsConfigEvent() const { - return false; -} - -} // namespace webrtc diff --git a/logging/rtc_event_log/events/rtc_event_logging_started.h b/logging/rtc_event_log/events/rtc_event_logging_started.h deleted file mode 100644 index 05dac6bdf3..0000000000 --- a/logging/rtc_event_log/events/rtc_event_logging_started.h +++ /dev/null @@ -1,29 +0,0 @@ -/* - * Copyright (c) 2017 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#ifndef LOGGING_RTC_EVENT_LOG_EVENTS_RTC_EVENT_LOGGING_STARTED_H_ -#define LOGGING_RTC_EVENT_LOG_EVENTS_RTC_EVENT_LOGGING_STARTED_H_ - -#include "logging/rtc_event_log/events/rtc_event.h" - -namespace webrtc { - -class RtcEventLoggingStarted final : public RtcEvent { - public: - ~RtcEventLoggingStarted() override = default; - - Type GetType() const override; - - bool IsConfigEvent() const override; -}; - -} // namespace webrtc - -#endif // LOGGING_RTC_EVENT_LOG_EVENTS_RTC_EVENT_LOGGING_STARTED_H_ diff --git a/logging/rtc_event_log/events/rtc_event_logging_stopped.cc b/logging/rtc_event_log/events/rtc_event_logging_stopped.cc deleted file mode 100644 index 8560cdb43d..0000000000 --- a/logging/rtc_event_log/events/rtc_event_logging_stopped.cc +++ /dev/null @@ -1,23 +0,0 @@ -/* - * Copyright (c) 2017 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include "logging/rtc_event_log/events/rtc_event_logging_stopped.h" - -namespace webrtc { - -RtcEvent::Type RtcEventLoggingStopped::GetType() const { - return RtcEvent::Type::LoggingStopped; -} - -bool RtcEventLoggingStopped::IsConfigEvent() const { - return false; -} - -} // namespace webrtc diff --git a/logging/rtc_event_log/events/rtc_event_logging_stopped.h b/logging/rtc_event_log/events/rtc_event_logging_stopped.h deleted file mode 100644 index 4b8575fa51..0000000000 --- a/logging/rtc_event_log/events/rtc_event_logging_stopped.h +++ /dev/null @@ -1,29 +0,0 @@ -/* - * Copyright (c) 2017 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#ifndef LOGGING_RTC_EVENT_LOG_EVENTS_RTC_EVENT_LOGGING_STOPPED_H_ -#define LOGGING_RTC_EVENT_LOG_EVENTS_RTC_EVENT_LOGGING_STOPPED_H_ - -#include "logging/rtc_event_log/events/rtc_event.h" - -namespace webrtc { - -class RtcEventLoggingStopped final : public RtcEvent { - public: - ~RtcEventLoggingStopped() override = default; - - Type GetType() const override; - - bool IsConfigEvent() const override; -}; - -} // namespace webrtc - -#endif // LOGGING_RTC_EVENT_LOG_EVENTS_RTC_EVENT_LOGGING_STOPPED_H_ diff --git a/logging/rtc_event_log/rtc_event_log.cc b/logging/rtc_event_log/rtc_event_log.cc index c6323c0d88..2173590662 100644 --- a/logging/rtc_event_log/rtc_event_log.cc +++ b/logging/rtc_event_log/rtc_event_log.cc @@ -19,8 +19,6 @@ #include #include "logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.h" -#include "logging/rtc_event_log/events/rtc_event_logging_started.h" -#include "logging/rtc_event_log/events/rtc_event_logging_stopped.h" #include "logging/rtc_event_log/output/rtc_event_log_output_file.h" #include "rtc_base/checks.h" #include "rtc_base/constructormagic.h" @@ -175,20 +173,15 @@ bool RtcEventLogImpl::StartLogging(std::unique_ptr output, RTC_LOG(LS_INFO) << "Starting WebRTC event log."; - // |start_event| captured by value. This is done here because we want the - // timestamp to reflect when StartLogging() was called; not the queueing - // delay of the TaskQueue. - // This is a bit inefficient - especially since we copy again to get it - // to comply with LogToOutput()'s signature - but it's a small problem. - RtcEventLoggingStarted start_event; + const int64_t timestamp_us = rtc::TimeMicros(); // Binding to |this| is safe because |this| outlives the |task_queue_|. - auto start = [this, start_event](std::unique_ptr output) { + auto start = [this, timestamp_us](std::unique_ptr output) { RTC_DCHECK_RUN_ON(&task_queue_); RTC_DCHECK(output->IsActive()); event_output_ = std::move(output); num_config_events_written_ = 0; - WriteToOutput(event_encoder_->Encode(start_event)); + WriteToOutput(event_encoder_->EncodeLogStart(timestamp_us)); LogEventsFromMemoryToOutput(); }; @@ -339,8 +332,8 @@ void RtcEventLogImpl::StopOutput() { void RtcEventLogImpl::StopLoggingInternal() { if (event_output_) { RTC_DCHECK(event_output_->IsActive()); - RtcEventLoggingStopped stop_event; - event_output_->Write(event_encoder_->Encode(stop_event)); + const int64_t timestamp_us = rtc::TimeMicros(); + event_output_->Write(event_encoder_->EncodeLogEnd(timestamp_us)); } StopOutput(); } diff --git a/logging/rtc_event_log/rtc_event_log_unittest.cc b/logging/rtc_event_log/rtc_event_log_unittest.cc index 156732317d..1cea3138ab 100644 --- a/logging/rtc_event_log/rtc_event_log_unittest.cc +++ b/logging/rtc_event_log/rtc_event_log_unittest.cc @@ -23,8 +23,6 @@ #include "logging/rtc_event_log/events/rtc_event_audio_send_stream_config.h" #include "logging/rtc_event_log/events/rtc_event_bwe_update_delay_based.h" #include "logging/rtc_event_log/events/rtc_event_bwe_update_loss_based.h" -#include "logging/rtc_event_log/events/rtc_event_logging_started.h" -#include "logging/rtc_event_log/events/rtc_event_logging_stopped.h" #include "logging/rtc_event_log/events/rtc_event_probe_cluster_created.h" #include "logging/rtc_event_log/events/rtc_event_probe_result_failure.h" #include "logging/rtc_event_log/events/rtc_event_probe_result_success.h"