Add VideoCodecProfile to WatchTime secondary properties.

Since we have been unable to join on the VideoDecoderStats table due to
processing times and lack of cross correlation per resolution, metrics
leadership has asked we add this field directly to watch time
calculations.

BUG=1002587
R=chcunningham

Change-Id: Id7ef8a28ec43607920885d62f3597b54aad647cd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1869681Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708765}

media/blink/watch_time_reporter_unittest.cc

@@ -12,6 +12,7 @@
 #include "base/test/test_mock_time_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "media/base/mock_media_log.h"
+#include "media/base/video_codecs.h"
 #include "media/base/watch_time_keys.h"
 #include "media/blink/watch_time_reporter.h"
 #include "media/mojo/mojom/media_metrics_provider.mojom.h"
@@ -804,6 +805,7 @@ TEST_P(WatchTimeReporterTest, WatchTimeReporterSecondaryProperties) {
   auto properties = mojom::SecondaryPlaybackProperties::New(
       has_audio_ ? kCodecAAC : kUnknownAudioCodec,
       has_video_ ? kCodecH264 : kUnknownVideoCodec,
+      has_video_ ? H264PROFILE_MAIN : VIDEO_CODEC_PROFILE_UNKNOWN,
       has_audio_ ? "FirstAudioDecoder" : "",
       has_video_ ? "FirstVideoDecoder" : "",
       has_audio_ ? EncryptionMode::kCenc : EncryptionMode::kUnencrypted,
@@ -840,8 +842,8 @@ TEST_P(WatchTimeReporterTest, SecondaryProperties_SizeIncreased) {
   EXPECT_CALL(*this, OnUpdateSecondaryProperties(_))
       .Times((has_audio_ && has_video_) ? 3 : 2);
   wtr_->UpdateSecondaryProperties(mojom::SecondaryPlaybackProperties::New(
-      kUnknownAudioCodec, kUnknownVideoCodec, "", "",
-      EncryptionMode::kUnencrypted, EncryptionMode::kUnencrypted,
+      kUnknownAudioCodec, kUnknownVideoCodec, VIDEO_CODEC_PROFILE_UNKNOWN, "",
+      "", EncryptionMode::kUnencrypted, EncryptionMode::kUnencrypted,
       kSizeJustRight));
   EXPECT_TRUE(IsMonitoring());
 
@@ -862,8 +864,8 @@ TEST_P(WatchTimeReporterTest, SecondaryProperties_SizeDecreased) {
   EXPECT_CALL(*this, OnUpdateSecondaryProperties(_))
       .Times((has_audio_ && has_video_) ? 3 : 2);
   wtr_->UpdateSecondaryProperties(mojom::SecondaryPlaybackProperties::New(
-      kUnknownAudioCodec, kUnknownVideoCodec, "", "",
-      EncryptionMode::kUnencrypted, EncryptionMode::kUnencrypted,
+      kUnknownAudioCodec, kUnknownVideoCodec, VIDEO_CODEC_PROFILE_UNKNOWN, "",
+      "", EncryptionMode::kUnencrypted, EncryptionMode::kUnencrypted,
       kSizeTooSmall));
   EXPECT_WATCH_TIME_FINALIZED();
   CycleReportingTimer();

media/blink/webmediaplayer_impl.cc

@@ -3187,8 +3187,9 @@ void WebMediaPlayerImpl::UpdateSecondaryProperties() {
   watch_time_reporter_->UpdateSecondaryProperties(
       mojom::SecondaryPlaybackProperties::New(
           pipeline_metadata_.audio_decoder_config.codec(),
-          pipeline_metadata_.video_decoder_config.codec(), audio_decoder_name_,
-          video_decoder_name_,
+          pipeline_metadata_.video_decoder_config.codec(),
+          pipeline_metadata_.video_decoder_config.profile(),
+          audio_decoder_name_, video_decoder_name_,
           DetermineEncryptionMode(
               pipeline_metadata_.audio_decoder_config.encryption_scheme()),
           DetermineEncryptionMode(

media/mojo/mojom/watch_time_recorder.mojom

@@ -27,6 +27,7 @@ struct PlaybackProperties {
 struct SecondaryPlaybackProperties {
   AudioCodec audio_codec;  // Note: We may not know the codec during all
   VideoCodec video_codec;  // playbacks (HLS, remoting, etc).
+  VideoCodecProfile video_codec_profile;
   string audio_decoder_name;
   string video_decoder_name;
   EncryptionMode audio_encryption_scheme;

media/mojo/services/watch_time_recorder.cc

@@ -11,6 +11,7 @@
 #include "base/metrics/histogram_functions.h"
 #include "base/strings/string_piece.h"
 #include "media/base/limits.h"
+#include "media/base/video_codecs.h"
 #include "media/base/video_decoder.h"
 #include "media/base/watch_time_keys.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
@@ -263,6 +264,8 @@ void WatchTimeRecorder::UpdateSecondaryProperties(
     // capture changes in encryption schemes.
     if (last_record.secondary_properties->audio_codec == kUnknownAudioCodec ||
         last_record.secondary_properties->video_codec == kUnknownVideoCodec ||
+        last_record.secondary_properties->video_codec_profile ==
+            VIDEO_CODEC_PROFILE_UNKNOWN ||
         last_record.secondary_properties->audio_decoder_name.empty() ||
         last_record.secondary_properties->video_decoder_name.empty()) {
       auto temp_props = last_record.secondary_properties.Clone();
@@ -270,6 +273,11 @@ void WatchTimeRecorder::UpdateSecondaryProperties(
         temp_props->audio_codec = secondary_properties->audio_codec;
       if (last_record.secondary_properties->video_codec == kUnknownVideoCodec)
         temp_props->video_codec = secondary_properties->video_codec;
+      if (last_record.secondary_properties->video_codec_profile ==
+          VIDEO_CODEC_PROFILE_UNKNOWN) {
+        temp_props->video_codec_profile =
+            secondary_properties->video_codec_profile;
+      }
       if (last_record.secondary_properties->audio_decoder_name.empty()) {
         temp_props->audio_decoder_name =
             secondary_properties->audio_decoder_name;
@@ -434,6 +442,8 @@ void WatchTimeRecorder::RecordUkmPlaybackData() {
     // See note in mojom::PlaybackProperties about why we have both of these.
     builder.SetAudioCodec(ukm_record.secondary_properties->audio_codec);
     builder.SetVideoCodec(ukm_record.secondary_properties->video_codec);
+    builder.SetVideoCodecProfile(
+        ukm_record.secondary_properties->video_codec_profile);
     builder.SetHasAudio(properties_->has_audio);
     builder.SetHasVideo(properties_->has_video);
 

tools/metrics/ukm/ukm.xml

@@ -3688,7 +3688,7 @@ be describing additional metrics about the same event.
   </summary>
   <metric name="AudioCodec">
     <summary>
-      media::AudioCodec enum value. Can be kUnknownAudioCodec even with HasAudio
+      media::AudioCodec enum value. Can be kUnknownAudioCodec even when HasAudio
       is true because we don't always know the codec.
     </summary>
   </metric>
@@ -3783,10 +3783,16 @@ be describing additional metrics about the same event.
   </metric>
   <metric name="VideoCodec">
     <summary>
-      media::VideoCodec enum value. Can be kUnknownVideoCodec even with HasVideo
+      media::VideoCodec enum value. Can be kUnknownVideoCodec even when HasVideo
       is true because we don't always know the codec.
     </summary>
   </metric>
+  <metric name="VideoCodecProfile">
+    <summary>
+      media::VideoCodecProfile enum value. Can be VIDEO_CODEC_PROFILE_UNKNOWN
+      even when HasVideo is true because we don't always know the codec profile.
+    </summary>
+  </metric>
   <metric name="VideoDecoderName">
     <summary>
       Enumeration of video decoder names, zero if none or unknown (Cast, HLS,
@@ -4156,7 +4162,7 @@ be describing additional metrics about the same event.
   <metric name="Video.CodecProfile">
     <summary>
       media::VideoCodecProfile enum value. Can be VIDEO_CODEC_PROFILE_UNKNOWN if
-      we don't know the audio codec.
+      we don't know the video codec.
     </summary>
   </metric>
   <metric name="Video.EME.KeySystem">