diff --git a/talk/media/base/fakenetworkinterface.h b/talk/media/base/fakenetworkinterface.h index 424101e419..5178e67eeb 100644 --- a/talk/media/base/fakenetworkinterface.h +++ b/talk/media/base/fakenetworkinterface.h @@ -123,11 +123,6 @@ class FakeNetworkInterface : public MediaChannel::NetworkInterface, return new rtc::Buffer(rtcp_packets_[index]); } - // Indicate that |n|'th packet for |ssrc| should be dropped. - void AddPacketDrop(uint32 ssrc, uint32 n) { - drop_map_[ssrc].insert(n); - } - int sendbuf_size() const { return sendbuf_size_; } int recvbuf_size() const { return recvbuf_size_; } rtc::DiffServCodePoint dscp() const { return dscp_; } @@ -143,15 +138,6 @@ class FakeNetworkInterface : public MediaChannel::NetworkInterface, } sent_ssrcs_[cur_ssrc]++; - // Check if we need to drop this packet. - std::map >::iterator itr = - drop_map_.find(cur_ssrc); - if (itr != drop_map_.end() && - itr->second.count(sent_ssrcs_[cur_ssrc]) > 0) { - // "Drop" the packet. - return true; - } - rtp_packets_.push_back(*packet); if (conf_) { rtc::Buffer buffer_copy(*packet); diff --git a/talk/media/base/videoengine_unittest.h b/talk/media/base/videoengine_unittest.h index 1e79d7fa61..388179587c 100644 --- a/talk/media/base/videoengine_unittest.h +++ b/talk/media/base/videoengine_unittest.h @@ -859,6 +859,19 @@ class VideoMediaChannelTest : public testing::Test, EXPECT_GT(info.receivers[0].framerate_decoded, 0); EXPECT_GT(info.receivers[0].framerate_output, 0); } + + cricket::VideoSenderInfo GetSenderStats(size_t i) { + cricket::VideoMediaInfo info; + EXPECT_TRUE(channel_->GetStats(&info)); + return info.senders[i]; + } + + cricket::VideoReceiverInfo GetReceiverStats(size_t i) { + cricket::VideoMediaInfo info; + EXPECT_TRUE(channel_->GetStats(&info)); + return info.receivers[i]; + } + // Test that stats work properly for a conf call with multiple recv streams. void GetStatsMultipleRecvStreams() { cricket::FakeVideoRenderer renderer1, renderer2; @@ -886,25 +899,28 @@ class VideoMediaChannelTest : public testing::Test, renderer1, 1, DefaultCodec().width, DefaultCodec().height, kTimeout); EXPECT_FRAME_ON_RENDERER_WAIT( renderer2, 1, DefaultCodec().width, DefaultCodec().height, kTimeout); + + EXPECT_TRUE(channel_->SetSend(false)); + cricket::VideoMediaInfo info; EXPECT_TRUE(channel_->GetStats(&info)); - ASSERT_EQ(1U, info.senders.size()); // TODO(whyuan): bytes_sent and bytes_rcvd are different. Are both payload? // For webrtc, bytes_sent does not include the RTP header length. - EXPECT_GT(info.senders[0].bytes_sent, 0); - EXPECT_EQ(NumRtpPackets(), info.senders[0].packets_sent); - EXPECT_EQ(DefaultCodec().width, info.senders[0].send_frame_width); - EXPECT_EQ(DefaultCodec().height, info.senders[0].send_frame_height); + EXPECT_GT(GetSenderStats(0).bytes_sent, 0); + EXPECT_EQ_WAIT(NumRtpPackets(), GetSenderStats(0).packets_sent, kTimeout); + EXPECT_EQ(DefaultCodec().width, GetSenderStats(0).send_frame_width); + EXPECT_EQ(DefaultCodec().height, GetSenderStats(0).send_frame_height); ASSERT_EQ(2U, info.receivers.size()); for (size_t i = 0; i < info.receivers.size(); ++i) { - EXPECT_EQ(1U, info.receivers[i].ssrcs().size()); - EXPECT_EQ(i + 1, info.receivers[i].ssrcs()[0]); - EXPECT_EQ(NumRtpBytes(), info.receivers[i].bytes_rcvd); - EXPECT_EQ(NumRtpPackets(), info.receivers[i].packets_rcvd); - EXPECT_EQ(DefaultCodec().width, info.receivers[i].frame_width); - EXPECT_EQ(DefaultCodec().height, info.receivers[i].frame_height); + EXPECT_EQ(1U, GetReceiverStats(i).ssrcs().size()); + EXPECT_EQ(i + 1, GetReceiverStats(i).ssrcs()[0]); + EXPECT_EQ_WAIT(NumRtpBytes(), GetReceiverStats(i).bytes_rcvd, kTimeout); + EXPECT_EQ_WAIT(NumRtpPackets(), GetReceiverStats(i).packets_rcvd, + kTimeout); + EXPECT_EQ(DefaultCodec().width, GetReceiverStats(i).frame_width); + EXPECT_EQ(DefaultCodec().height, GetReceiverStats(i).frame_height); } } // Test that stats work properly for a conf call with multiple send streams. @@ -1065,7 +1081,7 @@ class VideoMediaChannelTest : public testing::Test, EXPECT_TRUE(channel_->SetRenderer(kDefaultReceiveSsrc, &renderer_)); EXPECT_TRUE(SendFrame()); EXPECT_FRAME_WAIT(1, DefaultCodec().width, DefaultCodec().height, kTimeout); - EXPECT_GE(2, NumRtpPackets()); + EXPECT_GT(NumRtpPackets(), 0); uint32 ssrc = 0; size_t last_packet = NumRtpPackets() - 1; rtc::scoped_ptr @@ -1707,7 +1723,7 @@ class VideoMediaChannelTest : public testing::Test, SendAndReceive(codec); // Test sending and receiving on second stream. EXPECT_EQ_WAIT(1, renderer2_.num_rendered_frames(), kTimeout); - EXPECT_EQ(2, NumRtpPackets()); + EXPECT_GT(NumRtpPackets(), 0); EXPECT_EQ(1, renderer2_.num_rendered_frames()); } diff --git a/webrtc/modules/pacing/include/paced_sender.h b/webrtc/modules/pacing/include/paced_sender.h index 730d3b7028..645999d507 100644 --- a/webrtc/modules/pacing/include/paced_sender.h +++ b/webrtc/modules/pacing/include/paced_sender.h @@ -126,9 +126,6 @@ class PacedSender : public Module { // Process any pending packets in the queue(s). int32_t Process() override; - protected: - virtual bool ProbingExperimentIsEnabled() const; - private: // Updates the number of bytes that can be sent for the next time interval. void UpdateBytesPerInterval(int64_t delta_time_in_ms) diff --git a/webrtc/modules/pacing/paced_sender.cc b/webrtc/modules/pacing/paced_sender.cc index 7c842bff02..d5df480974 100644 --- a/webrtc/modules/pacing/paced_sender.cc +++ b/webrtc/modules/pacing/paced_sender.cc @@ -361,8 +361,11 @@ int32_t PacedSender::Process() { } } + if (!packets_->Empty()) + return 0; + size_t padding_needed; - if (prober_->IsProbing() && ProbingExperimentIsEnabled()) + if (prober_->IsProbing()) padding_needed = prober_->RecommendedPacketSize(); else padding_needed = padding_budget_->bytes_remaining(); @@ -407,9 +410,4 @@ void PacedSender::UpdateBytesPerInterval(int64_t delta_time_ms) { media_budget_->IncreaseBudget(delta_time_ms); padding_budget_->IncreaseBudget(delta_time_ms); } - -bool PacedSender::ProbingExperimentIsEnabled() const { - return webrtc::field_trial::FindFullName("WebRTC-BitrateProbing") == - "Enabled"; -} } // namespace webrtc diff --git a/webrtc/modules/pacing/paced_sender_unittest.cc b/webrtc/modules/pacing/paced_sender_unittest.cc index 0730062088..a00b5fa58d 100644 --- a/webrtc/modules/pacing/paced_sender_unittest.cc +++ b/webrtc/modules/pacing/paced_sender_unittest.cc @@ -743,22 +743,6 @@ TEST_F(PacedSenderTest, ProbingWithInitialFrame) { } } -class ProbingPacedSender : public PacedSender { - public: - ProbingPacedSender(Clock* clock, - Callback* callback, - int bitrate_kbps, - int max_bitrate_kbps, - int min_bitrate_kbps) - : PacedSender(clock, - callback, - bitrate_kbps, - max_bitrate_kbps, - min_bitrate_kbps) {} - - bool ProbingExperimentIsEnabled() const override { return true; } -}; - TEST_F(PacedSenderTest, ProbingWithTooSmallInitialFrame) { const int kNumPackets = 11; const int kNumDeltas = kNumPackets - 1; @@ -770,9 +754,8 @@ TEST_F(PacedSenderTest, ProbingWithTooSmallInitialFrame) { std::list expected_deltas_list(expected_deltas, expected_deltas + kNumPackets - 1); PacedSenderProbing callback(expected_deltas_list, &clock_); - send_bucket_.reset( - new ProbingPacedSender(&clock_, &callback, kInitialBitrateKbps, - kPaceMultiplier * kInitialBitrateKbps, 0)); + send_bucket_.reset(new PacedSender(&clock_, &callback, kInitialBitrateKbps, + kPaceMultiplier * kInitialBitrateKbps, 0)); for (int i = 0; i < kNumPackets - 5; ++i) { EXPECT_FALSE(send_bucket_->SendPacket( diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc index f683f14bd4..44ac965413 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc @@ -566,51 +566,38 @@ size_t RTPSender::TrySendRedundantPayloads(size_t bytes_to_send) { return bytes_to_send - bytes_left; } -size_t RTPSender::BuildPaddingPacket(uint8_t* packet, size_t header_length) { - size_t padding_bytes_in_packet = kMaxPaddingLength; +void RTPSender::BuildPaddingPacket(uint8_t* packet, + size_t header_length, + size_t padding_length) { packet[0] |= 0x20; // Set padding bit. int32_t *data = reinterpret_cast(&(packet[header_length])); // Fill data buffer with random data. - for (size_t j = 0; j < (padding_bytes_in_packet >> 2); ++j) { + for (size_t j = 0; j < (padding_length >> 2); ++j) { data[j] = rand(); // NOLINT } // Set number of padding bytes in the last byte of the packet. - packet[header_length + padding_bytes_in_packet - 1] = - static_cast(padding_bytes_in_packet); - return padding_bytes_in_packet; + packet[header_length + padding_length - 1] = + static_cast(padding_length); } -size_t RTPSender::TrySendPadData(size_t bytes) { - int64_t capture_time_ms; - uint32_t timestamp; - { - CriticalSectionScoped cs(send_critsect_.get()); - timestamp = timestamp_; - capture_time_ms = capture_time_ms_; - if (last_timestamp_time_ms_ > 0) { - timestamp += - (clock_->TimeInMilliseconds() - last_timestamp_time_ms_) * 90; - capture_time_ms += - (clock_->TimeInMilliseconds() - last_timestamp_time_ms_); - } - } - return SendPadData(timestamp, capture_time_ms, bytes); -} - -size_t RTPSender::SendPadData(uint32_t timestamp, - int64_t capture_time_ms, - size_t bytes) { - size_t padding_bytes_in_packet = 0; +size_t RTPSender::SendPadData(size_t bytes, + bool timestamp_provided, + uint32_t timestamp, + int64_t capture_time_ms) { + // Always send full padding packets. This is accounted for by the PacedSender, + // which will make sure we don't send too much padding even if a single packet + // is larger than requested. + size_t padding_bytes_in_packet = + std::min(MaxDataPayloadLength(), kMaxPaddingLength); size_t bytes_sent = 0; bool using_transport_seq = rtp_header_extension_map_.IsRegistered( kRtpExtensionTransportSequenceNumber) && packet_router_; for (; bytes > 0; bytes -= padding_bytes_in_packet) { - // Always send full padding packets. - if (bytes < kMaxPaddingLength) - bytes = kMaxPaddingLength; + if (bytes < padding_bytes_in_packet) + bytes = padding_bytes_in_packet; uint32_t ssrc; uint16_t sequence_number; @@ -618,8 +605,10 @@ size_t RTPSender::SendPadData(uint32_t timestamp, bool over_rtx; { CriticalSectionScoped cs(send_critsect_.get()); - // Only send padding packets following the last packet of a frame, - // indicated by the marker bit. + if (!timestamp_provided) { + timestamp = timestamp_; + capture_time_ms = capture_time_ms_; + } if (rtx_ == kRtxOff) { // Without RTX we can't send padding in the middle of frames. if (!last_packet_marker_bit_) @@ -635,6 +624,15 @@ size_t RTPSender::SendPadData(uint32_t timestamp, if (!media_has_been_sent_ && !rtp_header_extension_map_.IsRegistered( kRtpExtensionAbsoluteSendTime)) return 0; + // Only change change the timestamp of padding packets sent over RTX. + // Padding only packets over RTP has to be sent as part of a media + // frame (and therefore the same timestamp). + if (last_timestamp_time_ms_ > 0) { + timestamp += + (clock_->TimeInMilliseconds() - last_timestamp_time_ms_) * 90; + capture_time_ms += + (clock_->TimeInMilliseconds() - last_timestamp_time_ms_); + } ssrc = ssrc_rtx_; sequence_number = sequence_number_rtx_; ++sequence_number_rtx_; @@ -647,9 +645,7 @@ size_t RTPSender::SendPadData(uint32_t timestamp, size_t header_length = CreateRtpHeader(padding_packet, payload_type, ssrc, false, timestamp, sequence_number, std::vector()); - assert(header_length != static_cast(-1)); - padding_bytes_in_packet = BuildPaddingPacket(padding_packet, header_length); - assert(padding_bytes_in_packet <= bytes); + BuildPaddingPacket(padding_packet, header_length, padding_bytes_in_packet); size_t length = padding_bytes_in_packet + header_length; int64_t now_ms = clock_->TimeInMilliseconds(); @@ -999,7 +995,7 @@ size_t RTPSender::TimeToSendPadding(size_t bytes) { } size_t bytes_sent = TrySendRedundantPayloads(bytes); if (bytes_sent < bytes) - bytes_sent += TrySendPadData(bytes - bytes_sent); + bytes_sent += SendPadData(bytes - bytes_sent, false, 0, 0); return bytes_sent; } diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.h b/webrtc/modules/rtp_rtcp/source/rtp_sender.h index f10cb75100..6d24ee1376 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.h @@ -296,9 +296,10 @@ class RTPSender : public RTPSenderInterface { int32_t SetFecParameters(const FecProtectionParams *delta_params, const FecProtectionParams *key_params); - size_t SendPadData(uint32_t timestamp, - int64_t capture_time_ms, - size_t bytes); + size_t SendPadData(size_t bytes, + bool timestamp_provided, + uint32_t timestamp, + int64_t capture_time_ms); // Called on update of RTP statistics. void RegisterRtpStatisticsCallback(StreamDataCountersCallback* callback); @@ -340,9 +341,10 @@ class RTPSender : public RTPSenderInterface { // Return the number of bytes sent. Note that both of these functions may // return a larger value that their argument. size_t TrySendRedundantPayloads(size_t bytes); - size_t TrySendPadData(size_t bytes); - size_t BuildPaddingPacket(uint8_t* packet, size_t header_length); + void BuildPaddingPacket(uint8_t* packet, + size_t header_length, + size_t padding_length); void BuildRtxPacket(uint8_t* buffer, size_t* length, uint8_t* buffer_rtx); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc index 9086ea1cc5..555f2274b0 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -739,6 +739,7 @@ TEST_F(RtpSenderTest, SendPadding) { int64_t capture_time_ms = fake_clock_.TimeInMilliseconds(); int rtp_length_int = rtp_sender_->BuildRTPheader( packet_, kPayload, kMarkerBit, timestamp, capture_time_ms); + const uint32_t media_packet_timestamp = timestamp; ASSERT_NE(-1, rtp_length_int); size_t rtp_length = static_cast(rtp_length_int); @@ -779,11 +780,13 @@ TEST_F(RtpSenderTest, SendPadding) { &rtp_header)); EXPECT_EQ(kMaxPaddingLength, rtp_header.paddingLength); - // Verify sequence number and timestamp. + // Verify sequence number and timestamp. The timestamp should be the same + // as the last media packet. EXPECT_EQ(seq_num++, rtp_header.sequenceNumber); - EXPECT_EQ(timestamp, rtp_header.timestamp); + EXPECT_EQ(media_packet_timestamp, rtp_header.timestamp); // Verify transmission time offset. - EXPECT_EQ(0, rtp_header.extension.transmissionTimeOffset); + int offset = timestamp - media_packet_timestamp; + EXPECT_EQ(offset, rtp_header.extension.transmissionTimeOffset); uint64_t expected_send_time = ConvertMsToAbsSendTime(fake_clock_.TimeInMilliseconds()); EXPECT_EQ(expected_send_time, rtp_header.extension.absoluteSendTime); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc index f44cda157a..1de16404ec 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -320,7 +320,7 @@ int32_t RTPSenderVideo::SendVideo(const RtpVideoCodecTypes videoType, // a lock. It'll be a no-op if it's not registered. // TODO(guoweis): For now, all packets sent will carry the CVO such that // the RTP header length is consistent, although the receiver side will - // only exam the packets with market bit set. + // only exam the packets with marker bit set. size_t packetSize = payloadSize + rtp_header_length; RtpUtility::RtpHeaderParser rtp_parser(dataBuffer, packetSize); RTPHeader rtp_header; diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc index 7485dc9fc8..00e7279adb 100644 --- a/webrtc/video/end_to_end_tests.cc +++ b/webrtc/video/end_to_end_tests.cc @@ -382,7 +382,7 @@ TEST_F(EndToEndTest, ReceivesAndRetransmitsNack) { if (dropped_packets_.find(header.sequenceNumber) != dropped_packets_.end()) { retransmitted_packets_.insert(header.sequenceNumber); - if (nacks_left_ == 0 && + if (nacks_left_ <= 0 && retransmitted_packets_.size() == dropped_packets_.size()) { observation_complete_->Set(); } @@ -392,7 +392,7 @@ TEST_F(EndToEndTest, ReceivesAndRetransmitsNack) { ++sent_rtp_packets_; // Enough NACKs received, stop dropping packets. - if (nacks_left_ == 0) + if (nacks_left_ <= 0) return SEND_PACKET; // Check if it's time for a new loss burst. @@ -678,7 +678,9 @@ void EndToEndTest::TestReceivedFecPacketsNotNacked( // This test drops second RTP packet with a marker bit set, makes sure it's // retransmitted and renders. Retransmission SSRCs are also checked. void EndToEndTest::DecodesRetransmittedFrame(bool use_rtx, bool use_red) { - static const int kDroppedFrameNumber = 2; + // Must be set high enough to allow the bitrate probing to finish. + static const int kMinProbePackets = 30; + static const int kDroppedFrameNumber = kMinProbePackets + 1; class RetransmissionObserver : public test::EndToEndTest, public I420FrameCallback { public: @@ -688,6 +690,7 @@ void EndToEndTest::DecodesRetransmittedFrame(bool use_rtx, bool use_red) { retransmission_ssrc_(use_rtx ? kSendRtxSsrcs[0] : kSendSsrcs[0]), retransmission_payload_type_(GetPayloadType(use_rtx, use_red)), marker_bits_observed_(0), + num_packets_observed_(0), retransmitted_timestamp_(0), frame_retransmitted_(false) {} @@ -696,6 +699,14 @@ void EndToEndTest::DecodesRetransmittedFrame(bool use_rtx, bool use_red) { RTPHeader header; EXPECT_TRUE(parser_->Parse(packet, length, &header)); + // We accept some padding or RTX packets in the beginning to enable + // bitrate probing. + if (num_packets_observed_++ < kMinProbePackets && + header.payloadType != payload_type_) { + EXPECT_TRUE(retransmission_payload_type_ == header.payloadType || + length == header.headerLength + header.paddingLength); + return SEND_PACKET; + } if (header.timestamp == retransmitted_timestamp_) { EXPECT_EQ(retransmission_ssrc_, header.ssrc); EXPECT_EQ(retransmission_payload_type_, header.payloadType); @@ -706,8 +717,8 @@ void EndToEndTest::DecodesRetransmittedFrame(bool use_rtx, bool use_red) { EXPECT_EQ(kSendSsrcs[0], header.ssrc); EXPECT_EQ(payload_type_, header.payloadType); - // Found the second frame's final packet, drop this and expect a - // retransmission. + // Found the final packet of the frame to inflict loss to, drop this and + // expect a retransmission. if (header.markerBit && ++marker_bits_observed_ == kDroppedFrameNumber) { retransmitted_timestamp_ = header.timestamp; return DROP_PACKET; @@ -762,6 +773,7 @@ void EndToEndTest::DecodesRetransmittedFrame(bool use_rtx, bool use_red) { const uint32_t retransmission_ssrc_; const int retransmission_payload_type_; int marker_bits_observed_; + int num_packets_observed_; uint32_t retransmitted_timestamp_; bool frame_retransmitted_; } test(use_rtx, use_red); diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc index 2ff411d932..b396cd6836 100644 --- a/webrtc/video/video_send_stream_tests.cc +++ b/webrtc/video/video_send_stream_tests.cc @@ -539,10 +539,15 @@ void VideoSendStreamTest::TestPacketFragmentationSize(VideoFormat format, TriggerLossReport(header); if (test_generic_packetization_) { - size_t overhead = header.headerLength + header.paddingLength + - (1 /* Generic header */); - if (use_fec_) - overhead += 1; // RED for FEC header. + size_t overhead = header.headerLength + header.paddingLength; + // Only remove payload header and RED header if the packet actually + // contains payload. + if (length > overhead) { + overhead += (1 /* Generic header */); + if (use_fec_) + overhead += 1; // RED for FEC header. + } + EXPECT_GE(length, overhead); accumulated_payload_ += length - overhead; } @@ -1843,19 +1848,21 @@ class VP9HeaderObeserver : public test::SendTest { size_t payload_length = length - header.headerLength - header.paddingLength; - bool parse_vp9header_successful = - vp9depacketizer.Parse(&vp9payload, vp9_packet, payload_length); - bool is_vp9_codec_type = - vp9payload.type.Video.codec == RtpVideoCodecTypes::kRtpVideoVp9; - EXPECT_TRUE(parse_vp9header_successful); - EXPECT_TRUE(is_vp9_codec_type); + if (payload_length > 0) { + bool parse_vp9header_successful = + vp9depacketizer.Parse(&vp9payload, vp9_packet, payload_length); + bool is_vp9_codec_type = + vp9payload.type.Video.codec == RtpVideoCodecTypes::kRtpVideoVp9; + EXPECT_TRUE(parse_vp9header_successful); + EXPECT_TRUE(is_vp9_codec_type); - RTPVideoHeaderVP9* vp9videoHeader = - &vp9payload.type.Video.codecHeader.VP9; - if (parse_vp9header_successful && is_vp9_codec_type) { - InspectHeader(vp9videoHeader); - } else { - observation_complete_->Set(); + RTPVideoHeaderVP9* vp9videoHeader = + &vp9payload.type.Video.codecHeader.VP9; + if (parse_vp9header_successful && is_vp9_codec_type) { + InspectHeader(vp9videoHeader); + } else { + observation_complete_->Set(); + } } }