diff --git a/media/gpu/h264_decoder_unittest.cc b/media/gpu/h264_decoder_unittest.cc index d4b8b8fe77b24..623fdf02f9cea 100644 --- a/media/gpu/h264_decoder_unittest.cc +++ b/media/gpu/h264_decoder_unittest.cc @@ -809,6 +809,86 @@ TEST_F(H264DecoderTest, SetStreamRetry) { ASSERT_TRUE(decoder_->Flush()); } +// POC: Unvalidated first_mb_in_slice in subsequent slices reaches the +// accelerator. +// +// H264Decoder::PreprocessCurrentSlice() only validates first_mb_in_slice==0 +// when IsNewPrimaryCodedPicture() returns true. For the second (and later) +// slice of a multi-slice picture, IsNewPrimaryCodedPicture() returns false +// and the value is forwarded to accelerator_->SubmitSlice() with no upper +// bound check. The H.264 spec (7.4.3) requires first_mb_in_slice < +// PicSizeInMbs; the H.265 parser enforces the equivalent +// (h265_parser.cc:1148), but the H.264 path does not. +// +// On the VA-API backend the value is copied verbatim into +// VASliceParameterBufferH264.first_mb_in_slice and handed to the userspace +// GPU driver, which uses it as a macroblock-row write offset into the decode +// surface (GPU process, system memory on integrated GPUs). +// +// This test feeds a hand-crafted Annex-B stream: +// SPS : 320x240 (PicSizeInMbs = 20*15 = 300) +// PPS +// IDR slice 0 : first_mb_in_slice = 0 (passes the new-picture check) +// IDR slice 1 : first_mb_in_slice = 65535 (same frame_num/idr_pic_id -> +// IsNewPrimaryCodedPicture()=false) +// and asserts that SubmitSlice() is invoked with first_mb_in_slice == 65535, +// i.e. 218x larger than PicSizeInMbs, with no rejection by H264Decoder. +TEST_F(H264DecoderTest, UnvalidatedFirstMbInSliceReachesAccelerator) { + // Generated by gen_bitstream.py. + static const uint8_t kStream[] = { + 0x00, 0x00, 0x00, 0x01, 0x67, 0x42, 0x00, 0x1e, 0xda, 0x05, 0x07, 0xe4, + 0x00, 0x00, 0x00, 0x01, 0x68, 0xce, 0x38, 0x80, 0x00, 0x00, 0x00, 0x01, + 0x65, 0x88, 0x84, 0xd5, 0x55, 0x40, 0x00, 0x00, 0x00, 0x01, 0x65, 0x00, + 0x00, 0x80, 0x00, 0x08, 0x84, 0xd5, 0x55, 0x40, + }; + + scoped_refptr buf = + DecoderBuffer::CopyFrom(base::span(kStream)); + decoder_->SetStream(0, buf); + + // First Decode() processes the SPS and reports a config change. + ASSERT_EQ(AcceleratedVideoDecoder::kConfigChange, decoder_->Decode()); + EXPECT_EQ(gfx::Size(320, 240), decoder_->GetPicSize()); + + // PicSizeInMbs for the configured SPS. + constexpr int kPicSizeInMbs = (320 / 16) * (240 / 16); // = 300 + + std::vector observed_first_mb; + EXPECT_CALL(*accelerator_, SubmitSlice(_, _, _, _, _, _, _, _)) + .WillRepeatedly( + [&](const H264PPS*, const H264SliceHeader* slice_hdr, + const H264Picture::Vector&, const H264Picture::Vector&, + scoped_refptr, const uint8_t*, size_t, + const std::vector&) { + observed_first_mb.push_back(slice_hdr->first_mb_in_slice); + return H264Decoder::H264Accelerator::Status::kOk; + }); + + // Second Decode() processes PPS + both IDR slices of the same picture. + ASSERT_EQ(AcceleratedVideoDecoder::kRanOutOfStreamData, decoder_->Decode()); + ASSERT_TRUE(decoder_->Flush()); + + ASSERT_EQ(observed_first_mb.size(), 2u) + << "Expected both slices to reach SubmitSlice()"; + EXPECT_EQ(observed_first_mb[0], 0); + + // *** THE BUG *** + // 65535 >> PicSizeInMbs (300). H264Decoder performed no range check before + // forwarding it to the hardware accelerator. + EXPECT_EQ(observed_first_mb[1], 65535); + EXPECT_GT(observed_first_mb[1], kPicSizeInMbs) + << "first_mb_in_slice should have been rejected as >= PicSizeInMbs=" + << kPicSizeInMbs << " but reached SubmitSlice() unvalidated"; + + fprintf(stderr, + "\n[POC] H264Decoder forwarded first_mb_in_slice=%d to " + "accelerator->SubmitSlice() for a %d-macroblock picture " + "(legal max %d). On VA-API this becomes " + "VASliceParameterBufferH264.first_mb_in_slice and is used by the " + "GPU driver as a write offset into the decode surface.\n\n", + observed_first_mb[1], kPicSizeInMbs, kPicSizeInMbs - 1); +} + TEST_F(H264DecoderTest, ModifyReferencePicList_CompactionTime) { // Seed the decoder DPB and `curr_pic_` with a long term ref pic. scoped_refptr ref_pic = base::MakeRefCounted();