From ef615ea7a3960f13aeb341e9a76e627aff875238 Mon Sep 17 00:00:00 2001 From: philipel Date: Thu, 13 Sep 2018 11:07:48 +0200 Subject: [PATCH] Added is_last_packet_in_frame to match is_first_packet_in_frame. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Today we use |is_first_packet_in_frame| to know when a frame begins and the |markerBit| to know when it ends, but the markerbit does not actually mark the end of a frame, it marks the end of a picture. Bug: webrtc:9361 Change-Id: Icc70e6075590cdc31e875a4eb9d489868adbb67c Reviewed-on: https://webrtc-review.googlesource.com/100160 Reviewed-by: Danil Chapovalov Reviewed-by: Erik Språng Commit-Queue: Philip Eliasson Cr-Commit-Position: refs/heads/master@{#24722} --- modules/rtp_rtcp/source/rtp_video_header.h | 1 + modules/video_coding/frame_object.cc | 2 +- modules/video_coding/packet.cc | 2 ++ modules/video_coding/packet.h | 1 + modules/video_coding/packet_buffer.cc | 2 +- .../rtp_frame_reference_finder_unittest.cc | 14 +++++++------- .../video_coding/video_packet_buffer_unittest.cc | 16 ++++++++-------- video/rtp_video_stream_receiver.cc | 8 ++++++-- video/rtp_video_stream_receiver_unittest.cc | 14 +++++++------- 9 files changed, 34 insertions(+), 26 deletions(-) diff --git a/modules/rtp_rtcp/source/rtp_video_header.h b/modules/rtp_rtcp/source/rtp_video_header.h index 68b48847a2..288b7d03a0 100644 --- a/modules/rtp_rtcp/source/rtp_video_header.h +++ b/modules/rtp_rtcp/source/rtp_video_header.h @@ -52,6 +52,7 @@ struct RTPVideoHeader { VideoRotation rotation = VideoRotation::kVideoRotation_0; VideoContentType content_type = VideoContentType::UNSPECIFIED; bool is_first_packet_in_frame = false; + bool is_last_packet_in_frame = false; uint8_t simulcastIdx = 0; VideoCodecType codec = VideoCodecType::kVideoCodecGeneric; diff --git a/modules/video_coding/frame_object.cc b/modules/video_coding/frame_object.cc index 4f7724de3f..32b0cc125c 100644 --- a/modules/video_coding/frame_object.cc +++ b/modules/video_coding/frame_object.cc @@ -72,7 +72,7 @@ RtpFrameObject::RtpFrameObject(PacketBuffer* packet_buffer, VCMPacket* last_packet = packet_buffer_->GetPacket(last_seq_num); RTC_CHECK(last_packet); - RTC_CHECK(last_packet->markerBit); + RTC_CHECK(last_packet->is_last_packet_in_frame); // http://www.etsi.org/deliver/etsi_ts/126100_126199/126114/12.07.00_60/ // ts_126114v120700p.pdf Section 7.4.5. // The MTSI client shall add the payload bytes as defined in this clause diff --git a/modules/video_coding/packet.cc b/modules/video_coding/packet.cc index 691ec62011..dea72cb5ce 100644 --- a/modules/video_coding/packet.cc +++ b/modules/video_coding/packet.cc @@ -28,6 +28,7 @@ VCMPacket::VCMPacket() frameType(kEmptyFrame), codec(kVideoCodecGeneric), is_first_packet_in_frame(false), + is_last_packet_in_frame(false), completeNALU(kNaluUnset), insertStartCode(false), width(0), @@ -52,6 +53,7 @@ VCMPacket::VCMPacket(const uint8_t* ptr, codec(rtpHeader.video_header().codec), is_first_packet_in_frame( rtpHeader.video_header().is_first_packet_in_frame), + is_last_packet_in_frame(rtpHeader.video_header().is_last_packet_in_frame), completeNALU(kNaluIncomplete), insertStartCode(rtpHeader.video_header().codec == kVideoCodecH264 && rtpHeader.video_header().is_first_packet_in_frame), diff --git a/modules/video_coding/packet.h b/modules/video_coding/packet.h index 80a39cabbe..09ab983e8b 100644 --- a/modules/video_coding/packet.h +++ b/modules/video_coding/packet.h @@ -36,6 +36,7 @@ class VCMPacket { VideoCodecType codec; bool is_first_packet_in_frame; + bool is_last_packet_in_frame; VCMNaluCompleteness completeNALU; // Default is kNaluIncomplete. bool insertStartCode; // True if a start code should be inserted before this // packet. diff --git a/modules/video_coding/packet_buffer.cc b/modules/video_coding/packet_buffer.cc index e1172b3749..fe93fb312d 100644 --- a/modules/video_coding/packet_buffer.cc +++ b/modules/video_coding/packet_buffer.cc @@ -107,7 +107,7 @@ bool PacketBuffer::InsertPacket(VCMPacket* packet) { } sequence_buffer_[index].frame_begin = packet->is_first_packet_in_frame; - sequence_buffer_[index].frame_end = packet->markerBit; + sequence_buffer_[index].frame_end = packet->is_last_packet_in_frame; sequence_buffer_[index].seq_num = packet->seqNum; sequence_buffer_[index].continuous = false; sequence_buffer_[index].frame_created = false; diff --git a/modules/video_coding/rtp_frame_reference_finder_unittest.cc b/modules/video_coding/rtp_frame_reference_finder_unittest.cc index 63c94ed2fb..50a87799d1 100644 --- a/modules/video_coding/rtp_frame_reference_finder_unittest.cc +++ b/modules/video_coding/rtp_frame_reference_finder_unittest.cc @@ -88,7 +88,7 @@ class TestRtpFrameReferenceFinder : public ::testing::Test, ref_packet_buffer_->InsertPacket(&packet); packet.seqNum = seq_num_end; - packet.markerBit = true; + packet.is_last_packet_in_frame = true; ref_packet_buffer_->InsertPacket(&packet); std::unique_ptr frame(new RtpFrameObject( @@ -106,7 +106,7 @@ class TestRtpFrameReferenceFinder : public ::testing::Test, VCMPacket packet; packet.codec = kVideoCodecVP8; packet.seqNum = seq_num_start; - packet.markerBit = (seq_num_start == seq_num_end); + packet.is_last_packet_in_frame = (seq_num_start == seq_num_end); packet.frameType = keyframe ? kVideoFrameKey : kVideoFrameDelta; auto& vp8_header = packet.video_header.video_type_header.emplace(); @@ -118,7 +118,7 @@ class TestRtpFrameReferenceFinder : public ::testing::Test, if (seq_num_start != seq_num_end) { packet.seqNum = seq_num_end; - packet.markerBit = true; + packet.is_last_packet_in_frame = true; ref_packet_buffer_->InsertPacket(&packet); } @@ -142,7 +142,7 @@ class TestRtpFrameReferenceFinder : public ::testing::Test, packet.timestamp = pid; packet.codec = kVideoCodecVP9; packet.seqNum = seq_num_start; - packet.markerBit = (seq_num_start == seq_num_end); + packet.is_last_packet_in_frame = (seq_num_start == seq_num_end); packet.frameType = keyframe ? kVideoFrameKey : kVideoFrameDelta; vp9_header.flexible_mode = false; vp9_header.picture_id = pid % (1 << 15); @@ -157,7 +157,7 @@ class TestRtpFrameReferenceFinder : public ::testing::Test, ref_packet_buffer_->InsertPacket(&packet); if (seq_num_start != seq_num_end) { - packet.markerBit = true; + packet.is_last_packet_in_frame = true; vp9_header.ss_data_available = false; packet.seqNum = seq_num_end; ref_packet_buffer_->InsertPacket(&packet); @@ -182,7 +182,7 @@ class TestRtpFrameReferenceFinder : public ::testing::Test, packet.timestamp = pid; packet.codec = kVideoCodecVP9; packet.seqNum = seq_num_start; - packet.markerBit = (seq_num_start == seq_num_end); + packet.is_last_packet_in_frame = (seq_num_start == seq_num_end); packet.frameType = keyframe ? kVideoFrameKey : kVideoFrameDelta; vp9_header.inter_layer_predicted = inter; vp9_header.flexible_mode = true; @@ -197,7 +197,7 @@ class TestRtpFrameReferenceFinder : public ::testing::Test, if (seq_num_start != seq_num_end) { packet.seqNum = seq_num_end; - packet.markerBit = true; + packet.is_last_packet_in_frame = true; ref_packet_buffer_->InsertPacket(&packet); } diff --git a/modules/video_coding/video_packet_buffer_unittest.cc b/modules/video_coding/video_packet_buffer_unittest.cc index f588026ec9..50f5a5c492 100644 --- a/modules/video_coding/video_packet_buffer_unittest.cc +++ b/modules/video_coding/video_packet_buffer_unittest.cc @@ -68,7 +68,7 @@ class TestPacketBuffer : public ::testing::Test, packet.frameType = keyframe == kKeyFrame ? kVideoFrameKey : kVideoFrameDelta; packet.is_first_packet_in_frame = first == kFirst; - packet.markerBit = last == kLast; + packet.is_last_packet_in_frame = last == kLast; packet.sizeBytes = data_size; packet.dataPtr = data; @@ -157,7 +157,7 @@ TEST_F(TestPacketBuffer, NackCount) { packet.seqNum = seq_num; packet.frameType = kVideoFrameKey; packet.is_first_packet_in_frame = true; - packet.markerBit = false; + packet.is_last_packet_in_frame = false; packet.timesNacked = 0; packet_buffer_->InsertPacket(&packet); @@ -172,7 +172,7 @@ TEST_F(TestPacketBuffer, NackCount) { packet_buffer_->InsertPacket(&packet); packet.seqNum++; - packet.markerBit = true; + packet.is_last_packet_in_frame = true; packet.timesNacked = 1; packet_buffer_->InsertPacket(&packet); @@ -524,7 +524,7 @@ class TestPacketBufferH264 : public TestPacketBuffer { } } packet.is_first_packet_in_frame = first == kFirst; - packet.markerBit = last == kLast; + packet.is_last_packet_in_frame = last == kLast; packet.sizeBytes = data_size; packet.dataPtr = data; @@ -605,7 +605,7 @@ TEST_P(TestPacketBufferH264Parameterized, GetBitstreamBufferPadding) { packet.dataPtr = data; packet.sizeBytes = sizeof(data_data); packet.is_first_packet_in_frame = true; - packet.markerBit = true; + packet.is_last_packet_in_frame = true; packet_buffer_->InsertPacket(&packet); ASSERT_EQ(1UL, frames_from_callback_.size()); @@ -748,7 +748,7 @@ TEST_F(TestPacketBuffer, PacketTimestamps) { TEST_F(TestPacketBuffer, IncomingCodecChange) { VCMPacket packet; packet.is_first_packet_in_frame = true; - packet.markerBit = true; + packet.is_last_packet_in_frame = true; packet.sizeBytes = 0; packet.dataPtr = nullptr; @@ -783,7 +783,7 @@ TEST_F(TestPacketBuffer, TooManyNalusInPacket) { packet.seqNum = 1; packet.frameType = kVideoFrameKey; packet.is_first_packet_in_frame = true; - packet.markerBit = true; + packet.is_last_packet_in_frame = true; auto& h264_header = packet.video_header.video_type_header.emplace(); h264_header.nalus_length = kMaxNalusPerPacket; @@ -867,7 +867,7 @@ class TestPacketBufferH264XIsKeyframe : public TestPacketBufferH264 { packet_.seqNum = kSeqNum; packet_.is_first_packet_in_frame = true; - packet_.markerBit = true; + packet_.is_last_packet_in_frame = true; } VCMPacket packet_; diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc index 098f23d953..5939b524df 100644 --- a/video/rtp_video_stream_receiver.cc +++ b/video/rtp_video_stream_receiver.cc @@ -454,6 +454,8 @@ void RtpVideoStreamReceiver::ReceivePacket(const RtpPacketReceived& packet) { VideoSendTiming::kInvalid; webrtc_rtp_header.video_header().playout_delay.min_ms = -1; webrtc_rtp_header.video_header().playout_delay.max_ms = -1; + webrtc_rtp_header.video_header().is_last_packet_in_frame = + webrtc_rtp_header.header.markerBit; packet.GetExtension( &webrtc_rtp_header.video_header().rotation); @@ -470,8 +472,10 @@ void RtpVideoStreamReceiver::ReceivePacket(const RtpPacketReceived& packet) { webrtc_rtp_header.video_header().is_first_packet_in_frame = generic_descriptor_wire.FirstSubFrameInFrame() && generic_descriptor_wire.FirstPacketInSubFrame(); - // TODO(philipel): Add is_last_packet_in_frame to the RtpVideoHeader and use - // the information from the generic descriptor to set it. + webrtc_rtp_header.video_header().is_last_packet_in_frame = + webrtc_rtp_header.header.markerBit || + (generic_descriptor_wire.LastSubFrameInFrame() && + generic_descriptor_wire.LastPacketInSubFrame()); // For now we store the diffs in |generic_descirptor.dependencies|. They // are later recaculated when the frame id is unwrapped. diff --git a/video/rtp_video_stream_receiver_unittest.cc b/video/rtp_video_stream_receiver_unittest.cc index d74d4628d0..9891697e00 100644 --- a/video/rtp_video_stream_receiver_unittest.cc +++ b/video/rtp_video_stream_receiver_unittest.cc @@ -205,8 +205,8 @@ TEST_F(RtpVideoStreamReceiverTest, GenericKeyFrame) { WebRtcRTPHeader rtp_header = {}; const std::vector data({1, 2, 3, 4}); rtp_header.header.sequenceNumber = 1; - rtp_header.header.markerBit = 1; rtp_header.video_header().is_first_packet_in_frame = true; + rtp_header.video_header().is_last_packet_in_frame = true; rtp_header.frameType = kVideoFrameKey; rtp_header.video_header().codec = kVideoCodecGeneric; mock_on_complete_frame_callback_.AppendExpectedBitstream(data.data(), @@ -262,8 +262,8 @@ TEST_F(RtpVideoStreamReceiverTest, GenericKeyFrameBitstreamError) { WebRtcRTPHeader rtp_header = {}; const std::vector data({1, 2, 3, 4}); rtp_header.header.sequenceNumber = 1; - rtp_header.header.markerBit = 1; rtp_header.video_header().is_first_packet_in_frame = true; + rtp_header.video_header().is_last_packet_in_frame = true; rtp_header.frameType = kVideoFrameKey; rtp_header.video_header().codec = kVideoCodecGeneric; constexpr uint8_t expected_bitsteam[] = {1, 2, 3, 0xff}; @@ -315,9 +315,9 @@ TEST_P(RtpVideoStreamReceiverTestH264, InBandSpsPps) { std::vector idr_data; WebRtcRTPHeader idr_packet = GetDefaultPacket(); AddIdr(&idr_packet, 1); - idr_packet.video_header().is_first_packet_in_frame = true; idr_packet.header.sequenceNumber = 2; - idr_packet.header.markerBit = 1; + idr_packet.video_header().is_first_packet_in_frame = true; + idr_packet.video_header().is_last_packet_in_frame = true; idr_packet.frameType = kVideoFrameKey; idr_data.insert(idr_data.end(), {0x65, 1, 2, 3}); mock_on_complete_frame_callback_.AppendExpectedBitstream( @@ -357,8 +357,8 @@ TEST_P(RtpVideoStreamReceiverTestH264, OutOfBandFmtpSpsPps) { idr_packet.header.payloadType = kPayloadType; idr_packet.video_header().is_first_packet_in_frame = true; idr_packet.header.sequenceNumber = 2; - idr_packet.header.markerBit = 1; idr_packet.video_header().is_first_packet_in_frame = true; + idr_packet.video_header().is_last_packet_in_frame = true; idr_packet.frameType = kVideoFrameKey; idr_packet.video_header().codec = kVideoCodecH264; data.insert(data.end(), {1, 2, 3}); @@ -377,8 +377,8 @@ TEST_F(RtpVideoStreamReceiverTest, PaddingInMediaStream) { data.insert(data.end(), {1, 2, 3}); header.header.payloadType = 99; header.video_header().is_first_packet_in_frame = true; + header.video_header().is_last_packet_in_frame = true; header.header.sequenceNumber = 2; - header.header.markerBit = true; header.frameType = kVideoFrameKey; header.video_header().codec = kVideoCodecGeneric; mock_on_complete_frame_callback_.AppendExpectedBitstream(data.data(), @@ -410,8 +410,8 @@ TEST_F(RtpVideoStreamReceiverTest, RequestKeyframeIfFirstFrameIsDelta) { WebRtcRTPHeader rtp_header = {}; const std::vector data({1, 2, 3, 4}); rtp_header.header.sequenceNumber = 1; - rtp_header.header.markerBit = 1; rtp_header.video_header().is_first_packet_in_frame = true; + rtp_header.video_header().is_last_packet_in_frame = true; rtp_header.frameType = kVideoFrameDelta; rtp_header.video_header().codec = kVideoCodecGeneric;