Added -show_detector_state which show the detector state in the total bitrate graph.

webrtc/tools/event_log_visualizer/analyzer.cc

 /*
  *  Copyright (c) 2016 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.
  */
 
@@ skipping 865 matching lines @@
   plot->SetXAxis(0, call_duration_s_, "Time (s)", kLeftMargin, kRightMargin);
   plot->SetSuggestedYAxis(0, 10, "Percent lost packets", kBottomMargin,
                           kTopMargin);
   plot->SetTitle("Reported packet loss");
   plot->AppendTimeSeries(std::move(time_series));
 }
 
 // Plot the total bandwidth used by all RTP streams.
 void EventLogAnalyzer::CreateTotalBitrateGraph(
     PacketDirection desired_direction,
-    Plot* plot) {
+    Plot* plot,
+    bool show_detector_state) {
   struct TimestampSize {
     TimestampSize(uint64_t t, size_t s) : timestamp(t), size(s) {}
     uint64_t timestamp;
     size_t size;
   };
   std::vector<TimestampSize> packets;
 
   PacketDirection direction;
   size_t total_length;
 
@@ skipping 39 matching lines @@
   // Overlay the send-side bandwidth estimate over the outgoing bitrate.
   if (desired_direction == kOutgoingPacket) {
     TimeSeries loss_series("Loss-based estimate", LINE_STEP_GRAPH);
     for (auto& loss_update : bwe_loss_updates_) {
       float x =
           static_cast<float>(loss_update.timestamp - begin_time_) / 1000000;
       float y = static_cast<float>(loss_update.new_bitrate) / 1000;
       loss_series.points.emplace_back(x, y);
     }
 
+    TimeSeries overusing_series("Overusing", VSPAN_GRAPH);
+    overusing_series.color = "#ff8e82";

[terelius, 2017/04/24 11:58:06]

Color seems like a UI choice. I'd  prefer to not include that in the TimeSeries
object.

[philipel, 2017/04/26 12:56:28]

I think it makes sense to have defined colors for overuse/underuse/normal state.
If we used generated colors they might be different every time, but also overuse
might be marked as green, which seems counter intuitive to me.

[terelius, 2017/04/26 13:40:09]

But with the new code, every TimeSeries will have a color attribute which gets
ignored by the drawing code in almost all cases. This seems even more counter
intuitive to me.

If we want to hard-code colors, then I think it would be more appropriate to do
so in the drawing code. Maybe a VSPAN series labeled "Overusing" should always
be rendered in red? Or maybe the VSPAN shouldn't be a TimeSeries at all, but a
completely separate object?

[philipel, 2017/04/26 14:08:01]

I think the best would be to make color optional, and if it is set then we use
that, if not let the plot tool chose color.

[terelius, 2017/04/27 11:30:59]

That would make it hard to write the drawing code with a clean color scheme or
palette. For instance, you can't guarantee that different colors are clearly
distinguishable. Adding a color also mixes the semantic content of the data with
the visualization details.

Since the new series type seems to be a special case compared the existing ones,
I still think it would be better to either:
1) In the drawing code for VSPAN, set the color to #ff8e82, #5092fc or #c4ffc4
depending on the label used. It would still be a "hack", but at least it would
be localized to only the VSPAN type.
2) Create a new class to represent the colored background. This would open up
many new possibilities both for more efficient encodings, clearer variable
names, and alternative visualizations in the future.
For example,
struct Interval {
  double begin;
  double end;
}
struct IntervalSeries {
  string label;
  string color;
  vector<Interval> intervals;
}
In particular, we would get rid of the unused y-coordinates and we would no
longer have to interpret points differently depending on whether they are placed
in an even or odd position in the TimeSeries.

+    TimeSeries underusing_series("Underusing", VSPAN_GRAPH);
+    underusing_series.color = "#5092fc";
+    TimeSeries normal_series("Normal", VSPAN_GRAPH);
+    normal_series.color = "#c4ffc4";
+    normal_series.points.emplace_back(0, 0);
     TimeSeries delay_series("Delay-based estimate", LINE_STEP_GRAPH);
+    TimeSeries* last_series = &normal_series;
+    BandwidthUsage detector_state = BandwidthUsage::kBwNormal;

