MSE: Use video (AVC) keyframeness for SPS/PPS injection

In 1240638e, AVC keyframeness analysis result in MP4 was used instead of
MP4 keyframe metadata, but the conversion of the AVC frame to Annex-B
(and the conditional injection of SPS/PPS) still relied upon the MP4
keyframe metadata. Since that metadata could be incorrect, decode errors
could occur on some decoders due to missing SPS/PPS data for incorrectly
muxed AVC keyframes.

This change performs the bitstream analysis as part of frame conversion,
after converting to Annex-B, but before the conditional SPS/PPS
injection, and uses the analysis's keyframe result (if determined) to
condition the injection instead of the MP4 keyframe metadata.

BUG=889311

Change-Id: I1ef327ef7d399d5a9da552e0da4a58b0f0b04435
Reviewed-on: https://chromium-review.googlesource.com/1250171
Commit-Queue: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595176}

media/formats/mp4/avc.cc

@@ -331,10 +331,11 @@ AVCBitstreamConverter::AVCBitstreamConverter(
 
 AVCBitstreamConverter::~AVCBitstreamConverter() = default;
 
-bool AVCBitstreamConverter::ConvertFrame(
+bool AVCBitstreamConverter::ConvertAndAnalyzeFrame(
     std::vector<uint8_t>* frame_buf,
     bool is_keyframe,
-    std::vector<SubsampleEntry>* subsamples) const {
+    std::vector<SubsampleEntry>* subsamples,
+    AnalysisResult* analysis_result) const {
   // Convert the AVC NALU length fields to Annex B headers, as expected by
   // decoding libraries. Since this may enlarge the size of the buffer, we also
   // update the clear byte count for each subsample if encryption is used to
@@ -343,7 +344,13 @@ bool AVCBitstreamConverter::ConvertFrame(
   RCHECK(AVC::ConvertFrameToAnnexB(avc_config_->length_size, frame_buf,
                                    subsamples));
 
-  if (is_keyframe) {
+  // |is_keyframe| may be incorrect. Analyze the frame to see if it is a
+  // keyframe. |is_keyframe| will be used if the analysis is inconclusive.
+  // Also, provide the analysis result to the caller via out parameter
+  // |analysis_result|.
+  *analysis_result = Analyze(frame_buf, subsamples);
+
+  if (analysis_result->is_keyframe.value_or(is_keyframe)) {
     // If this is a keyframe, we (re-)inject SPS and PPS headers at the start of
     // a frame. If subsample info is present, we also update the clear byte
     // count for that first subsample.

media/formats/mp4/avc.h

@@ -80,16 +80,16 @@ class AVCBitstreamConverter : public BitstreamConverter {
 #endif  // BUILDFLAG(ENABLE_DOLBY_VISION_DEMUXING)
 
   // BitstreamConverter interface
-  bool ConvertFrame(std::vector<uint8_t>* frame_buf,
-                    bool is_keyframe,
-                    std::vector<SubsampleEntry>* subsamples) const override;
+  bool ConvertAndAnalyzeFrame(std::vector<uint8_t>* frame_buf,
+                              bool is_keyframe,
+                              std::vector<SubsampleEntry>* subsamples,
+                              AnalysisResult* analysis_result) const override;
 
+ private:
+  ~AVCBitstreamConverter() override;
   AnalysisResult Analyze(
       std::vector<uint8_t>* frame_buf,
       std::vector<SubsampleEntry>* subsamples) const override;
-
- private:
-  ~AVCBitstreamConverter() override;
   std::unique_ptr<AVCDecoderConfigurationRecord> avc_config_;
 
 #if BUILDFLAG(ENABLE_DOLBY_VISION_DEMUXING)

media/formats/mp4/bitstream_converter.h

@@ -47,9 +47,17 @@ class MEDIA_EXPORT BitstreamConverter
   // of input frame are encrypted and should update |subsamples| if necessary,
   // to make sure it correctly describes the converted output frame. See
   // SubsampleEntry definition in media/base/decrypt_config.h for more info.
-  virtual bool ConvertFrame(std::vector<uint8_t>* frame_buf,
-                            bool is_keyframe,
-                            std::vector<SubsampleEntry>* subsamples) const = 0;
+  // |analysis_result| is an output parameter that contains the AnalysisResult
+  // found during conversion.
+  virtual bool ConvertAndAnalyzeFrame(
+      std::vector<uint8_t>* frame_buf,
+      bool is_keyframe,
+      std::vector<SubsampleEntry>* subsamples,
+      AnalysisResult* analysis_result) const = 0;
+
+ protected:
+  friend class base::RefCountedThreadSafe<BitstreamConverter>;
+  virtual ~BitstreamConverter();
 
   // Inspects an already converted frame for conformance. If conformant,
   // inspects further to see if the converted frame appears to be a keyframe.
@@ -57,10 +65,6 @@ class MEDIA_EXPORT BitstreamConverter
   virtual AnalysisResult Analyze(
       std::vector<uint8_t>* frame_buf,
       std::vector<SubsampleEntry>* subsamples) const = 0;
-
- protected:
-  friend class base::RefCountedThreadSafe<BitstreamConverter>;
-  virtual ~BitstreamConverter();
 };
 
 }  // namespace mp4

media/formats/mp4/hevc.cc

@@ -236,14 +236,20 @@ HEVCBitstreamConverter::HEVCBitstreamConverter(
 HEVCBitstreamConverter::~HEVCBitstreamConverter() {
 }
 
-bool HEVCBitstreamConverter::ConvertFrame(
+bool HEVCBitstreamConverter::ConvertAndAnalyzeFrame(
     std::vector<uint8_t>* frame_buf,
     bool is_keyframe,
-    std::vector<SubsampleEntry>* subsamples) const {
+    std::vector<SubsampleEntry>* subsamples,
+    AnalysisResult* analysis_result) const {
   RCHECK(AVC::ConvertFrameToAnnexB(hevc_config_->lengthSizeMinusOne + 1,
                                    frame_buf, subsamples));
+  // |is_keyframe| may be incorrect. Analyze the frame to see if it is a
+  // keyframe. |is_keyframe| will be used if the analysis is inconclusive.
+  // Also, provide the analysis result to the caller via out parameter
+  // |analysis_result|.
+  *analysis_result = Analyze(frame_buf, subsamples);
 
-  if (is_keyframe) {
+  if (analysis_result->is_keyframe.value_or(is_keyframe)) {
     // If this is a keyframe, we (re-)inject HEVC params headers at the start of
     // a frame. If subsample info is present, we also update the clear byte
     // count for that first subsample.

media/formats/mp4/hevc.h

@@ -95,16 +95,16 @@ class HEVCBitstreamConverter : public BitstreamConverter {
       std::unique_ptr<HEVCDecoderConfigurationRecord> hevc_config);
 
   // BitstreamConverter interface
-  bool ConvertFrame(std::vector<uint8_t>* frame_buf,
-                    bool is_keyframe,
-                    std::vector<SubsampleEntry>* subsamples) const override;
+  bool ConvertAndAnalyzeFrame(std::vector<uint8_t>* frame_buf,
+                              bool is_keyframe,
+                              std::vector<SubsampleEntry>* subsamples,
+                              AnalysisResult* analysis_result) const override;
 
+ private:
+  ~HEVCBitstreamConverter() override;
   AnalysisResult Analyze(
       std::vector<uint8_t>* frame_buf,
       std::vector<SubsampleEntry>* subsamples) const override;
-
- private:
-  ~HEVCBitstreamConverter() override;
   std::unique_ptr<HEVCDecoderConfigurationRecord> hevc_config_;
 };
 

media/formats/mp4/mp4_stream_parser.cc

@@ -809,15 +809,15 @@ ParseResult MP4StreamParser::EnqueueSample(BufferQueueMap* buffers) {
         runs_->video_description().video_codec == kCodecHEVC ||
         runs_->video_description().video_codec == kCodecDolbyVision) {
       DCHECK(runs_->video_description().frame_bitstream_converter);
-      if (!runs_->video_description().frame_bitstream_converter->ConvertFrame(
-              &frame_buf, is_keyframe, &subsamples)) {
+      BitstreamConverter::AnalysisResult analysis;
+      if (!runs_->video_description()
+               .frame_bitstream_converter->ConvertAndAnalyzeFrame(
+                   &frame_buf, is_keyframe, &subsamples, &analysis)) {
         MEDIA_LOG(ERROR, media_log_)
             << "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.