diff --git a/webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework.cc b/webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework.cc index a9fd617118..ac0e53d585 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework.cc @@ -217,42 +217,47 @@ uint32_t PacketProcessor::bits_per_second() const { RateCounterFilter::RateCounterFilter(PacketProcessorListener* listener, int flow_id, const char* name, - const std::string& plot_name) + const std::string& algorithm_name) : PacketProcessor(listener, flow_id, kRegular), packets_per_second_stats_(), kbps_stats_(), start_plotting_time_ms_(0), - plot_name_(plot_name) { - std::stringstream ss; - ss << name << "_" << flow_id; - name_ = ss.str(); -} + flow_id_(flow_id), + name_(name), + algorithm_name_(algorithm_name) { + // Only used when compiling with BWE test logging enabled. + RTC_UNUSED(flow_id_); + } RateCounterFilter::RateCounterFilter(PacketProcessorListener* listener, const FlowIds& flow_ids, const char* name, - const std::string& plot_name) + const std::string& algorithm_name) : PacketProcessor(listener, flow_ids, kRegular), packets_per_second_stats_(), kbps_stats_(), start_plotting_time_ms_(0), - plot_name_(plot_name) { + flow_id_(0), + name_(name), + algorithm_name_(algorithm_name) { + // TODO(terelius): Appending the flow IDs to the algorithm name is a hack to + // keep the current plot functionality without having to print the full + // context for each PLOT line. It is unclear whether multiple flow IDs are + // needed at all in the long term. std::stringstream ss; - ss << name; - char delimiter = '_'; + ss << algorithm_name_; for (int flow_id : flow_ids) { - ss << delimiter << flow_id; - delimiter = ','; + ss << ',' << flow_id; } - name_ = ss.str(); + algorithm_name_ = ss.str(); } RateCounterFilter::RateCounterFilter(PacketProcessorListener* listener, const FlowIds& flow_ids, const char* name, int64_t start_plotting_time_ms, - const std::string& plot_name) - : RateCounterFilter(listener, flow_ids, name, plot_name) { + const std::string& algorithm_name) + : RateCounterFilter(listener, flow_ids, name, algorithm_name) { start_plotting_time_ms_ = start_plotting_time_ms; } @@ -277,11 +282,13 @@ void RateCounterFilter::Plot(int64_t timestamp_ms) { plot_kbps = rate_counter_.bits_per_second() / 1000.0; } BWE_TEST_LOGGING_CONTEXT(name_.c_str()); - if (plot_name_.empty()) { - BWE_TEST_LOGGING_PLOT(0, "Throughput_kbps#1", timestamp_ms, plot_kbps); + if (algorithm_name_.empty()) { + BWE_TEST_LOGGING_PLOT_WITH_SSRC(0, "Throughput_kbps#1", timestamp_ms, + plot_kbps, flow_id_); } else { - BWE_TEST_LOGGING_PLOT_WITH_NAME(0, "Throughput_kbps#1", timestamp_ms, - plot_kbps, plot_name_); + BWE_TEST_LOGGING_PLOT_WITH_NAME_AND_SSRC(0, "Throughput_kbps#1", + timestamp_ms, plot_kbps, flow_id_, + algorithm_name_); } RTC_UNUSED(plot_kbps); diff --git a/webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework.h b/webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework.h index 1fe3a228e4..5683d434ad 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework.h +++ b/webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework.h @@ -230,16 +230,16 @@ class RateCounterFilter : public PacketProcessor { RateCounterFilter(PacketProcessorListener* listener, int flow_id, const char* name, - const std::string& plot_name); + const std::string& algorithm_name); RateCounterFilter(PacketProcessorListener* listener, const FlowIds& flow_ids, const char* name, - const std::string& plot_name); + const std::string& algorithm_name); RateCounterFilter(PacketProcessorListener* listener, const FlowIds& flow_ids, const char* name, int64_t start_plotting_time_ms, - const std::string& plot_name); + const std::string& algorithm_name); virtual ~RateCounterFilter(); void LogStats(); @@ -250,10 +250,11 @@ class RateCounterFilter : public PacketProcessor { private: Stats packets_per_second_stats_; Stats kbps_stats_; - std::string name_; int64_t start_plotting_time_ms_; + int flow_id_; + std::string name_; // Algorithm name if single flow, Total link utilization if all flows. - std::string plot_name_; + std::string algorithm_name_; RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(RateCounterFilter); }; diff --git a/webrtc/modules/remote_bitrate_estimator/test/bwe_test_logging.cc b/webrtc/modules/remote_bitrate_estimator/test/bwe_test_logging.cc index 43818b08ca..12042a834d 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/bwe_test_logging.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/bwe_test_logging.cc @@ -18,6 +18,7 @@ #include #include +#include "webrtc/base/format_macros.h" #include "webrtc/base/platform_thread.h" #include "webrtc/system_wrappers/include/critical_section_wrapper.h" @@ -90,19 +91,26 @@ void Logging::Log(const char format[], ...) { } } -void Logging::Plot(int figure, double value) { - Plot(figure, value, 0, "-"); -} - -void Logging::Plot(int figure, double value, uint32_t ssrc) { - Plot(figure, value, ssrc, "-"); -} - -void Logging::Plot(int figure, double value, const std::string& alg_name) { - Plot(figure, value, 0, alg_name); +void Logging::Plot(int figure, const std::string& name, double value) { + Plot(figure, name, value, 0, "-"); } void Logging::Plot(int figure, + const std::string& name, + double value, + uint32_t ssrc) { + Plot(figure, name, value, ssrc, "-"); +} + +void Logging::Plot(int figure, + const std::string& name, + double value, + const std::string& alg_name) { + Plot(figure, name, value, 0, alg_name); +} + +void Logging::Plot(int figure, + const std::string& name, double value, uint32_t ssrc, const std::string& alg_name) { @@ -110,20 +118,9 @@ void Logging::Plot(int figure, ThreadMap::iterator it = thread_map_.find(rtc::CurrentThreadId()); assert(it != thread_map_.end()); const State& state = it->second.stack.top(); - std::stringstream ss; - ss << ssrc; - std::string label = state.tag + ':' + ss.str() + '@' + alg_name; - std::string prefix("Available"); - if (alg_name.compare(0, prefix.length(), prefix) == 0) { - std::string receiver("Receiver"); - size_t start_pos = label.find(receiver); - if (start_pos != std::string::npos) { - label.replace(start_pos, receiver.length(), "Sender"); - } - } if (state.enabled) { - printf("PLOT\t%d\t%s\t%f\t%f\n", figure, label.c_str(), - state.timestamp_ms * 0.001, value); + printf("PLOT\t%d\t%s:%" PRIu32 "@%s\t%f\t%f\n", figure, name.c_str(), ssrc, + alg_name.c_str(), state.timestamp_ms * 0.001, value); } } diff --git a/webrtc/modules/remote_bitrate_estimator/test/bwe_test_logging.h b/webrtc/modules/remote_bitrate_estimator/test/bwe_test_logging.h index 7a7bca1fce..9262164b2e 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/bwe_test_logging.h +++ b/webrtc/modules/remote_bitrate_estimator/test/bwe_test_logging.h @@ -183,26 +183,27 @@ _4, _5); \ } while (0) -#define BWE_TEST_LOGGING_PLOT(figure, name, time, value) \ - do { \ - __BWE_TEST_LOGGING_CONTEXT_DECLARE(__bwe_log_, __PLOT__, name, \ - static_cast(time), true); \ - webrtc::testing::bwe::Logging::GetInstance()->Plot(figure, value); \ +#define BWE_TEST_LOGGING_PLOT(figure, name, time, value) \ + do { \ + __BWE_TEST_LOGGING_CONTEXT_DECLARE(__bwe_log_, __PLOT__, name, \ + static_cast(time), true); \ + webrtc::testing::bwe::Logging::GetInstance()->Plot(figure, name, value); \ } while (0) #define BWE_TEST_LOGGING_PLOT_WITH_NAME(figure, name, time, value, alg_name) \ do { \ __BWE_TEST_LOGGING_CONTEXT_DECLARE(__bwe_log_, __PLOT__, name, \ static_cast(time), true); \ - webrtc::testing::bwe::Logging::GetInstance()->Plot(figure, value, \ + webrtc::testing::bwe::Logging::GetInstance()->Plot(figure, name, value, \ alg_name); \ } while (0) -#define BWE_TEST_LOGGING_PLOT_WITH_SSRC(figure, name, time, value, ssrc) \ - do { \ - __BWE_TEST_LOGGING_CONTEXT_DECLARE(__bwe_log_, __PLOT__, name, \ - static_cast(time), true); \ - webrtc::testing::bwe::Logging::GetInstance()->Plot(figure, value, ssrc); \ +#define BWE_TEST_LOGGING_PLOT_WITH_SSRC(figure, name, time, value, ssrc) \ + do { \ + __BWE_TEST_LOGGING_CONTEXT_DECLARE(__bwe_log_, __PLOT__, name, \ + static_cast(time), true); \ + webrtc::testing::bwe::Logging::GetInstance()->Plot(figure, name, value, \ + ssrc); \ } while (0) #define BWE_TEST_LOGGING_PLOT_WITH_NAME_AND_SSRC(figure, name, time, value, \ @@ -210,8 +211,8 @@ do { \ __BWE_TEST_LOGGING_CONTEXT_DECLARE(__bwe_log_, __PLOT__, name, \ static_cast(time), true); \ - webrtc::testing::bwe::Logging::GetInstance()->Plot(figure, value, ssrc, \ - alg_name); \ + webrtc::testing::bwe::Logging::GetInstance()->Plot(figure, name, value, \ + ssrc, alg_name); \ } while (0) #define BWE_TEST_LOGGING_BAR(figure, name, value, flow_id) \ @@ -279,10 +280,14 @@ class Logging { void SetGlobalEnable(bool enabled); void Log(const char format[], ...); - void Plot(int figure, double value); - void Plot(int figure, double value, const std::string& alg_name); - void Plot(int figure, double value, uint32_t ssrc); + void Plot(int figure, const std::string& name, double value); void Plot(int figure, + const std::string& name, + double value, + const std::string& alg_name); + void Plot(int figure, const std::string& name, double value, uint32_t ssrc); + void Plot(int figure, + const std::string& name, double value, uint32_t ssrc, const std::string& alg_name); diff --git a/webrtc/modules/remote_bitrate_estimator/test/metric_recorder.cc b/webrtc/modules/remote_bitrate_estimator/test/metric_recorder.cc index b5a2ba3f15..525f5fc273 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/metric_recorder.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/metric_recorder.cc @@ -124,13 +124,13 @@ void MetricRecorder::PlotAllDynamics() { void MetricRecorder::PlotDynamics(int metric) { if (metric == kTotalAvailable) { - BWE_TEST_LOGGING_PLOT_WITH_NAME( + BWE_TEST_LOGGING_PLOT_WITH_NAME_AND_SSRC( 0, plot_information_[kTotalAvailable].prefix, now_ms_, - GetTotalAvailableKbps(), "Available"); + GetTotalAvailableKbps(), flow_id_, "Available"); } else if (metric == kAvailablePerFlow) { - BWE_TEST_LOGGING_PLOT_WITH_NAME( + BWE_TEST_LOGGING_PLOT_WITH_NAME_AND_SSRC( 0, plot_information_[kAvailablePerFlow].prefix, now_ms_, - GetAvailablePerFlowKbps(), "Available_per_flow"); + GetAvailablePerFlowKbps(), flow_id_, "Available_per_flow"); } else { PlotLine(metric, plot_information_[metric].prefix, plot_information_[metric].time_ms, @@ -144,8 +144,9 @@ void MetricRecorder::PlotLine(int windows_id, const std::string& prefix, int64_t time_ms, T y) { - BWE_TEST_LOGGING_PLOT_WITH_NAME(windows_id, prefix, time_ms, - static_cast(y), algorithm_name_); + BWE_TEST_LOGGING_PLOT_WITH_NAME_AND_SSRC(windows_id, prefix, time_ms, + static_cast(y), flow_id_, + algorithm_name_); } void MetricRecorder::UpdateTimeMs(int64_t time_ms) { @@ -355,6 +356,8 @@ void MetricRecorder::PlotZero() { for (int i = kThroughput; i <= kLoss; ++i) { if (plot_information_[i].plot) { std::stringstream prefix; + // TODO(terelius): Since this does not use the BWE_TEST_LOGGING macros, + // it hasn't been kept up to date with the plot format. Remove or fix? prefix << "Receiver_" << flow_id_ << "_" + plot_information_[i].prefix; PlotLine(i, prefix.str(), now_ms_, 0); plot_information_[i].last_plot_ms = now_ms_; diff --git a/webrtc/modules/remote_bitrate_estimator/test/packet_receiver.cc b/webrtc/modules/remote_bitrate_estimator/test/packet_receiver.cc index 2f5dcee684..5bc6219cb5 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/packet_receiver.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/packet_receiver.cc @@ -44,15 +44,15 @@ PacketReceiver::PacketReceiver(PacketProcessorListener* listener, // Metric recorder plots them in separated figures, // alignment will take place with the #1 left axis. - prefixes.push_back("Throughput_kbps#1"); + prefixes.push_back("MetricRecorderThroughput_kbps#1"); prefixes.push_back("Sending_Estimate_kbps#1"); prefixes.push_back("Delay_ms_#1"); prefixes.push_back("Packet_Loss_#1"); prefixes.push_back("Objective_function_#1"); // Plot Total/PerFlow Available capacity together with throughputs. - prefixes.push_back("Throughput_kbps#1"); // Total Available. - prefixes.push_back("Throughput_kbps#1"); // Available per flow. + prefixes.push_back("Capacity_kbps#1"); // Total Available. + prefixes.push_back("PerFlowCapacity_kbps#1"); // Available per flow. bool plot_loss = plot_delay; // Plot loss if delay is plotted. metric_recorder_->SetPlotInformation(prefixes, plot_delay, plot_loss); diff --git a/webrtc/modules/remote_bitrate_estimator/test/plot_dynamics.py b/webrtc/modules/remote_bitrate_estimator/test/plot_dynamics.py index 164300141d..dac47626de 100755 --- a/webrtc/modules/remote_bitrate_estimator/test/plot_dynamics.py +++ b/webrtc/modules/remote_bitrate_estimator/test/plot_dynamics.py @@ -7,11 +7,15 @@ # in the file PATENTS. All contributing project authors may # be found in the AUTHORS file in the root of the source tree. -# This script is used to plot simulation dynamics. -# Able to plot each flow separately. Other plot boxes can be added, -# currently one for Throughput, one for Latency and one for Packet Loss. +# This script is used to plot simulation dynamics. The expected format is +# PLOT :@