Bug 1513651 - Collect telemetry on sample description box entries when parsing mp4s. r=jya

This patch adds telemetry to help us determine if mp4s with certain structures
can occur in the wild. Specifically:
- How often do mp4s have multiple entries in the sample description box?
- Do we ever see mp4s with multiple codecs in the sample description box?
- Do we ever see mp4s with multiple sets of crypto info in the sample
  description box?

This information is collected each time we parse mp4 metadata.

Remove some diagnostic asserts now that we're gracefully gathering info.

Reshuffle of DecoderData function order to align with the order in the header.

Differential Revision: https://phabricator.services.mozilla.com/D14426

--HG--
extra : moz-landing-system : lando

dom/media/mp4/DecoderData.cpp

@@ -8,6 +8,7 @@
 #include "DecoderData.h"
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/EndianUtils.h"
+#include "mozilla/Telemetry.h"
 #include "VideoUtils.h"
 
 // OpusDecoder header is really needed only by MP4 in rust
@@ -40,13 +41,6 @@ mozilla::Result<mozilla::Ok, nsresult> CryptoFile::DoUpdate(
   return mozilla::Ok();
 }
 
-bool MP4AudioInfo::IsValid() const {
-  return mChannels > 0 && mRate > 0 &&
-         // Accept any mime type here, but if it's aac, validate the profile.
-         (!mMimeType.EqualsLiteral("audio/mp4a-latm") || mProfile > 0 ||
-          mExtendedProfile > 0);
-}
-
 static MediaResult UpdateTrackProtectedInfo(mozilla::TrackInfo& aConfig,
                                             const Mp4parseSinfInfo& aSinf) {
   if (aSinf.is_encrypted != 0) {
@@ -72,40 +66,91 @@ static MediaResult UpdateTrackProtectedInfo(mozilla::TrackInfo& aConfig,
   return NS_OK;
 }
 
-MediaResult MP4AudioInfo::Update(const Mp4parseTrackInfo* track,
+// Verify various information shared by Mp4ParseTrackAudioInfo and
-                                 const Mp4parseTrackAudioInfo* audio) {
+// Mp4ParseTrackVideoInfo and record telemetry on that info. Returns an
-  MOZ_DIAGNOSTIC_ASSERT(audio->sample_info_count > 0,
+// appropriate MediaResult indicating if the info is valid or not.
-                        "Must have at least one audio sample info");
+// This verifies:
-  if (audio->sample_info_count == 0) {
+// - That we have a sample_info_count > 0 (valid tracks should have at least one
-    return MediaResult(
+//   sample description entry)
-        NS_ERROR_DOM_MEDIA_METADATA_ERR,
+// - That only a single codec is used across all sample infos, as we don't
-        RESULT_DETAIL("Got 0 audio sample info while updating audio track"));
+//   handle multiple.
-  }
+// - That only a single sample info contains crypto info, as we don't handle
+//  multiple.
+//
+// Telemetry is also recorded on the above. As of writing, the
+// telemetry is recorded to give us early warning if MP4s exist that we're not
+// handling. Note, if adding new checks and telemetry to this function,
+// telemetry should be recorded before returning to ensure it is gathered.
+template <typename Mp4ParseTrackAudioOrVideoInfo>
+static MediaResult VerifyAudioOrVideoInfoAndRecordTelemetry(
+    Mp4ParseTrackAudioOrVideoInfo* audioOrVideoInfo) {
+  Telemetry::Accumulate(
+      Telemetry::MEDIA_MP4_PARSE_NUM_SAMPLE_DESCRIPTION_ENTRIES,
+      audioOrVideoInfo->sample_info_count);
 
   bool hasCrypto = false;
-  Mp4parseCodec codecType = audio->sample_info[0].codec_type;
+  bool hasMultipleCodecs = false;
-  for (uint32_t i = 0; i < audio->sample_info_count; i++) {
+  bool hasMultipleCrypto = false;
-    if (audio->sample_info[0].codec_type != codecType) {
+  Mp4parseCodec codecType = audioOrVideoInfo->sample_info[0].codec_type;
-      // Different codecs in a single track. We don't handle this.
+  for (uint32_t i = 0; i < audioOrVideoInfo->sample_info_count; i++) {
-      return MediaResult(
+    if (audioOrVideoInfo->sample_info[0].codec_type != codecType) {
-          NS_ERROR_DOM_MEDIA_METADATA_ERR,
+      hasMultipleCodecs = true;
-          RESULT_DETAIL(
-              "Multiple codecs encountered while updating audio track"));
     }
 
     // Update our encryption info if any is present on the sample info.
-    if (audio->sample_info[i].protected_data.is_encrypted) {
+    if (audioOrVideoInfo->sample_info[i].protected_data.is_encrypted) {
       if (hasCrypto) {
-        // Multiple crypto entries found. We don't handle this.
+        hasMultipleCrypto = true;
-        return MediaResult(
-            NS_ERROR_DOM_MEDIA_METADATA_ERR,
-            RESULT_DETAIL(
-                "Multiple crypto info encountered while updating audio track"));
       }
+      hasCrypto = true;
+    }
+  }
+
+  Telemetry::Accumulate(
+      Telemetry::
+          MEDIA_MP4_PARSE_SAMPLE_DESCRIPTION_ENTRIES_HAVE_MULTIPLE_CODECS,
+      hasMultipleCodecs);
+
+  Telemetry::Accumulate(
+      Telemetry::
+          MEDIA_MP4_PARSE_SAMPLE_DESCRIPTION_ENTRIES_HAVE_MULTIPLE_CRYPTO,
+      hasMultipleCrypto);
+
+  if (audioOrVideoInfo->sample_info_count == 0) {
+    return MediaResult(
+        NS_ERROR_DOM_MEDIA_METADATA_ERR,
+        RESULT_DETAIL("Got 0 sample info while verifying track."));
+  }
+
+  if (hasMultipleCodecs) {
+    // Different codecs in a single track. We don't handle this.
+    return MediaResult(
+        NS_ERROR_DOM_MEDIA_METADATA_ERR,
+        RESULT_DETAIL("Multiple codecs encountered while verifying track."));
+  }
+
+  if (hasMultipleCrypto) {
+    // Multiple crypto entries found. We don't handle this.
+    return MediaResult(
+        NS_ERROR_DOM_MEDIA_METADATA_ERR,
+        RESULT_DETAIL(
+            "Multiple crypto info encountered while verifying track."));
+  }
+  return NS_OK;
+}
+
+MediaResult MP4AudioInfo::Update(const Mp4parseTrackInfo* track,
+                                 const Mp4parseTrackAudioInfo* audio) {
+  auto rv = VerifyAudioOrVideoInfoAndRecordTelemetry(audio);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  Mp4parseCodec codecType = audio->sample_info[0].codec_type;
+  for (uint32_t i = 0; i < audio->sample_info_count; i++) {
+    if (audio->sample_info[i].protected_data.is_encrypted) {
       auto rv =
           UpdateTrackProtectedInfo(*this, audio->sample_info[i].protected_data);
       NS_ENSURE_SUCCESS(rv, rv);
-      hasCrypto = true;
+      break;
     }
   }
 
@@ -158,40 +203,25 @@ MediaResult MP4AudioInfo::Update(const Mp4parseTrackInfo* track,
   return NS_OK;
 }
 
+bool MP4AudioInfo::IsValid() const {
+  return mChannels > 0 && mRate > 0 &&
+         // Accept any mime type here, but if it's aac, validate the profile.
+         (!mMimeType.EqualsLiteral("audio/mp4a-latm") || mProfile > 0 ||
+          mExtendedProfile > 0);
+}
+
 MediaResult MP4VideoInfo::Update(const Mp4parseTrackInfo* track,
                                  const Mp4parseTrackVideoInfo* video) {
-  MOZ_DIAGNOSTIC_ASSERT(video->sample_info_count > 0,
+  auto rv = VerifyAudioOrVideoInfoAndRecordTelemetry(video);
-                        "Must have at least one video sample info");
+  NS_ENSURE_SUCCESS(rv, rv);
-  if (video->sample_info_count == 0) {
-    return MediaResult(
-        NS_ERROR_DOM_MEDIA_METADATA_ERR,
-        RESULT_DETAIL("Got 0 audio sample info while updating video track"));
-  }
 
-  bool hasCrypto = false;
   Mp4parseCodec codecType = video->sample_info[0].codec_type;
   for (uint32_t i = 0; i < video->sample_info_count; i++) {
-    if (video->sample_info[0].codec_type != codecType) {
-      // Different codecs in a single track. We don't handle this.
-      return MediaResult(
-          NS_ERROR_DOM_MEDIA_METADATA_ERR,
-          RESULT_DETAIL(
-              "Multiple codecs encountered while updating video track"));
-    }
-
-    // Update our encryption info if any is present on the sample info.
     if (video->sample_info[i].protected_data.is_encrypted) {
-      if (hasCrypto) {
-        // Multiple crypto entries found. We don't handle this.
-        return MediaResult(
-            NS_ERROR_DOM_MEDIA_METADATA_ERR,
-            RESULT_DETAIL(
-                "Multiple crypto info encountered while updating video track"));
-      }
       auto rv =
           UpdateTrackProtectedInfo(*this, video->sample_info[i].protected_data);
       NS_ENSURE_SUCCESS(rv, rv);
-      hasCrypto = true;
+      break;
     }
   }
 

toolkit/components/telemetry/Histograms.json

@@ -8680,6 +8680,31 @@
     "n_values": 10,
     "description": "Counts types of deprecation warnings logged on every successful call to navigator.requestMediaKeySystemAccess(). 0=No warnings logged, 1=MediaEMENoCapabilitiesDeprecatedWarning, 2=MediaEMENoCodecsDeprecatedWarning."
   },
+  "MEDIA_MP4_PARSE_SAMPLE_DESCRIPTION_ENTRIES_HAVE_MULTIPLE_CODECS": {
+    "record_in_processes": ["main", "content"],
+    "alert_emails": ["bvandyk@mozilla.com", "jya@mozilla.com", "drno@ohlmeier.org"],
+    "bug_numbers": [1513651],
+    "expires_in_version": "never",
+    "kind": "boolean",
+    "description": "Records if multiple codecs are present in a track's sample description entries. Recorded each time we process a track's metadata while parsing mp4s."
+  },
+  "MEDIA_MP4_PARSE_SAMPLE_DESCRIPTION_ENTRIES_HAVE_MULTIPLE_CRYPTO": {
+    "record_in_processes": ["main", "content"],
+    "alert_emails": ["bvandyk@mozilla.com", "jya@mozilla.com", "drno@ohlmeier.org"],
+    "bug_numbers": [1513651],
+    "expires_in_version": "never",
+    "kind": "boolean",
+    "description": "Records if multiple sets of crypto info are present in a track's sample description entries. Recorded each time we process a track's metadata while parsing mp4s."
+  },
+  "MEDIA_MP4_PARSE_NUM_SAMPLE_DESCRIPTION_ENTRIES": {
+    "record_in_processes": ["main", "content"],
+    "alert_emails": ["bvandyk@mozilla.com", "jya@mozilla.com", "drno@ohlmeier.org"],
+    "bug_numbers": [1513651],
+    "expires_in_version": "never",
+    "kind": "enumerated",
+    "n_values": 5,
+    "description": "Counts the number of entries in the sample description box (stsd) for a track in an mp4. Recorded each time we process a track's metadata while parsing mp4s."
+  },
   "MEDIACACHE_WATERMARK_KB": {
     "record_in_processes": ["main", "content"],
     "alert_emails": ["gsquelart@mozilla.com"],