MSE: Log when MP4 keyframe-ness mismatches the compressed media

Logs to chrome://media-internals when the container's keyframe metadata
for a frame mismatches the contents of that frame (if the latter was
determined by the respective bitstream converter's Analyze()
implementation, of whom only AVC currently does any keyframe analysis).

BUG=860420,584384
TEST=Repro crbug 860420 shows logs, new MP4StreamParserTest.AVC_Key* and media

Change-Id: Ic87eab8726ad76b36b569df08ab2212b267008c2
Reviewed-on: https://chromium-review.googlesource.com/1149153Reviewed-by: Dan Sanders <sandersd@chromium.org>
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577751}

media/formats/mp4/mp4_stream_parser.cc

@@ -41,6 +41,7 @@ namespace {
 
 const int kMaxEmptySampleLogs = 20;
 const int kMaxInvalidConversionLogs = 20;
+const int kMaxVideoKeyframeMismatchLogs = 10;
 
 // Caller should be prepared to handle return of Unencrypted() in case of
 // unsupported scheme.
@@ -90,8 +91,8 @@ MP4StreamParser::MP4StreamParser(const std::set<int>& audio_object_types,
       has_sbr_(has_sbr),
       has_flac_(has_flac),
       num_empty_samples_skipped_(0),
-      num_invalid_conversions_(0) {
-}
+      num_invalid_conversions_(0),
+      num_video_keyframe_mismatches_(0) {}
 
 MP4StreamParser::~MP4StreamParser() = default;
 
@@ -814,9 +815,19 @@ ParseResult MP4StreamParser::EnqueueSample(BufferQueueMap* buffers) {
             << "Prepared video sample is not conformant";
       }
 
-      // TODO(wolenetz): Use |analysis.is_keyframe|, if it was actually
-      // performed, for at least logging if the result mismatches container's
-      // keyframe metadata for |frame_buf|.
+      // Use |analysis.is_keyframe|, if it was actually determined, for logging
+      // if the analysis mismatches the container's keyframe metadata for
+      // |frame_buf|.
+      if (analysis.is_keyframe.has_value() &&
+          runs_->is_keyframe() != analysis.is_keyframe.value()) {
+        LIMITED_MEDIA_LOG(DEBUG, media_log_, num_video_keyframe_mismatches_,
+                          kMaxVideoKeyframeMismatchLogs)
+            << "ISO-BMFF container metadata for video frame indicates that the "
+               "frame is "
+            << (runs_->is_keyframe() ? "" : "not ")
+            << "a keyframe, but the video frame contents indicate the "
+               "opposite.";
+      }
     }
   }
 

media/formats/mp4/mp4_stream_parser.h

@@ -148,6 +148,9 @@ class MEDIA_EXPORT MP4StreamParser : public StreamParser {
   // Tracks the number of MEDIA_LOGS for invalid bitstream conversion.
   int num_invalid_conversions_;
 
+  // Tracks the number of MEDIA_LOGS for video keyframe MP4<->frame mismatch.
+  int num_video_keyframe_mismatches_;
+
   DISALLOW_COPY_AND_ASSIGN(MP4StreamParser);
 };
 

media/formats/mp4/mp4_stream_parser_unittest.cc

@@ -50,7 +50,11 @@ MATCHER(SampleEncryptionInfoUnavailableLog, "") {
 }
 
 MATCHER_P(ErrorLog, error_string, "") {
-  return CONTAINS_STRING(arg, error_string);
+  return CONTAINS_STRING(arg, error_string) && CONTAINS_STRING(arg, "error");
+}
+
+MATCHER_P(DebugLog, debug_string, "") {
+  return CONTAINS_STRING(arg, debug_string) && CONTAINS_STRING(arg, "debug");
 }
 
 class MP4StreamParserTest : public testing::Test {
@@ -287,6 +291,51 @@ TEST_F(MP4StreamParserTest, UnknownDuration_V0_AllBitsSet) {
       512);
 }
 
