diff options
5 files changed, 83 insertions, 16 deletions
diff --git a/chromium/third_party/webrtc/common_video/h264/h264_bitstream_parser.cc b/chromium/third_party/webrtc/common_video/h264/h264_bitstream_parser.cc index d8f8a62378c..2e63b9d4a16 100644 --- a/chromium/third_party/webrtc/common_video/h264/h264_bitstream_parser.cc +++ b/chromium/third_party/webrtc/common_video/h264/h264_bitstream_parser.cc @@ -75,9 +75,9 @@ H264BitstreamParser::Result H264BitstreamParser::ParseNonParameterSetNalu( RETURN_INV_ON_FAIL(slice_reader.ReadBits(&bits_tmp, 2)); } // frame_num: u(v) - // Represented by log2_max_frame_num_minus4 + 4 bits. + // Represented by log2_max_frame_num bits. RETURN_INV_ON_FAIL( - slice_reader.ReadBits(&bits_tmp, sps_->log2_max_frame_num_minus4 + 4)); + slice_reader.ReadBits(&bits_tmp, sps_->log2_max_frame_num)); uint32_t field_pic_flag = 0; if (sps_->frame_mbs_only_flag == 0) { // field_pic_flag: u(1) @@ -92,10 +92,10 @@ H264BitstreamParser::Result H264BitstreamParser::ParseNonParameterSetNalu( RETURN_INV_ON_FAIL(slice_reader.ReadExponentialGolomb(&golomb_tmp)); } // pic_order_cnt_lsb: u(v) - // Represented by sps_.log2_max_pic_order_cnt_lsb_minus4 + 4 bits. + // Represented by sps_.log2_max_pic_order_cnt_lsb bits. if (sps_->pic_order_cnt_type == 0) { - RETURN_INV_ON_FAIL(slice_reader.ReadBits( - &bits_tmp, sps_->log2_max_pic_order_cnt_lsb_minus4 + 4)); + RETURN_INV_ON_FAIL( + slice_reader.ReadBits(&bits_tmp, sps_->log2_max_pic_order_cnt_lsb)); if (pps_->bottom_field_pic_order_in_frame_present_flag && field_pic_flag == 0) { // delta_pic_order_cnt_bottom: se(v) diff --git a/chromium/third_party/webrtc/common_video/h264/sps_parser.cc b/chromium/third_party/webrtc/common_video/h264/sps_parser.cc index b313f487883..647d5e01011 100644 --- a/chromium/third_party/webrtc/common_video/h264/sps_parser.cc +++ b/chromium/third_party/webrtc/common_video/h264/sps_parser.cc @@ -131,15 +131,30 @@ absl::optional<SpsParser::SpsState> SpsParser::ParseSpsUpToVui( } } } + // log2_max_frame_num and log2_max_pic_order_cnt_lsb are used with + // BitBuffer::ReadBits, which can read at most 32 bits at a time. We also have + // to avoid overflow when adding 4 to the on-wire golomb value, e.g., for evil + // input data, ReadExponentialGolomb might return 0xfffc. + const uint32_t kMaxLog2Minus4 = 32 - 4; + // log2_max_frame_num_minus4: ue(v) - RETURN_EMPTY_ON_FAIL( - buffer->ReadExponentialGolomb(&sps.log2_max_frame_num_minus4)); + uint32_t log2_max_frame_num_minus4; + if (!buffer->ReadExponentialGolomb(&log2_max_frame_num_minus4) || + log2_max_frame_num_minus4 > kMaxLog2Minus4) { + return OptionalSps(); + } + sps.log2_max_frame_num = log2_max_frame_num_minus4 + 4; + // pic_order_cnt_type: ue(v) RETURN_EMPTY_ON_FAIL(buffer->ReadExponentialGolomb(&sps.pic_order_cnt_type)); if (sps.pic_order_cnt_type == 0) { // log2_max_pic_order_cnt_lsb_minus4: ue(v) - RETURN_EMPTY_ON_FAIL( - buffer->ReadExponentialGolomb(&sps.log2_max_pic_order_cnt_lsb_minus4)); + uint32_t log2_max_pic_order_cnt_lsb_minus4; + if (!buffer->ReadExponentialGolomb(&log2_max_pic_order_cnt_lsb_minus4) || + log2_max_pic_order_cnt_lsb_minus4 > kMaxLog2Minus4) { + return OptionalSps(); + } + sps.log2_max_pic_order_cnt_lsb = log2_max_pic_order_cnt_lsb_minus4 + 4; } else if (sps.pic_order_cnt_type == 1) { // delta_pic_order_always_zero_flag: u(1) RETURN_EMPTY_ON_FAIL( diff --git a/chromium/third_party/webrtc/common_video/h264/sps_parser.h b/chromium/third_party/webrtc/common_video/h264/sps_parser.h index d4294b25589..000fcb1bfb5 100644 --- a/chromium/third_party/webrtc/common_video/h264/sps_parser.h +++ b/chromium/third_party/webrtc/common_video/h264/sps_parser.h @@ -32,8 +32,8 @@ class SpsParser { uint32_t delta_pic_order_always_zero_flag = 0; uint32_t separate_colour_plane_flag = 0; uint32_t frame_mbs_only_flag = 0; - uint32_t log2_max_frame_num_minus4 = 0; - uint32_t log2_max_pic_order_cnt_lsb_minus4 = 0; + uint32_t log2_max_frame_num = 4; // Smallest valid value. + uint32_t log2_max_pic_order_cnt_lsb = 4; // Smallest valid value. uint32_t pic_order_cnt_type = 0; uint32_t max_num_ref_frames = 0; uint32_t vui_params_present = 0; diff --git a/chromium/third_party/webrtc/common_video/h264/sps_parser_unittest.cc b/chromium/third_party/webrtc/common_video/h264/sps_parser_unittest.cc index 50227ed870a..dc1c7ba1343 100644 --- a/chromium/third_party/webrtc/common_video/h264/sps_parser_unittest.cc +++ b/chromium/third_party/webrtc/common_video/h264/sps_parser_unittest.cc @@ -43,6 +43,8 @@ static const size_t kSpsBufferMaxSize = 256; void GenerateFakeSps(uint16_t width, uint16_t height, int id, + uint32_t log2_max_frame_num_minus4, + uint32_t log2_max_pic_order_cnt_lsb_minus4, rtc::Buffer* out_buffer) { uint8_t rbsp[kSpsBufferMaxSize] = {0}; rtc::BitBufferWriter writer(rbsp, kSpsBufferMaxSize); @@ -57,12 +59,12 @@ void GenerateFakeSps(uint16_t width, // Profile is not special, so we skip all the chroma format settings. // Now some bit magic. - // log2_max_frame_num_minus4: ue(v). 0 is fine. - writer.WriteExponentialGolomb(0); + // log2_max_frame_num_minus4: ue(v). + writer.WriteExponentialGolomb(log2_max_frame_num_minus4); // pic_order_cnt_type: ue(v). 0 is the type we want. writer.WriteExponentialGolomb(0); // log2_max_pic_order_cnt_lsb_minus4: ue(v). 0 is fine. - writer.WriteExponentialGolomb(0); + writer.WriteExponentialGolomb(log2_max_pic_order_cnt_lsb_minus4); // max_num_ref_frames: ue(v). 0 is fine. writer.WriteExponentialGolomb(0); // gaps_in_frame_num_value_allowed_flag: u(1). @@ -104,9 +106,11 @@ void GenerateFakeSps(uint16_t width, byte_count++; } + out_buffer->Clear(); H264::WriteRbsp(rbsp, byte_count, out_buffer); } +// TODO(nisse): Delete test fixture. class H264SpsParserTest : public ::testing::Test { public: H264SpsParserTest() {} @@ -153,7 +157,7 @@ TEST_F(H264SpsParserTest, TestSampleSPSWeirdResolution) { TEST_F(H264SpsParserTest, TestSyntheticSPSQvgaLandscape) { rtc::Buffer buffer; - GenerateFakeSps(320u, 180u, 1, &buffer); + GenerateFakeSps(320u, 180u, 1, 0, 0, &buffer); EXPECT_TRUE(static_cast<bool>( sps_ = SpsParser::ParseSps(buffer.data(), buffer.size()))); EXPECT_EQ(320u, sps_->width); @@ -163,7 +167,7 @@ TEST_F(H264SpsParserTest, TestSyntheticSPSQvgaLandscape) { TEST_F(H264SpsParserTest, TestSyntheticSPSWeirdResolution) { rtc::Buffer buffer; - GenerateFakeSps(156u, 122u, 2, &buffer); + GenerateFakeSps(156u, 122u, 2, 0, 0, &buffer); EXPECT_TRUE(static_cast<bool>( sps_ = SpsParser::ParseSps(buffer.data(), buffer.size()))); EXPECT_EQ(156u, sps_->width); @@ -184,4 +188,48 @@ TEST_F(H264SpsParserTest, TestSampleSPSWithScalingLists) { EXPECT_EQ(1080u, sps_->height); } +TEST_F(H264SpsParserTest, TestLog2MaxFrameNumMinus4) { + rtc::Buffer buffer; + GenerateFakeSps(320u, 180u, 1, 0, 0, &buffer); + EXPECT_TRUE(static_cast<bool>( + sps_ = SpsParser::ParseSps(buffer.data(), buffer.size()))); + EXPECT_EQ(320u, sps_->width); + EXPECT_EQ(180u, sps_->height); + EXPECT_EQ(1u, sps_->id); + EXPECT_EQ(4u, sps_->log2_max_frame_num); + + GenerateFakeSps(320u, 180u, 1, 28, 0, &buffer); + EXPECT_TRUE(static_cast<bool>( + sps_ = SpsParser::ParseSps(buffer.data(), buffer.size()))); + EXPECT_EQ(320u, sps_->width); + EXPECT_EQ(180u, sps_->height); + EXPECT_EQ(1u, sps_->id); + EXPECT_EQ(32u, sps_->log2_max_frame_num); + + GenerateFakeSps(320u, 180u, 1, 29, 0, &buffer); + EXPECT_FALSE(SpsParser::ParseSps(buffer.data(), buffer.size())); +} + +TEST_F(H264SpsParserTest, TestLog2MaxPicOrderCntMinus4) { + rtc::Buffer buffer; + GenerateFakeSps(320u, 180u, 1, 0, 0, &buffer); + EXPECT_TRUE(static_cast<bool>( + sps_ = SpsParser::ParseSps(buffer.data(), buffer.size()))); + EXPECT_EQ(320u, sps_->width); + EXPECT_EQ(180u, sps_->height); + EXPECT_EQ(1u, sps_->id); + EXPECT_EQ(4u, sps_->log2_max_pic_order_cnt_lsb); + + GenerateFakeSps(320u, 180u, 1, 0, 28, &buffer); + EXPECT_TRUE(static_cast<bool>( + sps_ = SpsParser::ParseSps(buffer.data(), buffer.size()))); + EXPECT_EQ(320u, sps_->width); + EXPECT_EQ(180u, sps_->height); + EXPECT_EQ(1u, sps_->id); + EXPECT_EQ(32u, sps_->log2_max_pic_order_cnt_lsb); + + GenerateFakeSps(320u, 180u, 1, 0, 29, &buffer); + EXPECT_FALSE(SpsParser::ParseSps(buffer.data(), buffer.size())); +} + } // namespace webrtc diff --git a/chromium/third_party/webrtc/rtc_base/bitbuffer.cc b/chromium/third_party/webrtc/rtc_base/bitbuffer.cc index 025cfe1516a..be72d558f1e 100644 --- a/chromium/third_party/webrtc/rtc_base/bitbuffer.cc +++ b/chromium/third_party/webrtc/rtc_base/bitbuffer.cc @@ -108,6 +108,10 @@ bool BitBuffer::ReadUInt32(uint32_t* val) { } bool BitBuffer::PeekBits(uint32_t* val, size_t bit_count) { + // TODO(nisse): Could allow bit_count == 0 and always return success. But + // current code reads one byte beyond end of buffer in the case that + // RemainingBitCount() == 0 and bit_count == 0. + RTC_DCHECK(bit_count > 0); if (!val || bit_count > RemainingBitCount() || bit_count > 32) { return false; } |