Add UMA metrics for VideoCodec.MP4 and VideoCodec.WebM

media/filters/ffmpeg_demuxer.cc

Index: media/filters/ffmpeg_demuxer.cc
diff --git a/media/filters/ffmpeg_demuxer.cc b/media/filters/ffmpeg_demuxer.cc
index 081bcbf1a452be23ee45adb6a080e11bd5b55c77..d9ed87e0ee9059cb3290dd582791783f1cdff71c 100644
--- a/media/filters/ffmpeg_demuxer.cc
+++ b/media/filters/ffmpeg_demuxer.cc
@@ -58,9 +58,10 @@ void SetAVStreamDiscard(AVStream* stream, AVDiscard discard) {
 
 }  // namespace
 
-static base::Time ExtractTimelineOffset(AVFormatContext* format_context) {
-  if (strstr(format_context->iformat->name, "webm") ||
-      strstr(format_context->iformat->name, "matroska")) {
+static base::Time ExtractTimelineOffset(
+    container_names::MediaContainerName container,
+    const AVFormatContext* format_context) {
+  if (container == container_names::CONTAINER_WEBM) {
     const AVDictionaryEntry* entry =
         av_dict_get(format_context->metadata, "creation_time", NULL, 0);
 
@@ -160,7 +161,8 @@ static void RecordAudioCodecStats(const AudioDecoderConfig& audio_config) {
 }
 
 // Record video decoder config UMA stats corresponding to a src= playback.
-static void RecordVideoCodecStats(const VideoDecoderConfig& video_config,
+static void RecordVideoCodecStats(container_names::MediaContainerName container,
+                                  const VideoDecoderConfig& video_config,
                                   AVColorRange color_range,
                                   MediaLog* media_log) {
   media_log->RecordRapporWithSecurityOrigin("Media.OriginUrl.SRC.VideoCodec." +
@@ -168,6 +170,13 @@ static void RecordVideoCodecStats(const VideoDecoderConfig& video_config,
 
   UMA_HISTOGRAM_ENUMERATION("Media.VideoCodec", video_config.codec(),
                             kVideoCodecMax + 1);
+  if (container == container_names::CONTAINER_MOV) {
+    UMA_HISTOGRAM_ENUMERATION("Media.VideoCodec.MP4", video_config.codec(),

[xhwang, 2017/05/03 05:33:45]

Shall we just name this Media.SRC.VideoCodec.MP4 to avoid confusion? Then
probably add a TODO to fix other UMAs in this file and link the bug you filed.

[wolenetz, 2017/05/03 20:48:23]

I don't recall the previous reason why we didn't do this already, but I think it
was discussed years ago. Maybe it was related to dashboard queries with the old
(no "SRC") bucket names or just generally low pri? I agree with your suggestion;
kqyang@ please modify these SRC= container histograms to be Media.SRC.*.* and
add TODOs around l.171 and l.146 in this file, with a link to
https://crbug.com/717199 in the TODOs and CL description.

xhwang@, using "SRC" to mean a non-MSE, non-MediaStream playback is a bit of
tribal knowledge. I can't think of a better name, so SRC SGTM (just might
confuse folks unaware that "MediaSource Extensions" is *not* "SRC" :)

+                              kVideoCodecMax + 1);
+  } else if (container == container_names::CONTAINER_WEBM) {
+    UMA_HISTOGRAM_ENUMERATION("Media.VideoCodec.WebM", video_config.codec(),
+                              kVideoCodecMax + 1);
+  }
 
   // Drop UNKNOWN because U_H_E() uses one bucket for all values less than 1.
   if (video_config.profile() >= 0) {
@@ -1342,8 +1351,7 @@ void FFmpegDemuxer::OnFindStreamInfoDone(const PipelineStatusCB& status_cb,
     std::string track_language = streams_[i]->GetMetadata("language");
 
     // Some metadata is named differently in FFmpeg for webm files.
-    if (strstr(format_context->iformat->name, "webm") ||
-        strstr(format_context->iformat->name, "matroska")) {
+    if (glue_->container() == container_names::CONTAINER_WEBM) {
       // TODO(servolk): FFmpeg doesn't set stream->id correctly for webm files.
       // Need to fix that and use it as track id. crbug.com/323183
       track_id =
@@ -1385,8 +1393,8 @@ void FFmpegDemuxer::OnFindStreamInfoDone(const PipelineStatusCB& status_cb,
     } else if (codec_type == AVMEDIA_TYPE_VIDEO) {
       VideoDecoderConfig video_config = streams_[i]->video_decoder_config();
 
-      RecordVideoCodecStats(video_config, stream->codecpar->color_range,
-                            media_log_);
+      RecordVideoCodecStats(glue_->container(), video_config,
+                            stream->codecpar->color_range, media_log_);
 
       media_track = media_tracks->AddVideoTrack(video_config, track_id, "main",
                                                 track_label, track_language);
@@ -1454,7 +1462,7 @@ void FFmpegDemuxer::OnFindStreamInfoDone(const PipelineStatusCB& status_cb,
     const AVStream* audio_stream = stream->av_stream();
     DCHECK(audio_stream);
     if (audio_stream->codecpar->codec_id == AV_CODEC_ID_OPUS ||
-        (strcmp(format_context->iformat->name, "ogg") == 0 &&
+        (glue_->container() == container_names::CONTAINER_OGG &&
          audio_stream->codecpar->codec_id == AV_CODEC_ID_VORBIS)) {
       for (size_t i = 0; i < streams_.size(); ++i) {
         if (!streams_[i])
@@ -1478,12 +1486,14 @@ void FFmpegDemuxer::OnFindStreamInfoDone(const PipelineStatusCB& status_cb,
 
   // MPEG-4 B-frames cause grief for a simple container like AVI. Enable PTS
   // generation so we always get timestamps, see http://crbug.com/169570
-  if (strcmp(format_context->iformat->name, "avi") == 0)
+  if (glue_->container() == container_names::CONTAINER_AVI)
     format_context->flags |= AVFMT_FLAG_GENPTS;
 
   // For testing purposes, don't overwrite the timeline offset if set already.
-  if (timeline_offset_.is_null())
-    timeline_offset_ = ExtractTimelineOffset(format_context);
+  if (timeline_offset_.is_null()) {
+    timeline_offset_ =
+        ExtractTimelineOffset(glue_->container(), format_context);
+  }
 
   // Since we're shifting the externally visible start time to zero, we need to
   // adjust the timeline offset to compensate.