From d8b9ed77cf5263616703cc9fc159361e64222093 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20M=C3=B6ller?= Date: Wed, 8 May 2019 13:53:51 +0200 Subject: [PATCH] Promote RtcEventLogOutputFile to api/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Preparation for deleting PeerConnectionInterface::StartRtcEventLog method with a PlatformFile argument. Bug: webrtc:6463 Change-Id: Ia9fa1d99a3d87f3bf193e73382690b782ffea65c Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/135285 Commit-Queue: Niels Moller Reviewed-by: Björn Terelius Reviewed-by: Karl Wiberg Cr-Commit-Position: refs/heads/master@{#27879} --- api/BUILD.gn | 20 ++++++ api/DEPS | 7 ++- .../rtc_event_log_output_file.cc | 2 +- api/rtc_event_log_output_file.h | 62 +++++++++++++++++++ .../rtc_event_log_output_file_unittest.cc | 2 +- call/BUILD.gn | 2 +- call/rampup_tests.cc | 2 +- logging/BUILD.gn | 15 ++--- .../output/rtc_event_log_output_file.h | 49 +-------------- .../rtc_event_log/rtc_event_log_unittest.cc | 2 +- pc/BUILD.gn | 4 +- pc/peer_connection.cc | 2 +- pc/peer_connection_interface_unittest.cc | 2 +- test/pc/e2e/BUILD.gn | 2 +- test/pc/e2e/peer_connection_quality_test.cc | 2 +- test/scenario/BUILD.gn | 2 +- video/BUILD.gn | 2 +- video/video_quality_test.cc | 2 +- 18 files changed, 109 insertions(+), 72 deletions(-) rename {logging/rtc_event_log/output => api}/rtc_event_log_output_file.cc (98%) create mode 100644 api/rtc_event_log_output_file.h rename {logging/rtc_event_log/output => api}/rtc_event_log_output_file_unittest.cc (98%) diff --git a/api/BUILD.gn b/api/BUILD.gn index ec274094dd..2512bd4657 100644 --- a/api/BUILD.gn +++ b/api/BUILD.gn @@ -348,6 +348,22 @@ rtc_source_set("libjingle_logging_api") { ] } +rtc_source_set("rtc_event_log_output_file") { + visibility = [ "*" ] + sources = [ + "rtc_event_log_output_file.cc", + "rtc_event_log_output_file.h", + ] + + deps = [ + ":libjingle_logging_api", + "../logging:rtc_event_log_api", + "../rtc_base:checks", + "../rtc_base:rtc_base_approved", + "../rtc_base/system:file_wrapper", + ] +} + rtc_source_set("ortc_api") { visibility = [ "*" ] sources = [ @@ -804,6 +820,7 @@ if (rtc_include_tests) { "array_view_unittest.cc", "function_view_unittest.cc", "rtc_error_unittest.cc", + "rtc_event_log_output_file_unittest.cc", "rtp_parameters_unittest.cc", "test/loopback_media_transport_unittest.cc", ] @@ -813,13 +830,16 @@ if (rtc_include_tests) { ":function_view", ":libjingle_peerconnection_api", ":loopback_media_transport", + ":rtc_event_log_output_file", "../rtc_base:checks", "../rtc_base:gunit_helpers", "../rtc_base:rtc_base_approved", + "../test:fileutils", "../test:test_support", "task_queue:task_queue_default_factory_unittests", "units:units_unittests", "video:video_unittests", + "//third_party/abseil-cpp/absl/memory", ] } diff --git a/api/DEPS b/api/DEPS index 34bdfe03ce..9c174a1eb6 100644 --- a/api/DEPS +++ b/api/DEPS @@ -148,7 +148,12 @@ specific_include_rules = { "rtc_error\.h": [ "+rtc_base/logging.h", ], - + "rtc_event_log_output_file.h": [ + # TODO(bugs.webrtc.org/6463): Delete this dependency. + "+rtc_base/platform_file.h", + # For private member and constructor. + "+rtc_base/system/file_wrapper.h", + ], "rtp_receiver_interface\.h": [ "+rtc_base/ref_count.h", ], diff --git a/logging/rtc_event_log/output/rtc_event_log_output_file.cc b/api/rtc_event_log_output_file.cc similarity index 98% rename from logging/rtc_event_log/output/rtc_event_log_output_file.cc rename to api/rtc_event_log_output_file.cc index 3f329a4b79..3845071c8e 100644 --- a/logging/rtc_event_log/output/rtc_event_log_output_file.cc +++ b/api/rtc_event_log_output_file.cc @@ -11,7 +11,7 @@ #include #include -#include "logging/rtc_event_log/output/rtc_event_log_output_file.h" +#include "api/rtc_event_log_output_file.h" #include "logging/rtc_event_log/rtc_event_log.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" diff --git a/api/rtc_event_log_output_file.h b/api/rtc_event_log_output_file.h new file mode 100644 index 0000000000..e536dfc459 --- /dev/null +++ b/api/rtc_event_log_output_file.h @@ -0,0 +1,62 @@ +/* + * 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 API_RTC_EVENT_LOG_OUTPUT_FILE_H_ +#define API_RTC_EVENT_LOG_OUTPUT_FILE_H_ + +#include +#include +#include + +#include "api/rtc_event_log_output.h" +#include "rtc_base/platform_file.h" // Can't neatly forward PlatformFile. +#include "rtc_base/system/file_wrapper.h" + +namespace webrtc { + +class RtcEventLogOutputFile final : public RtcEventLogOutput { + public: + static const size_t kMaxReasonableFileSize; // Explanation at declaration. + + // Unlimited/limited-size output file (by filename). + explicit RtcEventLogOutputFile(const std::string& file_name); + RtcEventLogOutputFile(const std::string& file_name, size_t max_size_bytes); + + // Limited-size output file (by FILE*). This class takes ownership + // of the FILE*, and closes it on destruction. + RtcEventLogOutputFile(FILE* file, size_t max_size_bytes); + + // TODO(bugs.webrtc.org/6463): Deprecated, delete together with the + // corresponding PeerConnection::StartRtcEventLog override. + RtcEventLogOutputFile(rtc::PlatformFile file, size_t max_size_bytes); + + ~RtcEventLogOutputFile() override = default; + + bool IsActive() const override; + + bool Write(const std::string& output) override; + + private: + RtcEventLogOutputFile(FileWrapper file, size_t max_size_bytes); + + // IsActive() can be called either from outside or from inside, but we don't + // want to incur the overhead of a virtual function call if called from inside + // some other function of this class. + inline bool IsActiveInternal() const; + + // Maximum size, or zero for no limit. + const size_t max_size_bytes_; + size_t written_bytes_{0}; + FileWrapper file_; +}; + +} // namespace webrtc + +#endif // API_RTC_EVENT_LOG_OUTPUT_FILE_H_ diff --git a/logging/rtc_event_log/output/rtc_event_log_output_file_unittest.cc b/api/rtc_event_log_output_file_unittest.cc similarity index 98% rename from logging/rtc_event_log/output/rtc_event_log_output_file_unittest.cc rename to api/rtc_event_log_output_file_unittest.cc index 7d7cdf7a6b..bffda0c864 100644 --- a/logging/rtc_event_log/output/rtc_event_log_output_file_unittest.cc +++ b/api/rtc_event_log_output_file_unittest.cc @@ -8,7 +8,7 @@ * be found in the AUTHORS file in the root of the source tree. */ -#include "logging/rtc_event_log/output/rtc_event_log_output_file.h" +#include "api/rtc_event_log_output_file.h" #include #include diff --git a/call/BUILD.gn b/call/BUILD.gn index 692427ce80..dc89ab897c 100644 --- a/call/BUILD.gn +++ b/call/BUILD.gn @@ -422,6 +422,7 @@ if (rtc_include_tests) { ":call_interfaces", ":simulated_network", ":video_stream_api", + "../api:rtc_event_log_output_file", "../api:simulated_network_api", "../api/audio_codecs:builtin_audio_encoder_factory", "../api/task_queue", @@ -431,7 +432,6 @@ if (rtc_include_tests) { "../api/video_codecs:video_codecs_api", "../logging:rtc_event_log_api", "../logging:rtc_event_log_impl_base", - "../logging:rtc_event_log_impl_output", "../modules/audio_coding", "../modules/audio_device", "../modules/audio_device:audio_device_impl", diff --git a/call/rampup_tests.cc b/call/rampup_tests.cc index 85e8d9566a..71a22f9040 100644 --- a/call/rampup_tests.cc +++ b/call/rampup_tests.cc @@ -13,10 +13,10 @@ #include #include "absl/memory/memory.h" +#include "api/rtc_event_log_output_file.h" #include "api/task_queue/default_task_queue_factory.h" #include "api/task_queue/task_queue_factory.h" #include "call/fake_network_pipe.h" -#include "logging/rtc_event_log/output/rtc_event_log_output_file.h" #include "logging/rtc_event_log/rtc_event_log_factory.h" #include "rtc_base/checks.h" #include "rtc_base/flags.h" diff --git a/logging/BUILD.gn b/logging/BUILD.gn index 7504d6b9d5..6dc833fde8 100644 --- a/logging/BUILD.gn +++ b/logging/BUILD.gn @@ -21,7 +21,6 @@ group("logging") { ":rtc_event_bwe", ":rtc_event_log_impl_base", ":rtc_event_log_impl_encoder", - ":rtc_event_log_impl_output", ":rtc_event_pacing", ":rtc_event_rtp_rtcp", ":rtc_event_video", @@ -208,7 +207,6 @@ rtc_static_library("rtc_event_log_impl_encoder") { ":rtc_event_generic_packet_events", ":rtc_event_log2_proto", ":rtc_event_log_api", - ":rtc_event_log_impl_output", ":rtc_event_log_proto", ":rtc_event_pacing", ":rtc_event_rtp_rtcp", @@ -228,18 +226,14 @@ rtc_static_library("rtc_event_log_impl_encoder") { } } +# TODO(bugs.webrtc.org/6463): For backwards compatibility; delete as +# soon as downstream dependencies are updated. rtc_source_set("rtc_event_log_impl_output") { sources = [ - "rtc_event_log/output/rtc_event_log_output_file.cc", "rtc_event_log/output/rtc_event_log_output_file.h", ] - deps = [ - ":rtc_event_log_api", - "../api:libjingle_logging_api", - "../rtc_base:checks", - "../rtc_base:rtc_base_approved", - "../rtc_base/system:file_wrapper", + "../api:rtc_event_log_output_file", ] } @@ -361,7 +355,6 @@ if (rtc_enable_protobuf) { "rtc_event_log/encoder/delta_encoding_unittest.cc", "rtc_event_log/encoder/rtc_event_log_encoder_common_unittest.cc", "rtc_event_log/encoder/rtc_event_log_encoder_unittest.cc", - "rtc_event_log/output/rtc_event_log_output_file_unittest.cc", "rtc_event_log/rtc_event_log_unittest.cc", "rtc_event_log/rtc_event_log_unittest_helper.cc", "rtc_event_log/rtc_event_log_unittest_helper.h", @@ -376,7 +369,6 @@ if (rtc_enable_protobuf) { ":rtc_event_log_api", ":rtc_event_log_impl_base", ":rtc_event_log_impl_encoder", - ":rtc_event_log_impl_output", ":rtc_event_log_parser", ":rtc_event_log_proto", ":rtc_event_pacing", @@ -385,6 +377,7 @@ if (rtc_enable_protobuf) { ":rtc_stream_config", "../api:array_view", "../api:libjingle_peerconnection_api", + "../api:rtc_event_log_output_file", "../api:rtp_headers", "../api/task_queue:default_task_queue_factory", "../call", diff --git a/logging/rtc_event_log/output/rtc_event_log_output_file.h b/logging/rtc_event_log/output/rtc_event_log_output_file.h index 75a3af6e95..86be01d884 100644 --- a/logging/rtc_event_log/output/rtc_event_log_output_file.h +++ b/logging/rtc_event_log/output/rtc_event_log_output_file.h @@ -11,52 +11,9 @@ #ifndef LOGGING_RTC_EVENT_LOG_OUTPUT_RTC_EVENT_LOG_OUTPUT_FILE_H_ #define LOGGING_RTC_EVENT_LOG_OUTPUT_RTC_EVENT_LOG_OUTPUT_FILE_H_ -#include -#include -#include +// TODO(bugs.webrtc.org/6463): For backwards compatibility; delete as soon as +// downstream dependencies are updated. -#include "api/rtc_event_log_output.h" -#include "rtc_base/platform_file.h" // Can't neatly forward PlatformFile. -#include "rtc_base/system/file_wrapper.h" - -namespace webrtc { - -class RtcEventLogOutputFile final : public RtcEventLogOutput { - public: - static const size_t kMaxReasonableFileSize; // Explanation at declaration. - - // Unlimited/limited-size output file (by filename). - explicit RtcEventLogOutputFile(const std::string& file_name); - RtcEventLogOutputFile(const std::string& file_name, size_t max_size_bytes); - - // Limited-size output file (by FILE*). This class takes ownership - // of the FILE*, and closes it on destruction. - RtcEventLogOutputFile(FILE* file, size_t max_size_bytes); - - // TODO(bugs.webrtc.org/6463): Deprecated, delete together with the - // corresponding PeerConnection::StartRtcEventLog override. - RtcEventLogOutputFile(rtc::PlatformFile file, size_t max_size_bytes); - - ~RtcEventLogOutputFile() override = default; - - bool IsActive() const override; - - bool Write(const std::string& output) override; - - private: - RtcEventLogOutputFile(FileWrapper file, size_t max_size_bytes); - - // IsActive() can be called either from outside or from inside, but we don't - // want to incur the overhead of a virtual function call if called from inside - // some other function of this class. - inline bool IsActiveInternal() const; - - // Maximum size, or zero for no limit. - const size_t max_size_bytes_; - size_t written_bytes_{0}; - FileWrapper file_; -}; - -} // namespace webrtc +#include "api/rtc_event_log_output_file.h" #endif // LOGGING_RTC_EVENT_LOG_OUTPUT_RTC_EVENT_LOG_OUTPUT_FILE_H_ diff --git a/logging/rtc_event_log/rtc_event_log_unittest.cc b/logging/rtc_event_log/rtc_event_log_unittest.cc index bdd588b0ab..66d80956ed 100644 --- a/logging/rtc_event_log/rtc_event_log_unittest.cc +++ b/logging/rtc_event_log/rtc_event_log_unittest.cc @@ -18,6 +18,7 @@ #include #include "absl/memory/memory.h" +#include "api/rtc_event_log_output_file.h" #include "api/task_queue/default_task_queue_factory.h" #include "logging/rtc_event_log/events/rtc_event_audio_network_adaptation.h" #include "logging/rtc_event_log/events/rtc_event_audio_playout.h" @@ -39,7 +40,6 @@ #include "logging/rtc_event_log/events/rtc_event_rtp_packet_outgoing.h" #include "logging/rtc_event_log/events/rtc_event_video_receive_stream_config.h" #include "logging/rtc_event_log/events/rtc_event_video_send_stream_config.h" -#include "logging/rtc_event_log/output/rtc_event_log_output_file.h" #include "logging/rtc_event_log/rtc_event_log.h" #include "logging/rtc_event_log/rtc_event_log_factory.h" #include "logging/rtc_event_log/rtc_event_log_parser.h" diff --git a/pc/BUILD.gn b/pc/BUILD.gn index d4065208b2..f5ed3dd3c5 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -198,6 +198,7 @@ rtc_static_library("peerconnection") { "../api:fec_controller_api", "../api:libjingle_peerconnection_api", "../api:network_state_predictor_api", + "../api:rtc_event_log_output_file", "../api:rtc_stats_api", "../api:scoped_refptr", "../api/task_queue", @@ -208,7 +209,6 @@ rtc_static_library("peerconnection") { "../common_video", "../logging:ice_log", "../logging:rtc_event_log_api", - "../logging:rtc_event_log_impl_output", "../media:rtc_data", "../media:rtc_media_base", "../modules/rtp_rtcp:rtp_rtcp_format", @@ -536,6 +536,7 @@ if (rtc_include_tests) { ":pc_test_utils", "../api:callfactory_api", "../api:fake_media_transport", + "../api:rtc_event_log_output_file", "../api:rtc_stats_api", "../api/audio_codecs:audio_codecs_api", "../api/audio_codecs:builtin_audio_decoder_factory", @@ -548,7 +549,6 @@ if (rtc_include_tests) { "../call:call_interfaces", "../logging:rtc_event_log_api", "../logging:rtc_event_log_impl_base", - "../logging:rtc_event_log_impl_output", "../media:rtc_audio_video", "../media:rtc_data", # TODO(phoglund): AFAIK only used for one sctp constant. "../media:rtc_media_base", diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 8a6d0e57ac..b3247c6456 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -25,12 +25,12 @@ #include "api/media_stream_proxy.h" #include "api/media_stream_track_proxy.h" #include "api/rtc_error.h" +#include "api/rtc_event_log_output_file.h" #include "api/rtp_parameters.h" #include "api/uma_metrics.h" #include "api/video/builtin_video_bitrate_allocator_factory.h" #include "call/call.h" #include "logging/rtc_event_log/ice_logger.h" -#include "logging/rtc_event_log/output/rtc_event_log_output_file.h" #include "logging/rtc_event_log/rtc_event_log.h" #include "media/base/rid_description.h" #include "media/sctp/sctp_transport.h" diff --git a/pc/peer_connection_interface_unittest.cc b/pc/peer_connection_interface_unittest.cc index d73e45a53d..9bf33066c4 100644 --- a/pc/peer_connection_interface_unittest.cc +++ b/pc/peer_connection_interface_unittest.cc @@ -34,6 +34,7 @@ #include "api/peer_connection_interface.h" #include "api/rtc_error.h" #include "api/rtc_event_log_output.h" +#include "api/rtc_event_log_output_file.h" #include "api/rtp_receiver_interface.h" #include "api/rtp_sender_interface.h" #include "api/rtp_transceiver_interface.h" @@ -42,7 +43,6 @@ #include "api/video_codecs/builtin_video_encoder_factory.h" #include "api/video_codecs/video_decoder_factory.h" #include "api/video_codecs/video_encoder_factory.h" -#include "logging/rtc_event_log/output/rtc_event_log_output_file.h" #include "logging/rtc_event_log/rtc_event_log.h" #include "logging/rtc_event_log/rtc_event_log_factory.h" #include "logging/rtc_event_log/rtc_event_log_factory_interface.h" diff --git a/test/pc/e2e/BUILD.gn b/test/pc/e2e/BUILD.gn index b1297c9203..e0da5b89f2 100644 --- a/test/pc/e2e/BUILD.gn +++ b/test/pc/e2e/BUILD.gn @@ -256,6 +256,7 @@ if (rtc_include_tests) { "../../../api:audio_quality_analyzer_api", "../../../api:libjingle_peerconnection_api", "../../../api:peer_connection_quality_test_fixture_api", + "../../../api:rtc_event_log_output_file", "../../../api:scoped_refptr", "../../../api:video_quality_analyzer_api", "../../../api/task_queue", @@ -263,7 +264,6 @@ if (rtc_include_tests) { "../../../api/units:time_delta", "../../../api/units:timestamp", "../../../logging:rtc_event_log_api", - "../../../logging:rtc_event_log_impl_output", "../../../pc:pc_test_utils", "../../../pc:peerconnection", "../../../rtc_base", diff --git a/test/pc/e2e/peer_connection_quality_test.cc b/test/pc/e2e/peer_connection_quality_test.cc index 4653c7c812..325ac3944e 100644 --- a/test/pc/e2e/peer_connection_quality_test.cc +++ b/test/pc/e2e/peer_connection_quality_test.cc @@ -16,11 +16,11 @@ #include "absl/memory/memory.h" #include "api/media_stream_interface.h" #include "api/peer_connection_interface.h" +#include "api/rtc_event_log_output_file.h" #include "api/scoped_refptr.h" #include "api/task_queue/default_task_queue_factory.h" #include "api/test/video_quality_analyzer_interface.h" #include "api/units/time_delta.h" -#include "logging/rtc_event_log/output/rtc_event_log_output_file.h" #include "logging/rtc_event_log/rtc_event_log.h" #include "pc/sdp_utils.h" #include "pc/test/mock_peer_connection_observers.h" diff --git a/test/scenario/BUILD.gn b/test/scenario/BUILD.gn index 1918a2b414..e00683c281 100644 --- a/test/scenario/BUILD.gn +++ b/test/scenario/BUILD.gn @@ -83,6 +83,7 @@ if (rtc_include_tests) { "../:video_test_common", "../../api:fec_controller_api", "../../api:libjingle_peerconnection_api", + "../../api:rtc_event_log_output_file", "../../api:transport_api", "../../api/audio_codecs:builtin_audio_decoder_factory", "../../api/audio_codecs:builtin_audio_encoder_factory", @@ -105,7 +106,6 @@ if (rtc_include_tests) { "../../common_video", "../../logging:rtc_event_log_api", "../../logging:rtc_event_log_impl_base", - "../../logging:rtc_event_log_impl_output", "../../media:rtc_audio_video", "../../media:rtc_internal_video_codecs", "../../media:rtc_media_base", diff --git a/video/BUILD.gn b/video/BUILD.gn index 8d776d5dde..5e7cb47d44 100644 --- a/video/BUILD.gn +++ b/video/BUILD.gn @@ -246,6 +246,7 @@ if (rtc_include_tests) { deps = [ ":frame_dumping_decoder", "../api:fec_controller_api", + "../api:rtc_event_log_output_file", "../api:test_dependency_factory", "../api:video_quality_test_fixture_api", "../api/task_queue", @@ -258,7 +259,6 @@ if (rtc_include_tests) { "../common_video", "../logging:rtc_event_log_api", "../logging:rtc_event_log_impl_base", - "../logging:rtc_event_log_impl_output", "../media:rtc_audio_video", "../media:rtc_encoder_simulcast_proxy", "../media:rtc_internal_video_codecs", diff --git a/video/video_quality_test.cc b/video/video_quality_test.cc index 803bbb98e3..525650ea38 100644 --- a/video/video_quality_test.cc +++ b/video/video_quality_test.cc @@ -17,11 +17,11 @@ #include #include "absl/memory/memory.h" +#include "api/rtc_event_log_output_file.h" #include "api/task_queue/default_task_queue_factory.h" #include "api/video/builtin_video_bitrate_allocator_factory.h" #include "call/fake_network_pipe.h" #include "call/simulated_network.h" -#include "logging/rtc_event_log/output/rtc_event_log_output_file.h" #include "media/engine/adm_helpers.h" #include "media/engine/encoder_simulcast_proxy.h" #include "media/engine/fake_video_codec_factory.h"