Revert "MSE: refactor mp4 AnnexB validation to also report keyframe-ness"

This reverts commit 568cc6a9.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> MSE: refactor mp4 AnnexB validation to also report keyframe-ness
> 
> In preparation for logging when there appears to be a mismatch between
> MSE MP4 keyframe metadata and the encoded video bitstream's keyframe
> metadata, this change incorporates basic analysis of the latter as part
> of the existing Annex-B bitstream validation for MSE MP4 video.
> 
> Note that mp4::BitstreamConverter::AnalysisResult contains
> base::Optional<bool> for each of conformance and keyframe-ness fields.
> If a field doesn't have a value, that portion of the result was not
> analyzed.
> 
> Note that keyframe analysis is implemented only for AVC currently, but
> even that is skipped if the frame is detected as non-conformant before
> enough indications of whether or not it is a keyframe are detected.
> Neither AVC-DV nor HEVC AnnexB analyses do any actual keyframe analysis,
> since such was either skipped or not yet implemented previously,
> respectively.
> 
> A subsequent CL will use the newly reported MSE MP4 video bitstream
> converter's keyframe analysis results, if that analysis was done, in
> reporting to chrome://media-internals when the bitstream keyframe-ness
> mismatches that of the mp4 container for a coded frame.
> 
> BUG=860420,584384
> 
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
> Change-Id: I169c7774070ad232c86658bcd7803160323993ad
> Reviewed-on: https://chromium-review.googlesource.com/1144456
> Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
> Reviewed-by: Sergey Volk <servolk@chromium.org>
> Reviewed-by: Dan Sanders <sandersd@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#577717}

TBR=wolenetz@chromium.org,sandersd@chromium.org,servolk@chromium.org

Change-Id: If4d14f0cb6fb0779255b9d7eb3d5b2b8c20e5be1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 860420, 584384
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/1150244Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Commit-Queue: Zhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577969}

media/filters/ffmpeg_demuxer_unittest.cc

@@ -32,7 +32,6 @@
 #include "media/filters/ffmpeg_demuxer.h"
 #include "media/filters/file_data_source.h"
 #include "media/formats/mp4/avc.h"
-#include "media/formats/mp4/bitstream_converter.h"
 #include "media/media_buildflags.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -1200,8 +1199,8 @@ static void ValidateAnnexB(DemuxerStream* stream,
     subsamples = buffer->decrypt_config()->subsamples();
 
   bool is_valid =
