Only append SPS/PPS to bitstream if supplied out of band.
BUG=chromium:721597
Review-Url: https://codereview.webrtc.org/2945853002
Cr-Commit-Position: refs/heads/master@{#18701}
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc b/webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc
index ce3f340..68e54f1 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 @@
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 93c8887..8f0d9fe 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 @@
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 d1d52b4..88b440f 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 @@
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 @@
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();
+ break;
}
- default: {
- // Something other than an SPS/PPS nalu in this packet, then the SPS/PPS
- // should be appended.
- append_sps_pps = true;
- }
+ 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 @@
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 @@
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 37ab4ee..0aa1351 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 @@
return packet;
}
- void AddSps(VCMPacket* packet, int sps_id, std::vector<uint8_t>* data) {
+ void AddSps(VCMPacket* packet, uint8_t sps_id, std::vector<uint8_t>* 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 @@
}
void AddPps(VCMPacket* packet,
- int sps_id,
- int pps_id,
+ uint8_t sps_id,
+ uint8_t pps_id,
std::vector<uint8_t>* 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 @@
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 @@
std::vector<uint8_t> 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 @@
EXPECT_EQ(H264SpsPpsTracker::kInsert, tracker_.CopyAndFixBitstream(&packet));
std::vector<uint8_t> 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 @@
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 5f14089..8f42fbd 100644
--- a/webrtc/video/rtp_video_stream_receiver_unittest.cc
+++ b/webrtc/video/rtp_video_stream_receiver_unittest.cc
@@ -119,13 +119,13 @@
// TODO(Johan): refactor h264_sps_pps_tracker_unittests.cc to avoid duplicate
// code.
- void AddSps(WebRtcRTPHeader* packet, int sps_id, std::vector<uint8_t>* data) {
+ void AddSps(WebRtcRTPHeader* packet,
+ uint8_t sps_id,
+ std::vector<uint8_t>* 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 @@
}
void AddPps(WebRtcRTPHeader* packet,
- int sps_id,
- int pps_id,
+ uint8_t sps_id,
+ uint8_t pps_id,
std::vector<uint8_t>* 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 @@
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 @@
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 @@
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));
