[Video] Collect average keyframe distance for video

media/filters/decoder_stream_traits.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #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"
 #include "media/base/video_decoder.h"
 #include "media/base/video_frame.h"
 
 namespace media {
 
 // Audio decoder stream traits implementation.
 
@@ skipping 52 matching lines @@
   audio_ts_validator_->CheckForTimestampGap(buffer);
 }
 
 void DecoderStreamTraits<DemuxerStream::AUDIO>::OnDecodeDone(
     const scoped_refptr<OutputType>& buffer) {
   audio_ts_validator_->RecordOutputDuration(buffer);
 }
 
 // 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";
 }
 
 // static
 bool DecoderStreamTraits<DemuxerStream::VIDEO>::NeedsBitstreamConversion(
     DecoderType* decoder) {
   return decoder->NeedsBitstreamConversion();
 }
 
 // static
 void DecoderStreamTraits<DemuxerStream::VIDEO>::ReportStatistics(
     const StatisticsCB& statistics_cb,
     int bytes_decoded) {
   PipelineStatistics statistics;
   statistics.video_bytes_decoded = bytes_decoded;
   statistics_cb.Run(statistics);
 }
 
 // static
 scoped_refptr<DecoderStreamTraits<DemuxerStream::VIDEO>::OutputType>
     DecoderStreamTraits<DemuxerStream::VIDEO>::CreateEOSOutput() {
   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,
     CdmContext* cdm_context,
     const InitCB& init_cb,
     const OutputCB& output_cb) {
   DCHECK(stream->video_decoder_config().IsValidConfig());
   decoder->Initialize(stream->video_decoder_config(),
                       stream->liveness() == DemuxerStream::LIVENESS_LIVE,
                       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