-      mp4::AVC::AnalyzeAnnexB(buffer->data(), buffer->data_size(), subsamples)
-          .is_conformant.value_or(false);
+      mp4::AVC::IsValidAnnexB(buffer->data(), buffer->data_size(),
+                              subsamples);
   EXPECT_TRUE(is_valid);
 
   if (!is_valid) {

media/formats/mp4/avc.cc

@@ -171,21 +171,15 @@ bool AVC::ConvertConfigToAnnexB(const AVCDecoderConfigurationRecord& avc_config,
   return true;
 }
 
-// static
-BitstreamConverter::AnalysisResult AVC::AnalyzeAnnexB(
-    const uint8_t* buffer,
-    size_t size,
-    const std::vector<SubsampleEntry>& subsamples) {
+// Verifies AnnexB NALU order according to ISO/IEC 14496-10 Section 7.4.1.2.3
+bool AVC::IsValidAnnexB(const uint8_t* buffer,
+                        size_t size,
+                        const std::vector<SubsampleEntry>& subsamples) {
   DVLOG(3) << __func__;
   DCHECK(buffer);
 
-  BitstreamConverter::AnalysisResult result;
-  result.is_conformant = false;  // Will change if needed before return.
-
-  if (size == 0) {
-    result.is_conformant = true;
-    return result;
-  }
+  if (size == 0)
+    return true;
 
   H264Parser parser;
   parser.SetEncryptedStream(buffer, size, subsamples);
@@ -211,7 +205,7 @@ BitstreamConverter::AnalysisResult AVC::AnalyzeAnnexB(
           case H264NALU::kAUD:
             if (order_state > kAUDAllowed) {
               DVLOG(1) << "Unexpected AUD in order_state " << order_state;
-              return result;
+              return false;
             }
             order_state = kBeforeFirstVCL;
             break;
@@ -227,7 +221,7 @@ BitstreamConverter::AnalysisResult AVC::AnalyzeAnnexB(
             if (order_state > kBeforeFirstVCL) {
               DVLOG(1) << "Unexpected NALU type " << nalu.nal_unit_type
                        << " in order_state " << order_state;
-              return result;
+              return false;
             }
             order_state = kBeforeFirstVCL;
             break;
@@ -235,7 +229,7 @@ BitstreamConverter::AnalysisResult AVC::AnalyzeAnnexB(
           case H264NALU::kSPSExt:
             if (last_nalu_type != H264NALU::kSPS) {
               DVLOG(1) << "SPS extension does not follow an SPS.";
-              return result;
+              return false;
             }
             break;
 
@@ -246,26 +240,22 @@ BitstreamConverter::AnalysisResult AVC::AnalyzeAnnexB(
           case H264NALU::kIDRSlice:
             if (order_state > kAfterFirstVCL) {
               DVLOG(1) << "Unexpected VCL in order_state " << order_state;
-              return result;
+              return false;
             }
-
-            if (!result.is_keyframe.has_value())
-              result.is_keyframe = nalu.nal_unit_type == H264NALU::kIDRSlice;
-
             order_state = kAfterFirstVCL;
             break;
 
           case H264NALU::kCodedSliceAux:
             if (order_state != kAfterFirstVCL) {
               DVLOG(1) << "Unexpected extension in order_state " << order_state;
-              return result;
+              return false;
             }
             break;
 
           case H264NALU::kEOSeq:
             if (order_state != kAfterFirstVCL) {
               DVLOG(1) << "Unexpected EOSeq in order_state " << order_state;
-              return result;
+              return false;
             }
             order_state = kEOStreamAllowed;
             break;
@@ -273,7 +263,7 @@ BitstreamConverter::AnalysisResult AVC::AnalyzeAnnexB(
           case H264NALU::kEOStream:
             if (order_state < kAfterFirstVCL) {
               DVLOG(1) << "Unexpected EOStream in order_state " << order_state;
-              return result;
+              return false;
             }
             order_state = kNoMoreDataAllowed;
             break;
@@ -284,7 +274,7 @@ BitstreamConverter::AnalysisResult AVC::AnalyzeAnnexB(
                   order_state < kEOStreamAllowed)) {
               DVLOG(1) << "Unexpected NALU type " << nalu.nal_unit_type
                        << " in order_state " << order_state;
-              return result;
+              return false;
             }
             break;
 
@@ -294,30 +284,25 @@ BitstreamConverter::AnalysisResult AVC::AnalyzeAnnexB(
                 order_state != kAfterFirstVCL) {
               DVLOG(1) << "Unexpected NALU type " << nalu.nal_unit_type
                        << " in order_state " << order_state;
-              return result;
+              return false;
             }
         }
         last_nalu_type = nalu.nal_unit_type;
         break;
 
       case H264Parser::kInvalidStream:
-        return result;
+        return false;
 
       case H264Parser::kUnsupportedStream:
         NOTREACHED() << "AdvanceToNextNALU() returned kUnsupportedStream!";
-        return result;
+        return false;
 
       case H264Parser::kEOStream:
         done = true;
     }
   }
 
-  if (order_state < kAfterFirstVCL)
-    return result;
-
-  result.is_conformant = true;
-  DCHECK(result.is_keyframe.has_value());
-  return result;
+  return order_state >= kAfterFirstVCL;
 }
 
 AVCBitstreamConverter::AVCBitstreamConverter(
@@ -353,16 +338,14 @@ bool AVCBitstreamConverter::ConvertFrame(
   return true;
 }
 
-BitstreamConverter::AnalysisResult AVCBitstreamConverter::Analyze(
+bool AVCBitstreamConverter::IsValid(
     std::vector<uint8_t>* frame_buf,
     std::vector<SubsampleEntry>* subsamples) const {
 #if BUILDFLAG(ENABLE_DOLBY_VISION_DEMUXING)
-  if (disable_validation_) {
-    BitstreamConverter::AnalysisResult result;
-    return result;
-  }
+  if (disable_validation_)
+    return true;
 #endif  // BUILDFLAG(ENABLE_DOLBY_VISION_DEMUXING)
-  return AVC::AnalyzeAnnexB(frame_buf->data(), frame_buf->size(), *subsamples);
+  return AVC::IsValidAnnexB(frame_buf->data(), frame_buf->size(), *subsamples);
 }
 
 }  // namespace mp4

media/formats/mp4/avc.h

@@ -44,17 +44,14 @@ class MEDIA_EXPORT AVC {
       const AVCDecoderConfigurationRecord& avc_config,
       std::vector<uint8_t>* buffer);
 
-  // Analyzes the contents of |buffer| for conformance to Section 7.4.1.2.3 of
-  // ISO/IEC 14496-10. Also analyzes |buffer| and reports if it looks like a
-  // keyframe, if such can be determined. Determination of keyframe-ness is done
-  // only if |buffer| is conformant or if lack of conformance is detected after
-  // detecting keyframe-ness.
+  // Verifies that the contents of |buffer| conform to
+  // Section 7.4.1.2.3 of ISO/IEC 14496-10.
   // |subsamples| contains the information about what parts of the buffer are
   // encrypted and which parts are clear.
-  static BitstreamConverter::AnalysisResult AnalyzeAnnexB(
-      const uint8_t* buffer,
-      size_t size,
-      const std::vector<SubsampleEntry>& subsamples);
+  // Returns true if |buffer| contains conformant Annex B data
+  static bool IsValidAnnexB(const uint8_t* buffer,
+                            size_t size,
+                            const std::vector<SubsampleEntry>& subsamples);
 
   // Given a |buffer| and |subsamples| information and |pts| pointer into the
   // |buffer| finds the index of the subsample |ptr| is pointing into.
@@ -84,9 +81,8 @@ class AVCBitstreamConverter : public BitstreamConverter {
                     bool is_keyframe,
                     std::vector<SubsampleEntry>* subsamples) const override;
 
-  AnalysisResult Analyze(
-      std::vector<uint8_t>* frame_buf,
-      std::vector<SubsampleEntry>* subsamples) const override;
+  bool IsValid(std::vector<uint8_t>* frame_buf,
+               std::vector<SubsampleEntry>* subsamples) const override;
 
  private:
   ~AVCBitstreamConverter() override;

media/formats/mp4/avc_unittest.cc

@@ -6,16 +6,12 @@
 #include <stdint.h>
 #include <string.h>
 
-#include <ostream>
-
 #include "base/macros.h"
-#include "base/optional.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "media/base/decrypt_config.h"
 #include "media/base/stream_parser_buffer.h"
 #include "media/formats/mp4/avc.h"
-#include "media/formats/mp4/bitstream_converter.h"
 #include "media/formats/mp4/box_definitions.h"
 #include "media/video/h264_parser.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -40,15 +36,6 @@ static H264NALU::Type StringToNALUType(const std::string& name) {
   if (name == "I")
     return H264NALU::kIDRSlice;
 
-  if (name == "SDA")
-    return H264NALU::kSliceDataA;
-
-  if (name == "SDB")
-    return H264NALU::kSliceDataB;
-
-  if (name == "SDC")
-    return H264NALU::kSliceDataC;
-
   if (name == "SEI")
     return H264NALU::kSEIMessage;
 
@@ -145,9 +132,9 @@ static void WriteStartCodeAndNALUType(std::vector<uint8_t>* buffer,
 // The output buffer will contain a valid-looking Annex B (it's valid-looking in
 // the sense that it has start codes and correct NALU types, but the actual NALU
 // payload is junk).
-static void StringToAnnexB(const std::string& str,
-                           std::vector<uint8_t>* buffer,
-                           std::vector<SubsampleEntry>* subsamples) {
+void StringToAnnexB(const std::string& str,
+                    std::vector<uint8_t>* buffer,
+                    std::vector<SubsampleEntry>* subsamples) {
   DCHECK(!str.empty());
 
   std::vector<std::string> subsample_specs = base::SplitString(
@@ -189,30 +176,8 @@ static void StringToAnnexB(const std::string& str,
   }
 }
 
-// Helper to compare two results of AVC::Analyze().
-static bool AnalysesMatch(const BitstreamConverter::AnalysisResult& r1,
-                          const BitstreamConverter::AnalysisResult& r2) {
-  return r1.is_conformant == r2.is_conformant &&
-         r1.is_keyframe == r2.is_keyframe;
-}
-
-// Helper output operator, for debugging/testability.
-std::ostream& operator<<(std::ostream& os,
-                         const BitstreamConverter::AnalysisResult& r) {
-  os << "{ is_conformant: "
-     << (r.is_conformant.has_value()
-             ? (r.is_conformant.value() ? "true" : "false")
-             : "nullopt/unknown")
-     << ", is_keyframe: "
-     << (r.is_keyframe.has_value() ? (r.is_keyframe.value() ? "true" : "false")
-                                   : "nullopt/unknown")
-     << " }";
-  return os;
-}
-
-static std::string AnnexBToString(
-    const std::vector<uint8_t>& buffer,
-    const std::vector<SubsampleEntry>& subsamples) {
+std::string AnnexBToString(const std::vector<uint8_t>& buffer,
+                           const std::vector<SubsampleEntry>& subsamples) {
   std::stringstream ss;
 
   H264Parser parser;
@@ -265,14 +230,7 @@ TEST_P(AVCConversionTest, ParseCorrectly) {
   std::vector<SubsampleEntry> subsamples;
   MakeInputForLength(GetParam(), &buf);
   EXPECT_TRUE(AVC::ConvertFrameToAnnexB(GetParam(), &buf, &subsamples));
-
-  BitstreamConverter::AnalysisResult expected;
-  expected.is_conformant = true;
-  expected.is_keyframe = false;
-  EXPECT_PRED2(AnalysesMatch,
-               AVC::AnalyzeAnnexB(buf.data(), buf.size(), subsamples),
-               expected);
-
+  EXPECT_TRUE(AVC::IsValidAnnexB(buf.data(), buf.size(), subsamples));
   EXPECT_EQ(buf.size(), sizeof(kExpected));
   EXPECT_EQ(0, memcmp(kExpected, &buf[0], sizeof(kExpected)));
   EXPECT_EQ("P,SDC", AnnexBToString(buf, subsamples));
@@ -392,107 +350,63 @@ TEST_F(AVCConversionTest, StringConversionFunctions) {
   std::vector<uint8_t> buf;
   std::vector<SubsampleEntry> subsamples;
   StringToAnnexB(str, &buf, &subsamples);
-
-  BitstreamConverter::AnalysisResult expected;
-  expected.is_conformant = true;
-  expected.is_keyframe = true;
-  EXPECT_PRED2(AnalysesMatch,
-               AVC::AnalyzeAnnexB(buf.data(), buf.size(), subsamples),
-               expected);
+  EXPECT_TRUE(AVC::IsValidAnnexB(buf.data(), buf.size(), subsamples));
 
   EXPECT_EQ(str, AnnexBToString(buf, subsamples));
 }
 
 TEST_F(AVCConversionTest, ValidAnnexBConstructs) {
-  struct {
-    const char* case_string;
-    const bool is_keyframe;
-  } test_cases[] = {
-      {"I", true},
-      {"I I I I", true},
-      {"AUD I", true},
-      {"AUD SPS PPS I", true},
-      {"I EOSeq", true},
-      {"I EOSeq EOStr", true},
-      {"I EOStr", true},
-      {"P", false},
-      {"P P P P", false},
-      {"AUD SPS PPS P", false},
-      {"SEI SEI I", true},
-      {"SEI SEI R14 I", true},
-      {"SPS SPSExt SPS PPS I P", true},
-      {"R14 SEI I", true},
-      {"AUD,I", true},
-      {"AUD,SEI I", true},
-      {"AUD,SEI,SPS,PPS,I", true},
-
-      // In reality, these might not always be conformant/valid, but assuming
-      // they are, they're not keyframes because a non-IDR slice preceded the
-      // IDR slice, if any.
-      {"SDA SDB SDC", false},
-      {"P I", false},
-      {"SDA I", false},
-      {"SDB I", false},
-      {"SDC I", false},
+  const char* test_cases[] = {
+    "I",
+    "I I I I",
+    "AUD I",
+    "AUD SPS PPS I",
+    "I EOSeq",
+    "I EOSeq EOStr",
+    "I EOStr",
+    "P",
+    "P P P P",
+    "AUD SPS PPS P",
+    "SEI SEI I",
+    "SEI SEI R14 I",
+    "SPS SPSExt SPS PPS I P",
+    "R14 SEI I",
+    "AUD,I",
+    "AUD,SEI I",
+    "AUD,SEI,SPS,PPS,I"
   };
 
   for (size_t i = 0; i < arraysize(test_cases); ++i) {
     std::vector<uint8_t> buf;
     std::vector<SubsampleEntry> subsamples;
-    StringToAnnexB(test_cases[i].case_string, &buf, NULL);
-
-    BitstreamConverter::AnalysisResult expected;
-    expected.is_conformant = true;
-    expected.is_keyframe = test_cases[i].is_keyframe;
-    EXPECT_PRED2(AnalysesMatch,
-                 AVC::AnalyzeAnnexB(buf.data(), buf.size(), subsamples),
-                 expected)
-        << "'" << test_cases[i].case_string << "' failed";
+    StringToAnnexB(test_cases[i], &buf, NULL);
+    EXPECT_TRUE(AVC::IsValidAnnexB(buf.data(), buf.size(), subsamples))
+        << "'" << test_cases[i] << "' failed";
   }
 }
 
 TEST_F(AVCConversionTest, InvalidAnnexBConstructs) {
-  struct {
-    const char* case_string;
-    const base::Optional<bool> is_keyframe;
-  } test_cases[] = {
-      // For these cases, lack of conformance is determined before detecting any
-      // IDR or non-IDR slices, so the non-conformant frames' keyframe analysis
-      // reports base::nullopt (which means undetermined analysis result).
-      {"AUD", base::nullopt},            // No VCL present.
-      {"AUD,SEI", base::nullopt},        // No VCL present.
-      {"SPS PPS", base::nullopt},        // No VCL present.
-      {"SPS PPS AUD I", base::nullopt},  // Parameter sets must come after AUD.
-      {"SPSExt SPS P", base::nullopt},   // SPS must come before SPSExt.
-      {"SPS PPS SPSExt P", base::nullopt},  // SPSExt must follow an SPS.
-      {"EOSeq", base::nullopt},             // EOSeq must come after a VCL.
-      {"EOStr", base::nullopt},             // EOStr must come after a VCL.
-
-      // For these cases, IDR slice is first VCL and is detected before
-      // conformance failure, so the non-conformant frame is reported as a
-      // keyframe.
-      {"I EOStr EOSeq", true},  // EOSeq must come before EOStr.
-      {"I R14", true},          // Reserved14-18 must come before first VCL.
-      {"I SEI", true},          // SEI must come before first VCL.
-
-      // For this case, P slice is first VCL and is detected before conformance
-      // failure, so the non-conformant frame is reported as a non-keyframe.
-      {"P SPS P",
-       false},  // SPS after first VCL would indicate a new access unit.
+  static const char* test_cases[] = {
+    "AUD",  // No VCL present.
+    "AUD,SEI", // No VCL present.
+    "SPS PPS",  // No VCL present.
+    "SPS PPS AUD I",  // Parameter sets must come after AUD.
+    "SPSExt SPS P",  // SPS must come before SPSExt.
+    "SPS PPS SPSExt P",  // SPSExt must follow an SPS.
+    "EOSeq",  // EOSeq must come after a VCL.
+    "EOStr",  // EOStr must come after a VCL.
+    "I EOStr EOSeq",  // EOSeq must come before EOStr.
+    "I R14",  // Reserved14-18 must come before first VCL.
+    "I SEI",  // SEI must come before first VCL.
+    "P SPS P", // SPS after first VCL would indicate a new access unit.
   };
 
-  BitstreamConverter::AnalysisResult expected;
-  expected.is_conformant = false;
-
   for (size_t i = 0; i < arraysize(test_cases); ++i) {
     std::vector<uint8_t> buf;
     std::vector<SubsampleEntry> subsamples;
-    StringToAnnexB(test_cases[i].case_string, &buf, NULL);
-    expected.is_keyframe = test_cases[i].is_keyframe;
-    EXPECT_PRED2(AnalysesMatch,
-                 AVC::AnalyzeAnnexB(buf.data(), buf.size(), subsamples),
-                 expected)
-        << "'" << test_cases[i].case_string << "' failed";
+    StringToAnnexB(test_cases[i], &buf, NULL);
+    EXPECT_FALSE(AVC::IsValidAnnexB(buf.data(), buf.size(), subsamples))
+        << "'" << test_cases[i] << "' failed";
   }
 }
 
@@ -528,10 +442,6 @@ TEST_F(AVCConversionTest, InsertParamSetsAnnexB) {
   avc_config.pps_list[0].push_back(0x56);
   avc_config.pps_list[0].push_back(0x78);
 
-  BitstreamConverter::AnalysisResult expected;
-  expected.is_conformant = true;
-  expected.is_keyframe = true;
-
   for (size_t i = 0; i < arraysize(test_cases); ++i) {
     std::vector<uint8_t> buf;
     std::vector<SubsampleEntry> subsamples;
@@ -540,9 +450,7 @@ TEST_F(AVCConversionTest, InsertParamSetsAnnexB) {
 
     EXPECT_TRUE(AVC::InsertParamSetsAnnexB(avc_config, &buf, &subsamples))
         << "'" << test_cases[i].input << "' insert failed.";
-    EXPECT_PRED2(AnalysesMatch,
-                 AVC::AnalyzeAnnexB(buf.data(), buf.size(), subsamples),
-                 expected)
+    EXPECT_TRUE(AVC::IsValidAnnexB(buf.data(), buf.size(), subsamples))
         << "'" << test_cases[i].input << "' created invalid AnnexB.";
     EXPECT_EQ(test_cases[i].expected, AnnexBToString(buf, subsamples))
         << "'" << test_cases[i].input << "' generated unexpected output.";

media/formats/mp4/bitstream_converter.cc

@@ -7,13 +7,6 @@
 namespace media {
 namespace mp4 {
 
-BitstreamConverter::AnalysisResult::AnalysisResult(){};
-
-BitstreamConverter::AnalysisResult::AnalysisResult(const AnalysisResult& other)
-    : is_conformant(other.is_conformant), is_keyframe(other.is_keyframe) {}
-
-BitstreamConverter::AnalysisResult::~AnalysisResult() = default;
-
 BitstreamConverter::~BitstreamConverter() = default;
 
 }  // namespace mp4

media/formats/mp4/bitstream_converter.h

@@ -10,8 +10,6 @@
 #include <vector>
 
 #include "base/memory/ref_counted.h"
-#include "base/optional.h"
-#include "media/base/media_export.h"
 
 namespace media {
 
@@ -22,21 +20,9 @@ namespace mp4 {
 // BitstreamConverter provides a unified interface for performing some common
 // bitstream conversions (e.g. H.264 MP4 bitstream to Annex B, and elementary
 // AAC stream to ADTS).
-class MEDIA_EXPORT BitstreamConverter
+class BitstreamConverter
     : public base::RefCountedThreadSafe<BitstreamConverter> {
  public:
-  // Describes the result of Analyze(). Not all analyses are implemented or
-  // enabled across mp4::BitstreamConverter implementations, hence the use of
-  // base::Optional<>.
-  struct AnalysisResult {
-    AnalysisResult();
-    AnalysisResult(const AnalysisResult&);
-    ~AnalysisResult();
-
-    base::Optional<bool> is_conformant;
-    base::Optional<bool> is_keyframe;
-  };
-
   // Converts a single frame/buffer |frame_buf| into the output format.
   // Returns true iff the conversion was successful.
   // |frame_buf| is an input/output parameter, it contains input frame data and
@@ -51,12 +37,11 @@ class MEDIA_EXPORT BitstreamConverter
                             bool is_keyframe,
                             std::vector<SubsampleEntry>* subsamples) const = 0;
 
-  // Inspects an already converted frame for conformance. If conformant,
-  // inspects further to see if the converted frame appears to be a keyframe.
-  // Note, the checks may not be exhaustive (or implemented at all).
-  virtual AnalysisResult Analyze(
-      std::vector<uint8_t>* frame_buf,
-      std::vector<SubsampleEntry>* subsamples) const = 0;
+  // Checks a converted frame for conformance.
+  // Note: may return true even if the frame is not conformant; the checks may
+  // not be exhaustive (or implemented at all).
+  virtual bool IsValid(std::vector<uint8_t>* frame_buf,
+                       std::vector<SubsampleEntry>* subsamples) const = 0;
 
  protected:
   friend class base::RefCountedThreadSafe<BitstreamConverter>;

media/formats/mp4/hevc.cc

@@ -137,13 +137,11 @@ VideoCodecProfile HEVCDecoderConfigurationRecord::GetVideoProfile() const {
 static const uint8_t kAnnexBStartCode[] = {0, 0, 0, 1};
 static const int kAnnexBStartCodeSize = 4;
 
-// static
 bool HEVC::InsertParamSetsAnnexB(
     const HEVCDecoderConfigurationRecord& hevc_config,
     std::vector<uint8_t>* buffer,
     std::vector<SubsampleEntry>* subsamples) {
-  DCHECK(HEVC::AnalyzeAnnexB(buffer->data(), buffer->size(), *subsamples)
-             .is_conformant.value_or(true));
+  DCHECK(HEVC::IsValidAnnexB(buffer->data(), buffer->size(), *subsamples));
 
   std::unique_ptr<H265Parser> parser(new H265Parser());
   const uint8_t* start = buffer->data();
@@ -181,12 +179,10 @@ bool HEVC::InsertParamSetsAnnexB(
   buffer->insert(config_insert_point,
                  param_sets.begin(), param_sets.end());
 
-  DCHECK(HEVC::AnalyzeAnnexB(buffer->data(), buffer->size(), *subsamples)
-             .is_conformant.value_or(true));
+  DCHECK(HEVC::IsValidAnnexB(buffer->data(), buffer->size(), *subsamples));
   return true;
 }
 
-// static
 bool HEVC::ConvertConfigToAnnexB(
     const HEVCDecoderConfigurationRecord& hevc_config,
     std::vector<uint8_t>* buffer) {
@@ -208,23 +204,17 @@ bool HEVC::ConvertConfigToAnnexB(
   return true;
 }
 
-// static
-BitstreamConverter::AnalysisResult HEVC::AnalyzeAnnexB(
-    const uint8_t* buffer,
-    size_t size,
-    const std::vector<SubsampleEntry>& subsamples) {
+// Verifies AnnexB NALU order according to section 7.4.2.4.4 of ISO/IEC 23008-2.
+bool HEVC::IsValidAnnexB(const uint8_t* buffer,
+                         size_t size,
+                         const std::vector<SubsampleEntry>& subsamples) {
   DCHECK(buffer);
 
-  BitstreamConverter::AnalysisResult result;
+  if (size == 0)
+    return true;
 
-  if (size == 0) {
-    result.is_conformant = true;
-    return result;
-  }
-
-  // TODO(servolk): Implement this, see https://crbug.com/527595. For now, we
-  // report that neither conformance nor keyframe analyses were performed.
-  return result;
+  // TODO(servolk): Implement this, see crbug.com/527595
+  return true;
 }
 
 HEVCBitstreamConverter::HEVCBitstreamConverter(
@@ -253,10 +243,10 @@ bool HEVCBitstreamConverter::ConvertFrame(
   return true;
 }
 
-BitstreamConverter::AnalysisResult HEVCBitstreamConverter::Analyze(
+bool HEVCBitstreamConverter::IsValid(
     std::vector<uint8_t>* frame_buf,
     std::vector<SubsampleEntry>* subsamples) const {
-  return HEVC::AnalyzeAnnexB(frame_buf->data(), frame_buf->size(), *subsamples);
+  return HEVC::IsValidAnnexB(frame_buf->data(), frame_buf->size(), *subsamples);
 }
 
 }  // namespace mp4

media/formats/mp4/hevc.h

@@ -78,15 +78,14 @@ class MEDIA_EXPORT HEVC {
       std::vector<uint8_t>* buffer,
       std::vector<SubsampleEntry>* subsamples);
 
-  // Analyzes the contents of |buffer| for conformance to
-  // Section 7.4.2.4.4 of ISO/IEC 23008-2, and if conformant, further inspects
-  // |buffer| to report whether or not it looks like a keyframe.
+  // Verifies that the contents of |buffer| conform to
+  // Section 7.4.2.4.4 of ISO/IEC 23008-2.
   // |subsamples| contains the information about what parts of the buffer are
   // encrypted and which parts are clear.
-  static BitstreamConverter::AnalysisResult AnalyzeAnnexB(
-      const uint8_t* buffer,
-      size_t size,
-      const std::vector<SubsampleEntry>& subsamples);
+  // Returns true if |buffer| contains conformant Annex B data
+  static bool IsValidAnnexB(const uint8_t* buffer,
+                            size_t size,
+                            const std::vector<SubsampleEntry>& subsamples);
 };
 
 class HEVCBitstreamConverter : public BitstreamConverter {
@@ -99,9 +98,8 @@ class HEVCBitstreamConverter : public BitstreamConverter {
                     bool is_keyframe,
                     std::vector<SubsampleEntry>* subsamples) const override;
 
-  AnalysisResult Analyze(
-      std::vector<uint8_t>* frame_buf,
-      std::vector<SubsampleEntry>* subsamples) const override;
+  bool IsValid(std::vector<uint8_t>* frame_buf,
+               std::vector<SubsampleEntry>* subsamples) const override;
 
  private:
   ~HEVCBitstreamConverter() override;

media/formats/mp4/mp4_stream_parser.cc

@@ -802,21 +802,12 @@ ParseResult MP4StreamParser::EnqueueSample(BufferQueueMap* buffers) {
             << "Failed to prepare video sample for decode";
         return ParseResult::kError;
       }
-      BitstreamConverter::AnalysisResult analysis =
-          runs_->video_description().frame_bitstream_converter->Analyze(
-              &frame_buf, &subsamples);
-      // If conformance analysis was not actually performed, assume the frame is
-      // conformant.  If it was performed and found to be non-conformant, log
-      // it.
-      if (!analysis.is_conformant.value_or(true)) {
+      if (!runs_->video_description().frame_bitstream_converter->IsValid(
+              &frame_buf, &subsamples)) {
         LIMITED_MEDIA_LOG(DEBUG, media_log_, num_invalid_conversions_,
                           kMaxInvalidConversionLogs)
             << "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|.
     }
   }