From a75d12d09d5f9fdb4a14253ac2533fc7ab22d710 Mon Sep 17 00:00:00 2001 From: philipel Date: Mon, 7 Nov 2016 05:11:28 -0800 Subject: [PATCH] H264SpsPpsTracker now return PacketAction on CopyAndFixBitstream. To differentiate between when a packet should be dropped or when a keyframe is missing its SPS and/or SPS CopyAndFixBitstream now returns: - kInsert, the packet should be inserted into the PacketBuffer. - kDrop, the packet should be dropped. - kRequestKeyframe, a keyframe should be requested. BUG=webrtc:5514 Review-Url: https://codereview.webrtc.org/2477343002 Cr-Commit-Position: refs/heads/master@{#14948} --- .../video_coding/h264_sps_pps_tracker.cc | 15 ++++++----- .../video_coding/h264_sps_pps_tracker.h | 4 ++- .../h264_sps_pps_tracker_unittest.cc | 27 +++++++++++-------- 3 files changed, 27 insertions(+), 19 deletions(-) diff --git a/webrtc/modules/video_coding/h264_sps_pps_tracker.cc b/webrtc/modules/video_coding/h264_sps_pps_tracker.cc index b44f477427..7bcc2b7c5e 100644 --- a/webrtc/modules/video_coding/h264_sps_pps_tracker.cc +++ b/webrtc/modules/video_coding/h264_sps_pps_tracker.cc @@ -25,7 +25,8 @@ namespace { const uint8_t start_code_h264[] = {0, 0, 0, 1}; } // namespace -bool H264SpsPpsTracker::CopyAndFixBitstream(VCMPacket* packet) { +H264SpsPpsTracker::PacketAction H264SpsPpsTracker::CopyAndFixBitstream( + VCMPacket* packet) { RTC_DCHECK(packet->codec == kVideoCodecH264); const uint8_t* data = packet->dataPtr; @@ -69,21 +70,21 @@ bool H264SpsPpsTracker::CopyAndFixBitstream(VCMPacket* packet) { if (video_header.isFirstPacket) { if (nalu.pps_id == -1) { LOG(LS_WARNING) << "No PPS id in IDR nalu."; - return false; + return kRequestKeyframe; } auto pps = pps_data_.find(nalu.pps_id); if (pps == pps_data_.end()) { LOG(LS_WARNING) << "No PPS with id << " << nalu.pps_id << " received"; - return false; + return kRequestKeyframe; } auto sps = sps_data_.find(pps->second.sps_id); if (sps == sps_data_.end()) { LOG(LS_WARNING) << "No SPS with id << " << pps_data_[nalu.pps_id].sps_id << " received"; - return false; + return kRequestKeyframe; } pps_id = nalu.pps_id; @@ -101,7 +102,7 @@ bool H264SpsPpsTracker::CopyAndFixBitstream(VCMPacket* packet) { } if (!insert_packet) - return false; + return kDrop; // Calculate how much space we need for the rest of the bitstream. if (codec_header.packetization_type == kH264StapA) { @@ -158,7 +159,7 @@ bool H264SpsPpsTracker::CopyAndFixBitstream(VCMPacket* packet) { size_t copy_end = nalu_ptr - data + segment_length; if (copy_end > data_size) { delete[] buffer; - return false; + return kDrop; } memcpy(insert_at, nalu_ptr, segment_length); @@ -175,7 +176,7 @@ bool H264SpsPpsTracker::CopyAndFixBitstream(VCMPacket* packet) { packet->dataPtr = buffer; packet->sizeBytes = required_size; - return true; + return kInsert; } } // namespace video_coding diff --git a/webrtc/modules/video_coding/h264_sps_pps_tracker.h b/webrtc/modules/video_coding/h264_sps_pps_tracker.h index 6de092a365..b54afbb260 100644 --- a/webrtc/modules/video_coding/h264_sps_pps_tracker.h +++ b/webrtc/modules/video_coding/h264_sps_pps_tracker.h @@ -25,7 +25,9 @@ namespace video_coding { class H264SpsPpsTracker { public: - bool CopyAndFixBitstream(VCMPacket* packet); + enum PacketAction { kInsert, kDrop, kRequestKeyframe }; + + PacketAction CopyAndFixBitstream(VCMPacket* packet); private: struct PpsInfo { diff --git a/webrtc/modules/video_coding/h264_sps_pps_tracker_unittest.cc b/webrtc/modules/video_coding/h264_sps_pps_tracker_unittest.cc index f467028f16..340f4bd28a 100644 --- a/webrtc/modules/video_coding/h264_sps_pps_tracker_unittest.cc +++ b/webrtc/modules/video_coding/h264_sps_pps_tracker_unittest.cc @@ -87,7 +87,7 @@ TEST_F(TestH264SpsPpsTracker, NoNalus) { packet.dataPtr = data; packet.sizeBytes = sizeof(data); - EXPECT_TRUE(tracker_.CopyAndFixBitstream(&packet)); + EXPECT_EQ(H264SpsPpsTracker::kInsert, tracker_.CopyAndFixBitstream(&packet)); EXPECT_EQ(memcmp(packet.dataPtr, data, sizeof(data)), 0); delete[] packet.dataPtr; } @@ -100,7 +100,7 @@ TEST_F(TestH264SpsPpsTracker, FuAFirstPacket) { packet.dataPtr = data; packet.sizeBytes = sizeof(data); - EXPECT_TRUE(tracker_.CopyAndFixBitstream(&packet)); + EXPECT_EQ(H264SpsPpsTracker::kInsert, tracker_.CopyAndFixBitstream(&packet)); std::vector expected; expected.insert(expected.end(), start_code, start_code + sizeof(start_code)); expected.insert(expected.end(), {1, 2, 3}); @@ -116,7 +116,7 @@ TEST_F(TestH264SpsPpsTracker, StapAIncorrectSegmentLength) { packet.dataPtr = data; packet.sizeBytes = sizeof(data); - EXPECT_FALSE(tracker_.CopyAndFixBitstream(&packet)); + EXPECT_EQ(H264SpsPpsTracker::kDrop, tracker_.CopyAndFixBitstream(&packet)); } TEST_F(TestH264SpsPpsTracker, NoNalusFirstPacket) { @@ -126,7 +126,7 @@ TEST_F(TestH264SpsPpsTracker, NoNalusFirstPacket) { packet.dataPtr = data; packet.sizeBytes = sizeof(data); - EXPECT_TRUE(tracker_.CopyAndFixBitstream(&packet)); + EXPECT_EQ(H264SpsPpsTracker::kInsert, tracker_.CopyAndFixBitstream(&packet)); std::vector expected; expected.insert(expected.end(), start_code, start_code + sizeof(start_code)); expected.insert(expected.end(), {1, 2, 3}); @@ -143,7 +143,7 @@ TEST_F(TestH264SpsPpsTracker, IdrNoSpsPpsInserted) { packet.dataPtr = data.data(); packet.sizeBytes = data.size(); - EXPECT_TRUE(tracker_.CopyAndFixBitstream(&packet)); + EXPECT_EQ(H264SpsPpsTracker::kInsert, tracker_.CopyAndFixBitstream(&packet)); EXPECT_EQ(memcmp(packet.dataPtr, data.data(), data.size()), 0); delete[] packet.dataPtr; } @@ -157,7 +157,8 @@ TEST_F(TestH264SpsPpsTracker, IdrFirstPacketNoSpsPpsInserted) { packet.dataPtr = data.data(); packet.sizeBytes = data.size(); - EXPECT_FALSE(tracker_.CopyAndFixBitstream(&packet)); + EXPECT_EQ(H264SpsPpsTracker::kRequestKeyframe, + tracker_.CopyAndFixBitstream(&packet)); } TEST_F(TestH264SpsPpsTracker, IdrFirstPacketNoPpsInserted) { @@ -170,7 +171,8 @@ TEST_F(TestH264SpsPpsTracker, IdrFirstPacketNoPpsInserted) { packet.dataPtr = data.data(); packet.sizeBytes = data.size(); - EXPECT_FALSE(tracker_.CopyAndFixBitstream(&packet)); + EXPECT_EQ(H264SpsPpsTracker::kRequestKeyframe, + tracker_.CopyAndFixBitstream(&packet)); } TEST_F(TestH264SpsPpsTracker, IdrFirstPacketNoSpsInserted) { @@ -183,7 +185,8 @@ TEST_F(TestH264SpsPpsTracker, IdrFirstPacketNoSpsInserted) { packet.dataPtr = data.data(); packet.sizeBytes = data.size(); - EXPECT_FALSE(tracker_.CopyAndFixBitstream(&packet)); + EXPECT_EQ(H264SpsPpsTracker::kRequestKeyframe, + tracker_.CopyAndFixBitstream(&packet)); } TEST_F(TestH264SpsPpsTracker, SpsPpsPacketThenIdrFirstPacket) { @@ -195,7 +198,8 @@ TEST_F(TestH264SpsPpsTracker, SpsPpsPacketThenIdrFirstPacket) { AddPps(&sps_pps_packet, 0, 1, &data); sps_pps_packet.dataPtr = data.data(); sps_pps_packet.sizeBytes = data.size(); - EXPECT_FALSE(tracker_.CopyAndFixBitstream(&sps_pps_packet)); + EXPECT_EQ(H264SpsPpsTracker::kDrop, + tracker_.CopyAndFixBitstream(&sps_pps_packet)); data.clear(); // Insert first packet of the IDR @@ -205,7 +209,8 @@ TEST_F(TestH264SpsPpsTracker, SpsPpsPacketThenIdrFirstPacket) { data.insert(data.end(), {1, 2, 3}); idr_packet.dataPtr = data.data(); idr_packet.sizeBytes = data.size(); - EXPECT_TRUE(tracker_.CopyAndFixBitstream(&idr_packet)); + EXPECT_EQ(H264SpsPpsTracker::kInsert, + tracker_.CopyAndFixBitstream(&idr_packet)); std::vector expected; expected.insert(expected.end(), start_code, start_code + sizeof(start_code)); @@ -235,7 +240,7 @@ TEST_F(TestH264SpsPpsTracker, SpsPpsIdrInStapA) { packet.dataPtr = data.data(); packet.sizeBytes = data.size(); - EXPECT_TRUE(tracker_.CopyAndFixBitstream(&packet)); + EXPECT_EQ(H264SpsPpsTracker::kInsert, tracker_.CopyAndFixBitstream(&packet)); std::vector expected; // The SPS/PPS is repeated because this packet both contains the SPS/PPS