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 37 matching lines @@
   int num_ref_idx_lX_active_minus1;
   media::H264ModificationOfPicNum* list_mod;
 
   // 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;
+
+    // Cut off if we have more than requested in slice header.

[Pawel Osciak, 2015/09/01 11:00:36]

It would be nice to keep the comment from l.440 and change this slightly.

It's crucial to say here that it is possible and correct to actually not cut
off, but enlarge the list with this. The new entries would be empty, i.e.
nullptr. If it wasn't allowed, we would have to check that we are not actually
enlarging here. And of course nullptr entries on this list are handled in the
clients of this class.

[hshi1, 2015/09/01 13:16:55]

I see. Would the verbatim comment from l.440 (slightly reworded here) suffice,
or would you prefer it to be more verbose and elaborate on the point of
possibility of enlarging the list (and the fact that nullptr entries should not
result in a CHECK failure)?

[hshi1, 2015/09/01 17:17:10]

Done.

+    ref_pic_listx->resize(slice_hdr->num_ref_idx_l0_active_minus1 + 1);

[Pawel Osciak, 2015/09/01 11:00:36]

Per spec we should be trimming the list regardless of
ref_pic_list_modification_flag_lX, so this should happen before l.489.

Similarly for the other list below.

[hshi1, 2015/09/01 17:17:10]

Done.

   } else {
     if (!slice_hdr->ref_pic_list_modification_flag_l1)
       return true;
 
     list_mod = slice_hdr->ref_list_l1_modifications;
+
+    // Cut off if we have more than requested in slice header.
+    ref_pic_listx->resize(slice_hdr->num_ref_idx_l1_active_minus1 + 1);
   }
 
   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;
@@ skipping 766 matching lines @@
 
 gfx::Size H264Decoder::GetPicSize() const {
   return pic_size_;
 }
 
 size_t H264Decoder::GetRequiredNumOfPictures() const {
   return dpb_.max_num_pics() + kPicsInPipeline;
 }
 
 }  // namespace content