Detect H264 slices of the same frame in VEAUnittest

media/gpu/h264_decoder.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <algorithm>
 #include <limits>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback_helpers.h"
@@ skipping 92 matching lines @@
   pic->frame_num = pic->pic_num = frame_num;
   pic->adaptive_ref_pic_marking_mode_flag = false;
   pic->ref = true;
   pic->long_term_reference_flag = false;
   pic->field = H264Picture::FIELD_NONE;
 
   return CalculatePicOrderCounts(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.";
+  if (!InitPictureFromSliceHeader(curr_sps_id_, parser_, slice_hdr, curr_pic_))
     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)
-    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_))
     return false;
 
   curr_pic_->long_term_reference_flag = slice_hdr->long_term_reference_flag;
   curr_pic_->adaptive_ref_pic_marking_mode_flag =
       slice_hdr->adaptive_ref_pic_marking_mode_flag;
 
   // If the slice header indicates we will have to perform reference marking
   // process after this picture is decoded, store required data for that
@@ skipping 1002 matching lines @@
     if (!FinishPicture(pic))
       return false;
 
     unused_short_term_frame_num++;
     unused_short_term_frame_num %= max_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_, curr_pps_id_, curr_sps_id_, parser_,
+                               slice_hdr)) {
     // New picture, so first finish the previous one before processing it.
     if (!FinishPrevFrameIfPresent())
       return false;
 
     DCHECK(!curr_pic_);
 
     if (slice_hdr->first_mb_in_slice != 0) {
       DVLOG(1) << "ASO/invalid stream, first_mb_in_slice: "
                << slice_hdr->first_mb_in_slice;
       return false;
@@ skipping 192 matching lines @@
 }
 
 gfx::Size H264Decoder::GetPicSize() const {
   return pic_size_;
 }
 
 size_t H264Decoder::GetRequiredNumOfPictures() const {
   return dpb_.max_num_pics() + kPicsInPipeline;
 }
 
+// static
+bool H264Decoder::InitPictureFromSliceHeader(int curr_sps_id,

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

Perhaps we could instead return a scoped_refptr<H264Picture> to the new picture
and return nullptr on failure please? We wouldn't need to pass the existing
picture either, and also s/InitPicture/CreateH264Picture/ then.

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

Done.

+                                             const H264Parser& parser,

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

Perhaps we could pass the SPS directly to the method to avoid having to pass the
parser? In general the method requires an SPS, but not parser functionality. We
wouldn't need the sps id either then.

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

Done. Note that I kept param as |const H264SPS* sps| as it looks like there are
checks for nullptr within the function. 

+                                             const H264SliceHeader* slice_hdr,
+                                             scoped_refptr<H264Picture> pic) {
+  DCHECK(pic.get());
+
+  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;
+
+  DCHECK_NE(curr_sps_id, -1);
+  const H264SPS* sps = parser.GetSPS(curr_sps_id);
+  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(scoped_refptr<H264Picture> curr_pic,

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

const scoped_refptr<>& ?

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

I came across to this in an earlier review that passing scoped_refptr by value
is the preferred way now. I couldn't find any code guideline mentioning it
thouhg, but there is
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/0khIRhxDq6E/1lcud....

+                                           int curr_pps_id,
+                                           int curr_sps_id,
+                                           const H264Parser& parser,

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

Here we should be able to also pass an SPS only, without id and parser.

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

Done.

+                                           const H264SliceHeader* slice_hdr) {

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

Since we are not changing the structure, could we instead const H264SliceHeader&
please?

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

Done.

+  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;
+}
+
 }  // namespace media