MSE: Record counts of detected MSE audio, video and text tracks

media/filters/chunk_demuxer.cc

Index: media/filters/chunk_demuxer.cc
diff --git a/media/filters/chunk_demuxer.cc b/media/filters/chunk_demuxer.cc
index e13dcc1799b5b118a27720c7a4a89685babd789b..0957b8b78637fba491dce04a26941ef2423d64d4 100644
--- a/media/filters/chunk_demuxer.cc
+++ b/media/filters/chunk_demuxer.cc
@@ -13,6 +13,7 @@
 #include "base/callback_helpers.h"
 #include "base/location.h"
 #include "base/macros.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/stl_util.h"
 #include "media/base/audio_decoder_config.h"
 #include "media/base/bind_to_current_loop.h"
@@ -367,7 +368,10 @@ ChunkDemuxer::ChunkDemuxer(
       duration_(kNoTimestamp()),
       user_specified_duration_(-1),
       liveness_(DemuxerStream::LIVENESS_UNKNOWN),
-      splice_frames_enabled_(splice_frames_enabled) {
+      splice_frames_enabled_(splice_frames_enabled),
+      detected_audio_track_count_(0),
+      detected_video_track_count_(0),
+      detected_text_track_count_(0) {
   DCHECK(!open_cb_.is_null());
   DCHECK(!encrypted_media_init_data_cb_.is_null());
 }
@@ -961,12 +965,25 @@ void ChunkDemuxer::OnSourceInitDone(
       video_->SetLiveness(params.liveness);
   }
 
+  detected_audio_track_count_ += params.detected_audio_track_count;
+  detected_video_track_count_ += params.detected_video_track_count;
+  detected_text_track_count_ += params.detected_text_track_count;
+
   // Wait until all streams have initialized.

[chcunningham, 2016/03/23 23:05:56]

OK, we chatted f2f quite a bit. For posterity, let me state the problem and the
new plan:

PROBLEM:
This check is crucial to guarding the code below from over reporting (as well as
protecting code after that from executing too early). The gotcha is this check
is bad because:


1) the existence of video_ or audio_ does not guarantee that OnSourceInitDone is
called for those streams (which is really what you want to check). It happens
that the stream parser's (at least MP4) are written in such a way that the order
is correct (the callback for stream creation is made just a few lines above the
init done callback), but its really fragile and hard to verify for all parsers.

2) there is no check at all for text_ streams, so if you have a text stream in
separate source buffer, you will definitely over report here.


PLAN:
Add a counter for OnSourceInitDones such that you only proceed past this line
when inits_done_ == num_source_buffers_ (or whatever the equivalent variables
are).

[wolenetz, 2016/03/25 23:25:08]

All current Chromium stream parsers (not just mp4) adhere to the contract of
only calling init_cb *after* config_cb.

https://crbug.com/597447 filed to fix the text-only stream init race problem
described in 2), above.

Regarding (1), it'll need more than just a counter to make less fragile to code
change. Ideally, the init_cb could be removed from the stream parser interface,
and the contract changed such that the first config_cb_ executed by a stream
parser corresponds to that stream parser signalling "I'm initialized". That'd
make it simpler to enforce across all parsers, in
MediaSourceState::OnNewConfigs(), that the OnSourceInitDone() for that
MediaSourceState (1:1 with a SourceBuffer, and 1:1 with a StreamParser) is done
precisely after the first initialization segment has been reported by the parser
without producing error during OnNewConfigs() configs verification.

I'll add this bit into that same bug to get fixed at the same time (again,
ideally... :) )

[chcunningham, 2016/03/29 22:14:50]

Acknowledged.

   if ((!source_id_audio_.empty() && !audio_) ||
       (!source_id_video_.empty() && !video_)) {
     return;
   }
 
+  // Record detected track counts by type corresponding to an MSE playback.
+  // Counts are split into 50 buckets, capped into [0,100] range.
+  UMA_HISTOGRAM_COUNTS_100("Media.MSE.DetectedTrackCount.Audio",
+                           detected_audio_track_count_);
+  UMA_HISTOGRAM_COUNTS_100("Media.MSE.DetectedTrackCount.Video",
+                           detected_video_track_count_);
+  UMA_HISTOGRAM_COUNTS_100("Media.MSE.DetectedTrackCount.Text",
+                           detected_text_track_count_);
+
   SeekAllSources(GetStartTime());
   StartReturningData();