Add RtcEventLogFactory::Create variant that uses Environment
With intent to delete previous versions of the Create functions.
Bug: webrtc:15656
Change-Id: I972377701becca21b8ecfe15d41a10a4248f87ec
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/328420
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41275}
diff --git a/api/rtc_event_log/BUILD.gn b/api/rtc_event_log/BUILD.gn
index 158dc06..274c6ca 100644
--- a/api/rtc_event_log/BUILD.gn
+++ b/api/rtc_event_log/BUILD.gn
@@ -22,8 +22,10 @@
"..:libjingle_logging_api",
"../../rtc_base:checks",
"../../rtc_base:timeutils",
+ "../environment",
"../task_queue",
]
+ absl_deps = [ "//third_party/abseil-cpp/absl/base:nullability" ]
}
rtc_library("rtc_event_log_factory") {
@@ -35,12 +37,16 @@
deps = [
":rtc_event_log",
+ "..:field_trials_view",
"../../rtc_base:checks",
"../../rtc_base/system:rtc_export",
"../../system_wrappers:field_trial",
+ "../environment",
"../task_queue",
]
+ absl_deps = [ "//third_party/abseil-cpp/absl/base:nullability" ]
+
if (rtc_enable_protobuf) {
defines = [ "WEBRTC_ENABLE_RTC_EVENT_LOG" ]
deps += [ "../../logging:rtc_event_log_impl" ]
diff --git a/api/rtc_event_log/rtc_event_log_factory.cc b/api/rtc_event_log/rtc_event_log_factory.cc
index a3cb68c..c77011a 100644
--- a/api/rtc_event_log/rtc_event_log_factory.cc
+++ b/api/rtc_event_log/rtc_event_log_factory.cc
@@ -13,6 +13,7 @@
#include <memory>
#include <utility>
+#include "api/field_trials_view.h"
#include "rtc_base/checks.h"
#include "system_wrappers/include/field_trial.h"
@@ -27,9 +28,27 @@
RTC_DCHECK(task_queue_factory_);
}
+absl::Nonnull<std::unique_ptr<RtcEventLog>> RtcEventLogFactory::Create(
+ const Environment& env) const {
+#ifndef WEBRTC_ENABLE_RTC_EVENT_LOG
+ return std::make_unique<RtcEventLogNull>();
+#else
+ if (env.field_trials().IsEnabled("WebRTC-RtcEventLogKillSwitch")) {
+ return std::make_unique<RtcEventLogNull>();
+ }
+ RtcEventLog::EncodingType encoding_type =
+ env.field_trials().IsDisabled("WebRTC-RtcEventLogNewFormat")
+ ? RtcEventLog::EncodingType::Legacy
+ : RtcEventLog::EncodingType::NewFormat;
+ return std::make_unique<RtcEventLogImpl>(
+ RtcEventLogImpl::CreateEncoder(encoding_type), &env.task_queue_factory());
+#endif
+}
+
std::unique_ptr<RtcEventLog> RtcEventLogFactory::Create(
RtcEventLog::EncodingType encoding_type) const {
#ifdef WEBRTC_ENABLE_RTC_EVENT_LOG
+ RTC_DCHECK(task_queue_factory_);
if (field_trial::IsEnabled("WebRTC-RtcEventLogKillSwitch")) {
return std::make_unique<RtcEventLogNull>();
}
diff --git a/api/rtc_event_log/rtc_event_log_factory.h b/api/rtc_event_log/rtc_event_log_factory.h
index fd1db3c..f0cf6d7 100644
--- a/api/rtc_event_log/rtc_event_log_factory.h
+++ b/api/rtc_event_log/rtc_event_log_factory.h
@@ -13,6 +13,8 @@
#include <memory>
+#include "absl/base/nullability.h"
+#include "api/environment/environment.h"
#include "api/rtc_event_log/rtc_event_log.h"
#include "api/rtc_event_log/rtc_event_log_factory_interface.h"
#include "api/task_queue/task_queue_factory.h"
@@ -22,8 +24,16 @@
class RTC_EXPORT RtcEventLogFactory : public RtcEventLogFactoryInterface {
public:
+ RtcEventLogFactory() = default;
+
+ // TODO(bugs.webrtc.org/15656): deprecate and delete constructor taking
+ // task queue factory in favor of using task queue factory provided through
+ // the Environment parameter in Create function.
explicit RtcEventLogFactory(TaskQueueFactory* task_queue_factory);
- ~RtcEventLogFactory() override {}
+ ~RtcEventLogFactory() override = default;
+
+ absl::Nonnull<std::unique_ptr<RtcEventLog>> Create(
+ const Environment& env) const override;
std::unique_ptr<RtcEventLog> Create(
RtcEventLog::EncodingType encoding_type) const override;
@@ -31,7 +41,7 @@
RtcEventLog::EncodingType encoding_type) override;
private:
- TaskQueueFactory* const task_queue_factory_;
+ TaskQueueFactory* const task_queue_factory_ = nullptr;
};
} // namespace webrtc
diff --git a/api/rtc_event_log/rtc_event_log_factory_interface.h b/api/rtc_event_log/rtc_event_log_factory_interface.h
index a6f4dee..d0374fd 100644
--- a/api/rtc_event_log/rtc_event_log_factory_interface.h
+++ b/api/rtc_event_log/rtc_event_log_factory_interface.h
@@ -13,6 +13,8 @@
#include <memory>
+#include "absl/base/nullability.h"
+#include "api/environment/environment.h"
#include "api/rtc_event_log/rtc_event_log.h"
namespace webrtc {
@@ -24,6 +26,11 @@
public:
virtual ~RtcEventLogFactoryInterface() = default;
+ virtual absl::Nonnull<std::unique_ptr<RtcEventLog>> Create(
+ const Environment& env) const = 0;
+
+ // TODO(bugs.webrtc.org/15656): Delete functions below when all usage is
+ // migrated to the Create(const Environment&) function above.
virtual std::unique_ptr<RtcEventLog> Create(
RtcEventLog::EncodingType encoding_type) const = 0;
[[deprecated]] virtual std::unique_ptr<RtcEventLog> CreateRtcEventLog(
diff --git a/logging/BUILD.gn b/logging/BUILD.gn
index 95c2c90..92f55ed 100644
--- a/logging/BUILD.gn
+++ b/logging/BUILD.gn
@@ -482,10 +482,12 @@
]
deps = [
+ "../api/environment",
"../api/rtc_event_log",
"../rtc_base:macromagic",
"../rtc_base/synchronization:mutex",
]
+ absl_deps = [ "//third_party/abseil-cpp/absl/base:nullability" ]
}
if (rtc_enable_protobuf) {
@@ -604,9 +606,10 @@
"../api:rtc_event_log_output_file",
"../api:rtp_headers",
"../api:rtp_parameters",
+ "../api/environment",
+ "../api/environment:environment_factory",
"../api/rtc_event_log",
"../api/rtc_event_log:rtc_event_log_factory",
- "../api/task_queue:default_task_queue_factory",
"../api/units:time_delta",
"../api/units:timestamp",
"../call",
@@ -621,6 +624,7 @@
"../rtc_base:timeutils",
"../system_wrappers",
"../system_wrappers:field_trial",
+ "../test:explicit_key_value_config",
"../test:field_trial",
"../test:fileutils",
"../test:test_support",
diff --git a/logging/rtc_event_log/fake_rtc_event_log_factory.cc b/logging/rtc_event_log/fake_rtc_event_log_factory.cc
index 47db40c..9a27b63 100644
--- a/logging/rtc_event_log/fake_rtc_event_log_factory.cc
+++ b/logging/rtc_event_log/fake_rtc_event_log_factory.cc
@@ -17,17 +17,26 @@
namespace webrtc {
-std::unique_ptr<RtcEventLog> FakeRtcEventLogFactory::Create(
- RtcEventLog::EncodingType /*encoding_type*/) const {
+absl::Nonnull<std::unique_ptr<FakeRtcEventLog>>
+FakeRtcEventLogFactory::CreateFake() const {
auto fake_event_log = std::make_unique<FakeRtcEventLog>();
- const_cast<FakeRtcEventLogFactory*>(this)->last_log_created_ =
- fake_event_log.get();
+ const_cast<FakeRtcEventLog*&>(last_log_created_) = fake_event_log.get();
return fake_event_log;
}
+std::unique_ptr<RtcEventLog> FakeRtcEventLogFactory::Create(
+ const Environment& /*env*/) const {
+ return CreateFake();
+}
+
+std::unique_ptr<RtcEventLog> FakeRtcEventLogFactory::Create(
+ RtcEventLog::EncodingType /*encoding_type*/) const {
+ return CreateFake();
+}
+
std::unique_ptr<RtcEventLog> FakeRtcEventLogFactory::CreateRtcEventLog(
- RtcEventLog::EncodingType encoding_type) {
- return Create(encoding_type);
+ RtcEventLog::EncodingType /*encoding_type*/) {
+ return CreateFake();
}
} // namespace webrtc
diff --git a/logging/rtc_event_log/fake_rtc_event_log_factory.h b/logging/rtc_event_log/fake_rtc_event_log_factory.h
index c7ff33d..7110555 100644
--- a/logging/rtc_event_log/fake_rtc_event_log_factory.h
+++ b/logging/rtc_event_log/fake_rtc_event_log_factory.h
@@ -13,6 +13,8 @@
#include <memory>
+#include "absl/base/nullability.h"
+#include "api/environment/environment.h"
#include "api/rtc_event_log/rtc_event_log_factory_interface.h"
#include "logging/rtc_event_log/fake_rtc_event_log.h"
@@ -23,16 +25,21 @@
FakeRtcEventLogFactory() = default;
~FakeRtcEventLogFactory() override = default;
+ absl::Nonnull<std::unique_ptr<RtcEventLog>> Create(
+ const Environment& env) const override;
+
std::unique_ptr<RtcEventLog> Create(
RtcEventLog::EncodingType encoding_type) const override;
std::unique_ptr<RtcEventLog> CreateRtcEventLog(
RtcEventLog::EncodingType encoding_type) override;
- webrtc::FakeRtcEventLog* last_log_created() { return last_log_created_; }
+ FakeRtcEventLog* last_log_created() { return last_log_created_; }
private:
- webrtc::FakeRtcEventLog* last_log_created_;
+ absl::Nonnull<std::unique_ptr<FakeRtcEventLog>> CreateFake() const;
+
+ FakeRtcEventLog* last_log_created_ = nullptr;
};
} // namespace webrtc
diff --git a/logging/rtc_event_log/rtc_event_log_unittest.cc b/logging/rtc_event_log/rtc_event_log_unittest.cc
index 3730a08..a300572 100644
--- a/logging/rtc_event_log/rtc_event_log_unittest.cc
+++ b/logging/rtc_event_log/rtc_event_log_unittest.cc
@@ -19,8 +19,9 @@
#include <utility>
#include <vector>
+#include "api/environment/environment.h"
+#include "api/environment/environment_factory.h"
#include "api/rtc_event_log/rtc_event_log_factory.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"
#include "logging/rtc_event_log/events/rtc_event_audio_receive_stream_config.h"
@@ -50,6 +51,7 @@
#include "rtc_base/checks.h"
#include "rtc_base/fake_clock.h"
#include "rtc_base/random.h"
+#include "test/explicit_key_value_config.h"
#include "test/gtest.h"
#include "test/logging/memory_log_writer.h"
#include "test/testsupport/file_utils.h"
@@ -58,6 +60,8 @@
namespace {
+using test::ExplicitKeyValueConfig;
+
struct EventCounts {
size_t audio_send_streams = 0;
size_t audio_recv_streams = 0;
@@ -105,6 +109,21 @@
}
};
+std::unique_ptr<FieldTrialsView> CreateFieldTrialsFor(
+ RtcEventLog::EncodingType encoding_type) {
+ switch (encoding_type) {
+ case RtcEventLog::EncodingType::Legacy:
+ return std::make_unique<ExplicitKeyValueConfig>(
+ "WebRTC-RtcEventLogNewFormat/Disabled/");
+ case RtcEventLog::EncodingType::NewFormat:
+ return std::make_unique<ExplicitKeyValueConfig>(
+ "WebRTC-RtcEventLogNewFormat/Enabled/");
+ case RtcEventLog::EncodingType::ProtoFree:
+ RTC_CHECK(false);
+ return nullptr;
+ }
+}
+
class RtcEventLogSession
: public ::testing::TestWithParam<
std::tuple<uint64_t, int64_t, RtcEventLog::EncodingType>> {
@@ -336,14 +355,13 @@
void RtcEventLogSession::WriteLog(EventCounts count,
size_t num_events_before_start) {
- // TODO(terelius): Allow test to run with either a real or a fake clock_.
- // Maybe always use the ScopedFakeClock, but conditionally SleepMs()?
+ // TODO(terelius): Allow test to run with either a real or a fake clock_
+ // e.g. by using clock and task_queue_factory from TimeController
+ // when RtcEventLogImpl switches to use injected clock from the environment.
- auto task_queue_factory = CreateDefaultTaskQueueFactory();
- RtcEventLogFactory rtc_event_log_factory(task_queue_factory.get());
// The log will be flushed to output when the event_log goes out of scope.
- std::unique_ptr<RtcEventLog> event_log =
- rtc_event_log_factory.CreateRtcEventLog(encoding_type_);
+ std::unique_ptr<RtcEventLog> event_log = RtcEventLogFactory().Create(
+ CreateEnvironment(CreateFieldTrialsFor(encoding_type_)));
// We can't send or receive packets without configured streams.
RTC_CHECK_GE(count.video_recv_streams, 1);
@@ -934,12 +952,9 @@
int64_t start_time_us, utc_start_time_us, stop_time_us;
{
- auto task_queue_factory = CreateDefaultTaskQueueFactory();
- RtcEventLogFactory rtc_event_log_factory(task_queue_factory.get());
- // When `log` goes out of scope, the contents are flushed
- // to the output.
- std::unique_ptr<RtcEventLog> log =
- rtc_event_log_factory.CreateRtcEventLog(encoding_type_);
+ // When `log` goes out of scope, the contents are flushed to the output.
+ std::unique_ptr<RtcEventLog> log = RtcEventLogFactory().Create(
+ CreateEnvironment(CreateFieldTrialsFor(encoding_type_)));
for (size_t i = 0; i < kNumEvents; i++) {
// The purpose of the test is to verify that the log can handle
