Detect H264 slices of the same frame in VEAUnittest

media/gpu/video_encode_accelerator_unittest.cc

Index: media/gpu/video_encode_accelerator_unittest.cc
diff --git a/media/gpu/video_encode_accelerator_unittest.cc b/media/gpu/video_encode_accelerator_unittest.cc
index ded9b864e73252eda919cf9c65b2ab8755341d25..98fe7eeeec95c881b6f0fb7ccd38656d663aa733 100644
--- a/media/gpu/video_encode_accelerator_unittest.cc
+++ b/media/gpu/video_encode_accelerator_unittest.cc
@@ -46,6 +46,8 @@
 #include "media/filters/ffmpeg_video_decoder.h"
 #include "media/filters/h264_parser.h"
 #include "media/filters/ivf_parser.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/video_encode_accelerator.h"
@@ -605,11 +607,18 @@ class H264Validator : public StreamValidator {
   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_;
 };
 
@@ -637,9 +646,20 @@ 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))
+        H264SliceHeader slice_hdr;
+        const bool par_res = h264_parser_.ParseSliceHeader(nalu, &slice_hdr);

[Pawel Osciak, 2017/02/14 09:27:34]

s/bool/auto/

[emircan, 2017/02/14 22:58:45]

Moved it inside ASSERT_EQ.

+        if (par_res != H264Parser::kOk) {

[Pawel Osciak, 2017/02/14 09:27:34]

ASSERT_TRUE perhaps?

[emircan, 2017/02/14 22:58:45]

Moved the whole block into an ASSERT_EQ.

+          LOG(ERROR) << "Cannot parse H264 slice header.";
+          return;
+        }
+        if (IsNewPicture(slice_hdr)) {
+          if (!frame_cb_.Run(keyframe))
+            return;
+        }
+        if (!UpdateCurrentPicture(slice_hdr)) {

[Pawel Osciak, 2017/02/14 09:27:34]

This should also likely be a failure.

[emircan, 2017/02/14 22:58:45]

Done.

+          LOG(ERROR) << "Cannot update current picture.";
           return;
+        }
         break;
       }
 
@@ -664,6 +684,29 @@ void H264Validator::ProcessStreamBuffer(const uint8_t* stream, size_t size) {
   }
 }
 
+bool H264Validator::IsNewPicture(const H264SliceHeader& slice_hdr) {
+  return H264Decoder::IsNewPrimaryCodedPicture(
+      curr_pic_, 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;
+  curr_pic_ = H264Decoder::CreateH264PictureFromSliceHeader(

[Pawel Osciak, 2017/02/14 09:27:34]

Should we only be Creating if IsNewPicture() returned true, not otherwise?

[emircan, 2017/02/14 22:58:45]

Moved the call inside "if (IsNewPicture(slice_hdr))" block.

+      h264_parser_.GetSPS(curr_sps_id_), slice_hdr);

[Pawel Osciak, 2017/02/14 09:27:34]

For symmetry, perhaps we could check GetSPS()'s result similarly to GetPPS()
above and fail on that?

[emircan, 2017/02/14 22:58:45]

Done.

+  if (!curr_pic_) {
+    LOG(FATAL) << "Cannot initialize current frame.";
+    return false;
+  }
+  return true;
+}
+
 class VP8Validator : public StreamValidator {
  public:
   explicit VP8Validator(const FrameFoundCallback& frame_cb)
@@ -2233,6 +2276,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,