VTVDA: Optimize pic_order_cnt_type == 2.

media/video/h264_poc.cc

 // Copyright 2014 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 <stddef.h>
 
 #include <algorithm>
 
 #include "base/logging.h"
 #include "base/macros.h"
 #include "media/filters/h264_parser.h"
 #include "media/video/h264_poc.h"
 
 namespace media {
 
-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;
-}
+namespace {
 
 // Check if a slice includes memory management control operation 5, which
 // results in some |pic_order_cnt| state being cleared.
-static bool HasMMCO5(const media::H264SliceHeader& slice_hdr) {
+bool HasMMCO5(const media::H264SliceHeader& slice_hdr) {
   // Require that the frame actually has memory management control operations.
   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++) {
     int32_t op = slice_hdr.ref_pic_marking[i].memory_mgmnt_control_operation;
     if (op == 5)
       return true;
 
     // Stop at the end of the list.
     if (op == 0)
       return false;
   }
 
   // Should not get here, the list is always zero terminated.
   return false;
 }
 
+}  // namespace
+
+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;
+  pending_mmco5_ = false;
+}
+
 bool H264POC::ComputePicOrderCnt(
     const H264SPS* sps,
     const H264SliceHeader& slice_hdr,
     int32_t *pic_order_cnt) {

[Pawel Osciak, 2017/02/17 04:53:26]

On a side note, perhaps we should consider using base::Optional<> for return
value?

[sandersd (OOO until July 31), 2017/02/18 00:46:34]

Acknowledged.

   if (slice_hdr.field_pic_flag) {
     DLOG(ERROR) << "Interlaced frames are not supported";
     return false;
   }
 
   bool mmco5 = HasMMCO5(slice_hdr);

[Pawel Osciak, 2017/02/17 04:53:26]

Could we use pending_mmco5_ instead of mmco5 and remove the latter?

[sandersd (OOO until July 31), 2017/02/22 01:39:32]

Yes, it's the same. I did it this way so that member variable mutation only
occurs in the sections labelled 'Store state.'

   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);
 
-  // Note: Duplicate frame numbers are ignored. They occur in many videos

[Pawel Osciak, 2017/02/17 04:53:26]

Should this first note be removed as well?

[sandersd (OOO until July 31), 2017/02/18 00:46:34]

Yes, it was just plain wrong. (Duplicate frame numbers are allowed by the spec
for non-reference frames.)

-  // despite appearing to be invalid according to the spec.
-  // TODO(sandersd): Check if these videos are using slices or have redundant
-  // streams.
-
-  // Note: Gaps in frame numbers are also ignored. They do not affect POC
-  // computation.
-
   // Based on T-REC-H.264 8.2.1, "Decoding process for picture order
   // count", available from http://www.itu.int/rec/T-REC-H.264.
   //
   // Reorganized slightly from spec pseudocode to handle MMCO5 when storing
   // state instead of when loading it.
+  //
+  // Note: Gaps in frame numbers are ignored. They do not affect POC
+  // computation.
   switch (sps->pic_order_cnt_type) {
     case 0: {
       int32_t prev_pic_order_cnt_msb = ref_pic_order_cnt_msb_;
       int32_t prev_pic_order_cnt_lsb = ref_pic_order_cnt_lsb_;
 
       // For an IDR picture, clear the state.
       if (slice_hdr.idr_pic_flag) {
         prev_pic_order_cnt_msb = 0;
         prev_pic_order_cnt_lsb = 0;
       }
@@ skipping 11 matching lines @@
            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. Derive |top_field_order_count| and |bottom_field_order_cnt|
       //           (assuming no interlacing).
       int32_t top_foc = pic_order_cnt_msb + slice_hdr.pic_order_cnt_lsb;
       int32_t bottom_foc = top_foc + slice_hdr.delta_pic_order_cnt_bottom;
-      *pic_order_cnt = std::min(top_foc, bottom_foc);
+
+      // Compute POC.

[Pawel Osciak, 2017/02/17 04:53:26]

Perhaps we could replace this comment with an explanation why this is done
(possibly with a reference) please.

[sandersd (OOO until July 31), 2017/02/22 01:39:32]

Done. I went with something similar to what the header file says.

+      if (mmco5)
+        *pic_order_cnt = 0;
+      else
+        *pic_order_cnt = std::min(top_foc, bottom_foc);
 
       // Store state.
+      pending_mmco5_ = mmco5;
       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_foc;
         } else {
           ref_pic_order_cnt_msb_ = pic_order_cnt_msb;
           ref_pic_order_cnt_lsb_ = slice_hdr.pic_order_cnt_lsb;
         }
       }
@@ skipping 46 matching lines @@
           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. Derive |top_field_order_cnt| and |bottom_field_order_cnt|
       //       (assuming no interlacing).
       int32_t top_foc = expected_pic_order_cnt + slice_hdr.delta_pic_order_cnt0;
       int32_t bottom_foc = top_foc + sps->offset_for_top_to_bottom_field +
                            slice_hdr.delta_pic_order_cnt1;
-      *pic_order_cnt = std::min(top_foc, bottom_foc);
+
+      // Compute POC.
+      if (mmco5)
+        *pic_order_cnt = 0;
+      else
+        *pic_order_cnt = std::min(top_foc, bottom_foc);
 
       // Store state.
+      pending_mmco5_ = mmco5;
       prev_frame_num_ = slice_hdr.frame_num;
-      prev_frame_num_offset_ = frame_num_offset;
       if (mmco5)
         prev_frame_num_offset_ = 0;
+      else
+        prev_frame_num_offset_ = frame_num_offset;
 
       break;
     }
 
     case 2: {
       // 8-11. Derive |frame_num_offset|.
       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. Derive |temp_pic_order_count| (it's always the
       // |pic_order_cnt|, regardless of interlacing).
+      int32_t temp_pic_order_count;

[Pawel Osciak, 2017/02/17 04:53:26]

Perhaps we could say:

if (mmco5)
  *pic_order_cnt = 0;
else {
  // the whole if-else sequence below
}

?

[sandersd (OOO until July 31), 2017/02/18 00:46:34]

I wrote it that way originally, but ended up switching back for symmetry with
the other POC types.

I think it's more clear to break up the steps like this, as they are in the
spec, but the difference is trivial. I'll switch it if you ask again ;-).

       if (slice_hdr.idr_pic_flag)
+        temp_pic_order_count = 0;
+      else if (slice_hdr.nal_ref_idc == 0)
+        temp_pic_order_count = 2 * (frame_num_offset + slice_hdr.frame_num) - 1;
+      else
+        temp_pic_order_count = 2 * (frame_num_offset + slice_hdr.frame_num);
+
+      // Compute POC.
+      if (mmco5)
         *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);
+        *pic_order_cnt = temp_pic_order_count;
 
       // Store state.
+      pending_mmco5_ = mmco5;
       prev_frame_num_ = slice_hdr.frame_num;
-      prev_frame_num_offset_ = frame_num_offset;
       if (mmco5)
         prev_frame_num_offset_ = 0;
+      else
+        prev_frame_num_offset_ = frame_num_offset;
 
       break;
     }
 
     default:
       DLOG(ERROR) << "Invalid pic_order_cnt_type: " << sps->pic_order_cnt_type;
       return false;
   }
 
   return true;
 }
 
 }  // namespace media