MSE: Use video (AVC) keyframeness when it differs from MP4's

Due to prevalence of MSE support in other browsers for legacy
incorrectly muxed content where AVC keyframeness is incorrectly marked
in the MP4 container, this change expands the previous
chrome://media-internals logging when such mismatch is detected, to
trust the keyframe-ness of the AVC coded frame for buffering and
decoding.  Otherwise, such streams, especially those with frames
incorrectly marked as keyframes in the MP4, would be buffered as
out-of-order GOPS rather than out-of-order nonkeyframes, corrupting the
buffering and decode sequence on playback in the new, compliant,
MseBufferByPts logic.  Seek preroll from actual AVC nonkeyframes in
legacy MseBufferByDts logic should also be fixed by this change.

BUG=879734,844799,229412,760264

Change-Id: I90f53ec333e1aeeeaf39e96e176438cb13b4a195
Reviewed-on: https://chromium-review.googlesource.com/1205640
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589009}

media/formats/mp4/mp4_stream_parser.cc

@@ -799,6 +799,10 @@ ParseResult MP4StreamParser::EnqueueSample(BufferQueueMap* buffers) {
     subsamples = decrypt_config->subsamples();
   }
 
+  // This may change if analysis results indicate runs_->is_keyframe() is
+  // opposite of what the coded frame contains.
+  bool is_keyframe = runs_->is_keyframe();
+
   std::vector<uint8_t> frame_buf(buf, buf + sample_size);
   if (video) {
     if (runs_->video_description().video_codec == kCodecH264 ||
@@ -806,7 +810,7 @@ ParseResult MP4StreamParser::EnqueueSample(BufferQueueMap* buffers) {
         runs_->video_description().video_codec == kCodecDolbyVision) {
       DCHECK(runs_->video_description().frame_bitstream_converter);
       if (!runs_->video_description().frame_bitstream_converter->ConvertFrame(
-              &frame_buf, runs_->is_keyframe(), &subsamples)) {
+              &frame_buf, is_keyframe, &subsamples)) {
         MEDIA_LOG(ERROR, media_log_)
             << "Failed to prepare video sample for decode";
         return ParseResult::kError;
@@ -827,14 +831,25 @@ ParseResult MP4StreamParser::EnqueueSample(BufferQueueMap* buffers) {
       // 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()) {
+          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 ")
+            << (is_keyframe ? "" : "not ")
             << "a keyframe, but the video frame contents indicate the "
                "opposite.";
+        // As of September 2018, it appears that all of Edge, Firefox, Safari
+        // work with content that marks non-avc-keyframes as a keyframe in the
+        // container. Encoders/muxers/old streams still exist that produce
+        // all-keyframe mp4 video tracks, though many of the coded frames are
+        // not keyframes (likely workaround due to the impact on low-latency
+        // live streams until https://crbug.com/229412 was fixed).  We'll trust
+        // the AVC frame's keyframe-ness over the mp4 container's metadata if
+        // they mismatch. If other out-of-order codecs in mp4 (e.g. HEVC, DV)
+        // implement keyframe analysis in their frame_bitstream_converter, we'll
+        // similarly trust that analysis instead of the mp4.
+        is_keyframe = analysis.is_keyframe.value();
       }
     }
   }
@@ -868,9 +883,9 @@ ParseResult MP4StreamParser::EnqueueSample(BufferQueueMap* buffers) {
   StreamParserBuffer::Type buffer_type = audio ? DemuxerStream::AUDIO :
       DemuxerStream::VIDEO;
 
-  scoped_refptr<StreamParserBuffer> stream_buf = StreamParserBuffer::CopyFrom(
-      &frame_buf[0], frame_buf.size(), runs_->is_keyframe(), buffer_type,
-      runs_->track_id());
+  scoped_refptr<StreamParserBuffer> stream_buf =
+      StreamParserBuffer::CopyFrom(&frame_buf[0], frame_buf.size(), is_keyframe,
+                                   buffer_type, runs_->track_id());
 
   if (decrypt_config)
     stream_buf->set_decrypt_config(std::move(decrypt_config));
@@ -898,8 +913,7 @@ ParseResult MP4StreamParser::EnqueueSample(BufferQueueMap* buffers) {
   }
 
   DVLOG(3) << "Emit " << (audio ? "audio" : "video") << " frame: "
-           << " track_id=" << runs_->track_id()
-           << ", key=" << runs_->is_keyframe()
+           << " track_id=" << runs_->track_id() << ", key=" << is_keyframe
            << ", dur=" << runs_->duration().InMilliseconds()
            << ", dts=" << runs_->dts().InMilliseconds()
            << ", cts=" << runs_->cts().InMilliseconds()

media/formats/mp4/mp4_stream_parser_unittest.cc

@@ -62,7 +62,8 @@ class MP4StreamParserTest : public testing::Test {
   MP4StreamParserTest()
       : configs_received_(false),
         lower_bound_(
-            DecodeTimestamp::FromPresentationTime(base::TimeDelta::Max())) {
+            DecodeTimestamp::FromPresentationTime(base::TimeDelta::Max())),
+        verifying_keyframeness_sequence_(false) {
     std::set<int> audio_object_types;
     audio_object_types.insert(kISO_14496_3);
     parser_.reset(new MP4StreamParser(audio_object_types, false, false));
@@ -78,6 +79,7 @@ class MP4StreamParserTest : public testing::Test {
   DecodeTimestamp lower_bound_;
   StreamParser::TrackId audio_track_id_;
   StreamParser::TrackId video_track_id_;
+  bool verifying_keyframeness_sequence_;
 
   bool AppendData(const uint8_t* data, size_t length) {
     return parser_->Parse(data, length);
@@ -139,6 +141,11 @@ class MP4StreamParserTest : public testing::Test {
     return true;
   }
 
+  // Useful in single-track test media cases that need to verify
+  // keyframe/non-keyframe sequence in output of parse.
+  MOCK_METHOD0(ParsedKeyframe, void());
+  MOCK_METHOD0(ParsedNonKeyframe, void());
+
   bool NewBuffersF(const StreamParser::BufferQueueMap& buffer_queue_map) {
     DecodeTimestamp lowest_end_dts = kNoDecodeTimestamp();
     for (const auto& it : buffer_queue_map) {
@@ -157,6 +164,14 @@ class MP4StreamParserTest : public testing::Test {
                  << ", dur=" << buf->duration().InSecondsF();
         // Ensure that track ids are properly assigned on all emitted buffers.
         EXPECT_EQ(it.first, buf->track_id());
+
+        // Let single-track tests verify the sequence of keyframes/nonkeyframes.
+        if (verifying_keyframeness_sequence_) {
+          if (buf->is_key_frame())
+            ParsedKeyframe();
+          else
+            ParsedNonKeyframe();
+        }
       }
     }
 
@@ -298,9 +313,13 @@ TEST_F(MP4StreamParserTest, AVC_KeyAndNonKeyframeness_Match_Container) {
   // 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.
+  InSequence s;  // The EXPECT* sequence matters for this test.
   auto params = GetDefaultInitParametersExpectations();
   params.detected_audio_track_count = 0;
   InitializeParserWithInitParametersExpectations(params);
+  verifying_keyframeness_sequence_ = true;
+  EXPECT_CALL(*this, ParsedKeyframe());
+  EXPECT_CALL(*this, ParsedNonKeyframe());
   ParseMP4File("bear-640x360-v-2frames_frag.mp4", 512);
 }
 
@@ -310,12 +329,16 @@ TEST_F(MP4StreamParserTest, AVC_Keyframeness_Mismatches_Container) {
   //          others.
   // Frame 1: AVC Non-IDR, tfhd.default_sample_flags: not sync sample, depends
   //          on others.
+  InSequence s;  // The EXPECT* sequence matters for this test.
   auto params = GetDefaultInitParametersExpectations();
   params.detected_audio_track_count = 0;
   InitializeParserWithInitParametersExpectations(params);
+  verifying_keyframeness_sequence_ = true;
   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."));
+  EXPECT_CALL(*this, ParsedKeyframe());
+  EXPECT_CALL(*this, ParsedNonKeyframe());
   ParseMP4File("bear-640x360-v-2frames-keyframe-is-non-sync-sample_frag.mp4",
                512);
 }
@@ -326,12 +349,16 @@ TEST_F(MP4StreamParserTest, AVC_NonKeyframeness_Mismatches_Container) {
   //          depend on others.
   // Frame 1: AVC Non-IDR, tfhd.default_sample_flags: SYNC sample, DOES NOT
   //          depend on others.
+  InSequence s;  // The EXPECT* sequence matters for this test.
   auto params = GetDefaultInitParametersExpectations();
   params.detected_audio_track_count = 0;
   InitializeParserWithInitParametersExpectations(params);
+  verifying_keyframeness_sequence_ = true;
+  EXPECT_CALL(*this, ParsedKeyframe());
   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."));
+  EXPECT_CALL(*this, ParsedNonKeyframe());
   ParseMP4File("bear-640x360-v-2frames-nonkeyframe-is-sync-sample_frag.mp4",
                512);
 }