Fix dependent slice handling in H265 parser/decoder

This was not being handled correctly before. We now require passing back
in the prior decoded slice data so that dependent slice header parsing
will be correct. Previously we relied on the decoder to do this, but
even then we didn't handle all the fields properly. Fixes new fuzzer
case and removes need for prior fuzzer fix in this area.

This also fixes a range check issue where values needed to be in the 0
to 2^15 - 1 range for a few things that fixes another fuzzer case.

BUG=b:153111783,chrome:1149206,chrome:1148863,chrome:1149205
TEST=H265 playback still works, unit/fuzzer tests pass

Change-Id: I722dd5d2996685fd716f2218b3d9031a51fc4c79
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2542946
Commit-Queue: Jeffrey Kardatzke <jkardatzke@google.com>
Reviewed-by: Sergey Volk <servolk@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828369}

media/gpu/h265_decoder.cc

@@ -185,16 +185,8 @@ H265Decoder::DecodeResult H265Decoder::Decode() {
       case H265NALU::CRA_NUT:
         if (!curr_slice_hdr_) {
           curr_slice_hdr_.reset(new H265SliceHeader());
-          if (last_slice_hdr_) {
-            // This is a multi-slice picture, so we should copy all of the prior
-            // slice header data to the new slice and use those as the default
-            // values that don't have syntax elements present.
-            memcpy(curr_slice_hdr_.get(), last_slice_hdr_.get(),
-                   sizeof(H265SliceHeader));
-            last_slice_hdr_.reset();
-          }
-          par_res =
-              parser_.ParseSliceHeader(*curr_nalu_, curr_slice_hdr_.get());
+          par_res = parser_.ParseSliceHeader(*curr_nalu_, curr_slice_hdr_.get(),
+                                             last_slice_hdr_.get());
           if (par_res == H265Parser::kMissingParameterSet) {
             // We may still be able to recover if we skip until we find the
             // SPS/PPS.
@@ -387,16 +379,6 @@ H265Decoder::H265Accelerator::Status H265Decoder::PreprocessCurrentSlice() {
     DCHECK(!curr_pic_);
   }
 
-  // Validate that NumPicTotalCurr is non-zero for P/B slices. We do check this
-  // in the parser, but there's a way it can slip by when
-  // dependent_slice_segment_flag is set (and then we can't verify until we
-  // copy the defaults to the next slice in the decoder).
-  if ((slice_hdr->IsPSlice() || slice_hdr->IsBSlice()) &&
-      !slice_hdr->num_pic_total_curr) {
-    DVLOG(2) << "Zero valued NumPicTotalCurr for P/B slice";
-    return H265Accelerator::Status::kFail;
-  }
-
   return H265Accelerator::Status::kOk;
 }
 

media/video/h265_parser.cc

@@ -942,7 +942,8 @@ const H265PPS* H265Parser::GetPPS(int pps_id) const {
 }
 
 H265Parser::Result H265Parser::ParseSliceHeader(const H265NALU& nalu,
-                                                H265SliceHeader* shdr) {
+                                                H265SliceHeader* shdr,
+                                                H265SliceHeader* prior_shdr) {
   // 7.4.7 Slice segment header
   DVLOG(4) << "Parsing slice header";
   Result res = kOk;
@@ -969,20 +970,6 @@ H265Parser::Result H265Parser::ParseSliceHeader(const H265NALU& nalu,
   sps = GetSPS(pps->pps_seq_parameter_set_id);
   DCHECK(sps);  // We already validated this when we parsed the PPS.
 
-  // Set these defaults if they are not present here.
-  shdr->pic_output_flag = 1;
-  shdr->num_ref_idx_l0_active_minus1 =
-      pps->num_ref_idx_l0_default_active_minus1;
-  shdr->num_ref_idx_l1_active_minus1 =
-      pps->num_ref_idx_l1_default_active_minus1;
-  shdr->collocated_from_l0_flag = 1;
-  shdr->slice_deblocking_filter_disabled_flag =
-      pps->pps_deblocking_filter_disabled_flag;
-  shdr->slice_beta_offset_div2 = pps->pps_beta_offset_div2;
-  shdr->slice_tc_offset_div2 = pps->pps_tc_offset_div2;
-  shdr->slice_loop_filter_across_slices_enabled_flag =
-      pps->pps_loop_filter_across_slices_enabled_flag;
-
   if (!shdr->first_slice_segment_in_pic_flag) {
     if (pps->dependent_slice_segments_enabled_flag)
       READ_BOOL_OR_RETURN(&shdr->dependent_slice_segment_flag);
@@ -991,8 +978,33 @@ H265Parser::Result H265Parser::ParseSliceHeader(const H265NALU& nalu,
     IN_RANGE_OR_RETURN(shdr->slice_segment_address, 0,
                        sps->pic_size_in_ctbs_y - 1);
   }
-  shdr->curr_rps_idx = sps->num_short_term_ref_pic_sets;
-  if (!shdr->dependent_slice_segment_flag) {
+  if (shdr->dependent_slice_segment_flag) {
+    if (!prior_shdr) {
+      DVLOG(1) << "Cannot parse dependent slice w/out prior slice data";
+      return kInvalidStream;
+    }
+    // Copy everything in the structure starting at |slice_type| going forward.
+    // This is copying the dependent slice data that we do not parse below.
+    size_t skip_amount = offsetof(H265SliceHeader, slice_type);
+    memcpy(reinterpret_cast<uint8_t*>(shdr) + skip_amount,
+           reinterpret_cast<uint8_t*>(prior_shdr) + skip_amount,
+           sizeof(H265SliceHeader) - skip_amount);
+  } else {
+    // Set these defaults if they are not present here.
+    shdr->pic_output_flag = 1;
+    shdr->num_ref_idx_l0_active_minus1 =
+        pps->num_ref_idx_l0_default_active_minus1;
+    shdr->num_ref_idx_l1_active_minus1 =
+        pps->num_ref_idx_l1_default_active_minus1;
+    shdr->collocated_from_l0_flag = 1;
+    shdr->slice_deblocking_filter_disabled_flag =
+        pps->pps_deblocking_filter_disabled_flag;
+    shdr->slice_beta_offset_div2 = pps->pps_beta_offset_div2;
+    shdr->slice_tc_offset_div2 = pps->pps_tc_offset_div2;
+    shdr->slice_loop_filter_across_slices_enabled_flag =
+        pps->pps_loop_filter_across_slices_enabled_flag;
+    shdr->curr_rps_idx = sps->num_short_term_ref_pic_sets;
+
     // slice_reserved_flag
     SKIP_BITS_OR_RETURN(pps->num_extra_slice_header_bits);
     READ_UE_OR_RETURN(&shdr->slice_type);
@@ -1461,6 +1473,7 @@ H265Parser::Result H265Parser::ParseStRefPicSet(
     int abs_delta_rps_minus1;
     READ_BOOL_OR_RETURN(&delta_rps_sign);
     READ_UE_OR_RETURN(&abs_delta_rps_minus1);
+    IN_RANGE_OR_RETURN(abs_delta_rps_minus1, 0, 0x7FFF);
     int delta_rps = (1 - 2 * delta_rps_sign) * (abs_delta_rps_minus1 + 1);
     const H265StRefPicSet& ref_set = sps.st_ref_pic_set[ref_rps_idx];
     bool used_by_curr_pic_flag[kMaxShortTermRefPicSets];
@@ -1542,6 +1555,7 @@ H265Parser::Result H265Parser::ParseStRefPicSet(
     for (int i = 0; i < st_ref_pic_set->num_negative_pics; ++i) {
       int delta_poc_s0_minus1;
       READ_UE_OR_RETURN(&delta_poc_s0_minus1);
+      IN_RANGE_OR_RETURN(delta_poc_s0_minus1, 0, 0x7FFF);
       if (i == 0) {
         st_ref_pic_set->delta_poc_s0[i] = -(delta_poc_s0_minus1 + 1);
       } else {
@@ -1553,6 +1567,7 @@ H265Parser::Result H265Parser::ParseStRefPicSet(
     for (int i = 0; i < st_ref_pic_set->num_positive_pics; ++i) {
       int delta_poc_s1_minus1;
       READ_UE_OR_RETURN(&delta_poc_s1_minus1);
+      IN_RANGE_OR_RETURN(delta_poc_s1_minus1, 0, 0x7FFF);
       if (i == 0) {
         st_ref_pic_set->delta_poc_s1[i] = delta_poc_s1_minus1 + 1;
       } else {

media/video/h265_parser.h

@@ -479,7 +479,12 @@ class MEDIA_EXPORT H265Parser {
 
   // Parse a slice header, returning it in |*shdr|. |*nalu| must be set to
   // the NALU returned from AdvanceToNextNALU() and corresponding to |*shdr|.
-  Result ParseSliceHeader(const H265NALU& nalu, H265SliceHeader* shdr);
+  // |prior_shdr| should be the last parsed header in decoding order for
+  // handling dependent slice segments. If |prior_shdr| is null and this is a
+  // dependent slice segment, an error will be returned.
+  Result ParseSliceHeader(const H265NALU& nalu,
+                          H265SliceHeader* shdr,
+                          H265SliceHeader* prior_shdr);
 
   static VideoCodecProfile ProfileIDCToVideoCodecProfile(int profile_idc);
 

media/video/h265_parser_fuzzertest.cc

@@ -24,6 +24,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
       break;
 
     media::H265SliceHeader shdr;
+    media::H265SliceHeader prior_shdr;
     switch (nalu.nal_unit_type) {
       case media::H265NALU::SPS_NUT:
         int sps_id;
@@ -49,7 +50,8 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
       case media::H265NALU::IDR_W_RADL:
       case media::H265NALU::IDR_N_LP:
       case media::H265NALU::CRA_NUT:  // fallthrough
-        res = parser.ParseSliceHeader(nalu, &shdr);
+        res = parser.ParseSliceHeader(nalu, &shdr, &prior_shdr);
+        prior_shdr = shdr;
         break;
       default:
         // Skip any other NALU.

media/video/h265_parser_unittest.cc

@@ -79,6 +79,7 @@ TEST_F(H265ParserTest, RawHevcStreamFileParsing) {
       DVLOG(4) << "Found NALU " << nalu.nal_unit_type;
 
       H265SliceHeader shdr;
+      H265SliceHeader prior_shdr;
       switch (nalu.nal_unit_type) {
         case H265NALU::SPS_NUT:
           int sps_id;
@@ -106,7 +107,8 @@ TEST_F(H265ParserTest, RawHevcStreamFileParsing) {
         case H265NALU::IDR_W_RADL:
         case H265NALU::IDR_N_LP:
         case H265NALU::CRA_NUT:  // fallthrough
-          res = parser_.ParseSliceHeader(nalu, &shdr);
+          res = parser_.ParseSliceHeader(nalu, &shdr, &prior_shdr);
+          prior_shdr = shdr;
           break;
         default:
           break;
@@ -236,7 +238,8 @@ TEST_F(H265ParserTest, SliceHeaderParsing) {
   // Do an IDR slice header first.
   EXPECT_TRUE(ParseNalusUntilNut(&target_nalu, H265NALU::IDR_W_RADL));
   H265SliceHeader shdr;
-  EXPECT_EQ(H265Parser::kOk, parser_.ParseSliceHeader(target_nalu, &shdr));
+  EXPECT_EQ(H265Parser::kOk,
+            parser_.ParseSliceHeader(target_nalu, &shdr, nullptr));
   EXPECT_TRUE(shdr.first_slice_segment_in_pic_flag);
   EXPECT_FALSE(shdr.no_output_of_prior_pics_flag);
   EXPECT_EQ(shdr.slice_pic_parameter_set_id, 0);
@@ -249,7 +252,8 @@ TEST_F(H265ParserTest, SliceHeaderParsing) {
 
   // Then do a non-IDR slice header.
   EXPECT_TRUE(ParseNalusUntilNut(&target_nalu, H265NALU::TRAIL_R));
-  EXPECT_EQ(H265Parser::kOk, parser_.ParseSliceHeader(target_nalu, &shdr));
+  EXPECT_EQ(H265Parser::kOk,
+            parser_.ParseSliceHeader(target_nalu, &shdr, nullptr));
   EXPECT_TRUE(shdr.first_slice_segment_in_pic_flag);
   EXPECT_EQ(shdr.slice_pic_parameter_set_id, 0);
   EXPECT_FALSE(shdr.dependent_slice_segment_flag);