From 5c35f2fb1bc2325f0cd23db52d73db75fccc89c9 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Thu, 9 Jan 2020 12:26:18 +0100 Subject: [PATCH] Delete RtpDepacketizerVp9 in favor of VideoRtpDepacketizerVp9 Bug: webrtc:11152 Change-Id: Ic50f2dc49ca420b3406d4dea11ed20328aa59136 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/165382 Commit-Queue: Danil Chapovalov Reviewed-by: Sam Zackrisson Reviewed-by: Ilya Nikolaevskiy Cr-Commit-Position: refs/heads/master@{#30195} --- modules/rtp_rtcp/source/rtp_format_vp9.cc | 13 - modules/rtp_rtcp/source/rtp_format_vp9.h | 9 - .../source/rtp_format_vp9_unittest.cc | 268 +----------------- test/fuzzers/BUILD.gn | 2 + test/fuzzers/vp9_depacketizer_fuzzer.cc | 10 +- video/video_send_stream_tests.cc | 16 +- 6 files changed, 25 insertions(+), 293 deletions(-) diff --git a/modules/rtp_rtcp/source/rtp_format_vp9.cc b/modules/rtp_rtcp/source/rtp_format_vp9.cc index 0094075147..57ac44712c 100644 --- a/modules/rtp_rtcp/source/rtp_format_vp9.cc +++ b/modules/rtp_rtcp/source/rtp_format_vp9.cc @@ -421,17 +421,4 @@ bool RtpPacketizerVp9::WriteHeader(bool layer_begin, return true; } -bool RtpDepacketizerVp9::Parse(ParsedPayload* parsed_payload, - const uint8_t* payload, - size_t payload_length) { - RTC_DCHECK(parsed_payload); - int offset = VideoRtpDepacketizerVp9::ParseRtpPayload( - rtc::MakeArrayView(payload, payload_length), &parsed_payload->video); - if (offset == 0) - return false; - - parsed_payload->payload = payload + offset; - parsed_payload->payload_length = payload_length - offset; - return true; -} } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtp_format_vp9.h b/modules/rtp_rtcp/source/rtp_format_vp9.h index 3c4ae76df2..5e2d52a3c7 100644 --- a/modules/rtp_rtcp/source/rtp_format_vp9.h +++ b/modules/rtp_rtcp/source/rtp_format_vp9.h @@ -68,14 +68,5 @@ class RtpPacketizerVp9 : public RtpPacketizer { RTC_DISALLOW_COPY_AND_ASSIGN(RtpPacketizerVp9); }; -class RtpDepacketizerVp9 : public RtpDepacketizer { - public: - ~RtpDepacketizerVp9() override = default; - - bool Parse(ParsedPayload* parsed_payload, - const uint8_t* payload, - size_t payload_length) override; -}; - } // namespace webrtc #endif // MODULES_RTP_RTCP_SOURCE_RTP_FORMAT_VP9_H_ diff --git a/modules/rtp_rtcp/source/rtp_format_vp9_unittest.cc b/modules/rtp_rtcp/source/rtp_format_vp9_unittest.cc index 0a738ed23d..7fd5135a79 100644 --- a/modules/rtp_rtcp/source/rtp_format_vp9_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_format_vp9_unittest.cc @@ -15,6 +15,7 @@ #include "api/array_view.h" #include "modules/rtp_rtcp/source/rtp_packet_to_send.h" +#include "modules/rtp_rtcp/source/video_rtp_depacketizer_vp9.h" #include "test/gmock.h" #include "test/gtest.h" @@ -66,31 +67,19 @@ void VerifyHeader(const RTPVideoHeaderVP9& expected, } } -void VerifyPayload(const RtpDepacketizer::ParsedPayload& parsed, - const uint8_t* payload, - size_t payload_length) { - EXPECT_EQ(payload, parsed.payload); - EXPECT_EQ(payload_length, parsed.payload_length); - EXPECT_THAT(std::vector(parsed.payload, - parsed.payload + parsed.payload_length), - ::testing::ElementsAreArray(payload, payload_length)); -} - void ParseAndCheckPacket(const uint8_t* packet, const RTPVideoHeaderVP9& expected, - size_t expected_hdr_length, + int expected_hdr_length, size_t expected_length) { - std::unique_ptr depacketizer(new RtpDepacketizerVp9()); - RtpDepacketizer::ParsedPayload parsed; - ASSERT_TRUE(depacketizer->Parse(&parsed, packet, expected_length)); - EXPECT_EQ(kVideoCodecVP9, parsed.video_header().codec); + RTPVideoHeader video_header; + EXPECT_EQ(VideoRtpDepacketizerVp9::ParseRtpPayload( + rtc::MakeArrayView(packet, expected_length), &video_header), + expected_hdr_length); + EXPECT_EQ(kVideoCodecVP9, video_header.codec); auto& vp9_header = - absl::get(parsed.video_header().video_type_header); + absl::get(video_header.video_type_header); VerifyHeader(expected, vp9_header); - const size_t kExpectedPayloadLength = expected_length - expected_hdr_length; - VerifyPayload(parsed, packet + expected_hdr_length, kExpectedPayloadLength); } -} // namespace // Payload descriptor for flexible mode // 0 1 2 3 4 5 6 7 @@ -557,244 +546,5 @@ TEST_F(RtpPacketizerVp9Test, TestNonRefForInterLayerPred) { CreateParseAndCheckPackets(kExpectedHdrSizes, kExpectedSizes); } -class RtpDepacketizerVp9Test : public ::testing::Test { - protected: - RtpDepacketizerVp9Test() : depacketizer_(new RtpDepacketizerVp9()) {} - - void SetUp() override { expected_.InitRTPVideoHeaderVP9(); } - - RTPVideoHeaderVP9 expected_; - std::unique_ptr depacketizer_; -}; - -TEST_F(RtpDepacketizerVp9Test, ParseBasicHeader) { - const uint8_t kHeaderLength = 1; - uint8_t packet[4] = {0}; - packet[0] = 0x0C; // I:0 P:0 L:0 F:0 B:1 E:1 V:0 Z:0 - expected_.beginning_of_frame = true; - expected_.end_of_frame = true; - ParseAndCheckPacket(packet, expected_, kHeaderLength, sizeof(packet)); -} - -TEST_F(RtpDepacketizerVp9Test, ParseOneBytePictureId) { - const uint8_t kHeaderLength = 2; - uint8_t packet[10] = {0}; - packet[0] = 0x80; // I:1 P:0 L:0 F:0 B:0 E:0 V:0 Z:0 - packet[1] = kMaxOneBytePictureId; - - expected_.picture_id = kMaxOneBytePictureId; - expected_.max_picture_id = kMaxOneBytePictureId; - ParseAndCheckPacket(packet, expected_, kHeaderLength, sizeof(packet)); -} - -TEST_F(RtpDepacketizerVp9Test, ParseTwoBytePictureId) { - const uint8_t kHeaderLength = 3; - uint8_t packet[10] = {0}; - packet[0] = 0x80; // I:1 P:0 L:0 F:0 B:0 E:0 V:0 Z:0 - packet[1] = 0x80 | ((kMaxTwoBytePictureId >> 8) & 0x7F); - packet[2] = kMaxTwoBytePictureId & 0xFF; - - expected_.picture_id = kMaxTwoBytePictureId; - expected_.max_picture_id = kMaxTwoBytePictureId; - ParseAndCheckPacket(packet, expected_, kHeaderLength, sizeof(packet)); -} - -TEST_F(RtpDepacketizerVp9Test, ParseLayerInfoWithNonFlexibleMode) { - const uint8_t kHeaderLength = 3; - const uint8_t kTemporalIdx = 2; - const uint8_t kUbit = 1; - const uint8_t kSpatialIdx = 1; - const uint8_t kDbit = 1; - const uint8_t kTl0PicIdx = 17; - uint8_t packet[13] = {0}; - packet[0] = 0x20; // I:0 P:0 L:1 F:0 B:0 E:0 V:0 Z:0 - packet[1] = (kTemporalIdx << 5) | (kUbit << 4) | (kSpatialIdx << 1) | kDbit; - packet[2] = kTl0PicIdx; - - // T:2 U:1 S:1 D:1 - // TL0PICIDX:17 - expected_.temporal_idx = kTemporalIdx; - expected_.temporal_up_switch = kUbit ? true : false; - expected_.spatial_idx = kSpatialIdx; - expected_.inter_layer_predicted = kDbit ? true : false; - expected_.tl0_pic_idx = kTl0PicIdx; - ParseAndCheckPacket(packet, expected_, kHeaderLength, sizeof(packet)); -} - -TEST_F(RtpDepacketizerVp9Test, ParseLayerInfoWithFlexibleMode) { - const uint8_t kHeaderLength = 2; - const uint8_t kTemporalIdx = 2; - const uint8_t kUbit = 1; - const uint8_t kSpatialIdx = 0; - const uint8_t kDbit = 0; - uint8_t packet[13] = {0}; - packet[0] = 0x38; // I:0 P:0 L:1 F:1 B:1 E:0 V:0 Z:0 - packet[1] = (kTemporalIdx << 5) | (kUbit << 4) | (kSpatialIdx << 1) | kDbit; - - // I:0 P:0 L:1 F:1 B:1 E:0 V:0 Z:0 - // L: T:2 U:1 S:0 D:0 - expected_.beginning_of_frame = true; - expected_.flexible_mode = true; - expected_.temporal_idx = kTemporalIdx; - expected_.temporal_up_switch = kUbit ? true : false; - expected_.spatial_idx = kSpatialIdx; - expected_.inter_layer_predicted = kDbit ? true : false; - ParseAndCheckPacket(packet, expected_, kHeaderLength, sizeof(packet)); -} - -TEST_F(RtpDepacketizerVp9Test, ParseRefIdx) { - const uint8_t kHeaderLength = 6; - const int16_t kPictureId = 17; - const uint8_t kPdiff1 = 17; - const uint8_t kPdiff2 = 18; - const uint8_t kPdiff3 = 127; - uint8_t packet[13] = {0}; - packet[0] = 0xD8; // I:1 P:1 L:0 F:1 B:1 E:0 V:0 Z:0 - packet[1] = 0x80 | ((kPictureId >> 8) & 0x7F); // Two byte pictureID. - packet[2] = kPictureId; - packet[3] = (kPdiff1 << 1) | 1; // P_DIFF N:1 - packet[4] = (kPdiff2 << 1) | 1; // P_DIFF N:1 - packet[5] = (kPdiff3 << 1) | 0; // P_DIFF N:0 - - // I:1 P:1 L:0 F:1 B:1 E:0 V:0 Z:0 - // I: PICTURE ID:17 - // I: - // P,F: P_DIFF:17 N:1 => refPicId = 17 - 17 = 0 - // P,F: P_DIFF:18 N:1 => refPicId = (kMaxPictureId + 1) + 17 - 18 = 0x7FFF - // P,F: P_DIFF:127 N:0 => refPicId = (kMaxPictureId + 1) + 17 - 127 = 32658 - expected_.beginning_of_frame = true; - expected_.inter_pic_predicted = true; - expected_.flexible_mode = true; - expected_.picture_id = kPictureId; - expected_.num_ref_pics = 3; - expected_.pid_diff[0] = kPdiff1; - expected_.pid_diff[1] = kPdiff2; - expected_.pid_diff[2] = kPdiff3; - expected_.ref_picture_id[0] = 0; - expected_.ref_picture_id[1] = 0x7FFF; - expected_.ref_picture_id[2] = 32658; - ParseAndCheckPacket(packet, expected_, kHeaderLength, sizeof(packet)); -} - -TEST_F(RtpDepacketizerVp9Test, ParseRefIdxFailsWithNoPictureId) { - const uint8_t kPdiff = 3; - uint8_t packet[13] = {0}; - packet[0] = 0x58; // I:0 P:1 L:0 F:1 B:1 E:0 V:0 Z:0 - packet[1] = (kPdiff << 1); // P,F: P_DIFF:3 N:0 - - RtpDepacketizer::ParsedPayload parsed; - EXPECT_FALSE(depacketizer_->Parse(&parsed, packet, sizeof(packet))); -} - -TEST_F(RtpDepacketizerVp9Test, ParseRefIdxFailsWithTooManyRefPics) { - const uint8_t kPdiff = 3; - uint8_t packet[13] = {0}; - packet[0] = 0xD8; // I:1 P:1 L:0 F:1 B:1 E:0 V:0 Z:0 - packet[1] = kMaxOneBytePictureId; // I: PICTURE ID:127 - packet[2] = (kPdiff << 1) | 1; // P,F: P_DIFF:3 N:1 - packet[3] = (kPdiff << 1) | 1; // P,F: P_DIFF:3 N:1 - packet[4] = (kPdiff << 1) | 1; // P,F: P_DIFF:3 N:1 - packet[5] = (kPdiff << 1) | 0; // P,F: P_DIFF:3 N:0 - - RtpDepacketizer::ParsedPayload parsed; - EXPECT_FALSE(depacketizer_->Parse(&parsed, packet, sizeof(packet))); -} - -TEST_F(RtpDepacketizerVp9Test, ParseSsData) { - const uint8_t kHeaderLength = 6; - const uint8_t kYbit = 0; - const size_t kNs = 2; - const size_t kNg = 2; - uint8_t packet[23] = {0}; - packet[0] = 0x0A; // I:0 P:0 L:0 F:0 B:1 E:0 V:1 Z:0 - packet[1] = ((kNs - 1) << 5) | (kYbit << 4) | (1 << 3); // N_S Y G:1 - - packet[2] = kNg; // N_G - packet[3] = (0 << 5) | (1 << 4) | (0 << 2) | 0; // T:0 U:1 R:0 - - packet[4] = (2 << 5) | (0 << 4) | (1 << 2) | 0; // T:2 U:0 R:1 - - packet[5] = 33; - - expected_.beginning_of_frame = true; - expected_.ss_data_available = true; - expected_.num_spatial_layers = kNs; - expected_.spatial_layer_resolution_present = kYbit ? true : false; - expected_.gof.num_frames_in_gof = kNg; - expected_.gof.temporal_idx[0] = 0; - expected_.gof.temporal_idx[1] = 2; - expected_.gof.temporal_up_switch[0] = true; - expected_.gof.temporal_up_switch[1] = false; - expected_.gof.num_ref_pics[0] = 0; - expected_.gof.num_ref_pics[1] = 1; - expected_.gof.pid_diff[1][0] = 33; - ParseAndCheckPacket(packet, expected_, kHeaderLength, sizeof(packet)); -} - -TEST_F(RtpDepacketizerVp9Test, ParseFirstPacketInKeyFrame) { - uint8_t packet[2] = {0}; - packet[0] = 0x08; // I:0 P:0 L:0 F:0 B:1 E:0 V:0 Z:0 - - RtpDepacketizer::ParsedPayload parsed; - ASSERT_TRUE(depacketizer_->Parse(&parsed, packet, sizeof(packet))); - EXPECT_EQ(VideoFrameType::kVideoFrameKey, parsed.video_header().frame_type); - EXPECT_TRUE(parsed.video_header().is_first_packet_in_frame); -} - -TEST_F(RtpDepacketizerVp9Test, ParseLastPacketInDeltaFrame) { - uint8_t packet[2] = {0}; - packet[0] = 0x44; // I:0 P:1 L:0 F:0 B:0 E:1 V:0 Z:0 - - RtpDepacketizer::ParsedPayload parsed; - ASSERT_TRUE(depacketizer_->Parse(&parsed, packet, sizeof(packet))); - EXPECT_EQ(VideoFrameType::kVideoFrameDelta, parsed.video_header().frame_type); - EXPECT_FALSE(parsed.video_header().is_first_packet_in_frame); -} - -TEST_F(RtpDepacketizerVp9Test, ParseResolution) { - const uint16_t kWidth[2] = {640, 1280}; - const uint16_t kHeight[2] = {360, 720}; - uint8_t packet[20] = {0}; - packet[0] = 0x0A; // I:0 P:0 L:0 F:0 B:1 E:0 V:1 Z:0 - packet[1] = (1 << 5) | (1 << 4) | 0; // N_S:1 Y:1 G:0 - packet[2] = kWidth[0] >> 8; - packet[3] = kWidth[0] & 0xFF; - packet[4] = kHeight[0] >> 8; - packet[5] = kHeight[0] & 0xFF; - packet[6] = kWidth[1] >> 8; - packet[7] = kWidth[1] & 0xFF; - packet[8] = kHeight[1] >> 8; - packet[9] = kHeight[1] & 0xFF; - - RtpDepacketizer::ParsedPayload parsed; - ASSERT_TRUE(depacketizer_->Parse(&parsed, packet, sizeof(packet))); - EXPECT_EQ(kWidth[0], parsed.video_header().width); - EXPECT_EQ(kHeight[0], parsed.video_header().height); -} - -TEST_F(RtpDepacketizerVp9Test, ParseFailsForNoPayloadLength) { - uint8_t packet[1] = {0}; - RtpDepacketizer::ParsedPayload parsed; - EXPECT_FALSE(depacketizer_->Parse(&parsed, packet, 0)); -} - -TEST_F(RtpDepacketizerVp9Test, ParseFailsForTooShortBufferToFitPayload) { - const uint8_t kHeaderLength = 1; - uint8_t packet[kHeaderLength] = {0}; - RtpDepacketizer::ParsedPayload parsed; - EXPECT_FALSE(depacketizer_->Parse(&parsed, packet, sizeof(packet))); -} - -TEST_F(RtpDepacketizerVp9Test, ParseNonRefForInterLayerPred) { - uint8_t packet[2] = {0}; - - packet[0] = 0x08; // I:0 P:0 L:0 F:0 B:1 E:0 V:0 Z:0 - expected_.beginning_of_frame = true; - expected_.non_ref_for_inter_layer_pred = false; - ParseAndCheckPacket(packet, expected_, 1, sizeof(packet)); - - packet[0] = 0x05; // I:0 P:0 L:0 F:0 B:0 E:1 V:0 Z:1 - expected_.beginning_of_frame = false; - expected_.end_of_frame = true; - expected_.non_ref_for_inter_layer_pred = true; - ParseAndCheckPacket(packet, expected_, 1, sizeof(packet)); -} - +} // namespace } // namespace webrtc diff --git a/test/fuzzers/BUILD.gn b/test/fuzzers/BUILD.gn index b405d78c28..52bc4a7fa8 100644 --- a/test/fuzzers/BUILD.gn +++ b/test/fuzzers/BUILD.gn @@ -88,7 +88,9 @@ webrtc_fuzzer_test("vp9_depacketizer_fuzzer") { "vp9_depacketizer_fuzzer.cc", ] deps = [ + "../../api:array_view", "../../modules/rtp_rtcp", + "../../modules/rtp_rtcp:rtp_video_header", ] } diff --git a/test/fuzzers/vp9_depacketizer_fuzzer.cc b/test/fuzzers/vp9_depacketizer_fuzzer.cc index 8f62b429ff..ae36a94931 100644 --- a/test/fuzzers/vp9_depacketizer_fuzzer.cc +++ b/test/fuzzers/vp9_depacketizer_fuzzer.cc @@ -7,12 +7,14 @@ * in the file PATENTS. All contributing project authors may * be found in the AUTHORS file in the root of the source tree. */ -#include "modules/rtp_rtcp/source/rtp_format_vp9.h" +#include "api/array_view.h" +#include "modules/rtp_rtcp/source/rtp_video_header.h" +#include "modules/rtp_rtcp/source/video_rtp_depacketizer_vp9.h" namespace webrtc { void FuzzOneInput(const uint8_t* data, size_t size) { - RtpDepacketizerVp9 depacketizer; - RtpDepacketizer::ParsedPayload parsed_payload; - depacketizer.Parse(&parsed_payload, data, size); + RTPVideoHeader video_header; + VideoRtpDepacketizerVp9::ParseRtpPayload(rtc::MakeArrayView(data, size), + &video_header); } } // namespace webrtc diff --git a/video/video_send_stream_tests.cc b/video/video_send_stream_tests.cc index 41473f19cb..66d31465bd 100644 --- a/video/video_send_stream_tests.cc +++ b/video/video_send_stream_tests.cc @@ -27,9 +27,9 @@ #include "modules/rtp_rtcp/include/rtp_header_extension_map.h" #include "modules/rtp_rtcp/include/rtp_rtcp.h" #include "modules/rtp_rtcp/source/rtcp_sender.h" -#include "modules/rtp_rtcp/source/rtp_format_vp9.h" #include "modules/rtp_rtcp/source/rtp_header_extensions.h" #include "modules/rtp_rtcp/source/rtp_packet.h" +#include "modules/rtp_rtcp/source/video_rtp_depacketizer_vp9.h" #include "modules/video_coding/codecs/vp8/include/vp8.h" #include "modules/video_coding/codecs/vp9/include/vp9.h" #include "rtc_base/checks.h" @@ -3169,16 +3169,16 @@ class Vp9HeaderObserver : public test::SendTest { IsNewerSequenceNumber(rtp_packet.SequenceNumber(), last_packet_sequence_number_); if (!rtp_payload.empty() && new_packet) { - RtpDepacketizer::ParsedPayload parsed; - RtpDepacketizerVp9 depacketizer; - EXPECT_TRUE( - depacketizer.Parse(&parsed, rtp_payload.data(), rtp_payload.size())); - EXPECT_EQ(VideoCodecType::kVideoCodecVP9, parsed.video_header().codec); + RTPVideoHeader video_header; + EXPECT_NE( + VideoRtpDepacketizerVp9::ParseRtpPayload(rtp_payload, &video_header), + 0); + EXPECT_EQ(VideoCodecType::kVideoCodecVP9, video_header.codec); // Verify common fields for all configurations. const auto& vp9_header = - absl::get(parsed.video_header().video_type_header); + absl::get(video_header.video_type_header); VerifyCommonHeader(vp9_header); - CompareConsecutiveFrames(rtp_packet, parsed.video_header()); + CompareConsecutiveFrames(rtp_packet, video_header); // Verify configuration specific settings. InspectHeader(vp9_header);