H264 Decoder: move cut off logic from Construct to ModifyRefPicList().

content/common/gpu/media/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 344 matching lines @@
   size_t num_short_refs = ref_pic_list_p0_.size();
 
   // and sort them to get [1].
   std::sort(ref_pic_list_p0_.begin(), ref_pic_list_p0_.end(),
             PicNumDescCompare());
 
   // Now get long term pics and sort them by long_term_pic_num to get [2].
   dpb_.GetLongTermRefPicsAppending(&ref_pic_list_p0_);
   std::sort(ref_pic_list_p0_.begin() + num_short_refs, ref_pic_list_p0_.end(),
             LongTermPicNumAscCompare());
-
-  // Cut off if we have more than requested in slice header.
-  ref_pic_list_p0_.resize(slice_hdr->num_ref_idx_l0_active_minus1 + 1);
 }
 
 struct POCAscCompare {
   bool operator()(const scoped_refptr<H264Picture>& a,
                   const scoped_refptr<H264Picture>& b) const {
     return a->pic_order_cnt < b->pic_order_cnt;
   }
 };
 
 struct POCDescCompare {
@@ skipping 51 matching lines @@
   // Now add [3] and sort by ascending long_term_pic_num
   dpb_.GetShortTermRefPicsAppending(&ref_pic_list_b1_);
   std::sort(ref_pic_list_b1_.begin() + num_short_refs, ref_pic_list_b1_.end(),
             LongTermPicNumAscCompare());
 
   // If lists identical, swap first two entries in RefPicList1 (spec 8.2.4.2.3)
   if (ref_pic_list_b1_.size() > 1 &&
       std::equal(ref_pic_list_b0_.begin(), ref_pic_list_b0_.end(),
                  ref_pic_list_b1_.begin()))
     std::swap(ref_pic_list_b1_[0], ref_pic_list_b1_[1]);
-
-  // Per 8.2.4.2 it's possible for num_ref_idx_lX_active_minus1 to indicate
-  // there should be more ref pics on list than we constructed.
-  // Those superfluous ones should be treated as non-reference.
-  ref_pic_list_b0_.resize(slice_hdr->num_ref_idx_l0_active_minus1 + 1);
-  ref_pic_list_b1_.resize(slice_hdr->num_ref_idx_l1_active_minus1 + 1);
 }
 
 // See 8.2.4
 int H264Decoder::PicNumF(const scoped_refptr<H264Picture>& pic) {
   if (!pic)
     return -1;
 
   if (!pic->long_term)
     return pic->pic_num;
   else
@@ skipping 30 matching lines @@
 
   (*v)[from] = pic;
 }
 
 bool H264Decoder::ModifyReferencePicList(media::H264SliceHeader* slice_hdr,
                                          int list,
                                          H264Picture::Vector* ref_pic_listx) {
   int num_ref_idx_lX_active_minus1;
   media::H264ModificationOfPicNum* list_mod;
 
+  // Cut off if we have more than requested in slice header.

[Pawel Osciak, 2015/09/02 03:34:58]

Hm, this should probably say resize instead of cut off...

How about:

Resize the list to the size requested in the slice header.
Note that per 8.2.4.2 it's possible for num_ref_idx_lX_active_minus1 to
indicate there should be more ref pics on list than we constructed.
Those superfluous ones should be treated as non-reference and will be
initialized to nullptr, which must be handled by clients.

[hshi1, 2015/09/02 03:49:25]

Done.

+  // Note that per 8.2.4.2 it's possible for num_ref_idx_lX_active_minus1 to
+  // indicate there should be more ref pics on list than we constructed.
+  // Those superfluous ones should be treated as non-reference.
+  num_ref_idx_lX_active_minus1 = list == 0
+                                     ? slice_hdr->num_ref_idx_l0_active_minus1
+                                     : slice_hdr->num_ref_idx_l1_active_minus1;
+  ref_pic_listx->resize(num_ref_idx_lX_active_minus1 + 1);
+

[Pawel Osciak, 2015/09/02 03:34:58]

I'm on the fence here, so completely up to you, but what would you think about:

  bool ref_pic_list_modification_flag_lX;
  size_t num_ref_idx_lX_active_minus1;

  if (list == 0) {
    ref_pic_list_modification_flag_lX =
        slice_hdr->ref_pic_list_modification_flag_l0;
    num_ref_idx_lX_active_minus1 =
        slice_hdr->num_ref_idx_l0_active_minus1;
    list_mod = slice_hdr->ref_list_l0_modifications;
  } else {
    ref_pic_list_modification_flag_lX =
        slice_hdr->ref_pic_list_modification_flag_l1;
    num_ref_idx_lX_active_minus1 =
        slice_hdr->num_ref_idx_l1_active_minus1;
    list_mod = slice_hdr->ref_list_l1_modifications;
  }

  // Comment...
  ref_pic_listx->resize(num_ref_idx_lX_active_minus1 + 1);

  if (!ref_pic_list_modification_flag_lX)
    return true;

[hshi1, 2015/09/02 03:49:25]

Done.

   // This can process either ref_pic_list0 or ref_pic_list1, depending on
   // the list argument. Set up pointers to proper list to be processed here.
   if (list == 0) {
     if (!slice_hdr->ref_pic_list_modification_flag_l0)
       return true;
 
     list_mod = slice_hdr->ref_list_l0_modifications;
   } else {
     if (!slice_hdr->ref_pic_list_modification_flag_l1)
       return true;
 
     list_mod = slice_hdr->ref_list_l1_modifications;
   }
 
-  num_ref_idx_lX_active_minus1 = ref_pic_listx->size() - 1;
-  DCHECK_GE(num_ref_idx_lX_active_minus1, 0);
-
   // Spec 8.2.4.3:
   // Reorder pictures on the list in a way specified in the stream.
   int pic_num_lx_pred = curr_pic_->pic_num;
   int ref_idx_lx = 0;
   int pic_num_lx_no_wrap;
   int pic_num_lx;
   bool done = false;
   scoped_refptr<H264Picture> pic;
   for (int i = 0; i < media::H264SliceHeader::kRefListModSize && !done; ++i) {
     switch (list_mod->modification_of_pic_nums_idc) {
@@ skipping 761 matching lines @@
 
 gfx::Size H264Decoder::GetPicSize() const {
   return pic_size_;
 }
 
 size_t H264Decoder::GetRequiredNumOfPictures() const {
   return dpb_.max_num_pics() + kPicsInPipeline;
 }
 
 }  // namespace content