From c1600c56959fcf25ca9e3a93342155d781c1bcd2 Mon Sep 17 00:00:00 2001 From: danilchap Date: Wed, 26 Oct 2016 03:33:11 -0700 Subject: [PATCH] Follow standard sending CVO rtp header extension Include CVO in key frame. Include CVO in delta frame when rotation changes. Include CVO when it is non zero to support current receiver implementation. BUG=webrtc:6600 Review-Url: https://codereview.webrtc.org/2452583002 Cr-Commit-Position: refs/heads/master@{#14784} --- .../rtp_rtcp/source/rtp_sender_unittest.cc | 76 ++++++++++++------- .../rtp_rtcp/source/rtp_sender_video.cc | 17 ++++- .../rtp_rtcp/source/rtp_sender_video.h | 3 + 3 files changed, 65 insertions(+), 31 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc index cdaca2a829..8fa21d931d 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -242,21 +242,6 @@ class RtpSenderVideoTest : public RtpSenderTest { new RTPSenderVideo(&fake_clock_, rtp_sender_.get())); } std::unique_ptr rtp_sender_video_; - - void VerifyCVOPacket(const RtpPacketReceived& rtp_packet, - bool expect_cvo, - uint16_t seq_num, - VideoRotation expected_rotation) { - EXPECT_EQ(payload_, rtp_packet.PayloadType()); - EXPECT_EQ(seq_num, rtp_packet.SequenceNumber()); - EXPECT_EQ(kTimestamp, rtp_packet.Timestamp()); - EXPECT_EQ(rtp_sender_->SSRC(), rtp_packet.Ssrc()); - EXPECT_EQ(0U, rtp_packet.Csrcs().size()); - EXPECT_EQ(0U, rtp_packet.padding_size()); - VideoRotation actual_rotation; - EXPECT_TRUE(rtp_packet.GetExtension(&actual_rotation)); - EXPECT_EQ(expected_rotation, actual_rotation); - } }; TEST_F(RtpSenderTestWithoutPacer, @@ -1324,27 +1309,64 @@ TEST_F(RtpSenderTestWithoutPacer, RespectsNackBitrateLimit) { EXPECT_EQ(kNumPackets * 2, transport_.packets_sent()); } -// Verify that all packets of a frame have CVO byte set. -TEST_F(RtpSenderVideoTest, SendVideoWithCVO) { +TEST_F(RtpSenderVideoTest, KeyFrameHasCVO) { uint8_t kFrame[kMaxPacketLength]; - RTPVideoHeader hdr = {0}; - hdr.rotation = kVideoRotation_90; - EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension( kRtpExtensionVideoRotation, kVideoRotationExtensionId)); - EXPECT_EQ( - RtpUtility::Word32Align(kRtpOneByteHeaderLength + kVideoRotationLength), - rtp_sender_->RtpHeaderExtensionLength()); + RTPVideoHeader hdr = {0}; + hdr.rotation = kVideoRotation_0; rtp_sender_video_->SendVideo(kRtpVideoGeneric, kVideoFrameKey, kPayload, kTimestamp, 0, kFrame, sizeof(kFrame), nullptr, &hdr); - // Verify that this packet does have CVO byte. - VerifyCVOPacket(transport_.sent_packets_[0], true, kSeqNum, hdr.rotation); + VideoRotation rotation; + EXPECT_TRUE( + transport_.last_sent_packet().GetExtension(&rotation)); + EXPECT_EQ(kVideoRotation_0, rotation); +} - // Verify that this packet does have CVO byte. - VerifyCVOPacket(transport_.sent_packets_[1], true, kSeqNum + 1, hdr.rotation); +TEST_F(RtpSenderVideoTest, DeltaFrameHasCVOWhenChanged) { + uint8_t kFrame[kMaxPacketLength]; + EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension( + kRtpExtensionVideoRotation, kVideoRotationExtensionId)); + + RTPVideoHeader hdr = {0}; + hdr.rotation = kVideoRotation_90; + EXPECT_TRUE(rtp_sender_video_->SendVideo(kRtpVideoGeneric, kVideoFrameKey, + kPayload, kTimestamp, 0, kFrame, + sizeof(kFrame), nullptr, &hdr)); + + hdr.rotation = kVideoRotation_0; + EXPECT_TRUE(rtp_sender_video_->SendVideo(kRtpVideoGeneric, kVideoFrameDelta, + kPayload, kTimestamp + 1, 0, kFrame, + sizeof(kFrame), nullptr, &hdr)); + + VideoRotation rotation; + EXPECT_TRUE( + transport_.last_sent_packet().GetExtension(&rotation)); + EXPECT_EQ(kVideoRotation_0, rotation); +} + +TEST_F(RtpSenderVideoTest, DeltaFrameHasCVOWhenNonZero) { + uint8_t kFrame[kMaxPacketLength]; + EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension( + kRtpExtensionVideoRotation, kVideoRotationExtensionId)); + + RTPVideoHeader hdr = {0}; + hdr.rotation = kVideoRotation_90; + EXPECT_TRUE(rtp_sender_video_->SendVideo(kRtpVideoGeneric, kVideoFrameKey, + kPayload, kTimestamp, 0, kFrame, + sizeof(kFrame), nullptr, &hdr)); + + EXPECT_TRUE(rtp_sender_video_->SendVideo(kRtpVideoGeneric, kVideoFrameDelta, + kPayload, kTimestamp + 1, 0, kFrame, + sizeof(kFrame), nullptr, &hdr)); + + VideoRotation rotation; + EXPECT_TRUE( + transport_.last_sent_packet().GetExtension(&rotation)); + EXPECT_EQ(kVideoRotation_90, rotation); } // Make sure rotation is parsed correctly when the Camera (C) and Flip (F) bits diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc index a4a2b150b4..87a8226d23 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -49,7 +49,9 @@ RTPSenderVideo::RTPSenderVideo(Clock* clock, RTPSender* rtp_sender) : rtp_sender_(rtp_sender), clock_(clock), fec_bitrate_(1000, RateStatistics::kBpsScale), - video_bitrate_(1000, RateStatistics::kBpsScale) {} + video_bitrate_(1000, RateStatistics::kBpsScale) { + encoder_checker_.Detach(); +} RTPSenderVideo::~RTPSenderVideo() {} @@ -228,6 +230,7 @@ bool RTPSenderVideo::SendVideo(RtpVideoCodecTypes video_type, size_t payload_size, const RTPFragmentationHeader* fragmentation, const RTPVideoHeader* video_header) { + RTC_DCHECK_CALLED_SEQUENTIALLY(&encoder_checker_); if (payload_size == 0) return false; @@ -246,9 +249,15 @@ bool RTPSenderVideo::SendVideo(RtpVideoCodecTypes video_type, // packet in each group of packets which make up another type of frame // (e.g. a P-Frame) only if the current value is different from the previous // value sent. - // Here we are adding it to every packet of every frame at this point. - if (video_header && video_header->rotation != kVideoRotation_0) - rtp_header->SetExtension(video_header->rotation); + if (video_header) { + // Set rotation when key frame or when changed (to follow standard). + // Or when different from 0 (to follow current receiver implementation). + VideoRotation current_rotation = video_header->rotation; + if (frame_type == kVideoFrameKey || current_rotation != last_rotation_ || + current_rotation != kVideoRotation_0) + rtp_header->SetExtension(current_rotation); + last_rotation_ = current_rotation; + } size_t packet_capacity = rtp_sender_->MaxPayloadLength() - FecPacketOverhead() - diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.h b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.h index 96ddf41921..f69b8e370a 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.h @@ -17,6 +17,7 @@ #include "webrtc/base/criticalsection.h" #include "webrtc/base/onetimeevent.h" #include "webrtc/base/rate_statistics.h" +#include "webrtc/base/sequenced_task_checker.h" #include "webrtc/base/thread_annotations.h" #include "webrtc/common_types.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" @@ -89,9 +90,11 @@ class RTPSenderVideo { // Should never be held when calling out of this class. rtc::CriticalSection crit_; + rtc::SequencedTaskChecker encoder_checker_; RtpVideoCodecTypes video_type_ = kRtpVideoGeneric; int32_t retransmission_settings_ GUARDED_BY(crit_) = kRetransmitBaseLayer; + VideoRotation last_rotation_ GUARDED_BY(encoder_checker_) = kVideoRotation_0; // FEC bool fec_enabled_ GUARDED_BY(crit_) = false;