[terelius, 2017/04/24 11:58:06]

last_detector_state to make it correspond to last_series?

[philipel, 2017/04/26 12:56:28]

Having a detector state makes things so much easier to detect when the state has
changed (line 963).

[terelius, 2017/04/26 13:40:09]

I meant changing the name from detector_state to last_detector_state, since it
is not the current state but the state at the last BWE update.

[philipel, 2017/04/26 14:08:01]

Ah... Done :)

+
     for (auto& delay_update : bwe_delay_updates_) {
       float x =
           static_cast<float>(delay_update.timestamp - begin_time_) / 1000000;
       float y = static_cast<float>(delay_update.bitrate_bps) / 1000;
+
+      if (detector_state != delay_update.detector_state) {
+        RTC_CHECK(last_series);
+        last_series->points.emplace_back(x, 0);
+        last_series = nullptr;
+      }
+
+      switch (delay_update.detector_state) {
+        case BandwidthUsage::kBwNormal:
+          if (detector_state != BandwidthUsage::kBwNormal) {
+            normal_series.points.emplace_back(x, 0);
+            last_series = &normal_series;
+            detector_state = delay_update.detector_state;
+          }
+          break;
+        case BandwidthUsage::kBwUnderusing:
+          if (detector_state != BandwidthUsage::kBwUnderusing) {
+            underusing_series.points.emplace_back(x, 0);
+            last_series = &underusing_series;
+            detector_state = delay_update.detector_state;
+          }
+          break;
+        case BandwidthUsage::kBwOverusing:
+          if (detector_state != BandwidthUsage::kBwOverusing) {
+            overusing_series.points.emplace_back(x, 0);
+            last_series = &overusing_series;
+            detector_state = delay_update.detector_state;
+          }
+          break;
+      }
+
       delay_series.points.emplace_back(x, y);
     }
 
+    RTC_CHECK(last_series);
+    last_series->points.emplace_back(end_time_, 0);
+
     TimeSeries created_series("Probe cluster created.", DOT_GRAPH);
     for (auto& cluster : bwe_probe_cluster_created_events_) {
       float x = static_cast<float>(cluster.timestamp - begin_time_) / 1000000;
       float y = static_cast<float>(cluster.bitrate_bps) / 1000;
       created_series.points.emplace_back(x, y);
     }
 
     TimeSeries result_series("Probing results.", DOT_GRAPH);
     for (auto& result : bwe_probe_result_events_) {
       if (result.bitrate_bps) {
         float x = static_cast<float>(result.timestamp - begin_time_) / 1000000;
         float y = static_cast<float>(*result.bitrate_bps) / 1000;
         result_series.points.emplace_back(x, y);
       }
     }
+
+    if (show_detector_state) {
+      plot->AppendTimeSeries(std::move(underusing_series));
+      plot->AppendTimeSeries(std::move(normal_series));
+      plot->AppendTimeSeries(std::move(overusing_series));
+    }
+
+    plot->AppendTimeSeries(std::move(bitrate_series));
     plot->AppendTimeSeries(std::move(loss_series));
     plot->AppendTimeSeries(std::move(delay_series));
     plot->AppendTimeSeries(std::move(created_series));
     plot->AppendTimeSeries(std::move(result_series));
   }
 
   plot->SetXAxis(0, call_duration_s_, "Time (s)", kLeftMargin, kRightMargin);
   plot->SetSuggestedYAxis(0, 1, "Bitrate (kbps)", kBottomMargin, kTopMargin);
   if (desired_direction == webrtc::PacketDirection::kIncomingPacket) {
     plot->SetTitle("Incoming RTP bitrate");
@@ skipping 409 matching lines @@
       },
       audio_network_adaptation_events_, begin_time_, &time_series);
   plot->AppendTimeSeries(std::move(time_series));
   plot->SetXAxis(0, call_duration_s_, "Time (s)", kLeftMargin, kRightMargin);
   plot->SetSuggestedYAxis(0, 1, "Number of channels (1 (mono)/2 (stereo))",
                           kBottomMargin, kTopMargin);
   plot->SetTitle("Reported audio encoder number of channels");
 }
 }  // namespace plotting
 }  // namespace webrtc