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

content/common/gpu/media/h264_decoder.cc

Index: content/common/gpu/media/h264_decoder.cc
diff --git a/content/common/gpu/media/h264_decoder.cc b/content/common/gpu/media/h264_decoder.cc
index 9953ff21a593d28c9508f51715091f8b10f81203..b9532bb8bda13a48127f89b9101140191f47da97 100644
--- a/content/common/gpu/media/h264_decoder.cc
+++ b/content/common/gpu/media/h264_decoder.cc
@@ -362,9 +362,6 @@ void H264Decoder::ConstructReferencePicListsP(
   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 {
@@ -436,12 +433,6 @@ void H264Decoder::ConstructReferencePicListsB(
       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
@@ -489,25 +480,35 @@ static void ShiftRightAndInsert(H264Picture::Vector* v,
 bool H264Decoder::ModifyReferencePicList(media::H264SliceHeader* slice_hdr,
                                          int list,
                                          H264Picture::Vector* ref_pic_listx) {
+  bool ref_pic_list_modification_flag_lX;
   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;
-
+    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 {
-    if (!slice_hdr->ref_pic_list_modification_flag_l1)
-      return true;
-
+    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;
   }
 
-  num_ref_idx_lX_active_minus1 = ref_pic_listx->size() - 1;
-  DCHECK_GE(num_ref_idx_lX_active_minus1, 0);

[Pawel Osciak, 2015/09/02 03:57:18]

Please keep the DCHECK just in case.

[hshi1, 2015/09/02 03:58:43]

Are you sure this is needed? Note that after this CL,
num_ref_idx_lX_active_minus1 value is from the slice header, where the value
type is unsigned char, so it can never be negative.

+  // 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.
+  ref_pic_listx->resize(num_ref_idx_lX_active_minus1 + 1);
+
+  if (!ref_pic_list_modification_flag_lX)
+    return true;
 
   // Spec 8.2.4.3:
   // Reorder pictures on the list in a way specified in the stream.