Use base::Optional<int32_t> in H264POC::ComputePicOrderCnt(...)

This CL adds base::Optional<int32_t> return type for H264POC::ComputePicOrderCnt(...).

BUG=None
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2902693002
Cr-Commit-Position: refs/heads/master@{#473993}

media/gpu/vt_video_decode_accelerator_mac.cc

@@ -622,8 +622,9 @@ void VTVideoDecodeAccelerator::DecodeTask(const BitstreamBuffer& bitstream,
 
           // Compute and store frame properties. |image_size| gets filled in
           // later, since it comes from the decoder configuration.
-          int32_t pic_order_cnt;
-          if (!poc_.ComputePicOrderCnt(sps, slice_hdr, &pic_order_cnt)) {
+          base::Optional<int32_t> pic_order_cnt =
+              poc_.ComputePicOrderCnt(sps, slice_hdr);
+          if (!pic_order_cnt.has_value()) {
             DLOG(ERROR) << "Unable to compute POC";
             NotifyError(UNREADABLE_INPUT, SFT_INVALID_STREAM);
             return;
@@ -632,7 +633,7 @@ void VTVideoDecodeAccelerator::DecodeTask(const BitstreamBuffer& bitstream,
           frame->has_slice = true;
           frame->is_idr = nalu.nal_unit_type == media::H264NALU::kIDRSlice;
           frame->has_mmco5 = poc_.IsPendingMMCO5();
-          frame->pic_order_cnt = pic_order_cnt;
+          frame->pic_order_cnt = *pic_order_cnt;
           frame->reorder_window = ComputeReorderWindow(sps);
         }
 

media/gpu/vt_video_decode_accelerator_mac.h

@@ -157,12 +157,6 @@ class VTVideoDecodeAccelerator : public VideoDecodeAccelerator,
   // Methods for interacting with VideoToolbox. Run on |decoder_thread_|.
   //
 
-  // Compute the |pic_order_cnt| for a frame. Returns true or calls
-  // NotifyError() before returning false.
-  bool ComputePicOrderCnt(const H264SPS* sps,
-                          const H264SliceHeader& slice_hdr,
-                          Frame* frame);
-
   // Set up VideoToolbox using the current SPS and PPS. Returns true or calls
   // NotifyError() before returning false.
   bool ConfigureDecoder();

media/video/h264_poc.cc

@@ -57,15 +57,15 @@ void H264POC::Reset() {
   pending_mmco5_ = false;
 }
 
-bool H264POC::ComputePicOrderCnt(
+base::Optional<int32_t> H264POC::ComputePicOrderCnt(
     const H264SPS* sps,
-    const H264SliceHeader& slice_hdr,
-    int32_t *pic_order_cnt) {
+    const H264SliceHeader& slice_hdr) {
   if (slice_hdr.field_pic_flag) {
     DLOG(ERROR) << "Interlaced frames are not supported";
-    return false;
+    return base::nullopt;
   }
 
+  int32_t pic_order_cnt = 0;
   bool mmco5 = HasMMCO5(slice_hdr);
   int32_t max_frame_num = 1 << (sps->log2_max_frame_num_minus4 + 4);
   int32_t max_pic_order_cnt_lsb =
@@ -117,9 +117,9 @@ bool H264POC::ComputePicOrderCnt(
       // change to 0 after decoding; we change it immediately and set the
       // |pending_mmco5_| flag.
       if (mmco5)
-        *pic_order_cnt = 0;
+        pic_order_cnt = 0;
       else
-        *pic_order_cnt = std::min(top_foc, bottom_foc);
+        pic_order_cnt = std::min(top_foc, bottom_foc);
 
       // Store state.
       pending_mmco5_ = mmco5;
@@ -166,7 +166,7 @@ bool H264POC::ComputePicOrderCnt(
         // Moved inside 8-9 to avoid division when this check is not done.
         if (sps->num_ref_frames_in_pic_order_cnt_cycle == 0) {
           DLOG(ERROR) << "Invalid num_ref_frames_in_pic_order_cnt_cycle";
-          return false;
+          return base::nullopt;
         }
 
         // H264Parser checks that num_ref_frames_in_pic_order_cnt_cycle < 255.
@@ -192,9 +192,9 @@ bool H264POC::ComputePicOrderCnt(
 
       // Compute POC. MMCO5 handling is the same as |pic_order_cnt_type| == 0.
       if (mmco5)
-        *pic_order_cnt = 0;
+        pic_order_cnt = 0;
       else
-        *pic_order_cnt = std::min(top_foc, bottom_foc);
+        pic_order_cnt = std::min(top_foc, bottom_foc);
 
       // Store state.
       pending_mmco5_ = mmco5;
@@ -229,9 +229,9 @@ bool H264POC::ComputePicOrderCnt(
 
       // Compute POC. MMCO5 handling is the same as |pic_order_cnt_type| == 0.
       if (mmco5)
-        *pic_order_cnt = 0;
+        pic_order_cnt = 0;
       else
-        *pic_order_cnt = temp_pic_order_count;
+        pic_order_cnt = temp_pic_order_count;
 
       // Store state.
       pending_mmco5_ = mmco5;
@@ -246,10 +246,10 @@ bool H264POC::ComputePicOrderCnt(
 
     default:
       DLOG(ERROR) << "Invalid pic_order_cnt_type: " << sps->pic_order_cnt_type;
-      return false;
+      return base::nullopt;
   }
 
-  return true;
+  return pic_order_cnt;
 }
 
 }  // namespace media

media/video/h264_poc.h

@@ -19,13 +19,9 @@ class MEDIA_EXPORT H264POC {
   H264POC();
   ~H264POC();
 
-  // Compute the picture order count for a slice, storing the result into
-  // |*pic_order_cnt|.
-  // TODO(sandersd): Switch to a base::Optional<int32_t> return type.
-  bool ComputePicOrderCnt(
-      const H264SPS* sps,
-      const H264SliceHeader& slice_hdr,
-      int32_t* pic_order_cnt);
+  // Returns the picture order count for a slice.
+  base::Optional<int32_t> ComputePicOrderCnt(const H264SPS* sps,
+                                             const H264SliceHeader& slice_hdr);
 
   // As specified, the POC of a frame with MMCO5 changes (to zero) after
   // decoding. We instead return 0 immediately, and flag that this has occurred

media/video/h264_poc_unittest.cc

@@ -22,16 +22,14 @@ class H264POCTest : public testing::Test {
   }
 
  protected:
-  bool ComputePOC() {
-    bool result = h264_poc_.ComputePicOrderCnt(&sps_, slice_hdr_, &poc_);
+  void ComputePOC() {
+    poc_ = h264_poc_.ComputePicOrderCnt(&sps_, slice_hdr_);
 
     // Clear MMCO5.
     slice_hdr_.adaptive_ref_pic_marking_mode_flag = false;
     slice_hdr_.ref_pic_marking[0].memory_mgmnt_control_operation = 0;
     slice_hdr_.ref_pic_marking[1].memory_mgmnt_control_operation = 0;
     slice_hdr_.ref_pic_marking[2].memory_mgmnt_control_operation = 0;
-
-    return result;
   }
 
   // Also sets as a reference frame and unsets IDR, which is required for
@@ -45,7 +43,7 @@ class H264POCTest : public testing::Test {
     slice_hdr_.ref_pic_marking[2].memory_mgmnt_control_operation = 0;
   }
 
-  int32_t poc_;
+  base::Optional<int32_t> poc_;
 
   H264SPS sps_;
   H264SliceHeader slice_hdr_;
@@ -61,29 +59,33 @@ TEST_F(H264POCTest, PicOrderCntType0) {
   // Initial IDR with POC 0.
   slice_hdr_.idr_pic_flag = true;
   slice_hdr_.frame_num = 0;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(0, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(0, *poc_);
 
   // Ref frame with POC lsb 8.
   slice_hdr_.idr_pic_flag = false;
   slice_hdr_.frame_num = 1;
   slice_hdr_.pic_order_cnt_lsb = 8;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(8, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(8, *poc_);
 
   // Ref frame with POC lsb 0. This should be detected as wrapping, as the
   // (negative) gap is at least half the maximum.
   slice_hdr_.pic_order_cnt_lsb = 0;
   slice_hdr_.frame_num = 2;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(16, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(16, *poc_);
 
   // Ref frame with POC lsb 9. This should be detected as negative wrapping,
   // as the (positive) gap is more than half the maximum.
   slice_hdr_.pic_order_cnt_lsb = 9;
   slice_hdr_.frame_num = 3;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(9, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(9, *poc_);
 }
 
 TEST_F(H264POCTest, PicOrderCntType0_WithMMCO5) {
@@ -93,38 +95,44 @@ TEST_F(H264POCTest, PicOrderCntType0_WithMMCO5) {
   // Initial IDR with POC 0.
   slice_hdr_.idr_pic_flag = true;
   slice_hdr_.frame_num = 0;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(0, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(0, *poc_);
 
   // Skip ahead.
   slice_hdr_.idr_pic_flag = false;
   slice_hdr_.frame_num = 1;
   slice_hdr_.pic_order_cnt_lsb = 8;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(8, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(8, *poc_);
 
   slice_hdr_.frame_num = 2;
   slice_hdr_.pic_order_cnt_lsb = 0;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(16, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(16, *poc_);
 
   slice_hdr_.frame_num = 3;
   slice_hdr_.pic_order_cnt_lsb = 8;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(24, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(24, *poc_);
 
   // MMCO5 resets to 0.
   slice_hdr_.frame_num = 4;
   slice_hdr_.pic_order_cnt_lsb = 0;
   SetMMCO5();
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(0, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(0, *poc_);
 
   // Still detected as positive wrapping.
   slice_hdr_.frame_num = 5;
   slice_hdr_.pic_order_cnt_lsb = 8;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(24, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(24, *poc_);
 }
 
 TEST_F(H264POCTest, PicOrderCntType1) {
@@ -138,34 +146,40 @@ TEST_F(H264POCTest, PicOrderCntType1) {
   // Initial IDR with POC 0.
   slice_hdr_.idr_pic_flag = true;
   slice_hdr_.frame_num = 0;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(0, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(0, *poc_);
 
   // Ref frame.
   slice_hdr_.idr_pic_flag = false;
   slice_hdr_.frame_num = 1;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(1, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(1, *poc_);
 
   // Ref frame.
   slice_hdr_.frame_num = 2;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(3, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(3, *poc_);
 
   // Ref frame.
   slice_hdr_.frame_num = 3;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(4, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(4, *poc_);
 
   // Ref frame.
   slice_hdr_.frame_num = 4;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(6, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(6, *poc_);
 
   // Ref frame, detected as wrapping (ie, this is frame 16).
   slice_hdr_.frame_num = 0;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(24, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(24, *poc_);
 }
 
 TEST_F(H264POCTest, PicOrderCntType1_WithMMCO5) {
@@ -179,25 +193,29 @@ TEST_F(H264POCTest, PicOrderCntType1_WithMMCO5) {
   // Initial IDR with POC 0.
   slice_hdr_.idr_pic_flag = true;
   slice_hdr_.frame_num = 0;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(0, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(0, *poc_);
 
   // Ref frame.
   slice_hdr_.idr_pic_flag = false;
   slice_hdr_.frame_num = 1;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(1, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(1, *poc_);
 
   // Ref frame, detected as wrapping.
   SetMMCO5();
   slice_hdr_.frame_num = 0;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(0, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(0, *poc_);
 
   // Ref frame, wrapping from before has been cleared.
   slice_hdr_.frame_num = 1;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(1, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(1, *poc_);
 }
 
 // |frame_num| values may be duplicated by non-reference frames.
@@ -212,21 +230,24 @@ TEST_F(H264POCTest, PicOrderCntType1_DupFrameNum) {
   // Initial IDR with POC 0.
   slice_hdr_.idr_pic_flag = true;
   slice_hdr_.frame_num = 0;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(0, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(0, *poc_);
 
   // Ref frame.
   slice_hdr_.idr_pic_flag = false;
   slice_hdr_.frame_num = 1;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(1, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(1, *poc_);
 
   // Duplicate |frame_num| frame.
   slice_hdr_.nal_ref_idc = 0;
   slice_hdr_.frame_num = 1;
   slice_hdr_.delta_pic_order_cnt0 = 2;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(2, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(2, *poc_);
 }
 
 TEST_F(H264POCTest, PicOrderCntType2) {
@@ -235,34 +256,40 @@ TEST_F(H264POCTest, PicOrderCntType2) {
   // Initial IDR with POC 0.
   slice_hdr_.idr_pic_flag = true;
   slice_hdr_.frame_num = 0;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(0, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(0, *poc_);
 
   // Ref frame.
   slice_hdr_.idr_pic_flag = false;
   slice_hdr_.frame_num = 1;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(2, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(2, *poc_);
 
   // Ref frame.
   slice_hdr_.frame_num = 2;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(4, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(4, *poc_);
 
   // Ref frame.
   slice_hdr_.frame_num = 3;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(6, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(6, *poc_);
 
   // Ref frame.
   slice_hdr_.frame_num = 4;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(8, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(8, *poc_);
 
   // Ref frame, detected as wrapping (ie, this is frame 16).
   slice_hdr_.frame_num = 0;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(32, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(32, *poc_);
 }
 
 TEST_F(H264POCTest, PicOrderCntType2_WithMMCO5) {
@@ -271,25 +298,29 @@ TEST_F(H264POCTest, PicOrderCntType2_WithMMCO5) {
   // Initial IDR with POC 0.
   slice_hdr_.idr_pic_flag = true;
   slice_hdr_.frame_num = 0;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(0, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(0, *poc_);
 
   // Ref frame.
   slice_hdr_.idr_pic_flag = false;
   slice_hdr_.frame_num = 1;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(2, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(2, *poc_);
 
   // Ref frame, detected as wrapping.
   SetMMCO5();
   slice_hdr_.frame_num = 0;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(0, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(0, *poc_);
 
   // Ref frame, wrapping from before has been cleared.
   slice_hdr_.frame_num = 1;
-  ASSERT_TRUE(ComputePOC());
-  ASSERT_EQ(2, poc_);
+  ComputePOC();
+  ASSERT_TRUE(poc_.has_value());
+  ASSERT_EQ(2, *poc_);
 }
 
 }  // namespace media