Implement |pic_order_cnt| computation for VTVideoDecode accelerator.

content/common/gpu/media/h264_poc.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.

[DaleCurtis, 2014/11/20 23:03:30]

Definitely needs a unit test in this CL, this is self-contained enough that it
should be easy.

[sandersd (OOO until July 31), 2014/11/20 23:58:25]

Acknowledged. I'll ping when I have added that.

I won't try for full coverage.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <algorithm>
+
+#include "base/logging.h"
+#include "content/common/gpu/media/h264_poc.h"
+#include "media/filters/h264_parser.h"
+
+namespace content {
+
+H264POC::H264POC() {
+  Reset();
+}
+
+H264POC::~H264POC() {
+}
+
+void H264POC::Reset() {
+  // It shouldn't be necessary to reset these values, but doing so will improve
+  // reproducibility for buggy streams.
+  ref_pic_order_cnt_msb_ = 0;
+  ref_pic_order_cnt_lsb_ = 0;
+  prev_frame_num_ = 0;
+  prev_frame_num_offset_ = 0;
+}
+
+static bool HasMMCO5(const media::H264SliceHeader& slice_hdr) {

[DaleCurtis, 2014/11/20 23:03:30]

Needs docs.

[sandersd (OOO until July 31), 2014/11/20 23:58:24]

Done.

+  if (slice_hdr.nal_ref_idc == 0 ||
+      slice_hdr.idr_pic_flag ||
+      !slice_hdr.adaptive_ref_pic_marking_mode_flag) {
+    return false;
+  }
+
+  for (size_t i = 0; i < arraysize(slice_hdr.ref_pic_marking); i++) {

[DaleCurtis, 2014/11/20 23:03:29]

Why is this a loop? It seems this is all just:

return slice_hdr.ref_pic_marking[0].memory_mgmnt_control_operation == 0 || 5.

[sandersd (OOO until July 31), 2014/11/20 23:58:24]

No, it loops until ref_pic_marking[i].mmco == 0, returning true only if it finds
a 5 first. I've rewritten it using ifs, hopefully it is more clear.

+    switch (slice_hdr.ref_pic_marking[i].memory_mgmnt_control_operation) {
+      case 0:
+        return false;
+      case 5:
+        return true;
+      default:
+        break;
+    }
+  }
+
+  // Should not get here, the list is always zero terminated.
+  return false;
+}
+
+bool H264POC::ComputePicOrderCnt(
+    const media::H264SPS* sps,
+    const media::H264SliceHeader& slice_hdr,
+    int32_t *pic_order_cnt) {
+  if (slice_hdr.field_pic_flag) {
+    DLOG(ERROR) << "Interlaced frames are not supported";
+    return false;
+  }
+
+  // TODO(sandersd): Handle |gaps_in_frame_num_value|.
+  if (prev_frame_num_ > 0 && prev_frame_num_ < slice_hdr.frame_num - 1) {
+    DLOG(ERROR) << "Gaps in frame_num are not supported";
+    return false;
+  }
+
+  bool mmco5 = HasMMCO5(slice_hdr);

[DaleCurtis, 2014/11/20 23:03:30]

Docs or a link to the spec this is implementing?

[sandersd (OOO until July 31), 2014/11/20 23:58:24]

Done.

+  int32_t max_frame_num = 1 << (sps->log2_max_frame_num_minus4 + 4);
+  int32_t max_pic_order_cnt_lsb =
+      1 << (sps->log2_max_pic_order_cnt_lsb_minus4 + 4);
+
+  // Reorganized slightly from spec pseudocode to handle MMCO5 when storing.
+  switch (sps->pic_order_cnt_type) {

[DaleCurtis, 2014/11/20 23:03:30]

nit: This actually seems like it would look better as a series of if/else
blocks... certainly it would save you some indentation.

[sandersd (OOO until July 31), 2014/11/20 23:58:24]

I tried it both ways, I think it's more clear as a switch.

+    case 0:
+      {

[DaleCurtis, 2014/11/20 23:03:29]

Move { to above line and save 2 spaces of indent.

[sandersd (OOO until July 31), 2014/11/20 23:58:24]

Done.

+        int32_t prev_pic_order_cnt_msb = ref_pic_order_cnt_msb_;
+        int32_t prev_pic_order_cnt_lsb = ref_pic_order_cnt_lsb_;
+
+        if (slice_hdr.idr_pic_flag) {
+          prev_pic_order_cnt_msb = 0;
+          prev_pic_order_cnt_lsb = 0;
+        }
+
+        // 8-3.

[DaleCurtis, 2014/11/20 23:03:29]

Helps to include summary of this step :) Ditto for all other steps.

[sandersd (OOO until July 31), 2014/11/20 23:58:24]

Done. At least as much as possible, the steps are rather opaque even with a copy
of the spec handy.

+        int32_t pic_order_cnt_msb;
+        if ((slice_hdr.pic_order_cnt_lsb < prev_pic_order_cnt_lsb) &&
+            (prev_pic_order_cnt_lsb - slice_hdr.pic_order_cnt_lsb >=
+             max_pic_order_cnt_lsb / 2)) {
+          pic_order_cnt_msb = prev_pic_order_cnt_msb + max_pic_order_cnt_lsb;
+        } else if ((slice_hdr.pic_order_cnt_lsb > prev_pic_order_cnt_lsb) &&
+            (slice_hdr.pic_order_cnt_lsb - prev_pic_order_cnt_lsb >
+             max_pic_order_cnt_lsb / 2)) {
+          pic_order_cnt_msb = prev_pic_order_cnt_msb - max_pic_order_cnt_lsb;
+        } else {
+          pic_order_cnt_msb = prev_pic_order_cnt_msb;
+        }
+
+        // 8-4, 8-5.
+        int32_t top_poc = pic_order_cnt_msb + slice_hdr.pic_order_cnt_lsb;
+        int32_t bottom_poc = top_poc + slice_hdr.delta_pic_order_cnt_bottom;
+        *pic_order_cnt = std::min(top_poc, bottom_poc);
+
+        // Store state.
+        prev_frame_num_ = slice_hdr.frame_num;
+        if (slice_hdr.nal_ref_idc != 0) {
+          if (mmco5) {
+            ref_pic_order_cnt_msb_ = 0;
+            ref_pic_order_cnt_lsb_ = top_poc;
+          } else {
+            ref_pic_order_cnt_msb_ = pic_order_cnt_msb;
+            ref_pic_order_cnt_lsb_ = slice_hdr.pic_order_cnt_lsb;
+          }
+        }
+
+        break;
+      }
+
+    case 1:
+      {
+        // AVC 8.2.1.2.

[DaleCurtis, 2014/11/20 23:03:30]

Again, this is all gibberish to me, so you'll need to include some summary
comments and possibly a link to the spec at the top of the file if possible.

[sandersd (OOO until July 31), 2014/11/20 23:58:24]

Acknowledged.

+        // 8-6.
+        int32_t frame_num_offset;
+        if (slice_hdr.idr_pic_flag)
+          frame_num_offset = 0;
+        else if (prev_frame_num_ > slice_hdr.frame_num)
+          frame_num_offset = prev_frame_num_offset_ + max_frame_num;
+        else
+          frame_num_offset = prev_frame_num_offset_;
+
+        // 8-7.
+        int32_t abs_frame_num;
+        if (sps->num_ref_frames_in_pic_order_cnt_cycle != 0)
+          abs_frame_num = frame_num_offset + slice_hdr.frame_num;
+        else
+          abs_frame_num = 0;
+
+        if (slice_hdr.nal_ref_idc == 0 && abs_frame_num > 0)
+          abs_frame_num--;
+
+        // 8-9.
+        int32_t expected_pic_order_cnt = 0;
+        if (abs_frame_num > 0) {
+          // 8-8.
+          if (sps->num_ref_frames_in_pic_order_cnt_cycle == 0) {
+            DLOG(ERROR) << "Invalid num_ref_frames_in_pic_order_cnt_cycle";
+            return false;
+          }
+
+          int32_t pic_order_cnt_cycle_cnt =
+              (abs_frame_num - 1) / sps->num_ref_frames_in_pic_order_cnt_cycle;
+          int32_t frame_num_in_pic_order_cnt_cycle =
+              (abs_frame_num - 1) % sps->num_ref_frames_in_pic_order_cnt_cycle;
+          expected_pic_order_cnt = pic_order_cnt_cycle_cnt *
+                                   sps->expected_delta_per_pic_order_cnt_cycle;
+
+          // H264Parser checks that num_ref_frames_in_pic_order_cnt_cycle < 255.
+          for (int32_t i = 0; i <= frame_num_in_pic_order_cnt_cycle; i++)
+            expected_pic_order_cnt += sps->offset_for_ref_frame[i];
+        }
+        if (slice_hdr.nal_ref_idc == 0)
+          expected_pic_order_cnt += sps->offset_for_non_ref_pic;
+
+        // 8-10.
+        int32_t top_poc = expected_pic_order_cnt +
+                          slice_hdr.delta_pic_order_cnt[0];
+        int32_t bottom_poc = top_poc + sps->offset_for_top_to_bottom_field +
+                             slice_hdr.delta_pic_order_cnt[1];
+        *pic_order_cnt = std::min(top_poc, bottom_poc);
+
+        // Store state.
+        prev_frame_num_ = slice_hdr.frame_num;
+        prev_frame_num_offset_ = frame_num_offset;
+        if (mmco5)
+          prev_frame_num_offset_ = 0;
+
+        break;
+      }
+
+    case 2:
+      {
+        // AVC 8.2.1.3.
+        // 8-11.
+        int32_t frame_num_offset;
+        if (slice_hdr.idr_pic_flag)
+          frame_num_offset = 0;
+        else if (prev_frame_num_ > slice_hdr.frame_num)
+          frame_num_offset = prev_frame_num_offset_ + max_frame_num;
+        else
+          frame_num_offset = prev_frame_num_offset_;
+
+        // 8-12, 8-13.
+        if (slice_hdr.idr_pic_flag)
+          *pic_order_cnt = 0;
+        else if (slice_hdr.nal_ref_idc == 0)
+          *pic_order_cnt = 2 * (frame_num_offset + slice_hdr.frame_num) - 1;
+        else
+          *pic_order_cnt = 2 * (frame_num_offset + slice_hdr.frame_num);
+
+        // Store state.
+        prev_frame_num_ = slice_hdr.frame_num;
+        prev_frame_num_offset_ = frame_num_offset;
+        if (mmco5)
+          prev_frame_num_offset_ = 0;
+
+        break;
+      }
+
+    default:
+      DLOG(ERROR) << "Invalid pic_order_cnt_type: " << sps->pic_order_cnt_type;
+      return false;
+  }
+
+  return true;
+}
+
+}  // namespace content