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 4e0dd6ad8ec14bcdf064266d6eb131e9d8a9b3d5..a7f9fe7a5379b77660b98275917e0873d169e3c5 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"
@@ -599,22 +601,33 @@ class H264Validator : public StreamValidator {
       : StreamValidator(frame_cb),
         seen_sps_(false),
         seen_pps_(false),
-        seen_idr_(false) {}
+        seen_idr_(false),
+        curr_pic_(new H264Picture()),

[Pawel Osciak, 2016/12/20 08:34:22]

Perhaps we could create this when a new frame is parsed.

[emircan, 2017/01/03 19:29:00]

Done.

+        curr_sps_id_(-1),

[Pawel Osciak, 2016/12/20 08:34:22]

Nit: perhaps initialize at definition site below?

[emircan, 2017/01/03 19:29:00]

Done.

+        curr_pps_id_(-1) {}
 
   void ProcessStreamBuffer(const uint8_t* stream, size_t size) override;
 
  private:
+  bool IsNewFrame(const H264NALU& nalu, int sps_id, int pps_id);
+
   // 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_;
+  int curr_pps_id_;
+
   H264Parser h264_parser_;
 };
 
 void H264Validator::ProcessStreamBuffer(const uint8_t* stream, size_t size) {
   h264_parser_.SetStream(stream, static_cast<off_t>(size));
 
+  int sps_id = -1;
+  int pps_id = -1;
   while (1) {
     H264NALU nalu;
     H264Parser::Result result;
@@ -624,7 +637,6 @@ void H264Validator::ProcessStreamBuffer(const uint8_t* stream, size_t size) {
       break;
 
     ASSERT_EQ(H264Parser::kOk, result);
-
     bool keyframe = false;
 
     switch (nalu.nal_unit_type) {
@@ -636,14 +648,12 @@ 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))
+        if (IsNewFrame(nalu, sps_id, pps_id) && !frame_cb_.Run(keyframe))

[Pawel Osciak, 2016/12/20 08:34:22]

For readability, please:

if (IsNewFrame()) {
  if (!frame_cb_.Run(keyframe))
    return;
}


Separately, even if we just saw an SPS and/or PPS in the stream, it does not
mean they are being used by the slice that follows them (or, really, by any of
the subsequent slices in the stream for that matter).

So for the new frame logic, we should instead compare pic_parameter_set_id from
the parsed H264SliceHeader and the corresponding seq_parameter_set_id (might be
useful to add it to H264SliceHeader as well), to the curr_sps_id_ and
curr_pps_id_, and update curr_sps_id_ and curr_pps_id_ with the values from the
slice unconditionally, immediately after processing it.

[emircan, 2017/01/03 19:29:00]

Done.

           return;
         break;
       }
 
       case H264NALU::kSPS: {
-        int sps_id;
         ASSERT_EQ(H264Parser::kOk, h264_parser_.ParseSPS(&sps_id));
         seen_sps_ = true;
         break;
@@ -651,7 +661,6 @@ void H264Validator::ProcessStreamBuffer(const uint8_t* stream, size_t size) {
 
       case H264NALU::kPPS: {
         ASSERT_TRUE(seen_sps_);
-        int pps_id;
         ASSERT_EQ(H264Parser::kOk, h264_parser_.ParsePPS(&pps_id));
         seen_pps_ = true;
         break;
@@ -663,6 +672,25 @@ void H264Validator::ProcessStreamBuffer(const uint8_t* stream, size_t size) {
   }
 }
 
+bool H264Validator::IsNewFrame(const H264NALU& nalu, int sps_id, int pps_id) {
+  std::unique_ptr<H264SliceHeader> slice_hdr(new H264SliceHeader());
+  const bool par_res = h264_parser_.ParseSliceHeader(nalu, slice_hdr.get());
+  if (par_res != H264Parser::kOk) {
+    LOG(ERROR) << "Cannot parse H264 slice header.";
+    return true;

[Pawel Osciak, 2016/12/20 08:34:22]

Should this be s/true/false/?

[emircan, 2017/01/03 19:29:00]

Done.

+  }
+  const bool is_new_frame = H264Decoder::IsNewPrimaryCodedPicture(
+      curr_pic_, curr_sps_id_, curr_pps_id_, h264_parser_, slice_hdr.get());
+
+  curr_sps_id_ = sps_id != -1 ? sps_id : curr_sps_id_;
+  curr_pps_id_ = pps_id != -1 ? pps_id : curr_pps_id_;
+  if (!H264Decoder::InitPictureFromSliceHeader(curr_sps_id_, h264_parser_,
+                                               slice_hdr.get(), curr_pic_)) {
+    LOG(FATAL) << "Cannot initialize current frame.";
+  }
+  return is_new_frame;
+}
+
 class VP8Validator : public StreamValidator {
  public:
   explicit VP8Validator(const FrameFoundCallback& frame_cb)
@@ -2101,6 +2129,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,