diff --git a/webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc b/webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc index ce3f340084..68e54f1432 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc @@ -488,8 +488,6 @@ bool RtpDepacketizerH264::ProcessStapAOrSingleNalu( NaluInfo nalu; nalu.type = payload_data[start_offset] & kTypeMask; - nalu.offset = start_offset; - nalu.size = end_offset - start_offset; nalu.sps_id = -1; nalu.pps_id = -1; start_offset += H264::kNaluTypeSize; diff --git a/webrtc/modules/video_coding/codecs/h264/include/h264_globals.h b/webrtc/modules/video_coding/codecs/h264/include/h264_globals.h index 93c8887dcd..8f0d9fe159 100644 --- a/webrtc/modules/video_coding/codecs/h264/include/h264_globals.h +++ b/webrtc/modules/video_coding/codecs/h264/include/h264_globals.h @@ -57,10 +57,6 @@ struct NaluInfo { uint8_t type; int sps_id; int pps_id; - - // Offset and size are only valid for non-FuA packets. - size_t offset; - size_t size; }; const size_t kMaxNalusPerPacket = 10; diff --git a/webrtc/modules/video_coding/h264_sps_pps_tracker.cc b/webrtc/modules/video_coding/h264_sps_pps_tracker.cc index d1d52b4ed2..88b440f0b8 100644 --- a/webrtc/modules/video_coding/h264_sps_pps_tracker.cc +++ b/webrtc/modules/video_coding/h264_sps_pps_tracker.cc @@ -37,30 +37,20 @@ H264SpsPpsTracker::PacketAction H264SpsPpsTracker::CopyAndFixBitstream( const RTPVideoHeader& video_header = packet->video_header; const RTPVideoHeaderH264& codec_header = video_header.codecHeader.H264; - int pps_id = -1; - int sps_id = -1; - bool append_sps_pps = codec_header.nalus_length == 0; - size_t required_size = 0; + bool append_sps_pps = false; + auto sps = sps_data_.end(); + auto pps = pps_data_.end(); + for (size_t i = 0; i < codec_header.nalus_length; ++i) { const NaluInfo& nalu = codec_header.nalus[i]; switch (nalu.type) { case H264::NaluType::kSps: { - // Save SPS. - sps_data_[nalu.sps_id].size = nalu.size; - sps_data_[nalu.sps_id].data.reset(new uint8_t[nalu.size]); - memcpy(sps_data_[nalu.sps_id].data.get(), data + nalu.offset, - nalu.size); sps_data_[nalu.sps_id].width = packet->width; sps_data_[nalu.sps_id].height = packet->height; break; } case H264::NaluType::kPps: { - // Save PPS. pps_data_[nalu.pps_id].sps_id = nalu.sps_id; - pps_data_[nalu.pps_id].size = nalu.size; - pps_data_[nalu.pps_id].data.reset(new uint8_t[nalu.size]); - memcpy(pps_data_[nalu.pps_id].data.get(), data + nalu.offset, - nalu.size); break; } case H264::NaluType::kIdr: { @@ -73,52 +63,52 @@ H264SpsPpsTracker::PacketAction H264SpsPpsTracker::CopyAndFixBitstream( return kRequestKeyframe; } - auto pps = pps_data_.find(nalu.pps_id); + pps = pps_data_.find(nalu.pps_id); if (pps == pps_data_.end()) { LOG(LS_WARNING) << "No PPS with id << " << nalu.pps_id << " received"; return kRequestKeyframe; } - sps_id = pps->second.sps_id; - auto sps = sps_data_.find(sps_id); + 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"; + LOG(LS_WARNING) + << "No SPS with id << " << pps->second.sps_id << " received"; return kRequestKeyframe; } - pps_id = nalu.pps_id; - required_size += pps->second.size + sizeof(start_code_h264); - required_size += sps->second.size + sizeof(start_code_h264); + // Since the first packet of every keyframe should have its width and + // height set we set it here in the case of it being supplied out of + // band. + packet->width = sps->second.width; + packet->height = sps->second.height; + + // If the SPS/PPS was supplied out of band then we will have saved + // the actual bitstream in |data|. + if (sps->second.data && pps->second.data) { + RTC_DCHECK_GT(sps->second.size, 0); + RTC_DCHECK_GT(pps->second.size, 0); + append_sps_pps = true; + } } - FALLTHROUGH(); - } - default: { - // Something other than an SPS/PPS nalu in this packet, then the SPS/PPS - // should be appended. - append_sps_pps = true; + break; } + default: + break; } } - if (!append_sps_pps) { - // Two things: Firstly, when we receive a packet the data pointed at by - // |dataPtr| is volatile, meaning we have to copy the data into our own - // buffer if we want to use it at a later stage. Secondly, when a packet is - // inserted into the PacketBuffer it expects the packet to own its own - // buffer, and this function copies (and fix) the bitstream of the packet - // into its own buffer. - // - // SPS/PPS packets is a special case. Since we save the SPS/PPS NALU and - // append it to the first packet of every IDR frame the SPS/PPS packet - // doesn't actually need to contain any bitstream data. - packet->dataPtr = nullptr; - packet->sizeBytes = 0; - return kInsert; - } + RTC_CHECK(!append_sps_pps || + (sps != sps_data_.end() && pps != pps_data_.end())); // Calculate how much space we need for the rest of the bitstream. + size_t required_size = 0; + + if (append_sps_pps) { + required_size += sps->second.size + sizeof(start_code_h264); + required_size += pps->second.size + sizeof(start_code_h264); + } + if (codec_header.packetization_type == kH264StapA) { const uint8_t* nalu_ptr = data + 1; while (nalu_ptr < data + data_size) { @@ -142,21 +132,18 @@ H264SpsPpsTracker::PacketAction H264SpsPpsTracker::CopyAndFixBitstream( uint8_t* buffer = new uint8_t[required_size]; uint8_t* insert_at = buffer; - // If pps_id != -1 then we have the SPS/PPS and they should be prepended - // to the bitstream with start codes inserted. - if (pps_id != -1) { + if (append_sps_pps) { // Insert SPS. memcpy(insert_at, start_code_h264, sizeof(start_code_h264)); insert_at += sizeof(start_code_h264); - memcpy(insert_at, sps_data_[pps_data_[pps_id].sps_id].data.get(), - sps_data_[pps_data_[pps_id].sps_id].size); - insert_at += sps_data_[pps_data_[pps_id].sps_id].size; + memcpy(insert_at, sps->second.data.get(), sps->second.size); + insert_at += sps->second.size; // Insert PPS. memcpy(insert_at, start_code_h264, sizeof(start_code_h264)); insert_at += sizeof(start_code_h264); - memcpy(insert_at, pps_data_[pps_id].data.get(), pps_data_[pps_id].size); - insert_at += pps_data_[pps_id].size; + memcpy(insert_at, pps->second.data.get(), pps->second.size); + insert_at += pps->second.size; } // Copy the rest of the bitstream and insert start codes. @@ -188,11 +175,6 @@ H264SpsPpsTracker::PacketAction H264SpsPpsTracker::CopyAndFixBitstream( memcpy(insert_at, data, data_size); } - if (sps_id != -1) { - packet->width = sps_data_[sps_id].width; - packet->height = sps_data_[sps_id].height; - } - packet->dataPtr = buffer; packet->sizeBytes = required_size; return kInsert; 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 37ab4eed10..0aa1351440 100644 --- a/webrtc/modules/video_coding/h264_sps_pps_tracker_unittest.cc +++ b/webrtc/modules/video_coding/h264_sps_pps_tracker_unittest.cc @@ -35,13 +35,11 @@ class TestH264SpsPpsTracker : public ::testing::Test { return packet; } - void AddSps(VCMPacket* packet, int sps_id, std::vector* data) { + void AddSps(VCMPacket* packet, uint8_t sps_id, std::vector* data) { NaluInfo info; info.type = H264::NaluType::kSps; info.sps_id = sps_id; info.pps_id = -1; - info.offset = data->size(); - info.size = 2; data->push_back(H264::NaluType::kSps); data->push_back(sps_id); // The sps data, just a single byte. @@ -50,15 +48,13 @@ class TestH264SpsPpsTracker : public ::testing::Test { } void AddPps(VCMPacket* packet, - int sps_id, - int pps_id, + uint8_t sps_id, + uint8_t pps_id, std::vector* data) { NaluInfo info; info.type = H264::NaluType::kPps; info.sps_id = sps_id; info.pps_id = pps_id; - info.offset = data->size(); - info.size = 2; data->push_back(H264::NaluType::kPps); data->push_back(pps_id); // The pps data, just a single byte. @@ -200,6 +196,7 @@ TEST_F(TestH264SpsPpsTracker, SpsPpsPacketThenIdrFirstPacket) { sps_pps_packet.sizeBytes = data.size(); EXPECT_EQ(H264SpsPpsTracker::kInsert, tracker_.CopyAndFixBitstream(&sps_pps_packet)); + delete[] sps_pps_packet.dataPtr; data.clear(); // Insert first packet of the IDR @@ -214,10 +211,6 @@ TEST_F(TestH264SpsPpsTracker, SpsPpsPacketThenIdrFirstPacket) { std::vector expected; expected.insert(expected.end(), start_code, start_code + sizeof(start_code)); - expected.insert(expected.end(), {H264::NaluType::kSps, 0}); - expected.insert(expected.end(), start_code, start_code + sizeof(start_code)); - expected.insert(expected.end(), {H264::NaluType::kPps, 1}); - expected.insert(expected.end(), start_code, start_code + sizeof(start_code)); expected.insert(expected.end(), {1, 2, 3}); EXPECT_EQ(memcmp(idr_packet.dataPtr, expected.data(), expected.size()), 0); delete[] idr_packet.dataPtr; @@ -243,13 +236,6 @@ TEST_F(TestH264SpsPpsTracker, SpsPpsIdrInStapA) { EXPECT_EQ(H264SpsPpsTracker::kInsert, tracker_.CopyAndFixBitstream(&packet)); std::vector expected; - // The SPS/PPS is repeated because this packet both contains the SPS/PPS - // and it is the first packet of an IDR, which will cause the SPS/PPS to be - // prepended to the bitstream. - expected.insert(expected.end(), start_code, start_code + sizeof(start_code)); - expected.insert(expected.end(), {H264::NaluType::kSps, 13}); - expected.insert(expected.end(), start_code, start_code + sizeof(start_code)); - expected.insert(expected.end(), {H264::NaluType::kPps, 27}); expected.insert(expected.end(), start_code, start_code + sizeof(start_code)); expected.insert(expected.end(), {H264::NaluType::kSps, 13}); expected.insert(expected.end(), start_code, start_code + sizeof(start_code)); @@ -344,6 +330,7 @@ TEST_F(TestH264SpsPpsTracker, SaveRestoreWidthHeight) { sps_pps_packet.height = 240; EXPECT_EQ(H264SpsPpsTracker::kInsert, tracker_.CopyAndFixBitstream(&sps_pps_packet)); + delete[] sps_pps_packet.dataPtr; VCMPacket idr_packet = GetDefaultPacket(); idr_packet.video_header.is_first_packet_in_frame = true; diff --git a/webrtc/video/rtp_video_stream_receiver_unittest.cc b/webrtc/video/rtp_video_stream_receiver_unittest.cc index 5f14089780..8f42fbdfbd 100644 --- a/webrtc/video/rtp_video_stream_receiver_unittest.cc +++ b/webrtc/video/rtp_video_stream_receiver_unittest.cc @@ -119,13 +119,13 @@ class RtpVideoStreamReceiverTest : public testing::Test { // TODO(Johan): refactor h264_sps_pps_tracker_unittests.cc to avoid duplicate // code. - void AddSps(WebRtcRTPHeader* packet, int sps_id, std::vector* data) { + void AddSps(WebRtcRTPHeader* packet, + uint8_t sps_id, + std::vector* data) { NaluInfo info; info.type = H264::NaluType::kSps; info.sps_id = sps_id; info.pps_id = -1; - info.offset = data->size(); - info.size = 2; data->push_back(H264::NaluType::kSps); data->push_back(sps_id); packet->type.Video.codecHeader.H264 @@ -133,15 +133,13 @@ class RtpVideoStreamReceiverTest : public testing::Test { } void AddPps(WebRtcRTPHeader* packet, - int sps_id, - int pps_id, + uint8_t sps_id, + uint8_t pps_id, std::vector* data) { NaluInfo info; info.type = H264::NaluType::kPps; info.sps_id = sps_id; info.pps_id = pps_id; - info.offset = data->size(); - info.size = 2; data->push_back(H264::NaluType::kPps); data->push_back(pps_id); packet->type.Video.codecHeader.H264 @@ -217,6 +215,7 @@ TEST_F(RtpVideoStreamReceiverTest, InBandSpsPps) { WebRtcRTPHeader sps_packet = GetDefaultPacket(); AddSps(&sps_packet, 0, &sps_data); sps_packet.header.sequenceNumber = 0; + sps_packet.type.Video.is_first_packet_in_frame = true; mock_on_complete_frame_callback_.AppendExpectedBitstream( kH264StartCode, sizeof(kH264StartCode)); mock_on_complete_frame_callback_.AppendExpectedBitstream(sps_data.data(), @@ -228,6 +227,7 @@ TEST_F(RtpVideoStreamReceiverTest, InBandSpsPps) { WebRtcRTPHeader pps_packet = GetDefaultPacket(); AddPps(&pps_packet, 0, 1, &pps_data); pps_packet.header.sequenceNumber = 1; + pps_packet.type.Video.is_first_packet_in_frame = true; mock_on_complete_frame_callback_.AppendExpectedBitstream( kH264StartCode, sizeof(kH264StartCode)); mock_on_complete_frame_callback_.AppendExpectedBitstream(pps_data.data(), @@ -241,9 +241,7 @@ TEST_F(RtpVideoStreamReceiverTest, InBandSpsPps) { idr_packet.type.Video.is_first_packet_in_frame = true; idr_packet.header.sequenceNumber = 2; idr_packet.header.markerBit = 1; - idr_packet.type.Video.is_first_packet_in_frame = true; idr_packet.frameType = kVideoFrameKey; - idr_packet.type.Video.codec = kRtpVideoH264; idr_data.insert(idr_data.end(), {0x65, 1, 2, 3}); mock_on_complete_frame_callback_.AppendExpectedBitstream( kH264StartCode, sizeof(kH264StartCode));