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..7cb00f6868477323ae4c393c5f549d347a416ea0 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
@@ -499,11 +490,17 @@ bool H264Decoder::ModifyReferencePicList(media::H264SliceHeader* slice_hdr,
       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;