+TEST_F(MP4StreamParserTest, AVC_KeyAndNonKeyframeness_Match_Container) {
+  // Both AVC video frames' keyframe-ness metadata matches the MP4:
+  // Frame 0: AVC IDR, trun.first_sample_flags: sync sample that doesn't
+  //          depend on others.
+  // Frame 1: AVC Non-IDR, tfhd.default_sample_flags: not sync sample, depends
+  //          on others.
+  // This is the base case; see also the "Mismatches" cases, below.
+  auto params = GetDefaultInitParametersExpectations();
+  params.detected_audio_track_count = 0;
+  InitializeParserWithInitParametersExpectations(params);
+  ParseMP4File("bear-640x360-v-2frames_frag.mp4", 512);
+}
+
+TEST_F(MP4StreamParserTest, AVC_Keyframeness_Mismatches_Container) {
+  // The first AVC video frame's keyframe-ness metadata matches the MP4:
+  // Frame 0: AVC IDR, trun.first_sample_flags: NOT sync sample, DEPENDS on
+  //          others.
+  // Frame 1: AVC Non-IDR, tfhd.default_sample_flags: not sync sample, depends
+  //          on others.
+  auto params = GetDefaultInitParametersExpectations();
+  params.detected_audio_track_count = 0;
+  InitializeParserWithInitParametersExpectations(params);
+  EXPECT_MEDIA_LOG(DebugLog(
+      "ISO-BMFF container metadata for video frame indicates that the frame is "
+      "not a keyframe, but the video frame contents indicate the opposite."));
+  ParseMP4File("bear-640x360-v-2frames-keyframe-is-non-sync-sample_frag.mp4",
+               512);
+}
+
+TEST_F(MP4StreamParserTest, AVC_NonKeyframeness_Mismatches_Container) {
+  // The second AVC video frame's keyframe-ness metadata matches the MP4:
+  // Frame 0: AVC IDR, trun.first_sample_flags: sync sample that doesn't
+  //          depend on others.
+  // Frame 1: AVC Non-IDR, tfhd.default_sample_flags: SYNC sample, DOES NOT
+  //          depend on others.
+  auto params = GetDefaultInitParametersExpectations();
+  params.detected_audio_track_count = 0;
+  InitializeParserWithInitParametersExpectations(params);
+  EXPECT_MEDIA_LOG(DebugLog(
+      "ISO-BMFF container metadata for video frame indicates that the frame is "
+      "a keyframe, but the video frame contents indicate the opposite."));
+  ParseMP4File("bear-640x360-v-2frames-nonkeyframe-is-sync-sample_frag.mp4",
+               512);
+}
+
 TEST_F(MP4StreamParserTest, MPEG2_AAC_LC) {
   InSequence s;
   std::set<int> audio_object_types;

media/test/data/README

@@ -74,6 +74,20 @@ vorbis-packet-1  - timestamp: 0ms, duration: 0ms
 vorbis-packet-2  - timestamp: 0ms, duration: 0ms
 vorbis-packet-3  - timestamp: 2902ms, duration: 0ms
 
+// MSE MP4 keyframe-metadata versus encoded AVC keyframe-ness test media:
+bear-640x360-v-2frames_frag.mp4 - Just first 2 video frames of bear-640x360-v_frag.mp4, created with:
+  ffmpeg -i bear-640x360-v_frag.mp4 -vcodec copy -movflags frag_keyframe+empty_moov+default_base_moof \
+    -vframes 2 bear-640x360-v-2frames_frag.mp4
+  It's 1 keyframe + 1 non-keyframe, with container's frame keyframe-ness correct.
+bear-640x360-v-2frames-keyframe-is-non-sync-sample_frag.mp4
+  This is bear-640x360-v-2frames_frag.mp4, with manually updated trun.first_sample_flags:
+    s/0x02000000/0x01010000 (first frame is non-sync-sample, depends on another
+    frame, mismatches compressed h264 first frame's keyframe-ness).
+bear-640x360-v-2frames-nonkeyframe-is-sync-sample_frag.mp4
+  This is bear-640x360-v-2frames_frag.mp4, with manually updated tfhd.default_sample_flags:
+    s/0x01010000/0x02000000 (second frame is sync-sample, doesn't depend on other
+    frames, mismatches compressed h264 second frame's nonkeyframe-ness).
+
 // 10-bit test file(s)
 bear-320x180-hi10p.mp4
 bear-320x240-vp9_profile2.webm - VP9 encoded video with profile 2 (10-bit, 4:2:0). Codec string: vp09.02.10.10.01.02.02.02.00.