[Video] Collect average keyframe distance for video

media/filters/decoder_stream_traits.cc

Index: media/filters/decoder_stream_traits.cc
diff --git a/media/filters/decoder_stream_traits.cc b/media/filters/decoder_stream_traits.cc
index ec6c7993a1464c4c1347ae0af971a8ad288d85e8..b298f6f6fb28d1faf9f2c0fa81ff1b9106a77df0 100644
--- a/media/filters/decoder_stream_traits.cc
+++ b/media/filters/decoder_stream_traits.cc
@@ -5,6 +5,7 @@
 #include "media/filters/decoder_stream_traits.h"
 
 #include "base/logging.h"
+#include "base/metrics/histogram_macros.h"
 #include "media/base/audio_buffer.h"
 #include "media/base/audio_decoder.h"
 #include "media/base/audio_decoder_config.h"
@@ -77,6 +78,12 @@ void DecoderStreamTraits<DemuxerStream::AUDIO>::OnDecodeDone(
 
 // Video decoder stream traits implementation.
 
+namespace {

[DaleCurtis, 2016/12/03 01:24:57]

Just inline this if it's only used in one spot. Add a comment there about why
you chose this value.

[whywhat, 2016/12/05 19:20:20]

I just guessed :) How would you choose the number here?

+
+const size_t kKeyframeDistanceAverageDepth = 10;
+
+}  // anonymous namespace
+
 // static
 std::string DecoderStreamTraits<DemuxerStream::VIDEO>::ToString() {
   return "video";
@@ -103,6 +110,10 @@ scoped_refptr<DecoderStreamTraits<DemuxerStream::VIDEO>::OutputType>
   return OutputType::CreateEOSFrame();
 }
 
+DecoderStreamTraits<DemuxerStream::VIDEO>::DecoderStreamTraits(
+    const scoped_refptr<MediaLog>& media_log)
+    : keyframe_distance_average_(kKeyframeDistanceAverageDepth) {}
+
 void DecoderStreamTraits<DemuxerStream::VIDEO>::InitializeDecoder(
     DecoderType* decoder,
     DemuxerStream* stream,
@@ -115,4 +126,47 @@ void DecoderStreamTraits<DemuxerStream::VIDEO>::InitializeDecoder(
                       cdm_context, init_cb, output_cb);
 }
 
+void DecoderStreamTraits<DemuxerStream::VIDEO>::OnStreamReset(
+    DemuxerStream* stream) {
+  DCHECK(stream);
+  ResetMetrics();

[DaleCurtis, 2016/12/03 01:24:57]

Why bother if we only care about the max value? This is all still within the
same stream.

[whywhat, 2016/12/05 19:20:20]

I'm not entirely sure when OnStreamReset is called.
I'm worried about the case when we disable the video track for a while - it
seems like the next decoded frame timestamp might be far from the last one which
would bump the max distance up by the time the video was not decoded. At least,
that's what I saw while testing my change -- although it seems like the reset in
end_of_stream() case in OnDecode took care of that.

Perhaps, I don't need to reset |max| at least.

+  max_average_ = base::TimeDelta();
+}
+
+void DecoderStreamTraits<DemuxerStream::VIDEO>::OnDecode(
+    const scoped_refptr<DecoderBuffer>& buffer) {
+  if (!buffer.get())

[DaleCurtis, 2016/12/03 01:24:57]

Drop .get()

+    return;
+
+  if (buffer->end_of_stream()) {
+    ResetMetrics();
+    return;
+  }
+
+  if (!buffer->is_key_frame())
+    return;
+
+  base::TimeDelta current_frame_timestamp = buffer->timestamp();
+  if (last_keyframe_timestamp_.is_zero()) {
+    last_keyframe_timestamp_ = current_frame_timestamp;
+    return;
+  }
+
+  base::TimeDelta last_keyframe_distance =
+      current_frame_timestamp - last_keyframe_timestamp_;
+  last_keyframe_timestamp_ = current_frame_timestamp;
+  keyframe_distance_average_.AddSample(last_keyframe_distance);
+  base::TimeDelta new_average = keyframe_distance_average_.Average();
+  if (new_average > max_average_) {
+    max_average_ = new_average;
+    UMA_HISTOGRAM_MEDIUM_TIMES("Media.Video.KeyFrameDistanceAverage",

[DaleCurtis, 2016/12/03 01:24:57]

I wonder if instead you just want to record every delta and let the UMA sampling
process take care of the actual averaging. The question we're trying to answer
is if a given stream has keyframes near enough for us to enable the
disable-track optimization.

[whywhat, 2016/12/05 19:20:20]

For UMA, perhaps, but when we try to decide whether to disable the video track
or not, won't we need to calculate the average across a few frames to decide?
I'm okay re-adding it in some next patch.

+                               new_average);
+  }
+}
+
+void DecoderStreamTraits<DemuxerStream::VIDEO>::ResetMetrics() {
+  keyframe_distance_average_.Reset();
+  last_keyframe_timestamp_ = base::TimeDelta();
+}
+
 }  // namespace media