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;