From 2234121cfb1f96e6067873688c0b0d06cbfb4260 Mon Sep 17 00:00:00 2001 From: Magnus Jedvert Date: Wed, 11 Jul 2018 14:27:49 +0200 Subject: [PATCH] VCMPacket: Set VCMNaluCompleteness for generic codecs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We previously always set VCMNaluCompleteness to kNaluComplete for kVideoCodecGeneric, which seems wrong. This CL fixes that and also cleans up the code a bit. The logic for VP8, VP9, and H264 should be exactly preserved. This CL also updates the test to use kVideoCodecGeneric instead of kVideoCodecUnknown. kVideoCodecUnknown has no purpose and should be removed. Bug: webrtc:9516 Change-Id: Ib8d2bf6a04d41b91c5774531f3a669edce3c6cb2 Reviewed-on: https://webrtc-review.googlesource.com/88181 Reviewed-by: Philip Eliasson Reviewed-by: Sami Kalliomäki Commit-Queue: Magnus Jedvert Cr-Commit-Position: refs/heads/master@{#23933} --- .../video_coding/jitter_buffer_unittest.cc | 7 +- modules/video_coding/packet.cc | 87 +++---------------- modules/video_coding/packet.h | 5 -- modules/video_coding/session_info_unittest.cc | 1 - 4 files changed, 17 insertions(+), 83 deletions(-) diff --git a/modules/video_coding/jitter_buffer_unittest.cc b/modules/video_coding/jitter_buffer_unittest.cc index db4ede0ff2..adb495d6fe 100644 --- a/modules/video_coding/jitter_buffer_unittest.cc +++ b/modules/video_coding/jitter_buffer_unittest.cc @@ -242,7 +242,8 @@ class TestBasicJitterBuffer : public ::testing::TestWithParam, rtpHeader.header.timestamp = timestamp_; rtpHeader.header.markerBit = true; rtpHeader.frameType = kVideoFrameDelta; - rtpHeader.video_header().codec = kVideoCodecUnknown; + rtpHeader.video_header().codec = kVideoCodecGeneric; + rtpHeader.video_header().is_first_packet_in_frame = true; packet_.reset(new VCMPacket(data_, size_, rtpHeader)); } @@ -800,7 +801,7 @@ TEST_F(TestBasicJitterBuffer, TestReorderingWithPadding) { rtpHeader.header.sequenceNumber = seq_num_ + 2; rtpHeader.header.timestamp = timestamp_ + (33 * 90); rtpHeader.header.markerBit = false; - rtpHeader.video_header().codec = kVideoCodecUnknown; + rtpHeader.video_header().codec = kVideoCodecGeneric; VCMPacket empty_packet(data_, 0, rtpHeader); EXPECT_EQ(kOldPacket, jitter_buffer_->InsertPacket(empty_packet, &retransmitted)); @@ -2164,7 +2165,7 @@ TEST_F(TestBasicJitterBuffer, H264IncompleteNalu) { timestamp_ += 33 * 90; WebRtcRTPHeader rtpHeader; memset(&rtpHeader, 0, sizeof(rtpHeader)); - rtpHeader.video_header().codec = kVideoCodecUnknown; + rtpHeader.video_header().codec = kVideoCodecGeneric; VCMPacket emptypacket(data_, 0, rtpHeader); emptypacket.seqNum = seq_num_; emptypacket.timestamp = timestamp_; diff --git a/modules/video_coding/packet.cc b/modules/video_coding/packet.cc index 6afadc339a..5ffa45b339 100644 --- a/modules/video_coding/packet.cc +++ b/modules/video_coding/packet.cc @@ -49,15 +49,24 @@ VCMPacket::VCMPacket(const uint8_t* ptr, markerBit(rtpHeader.header.markerBit), timesNacked(-1), frameType(rtpHeader.frameType), - codec(kVideoCodecUnknown), + codec(rtpHeader.video_header().codec), is_first_packet_in_frame( rtpHeader.video_header().is_first_packet_in_frame), - completeNALU(kNaluComplete), - insertStartCode(false), + completeNALU(kNaluIncomplete), + insertStartCode(rtpHeader.video_header().codec == kVideoCodecH264 && + rtpHeader.video_header().is_first_packet_in_frame), width(rtpHeader.video_header().width), height(rtpHeader.video_header().height), video_header(rtpHeader.video_header()) { - CopyCodecSpecifics(rtpHeader.video_header()); + if (is_first_packet_in_frame && markerBit) { + completeNALU = kNaluComplete; + } else if (is_first_packet_in_frame) { + completeNALU = kNaluStart; + } else if (markerBit) { + completeNALU = kNaluEnd; + } else { + completeNALU = kNaluIncomplete; + } if (markerBit) { video_header.rotation = rtpHeader.video_header().rotation; @@ -70,74 +79,4 @@ VCMPacket::VCMPacket(const uint8_t* ptr, } } -void VCMPacket::Reset() { - payloadType = 0; - timestamp = 0; - ntp_time_ms_ = 0; - seqNum = 0; - dataPtr = NULL; - sizeBytes = 0; - markerBit = false; - timesNacked = -1; - frameType = kEmptyFrame; - codec = kVideoCodecUnknown; - is_first_packet_in_frame = false; - completeNALU = kNaluUnset; - insertStartCode = false; - width = 0; - height = 0; - video_header = {}; -} - -void VCMPacket::CopyCodecSpecifics(const RTPVideoHeader& videoHeader) { - codec = videoHeader.codec; - switch (videoHeader.codec) { - case kVideoCodecVP8: - // Handle all packets within a frame as depending on the previous packet - // TODO(holmer): This should be changed to make fragments independent - // when the VP8 RTP receiver supports fragments. - if (is_first_packet_in_frame && markerBit) - completeNALU = kNaluComplete; - else if (is_first_packet_in_frame) - completeNALU = kNaluStart; - else if (markerBit) - completeNALU = kNaluEnd; - else - completeNALU = kNaluIncomplete; - - return; - case kVideoCodecVP9: - if (is_first_packet_in_frame && markerBit) - completeNALU = kNaluComplete; - else if (is_first_packet_in_frame) - completeNALU = kNaluStart; - else if (markerBit) - completeNALU = kNaluEnd; - else - completeNALU = kNaluIncomplete; - - return; - case kVideoCodecH264: - is_first_packet_in_frame = videoHeader.is_first_packet_in_frame; - if (is_first_packet_in_frame) - insertStartCode = true; - - if (is_first_packet_in_frame && markerBit) { - completeNALU = kNaluComplete; - } else if (is_first_packet_in_frame) { - completeNALU = kNaluStart; - } else if (markerBit) { - completeNALU = kNaluEnd; - } else { - completeNALU = kNaluIncomplete; - } - return; - case kVideoCodecGeneric: - return; - default: - codec = kVideoCodecUnknown; - return; - } -} - } // namespace webrtc diff --git a/modules/video_coding/packet.h b/modules/video_coding/packet.h index 53b69a372d..c67cc2f0fa 100644 --- a/modules/video_coding/packet.h +++ b/modules/video_coding/packet.h @@ -23,8 +23,6 @@ class VCMPacket { const size_t size, const WebRtcRTPHeader& rtpHeader); - void Reset(); - uint8_t payloadType; uint32_t timestamp; // NTP time of the capture time in local timebase in milliseconds. @@ -47,9 +45,6 @@ class VCMPacket { RTPVideoHeader video_header; int64_t receive_time_ms; - - protected: - void CopyCodecSpecifics(const RTPVideoHeader& videoHeader); }; } // namespace webrtc diff --git a/modules/video_coding/session_info_unittest.cc b/modules/video_coding/session_info_unittest.cc index 1863ad7eef..2846653b3f 100644 --- a/modules/video_coding/session_info_unittest.cc +++ b/modules/video_coding/session_info_unittest.cc @@ -23,7 +23,6 @@ class TestSessionInfo : public ::testing::Test { memset(packet_buffer_, 0, sizeof(packet_buffer_)); memset(frame_buffer_, 0, sizeof(frame_buffer_)); session_.Reset(); - packet_.Reset(); packet_.frameType = kVideoFrameDelta; packet_.sizeBytes = packet_buffer_size(); packet_.dataPtr = packet_buffer_;