zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert of H264SpsPpsTracker.InsertSpsPpsNalus() should accept Nalus with header. (patchset #3 id:40001 of https://codereview.webrtc.org/2638933002/ )

Browse Source 
Reason for revert:
Triggers leak on Linux memcheck (non-default trybot):

### BEGIN MEMORY TOOL REPORT (error hash=#0112A395AF2326BC#)
Command: ../Release/./modules_unittests --isolated-script-test-output=/b/s/w/ioUlJCnu/output.json --isolated-script-test-chartjson-output=/b/s/w/ioUlJCnu/chartjson-output.json --gtest_filter=-CommonFormats/AudioProcessingTest*
Leak_DefinitelyLost
45 bytes in 1 blocks are definitely lost in loss record 118 of 277
  operator new[](unsigned long) (m_replacemalloc/vg_replace_malloc.c:363)
  webrtc::video_coding::H264SpsPpsTracker::CopyAndFixBitstream(webrtc::VCMPacket*) (/b/s/w/irJgAGsR/out/Release/modules_unittests)
  webrtc::video_coding::TestH264SpsPpsTracker_SpsPpsOutOfBand_Test::TestBody() (/b/s/w/irJgAGsR/out/Release/modules_unittests)
Suppression (error hash=#0112A395AF2326BC#):
  For more info on using suppressions see http://dev.chromium.org/developers/tree-sheriffs/sheriff-details-chromium/memory-sheriff#TOC-Suppressing-memory-reports
{
   <insert_a_suppression_name_here>
   Memcheck:Leak
   fun:_Zna*
   fun:_ZN6webrtc12video_coding17H264SpsPpsTracker19CopyAndFixBitstreamEPNS_9VCMPacketE
   fun:_ZN6webrtc12video_coding42TestH264SpsPpsTracker_SpsPpsOutOfBand_Test8TestBodyEv
}
### END MEMORY TOOL REPORT (error hash=#0112A395AF2326BC#)

Original issue's description:
> H264SpsPpsTracker.InsertSpsPpsNalus() should accept Nalus with header.
>
> - Changed method name to clarify that entire Nalus are expected.
> - Added unit test code.
> - Adjusted InsetSpsPpsNalus() implementation to above requirement.
>
> BUG=webrtc:5948
>
> Review-Url: https://codereview.webrtc.org/2638933002
> Cr-Commit-Position: refs/heads/master@{#16221}
> Committed: f53d7374cf

TBR=philipel@webrtc.org,sprang@webrtc.org,johan@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:5948

Review-Url: https://codereview.webrtc.org/2649113003
Cr-Commit-Position: refs/heads/master@{#16225}
This commit is contained in:
kjellander
2017-01-23 20:16:58 -08:00
committed by Commit bot
parent 1b54a5f018
commit 914d49d0fd
4 changed files with 10 additions and 102 deletions
Download Patch File Download Diff File Expand all files Collapse all files

44
webrtc/modules/video_coding/h264_sps_pps_tracker.cc
View File

@ -182,41 +182,15 @@ H264SpsPpsTracker::PacketAction H264SpsPpsTracker::CopyAndFixBitstream(
return kInsert;
}
void H264SpsPpsTracker::InsertSpsPpsNalus(const std::vector<uint8_t>& sps,
const std::vector<uint8_t>& pps) {
constexpr size_t kNaluHeaderOffset = 1;
if (sps.size() < kNaluHeaderOffset) {
LOG(LS_WARNING) << "SPS size " << sps.size() << " is smaller than "
<< kNaluHeaderOffset;
return;
}
if ((sps[0] & 0x1f) != H264::NaluType::kSps) {
LOG(LS_WARNING) << "SPS Nalu header missing";
return;
}
if (pps.size() < kNaluHeaderOffset) {
LOG(LS_WARNING) << "PPS size " << pps.size() << " is smaller than "
<< kNaluHeaderOffset;
return;
}
if ((pps[0] & 0x1f) != H264::NaluType::kPps) {
LOG(LS_WARNING) << "SPS Nalu header missing";
return;
}
rtc::Optional<SpsParser::SpsState> parsed_sps = SpsParser::ParseSps(
sps.data() + kNaluHeaderOffset, sps.size() - kNaluHeaderOffset);
rtc::Optional<PpsParser::PpsState> parsed_pps = PpsParser::ParsePps(
pps.data() + kNaluHeaderOffset, pps.size() - kNaluHeaderOffset);
if (!parsed_sps) {
LOG(LS_WARNING) << "Failed to parse SPS.";
}
if (!parsed_pps) {
LOG(LS_WARNING) << "Failed to parse PPS.";
}
void H264SpsPpsTracker::InsertSpsPps(const std::vector<uint8_t>& sps,
const std::vector<uint8_t>& pps) {
rtc::Optional<SpsParser::SpsState> parsed_sps =
SpsParser::ParseSps(sps.data(), sps.size());
rtc::Optional<PpsParser::PpsState> parsed_pps =
PpsParser::ParsePps(pps.data(), pps.size());
if (!parsed_pps || !parsed_sps) {
LOG(LS_WARNING) << "Failed to parse SPS or PPS parameters.";
return;
}
@ -234,10 +208,6 @@ void H264SpsPpsTracker::InsertSpsPpsNalus(const std::vector<uint8_t>& sps,
memcpy(pps_data, pps.data(), pps_info.size);
pps_info.data.reset(pps_data);
pps_data_[parsed_pps->id] = std::move(pps_info);
LOG(LS_INFO) << "Inserted SPS id " << parsed_sps->id << " and PPS id "
<< parsed_pps->id << " (referencing SPS " << parsed_pps->sps_id
<< ")";
}
} // namespace video_coding

5
webrtc/modules/video_coding/h264_sps_pps_tracker.h
View File

@ -29,9 +29,8 @@ class H264SpsPpsTracker {
enum PacketAction { kInsert, kDrop, kRequestKeyframe };
PacketAction CopyAndFixBitstream(VCMPacket* packet);
void InsertSpsPpsNalus(const std::vector<uint8_t>& sps,
const std::vector<uint8_t>& pps);
void InsertSpsPps(const std::vector<uint8_t>& sps,
const std::vector<uint8_t>& pps);
private:
struct PpsInfo {

60
webrtc/modules/video_coding/h264_sps_pps_tracker_unittest.cc
View File

@ -261,65 +261,5 @@ TEST_F(TestH264SpsPpsTracker, SpsPpsIdrInStapA) {
delete[] packet.dataPtr;
}
TEST_F(TestH264SpsPpsTracker, SpsPpsOutOfBand) {
constexpr uint8_t kData[] = {1, 2, 3};
// Generated by "ffmpeg -r 30 -f avfoundation -i "default" out.h264" on macos.
const std::vector<uint8_t> sps(
{0x67, 0x7a, 0x00, 0x0d, 0xbc, 0xd9, 0x41, 0x41, 0xfa, 0x10, 0x00, 0x00,
0x03, 0x00, 0x10, 0x00, 0x00, 0x03, 0x03, 0xc0, 0xf1, 0x42, 0x99, 0x60});
const std::vector<uint8_t> pps({0x68, 0xeb, 0xe3, 0xcb, 0x22, 0xc0});
tracker_.InsertSpsPpsNalus(sps, pps);
// Insert first packet of the IDR.
VCMPacket idr_packet = GetDefaultPacket();
idr_packet.video_header.is_first_packet_in_frame = true;
AddIdr(&idr_packet, 0);
idr_packet.dataPtr = kData;
idr_packet.sizeBytes = sizeof(kData);
EXPECT_EQ(H264SpsPpsTracker::kInsert,
tracker_.CopyAndFixBitstream(&idr_packet));
}
TEST_F(TestH264SpsPpsTracker, SpsPpsOutOfBandWrongNaluHeader) {
constexpr uint8_t kData[] = {1, 2, 3};
// Generated by "ffmpeg -r 30 -f avfoundation -i "default" out.h264" on macos.
// Nalu headers manupilated afterwards.
const std::vector<uint8_t> sps(
{0xff, 0x7a, 0x00, 0x0d, 0xbc, 0xd9, 0x41, 0x41, 0xfa, 0x10, 0x00, 0x00,
0x03, 0x00, 0x10, 0x00, 0x00, 0x03, 0x03, 0xc0, 0xf1, 0x42, 0x99, 0x60});
const std::vector<uint8_t> pps({0xff, 0xeb, 0xe3, 0xcb, 0x22, 0xc0});
tracker_.InsertSpsPpsNalus(sps, pps);
// Insert first packet of the IDR.
VCMPacket idr_packet = GetDefaultPacket();
idr_packet.video_header.is_first_packet_in_frame = true;
AddIdr(&idr_packet, 0);
idr_packet.dataPtr = kData;
idr_packet.sizeBytes = sizeof(kData);
EXPECT_EQ(H264SpsPpsTracker::kRequestKeyframe,
tracker_.CopyAndFixBitstream(&idr_packet));
}
TEST_F(TestH264SpsPpsTracker, SpsPpsOutOfBandIncompleteNalu) {
constexpr uint8_t kData[] = {1, 2, 3};
// Generated by "ffmpeg -r 30 -f avfoundation -i "default" out.h264" on macos.
// Nalus damaged afterwards.
const std::vector<uint8_t> sps({0x67, 0x7a, 0x00, 0x0d, 0xbc, 0xd9});
const std::vector<uint8_t> pps({0x68, 0xeb, 0xe3, 0xcb, 0x22, 0xc0});
tracker_.InsertSpsPpsNalus(sps, pps);
// Insert first packet of the IDR.
VCMPacket idr_packet = GetDefaultPacket();
idr_packet.video_header.is_first_packet_in_frame = true;
AddIdr(&idr_packet, 0);
idr_packet.dataPtr = kData;
idr_packet.sizeBytes = sizeof(kData);
EXPECT_EQ(H264SpsPpsTracker::kRequestKeyframe,
tracker_.CopyAndFixBitstream(&idr_packet));
}
} // namespace video_coding
} // namespace webrtc