Detect H264 slices of the same frame in VEAUnittest

This CL is based on https://codereview.chromium.org/2538883002/.

This CL adds checks similar to H264Decoder::IsNewPrimaryCodedPicture() to
determine if the given H264 slice belongs to a new frame. On Mac and Win,
we can have multiple slices per frame and this causes problems.

Bug: 669678, 828176
Test: Tested VEAUnittest on MBP 2016.
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;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I4781bacd199ae0545a3f46b37765b126f947b92e
Reviewed-on: https://chromium-review.googlesource.com/991198Reviewed-by: Dan Sanders <sandersd@chromium.org>
Commit-Queue: Emircan Uysaler <emircan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548590}

media/gpu/h264_decoder.cc

@@ -110,53 +110,9 @@ bool H264Decoder::InitNonexistingPicture(scoped_refptr<H264Picture> pic,
 }
 
 bool H264Decoder::InitCurrPicture(const H264SliceHeader* slice_hdr) {
-  DCHECK(curr_pic_.get());
-
-  curr_pic_->idr = slice_hdr->idr_pic_flag;
-  if (curr_pic_->idr)
-    curr_pic_->idr_pic_id = slice_hdr->idr_pic_id;
-
-  if (slice_hdr->field_pic_flag) {
-    curr_pic_->field = slice_hdr->bottom_field_flag ? H264Picture::FIELD_BOTTOM
-                                                    : H264Picture::FIELD_TOP;
-  } else {
-    curr_pic_->field = H264Picture::FIELD_NONE;
-  }
-
-  if (curr_pic_->field != H264Picture::FIELD_NONE) {
-    DVLOG(1) << "Interlaced video not supported.";
-    return false;
-  }
-
-  curr_pic_->nal_ref_idc = slice_hdr->nal_ref_idc;
-  curr_pic_->ref = slice_hdr->nal_ref_idc != 0;
-  // This assumes non-interlaced stream.
-  curr_pic_->frame_num = curr_pic_->pic_num = slice_hdr->frame_num;
-
-  DCHECK_NE(curr_sps_id_, -1);
-  const H264SPS* sps = parser_.GetSPS(curr_sps_id_);
-  if (!sps)
+  if (!FillH264PictureFromSliceHeader(parser_.GetSPS(curr_sps_id_), *slice_hdr,
+                                      curr_pic_.get())) {
     return false;
-
-  curr_pic_->pic_order_cnt_type = sps->pic_order_cnt_type;
-  switch (curr_pic_->pic_order_cnt_type) {
-    case 0:
-      curr_pic_->pic_order_cnt_lsb = slice_hdr->pic_order_cnt_lsb;
-      curr_pic_->delta_pic_order_cnt_bottom =
-          slice_hdr->delta_pic_order_cnt_bottom;
-      break;
-
-    case 1:
-      curr_pic_->delta_pic_order_cnt0 = slice_hdr->delta_pic_order_cnt0;
-      curr_pic_->delta_pic_order_cnt1 = slice_hdr->delta_pic_order_cnt1;
-      break;
-
-    case 2:
-      break;
-
-    default:
-      NOTREACHED();
-      return false;
   }
 
   if (!CalculatePicOrderCounts(curr_pic_))
@@ -1200,53 +1156,12 @@ bool H264Decoder::HandleFrameNumGap(int frame_num) {
   return true;
 }
 
-bool H264Decoder::IsNewPrimaryCodedPicture(
-    const H264SliceHeader* slice_hdr) const {
-  if (!curr_pic_)
-    return true;
-
-  // 7.4.1.2.4, assumes non-interlaced.
-  if (slice_hdr->frame_num != curr_pic_->frame_num ||
-      slice_hdr->pic_parameter_set_id != curr_pps_id_ ||
-      slice_hdr->nal_ref_idc != curr_pic_->nal_ref_idc ||
-      slice_hdr->idr_pic_flag != curr_pic_->idr ||
-      (slice_hdr->idr_pic_flag &&
-       (slice_hdr->idr_pic_id != curr_pic_->idr_pic_id ||
-        // If we have two consecutive IDR slices, and the second one has
-        // first_mb_in_slice == 0, treat it as a new picture.
-        // Per spec, idr_pic_id should not be equal in this case (and we should
-        // have hit the condition above instead, see spec 7.4.3 on idr_pic_id),
-        // but some encoders neglect changing idr_pic_id for two consecutive
-        // IDRs. Work around this by checking if the next slice contains the
-        // zeroth macroblock, i.e. data that belongs to the next picture.
-        slice_hdr->first_mb_in_slice == 0)))
-    return true;
-
-  const H264SPS* sps = parser_.GetSPS(curr_sps_id_);
-  if (!sps)
-    return false;
-
-  if (sps->pic_order_cnt_type == curr_pic_->pic_order_cnt_type) {
-    if (curr_pic_->pic_order_cnt_type == 0) {
-      if (slice_hdr->pic_order_cnt_lsb != curr_pic_->pic_order_cnt_lsb ||
-          slice_hdr->delta_pic_order_cnt_bottom !=
-              curr_pic_->delta_pic_order_cnt_bottom)
-        return true;
-    } else if (curr_pic_->pic_order_cnt_type == 1) {
-      if (slice_hdr->delta_pic_order_cnt0 != curr_pic_->delta_pic_order_cnt0 ||
-          slice_hdr->delta_pic_order_cnt1 != curr_pic_->delta_pic_order_cnt1)
-        return true;
-    }
-  }
-
-  return false;
-}
-
 bool H264Decoder::PreprocessCurrentSlice() {
   const H264SliceHeader* slice_hdr = curr_slice_hdr_.get();
   DCHECK(slice_hdr);
 
-  if (IsNewPrimaryCodedPicture(slice_hdr)) {
+  if (IsNewPrimaryCodedPicture(curr_pic_.get(), curr_pps_id_,
+                               parser_.GetSPS(curr_sps_id_), *slice_hdr)) {
     // New picture, so first finish the previous one before processing it.
     if (!FinishPrevFrameIfPresent())
       return false;
@@ -1458,4 +1373,101 @@ size_t H264Decoder::GetRequiredNumOfPictures() const {
   return dpb_.max_num_pics() + kPicsInPipeline;
 }
 
+// static
+bool H264Decoder::FillH264PictureFromSliceHeader(
+    const H264SPS* sps,
+    const H264SliceHeader& slice_hdr,
+    H264Picture* pic) {
+  DCHECK(pic);
+
+  pic->idr = slice_hdr.idr_pic_flag;
+  if (pic->idr)
+    pic->idr_pic_id = slice_hdr.idr_pic_id;
+
+  if (slice_hdr.field_pic_flag) {
+    pic->field = slice_hdr.bottom_field_flag ? H264Picture::FIELD_BOTTOM
+                                             : H264Picture::FIELD_TOP;
+  } else {
+    pic->field = H264Picture::FIELD_NONE;
+  }
+
+  if (pic->field != H264Picture::FIELD_NONE) {
+    DVLOG(1) << "Interlaced video not supported.";
+    return false;
+  }
+
+  pic->nal_ref_idc = slice_hdr.nal_ref_idc;
+  pic->ref = slice_hdr.nal_ref_idc != 0;
+  // This assumes non-interlaced stream.
+  pic->frame_num = pic->pic_num = slice_hdr.frame_num;
+
+  if (!sps)
+    return false;
+
+  pic->pic_order_cnt_type = sps->pic_order_cnt_type;
+  switch (pic->pic_order_cnt_type) {
+    case 0:
+      pic->pic_order_cnt_lsb = slice_hdr.pic_order_cnt_lsb;
+      pic->delta_pic_order_cnt_bottom = slice_hdr.delta_pic_order_cnt_bottom;
+      break;
+
+    case 1:
+      pic->delta_pic_order_cnt0 = slice_hdr.delta_pic_order_cnt0;
+      pic->delta_pic_order_cnt1 = slice_hdr.delta_pic_order_cnt1;
+      break;
+
+    case 2:
+      break;
+
+    default:
+      NOTREACHED();
+      return false;
+  }
+  return true;
+}
+
+// static
+bool H264Decoder::IsNewPrimaryCodedPicture(const H264Picture* curr_pic,
+                                           int curr_pps_id,
+                                           const H264SPS* sps,
+                                           const H264SliceHeader& slice_hdr) {
+  if (!curr_pic)
+    return true;
+
+  // 7.4.1.2.4, assumes non-interlaced.
+  if (slice_hdr.frame_num != curr_pic->frame_num ||
+      slice_hdr.pic_parameter_set_id != curr_pps_id ||
+      slice_hdr.nal_ref_idc != curr_pic->nal_ref_idc ||
+      slice_hdr.idr_pic_flag != curr_pic->idr ||
+      (slice_hdr.idr_pic_flag &&
+       (slice_hdr.idr_pic_id != curr_pic->idr_pic_id ||
+        // If we have two consecutive IDR slices, and the second one has
+        // first_mb_in_slice == 0, treat it as a new picture.
+        // Per spec, idr_pic_id should not be equal in this case (and we should
+        // have hit the condition above instead, see spec 7.4.3 on idr_pic_id),
+        // but some encoders neglect changing idr_pic_id for two consecutive
+        // IDRs. Work around this by checking if the next slice contains the
+        // zeroth macroblock, i.e. data that belongs to the next picture.
+        slice_hdr.first_mb_in_slice == 0)))
+    return true;
+
+  if (!sps)
+    return false;
+
+  if (sps->pic_order_cnt_type == curr_pic->pic_order_cnt_type) {
+    if (curr_pic->pic_order_cnt_type == 0) {
+      if (slice_hdr.pic_order_cnt_lsb != curr_pic->pic_order_cnt_lsb ||
+          slice_hdr.delta_pic_order_cnt_bottom !=
+              curr_pic->delta_pic_order_cnt_bottom)
+        return true;
+    } else if (curr_pic->pic_order_cnt_type == 1) {
+      if (slice_hdr.delta_pic_order_cnt0 != curr_pic->delta_pic_order_cnt0 ||
+          slice_hdr.delta_pic_order_cnt1 != curr_pic->delta_pic_order_cnt1)
+        return true;
+    }
+  }
+
+  return false;
+}
+
 }  // namespace media

media/gpu/h264_decoder.h

@@ -112,6 +112,18 @@ class MEDIA_GPU_EXPORT H264Decoder : public AcceleratedVideoDecoder {
   gfx::Size GetPicSize() const override;
   size_t GetRequiredNumOfPictures() const override;
 
+  // Return true if we need to start a new picture.
+  static bool IsNewPrimaryCodedPicture(const H264Picture* curr_pic,
+                                       int curr_pps_id,
+                                       const H264SPS* sps,
+                                       const H264SliceHeader& slice_hdr);
+
+  // Fill a H264Picture in |pic| from given |sps| and |slice_hdr|. Return false
+  // when there is an error.
+  static bool FillH264PictureFromSliceHeader(const H264SPS* sps,
+                                             const H264SliceHeader& slice_hdr,
+                                             H264Picture* pic);
+
  private:
   // We need to keep at most kDPBMaxSize pictures in DPB for
   // reference/to display later and an additional one for the one currently
@@ -140,9 +152,6 @@ class MEDIA_GPU_EXPORT H264Decoder : public AcceleratedVideoDecoder {
   // Process current slice as a slice of the current picture.
   bool ProcessCurrentSlice();
 
-  // Return true if we need to start a new picture.
-  bool IsNewPrimaryCodedPicture(const H264SliceHeader* slice_hdr) const;
-
   // Initialize the current picture according to data in |slice_hdr|.
   bool InitCurrPicture(const H264SliceHeader* slice_hdr);
 

media/gpu/video_encode_accelerator_unittest.cc

@@ -48,6 +48,8 @@
 #include "media/filters/ivf_parser.h"
 #include "media/gpu/buildflags.h"
 #include "media/gpu/gpu_video_encode_accelerator_factory.h"
+#include "media/gpu/h264_decoder.h"
+#include "media/gpu/h264_dpb.h"
 #include "media/gpu/video_accelerator_unittest_helpers.h"
 #include "media/video/fake_video_encode_accelerator.h"
 #include "media/video/h264_parser.h"
@@ -555,16 +557,24 @@ class H264Validator : public StreamValidator {
       : StreamValidator(frame_cb),
         seen_sps_(false),
         seen_pps_(false),
-        seen_idr_(false) {}
+        seen_idr_(false),
+        curr_pic_(new H264Picture) {}
 
   void ProcessStreamBuffer(const uint8_t* stream, size_t size) override;
 
  private:
+  bool IsNewPicture(const H264SliceHeader& slice_hdr);
+  bool UpdateCurrentPicture(const H264SliceHeader& slice_hdr);
+
   // Set to true when encoder provides us with the corresponding NALU type.
   bool seen_sps_;
   bool seen_pps_;
   bool seen_idr_;
 
+  scoped_refptr<H264Picture> curr_pic_;
+  int curr_sps_id_ = -1;
+  int curr_pps_id_ = -1;
+
   H264Parser h264_parser_;
 };
 
@@ -592,9 +602,14 @@ void H264Validator::ProcessStreamBuffer(const uint8_t* stream, size_t size) {
         FALLTHROUGH;
       case H264NALU::kNonIDRSlice: {
         ASSERT_TRUE(seen_idr_);
-        seen_sps_ = seen_pps_ = false;
-        if (!frame_cb_.Run(keyframe))
-          return;
+        H264SliceHeader slice_hdr;
+        ASSERT_EQ(H264Parser::kOk,
+                  h264_parser_.ParseSliceHeader(nalu, &slice_hdr));
+        if (IsNewPicture(slice_hdr)) {
+          if (!frame_cb_.Run(keyframe))
+            return;
+          ASSERT_TRUE(UpdateCurrentPicture(slice_hdr));
+        }
         break;
       }
 
@@ -619,6 +634,37 @@ void H264Validator::ProcessStreamBuffer(const uint8_t* stream, size_t size) {
   }
 }
 
+bool H264Validator::IsNewPicture(const H264SliceHeader& slice_hdr) {
+  if (!curr_pic_)
+    return true;
+  return H264Decoder::IsNewPrimaryCodedPicture(
+      curr_pic_.get(), curr_pps_id_, h264_parser_.GetSPS(curr_sps_id_),
+      slice_hdr);
+}
+
+bool H264Validator::UpdateCurrentPicture(const H264SliceHeader& slice_hdr) {
+  curr_pps_id_ = slice_hdr.pic_parameter_set_id;
+  const H264PPS* pps = h264_parser_.GetPPS(curr_pps_id_);
+  if (!pps) {
+    LOG(ERROR) << "Cannot parse pps.";
+    return false;
+  }
+
+  curr_sps_id_ = pps->seq_parameter_set_id;
+  const H264SPS* sps = h264_parser_.GetSPS(curr_sps_id_);
+  if (!sps) {
+    LOG(ERROR) << "Cannot parse sps.";
+    return false;
+  }
+
+  if (!H264Decoder::FillH264PictureFromSliceHeader(sps, slice_hdr,
+                                                   curr_pic_.get())) {
+    LOG(FATAL) << "Cannot initialize current frame.";
+    return false;
+  }
+  return true;
+}
+
 class VP8Validator : public StreamValidator {
  public:
   explicit VP8Validator(const FrameFoundCallback& frame_cb)
@@ -2575,6 +2621,13 @@ INSTANTIATE_TEST_CASE_P(MultipleEncoders,
                                                           false,
                                                           false,
                                                           false)));
+
+INSTANTIATE_TEST_CASE_P(
+    VerifyTimestamp,
+    VideoEncodeAcceleratorTest,
+    ::testing::Values(
+        std::make_tuple(1, false, 0, false, false, false, false, false, true)));
+
 #if defined(OS_WIN)
 INSTANTIATE_TEST_CASE_P(
     ForceBitrate,