From 1788dcb506c5a28a10db07d5a9139a29ce72f350 Mon Sep 17 00:00:00 2001 From: Niels Moller Date: Thu, 9 Aug 2018 06:18:57 +0000 Subject: [PATCH] Revert "Refactor RtpVideoStreamReceiver without RtpReceiver." MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 0b9e01d605a174a52635626c885738a222abff46. Reason for revert: Appears to breaks AV sync, failing perftests: CallPerfTest.PlaysOutAudioAndVideoInSyncWithVideoNtpDrift CallPerfTest.PlaysOutAudioAndVideoInSyncWithAudioFasterThanVideoDrift CallPerfTest.PlaysOutAudioAndVideoInSyncWithVideoFasterThanAudioDrift Original change's description: > Refactor RtpVideoStreamReceiver without RtpReceiver. > > Bug: webrtc:7135 > Change-Id: Iabf3330e579b892efc160683f9f90efbf6ff9a40 > Reviewed-on: https://webrtc-review.googlesource.com/92398 > Commit-Queue: Niels Moller > Reviewed-by: Erik Språng > Reviewed-by: Magnus Jedvert > Reviewed-by: Danil Chapovalov > Cr-Commit-Position: refs/heads/master@{#24232} TBR=danilchap@webrtc.org,magjed@webrtc.org,nisse@webrtc.org,sprang@webrtc.org Change-Id: I70a1dcb519c51937e35e04ac82b2ab495bec0a3d No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: webrtc:7135 Reviewed-on: https://webrtc-review.googlesource.com/93260 Reviewed-by: Niels Moller Commit-Queue: Niels Moller Cr-Commit-Position: refs/heads/master@{#24235} --- call/call.cc | 2 - modules/rtp_rtcp/source/rtp_format.cc | 5 +- video/rtp_video_stream_receiver.cc | 111 +++++++++----------------- video/rtp_video_stream_receiver.h | 20 ++--- video/video_receive_stream.cc | 4 +- 5 files changed, 51 insertions(+), 91 deletions(-) diff --git a/call/call.cc b/call/call.cc index 113109a823..a9fbceef58 100644 --- a/call/call.cc +++ b/call/call.cc @@ -1277,7 +1277,6 @@ PacketReceiver::DeliveryStatus Call::DeliverRtp(MediaType media_type, return DELIVERY_OK; } } else if (media_type == MediaType::VIDEO) { - parsed_packet.set_payload_type_frequency(kVideoPayloadTypeFrequency); if (video_receiver_controller_.OnRtpPacket(parsed_packet)) { received_bytes_per_second_counter_.Add(length); received_video_bytes_per_second_counter_.Add(length); @@ -1328,7 +1327,6 @@ void Call::OnRecoveredPacket(const uint8_t* packet, size_t length) { parsed_packet.IdentifyExtensions(it->second.extensions); // TODO(brandtr): Update here when we support protecting audio packets too. - parsed_packet.set_payload_type_frequency(kVideoPayloadTypeFrequency); video_receiver_controller_.OnRtpPacket(parsed_packet); } diff --git a/modules/rtp_rtcp/source/rtp_format.cc b/modules/rtp_rtcp/source/rtp_format.cc index a24c132ad5..3f2038e1c6 100644 --- a/modules/rtp_rtcp/source/rtp_format.cc +++ b/modules/rtp_rtcp/source/rtp_format.cc @@ -54,8 +54,11 @@ RtpDepacketizer* RtpDepacketizer::Create(VideoCodecType type) { return new RtpDepacketizerVp8(); case kVideoCodecVP9: return new RtpDepacketizerVp9(); - default: + case kVideoCodecGeneric: return new RtpDepacketizerGeneric(); + default: + RTC_NOTREACHED(); } + return nullptr; } } // namespace webrtc diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc index 1ccc3a62e7..f99aecde66 100644 --- a/video/rtp_video_stream_receiver.cc +++ b/video/rtp_video_stream_receiver.cc @@ -14,20 +14,17 @@ #include #include -#include "absl/memory/memory.h" - #include "common_types.h" // NOLINT(build/include) #include "media/base/mediaconstants.h" #include "modules/pacing/packet_router.h" #include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" #include "modules/rtp_rtcp/include/receive_statistics.h" #include "modules/rtp_rtcp/include/rtp_cvo.h" +#include "modules/rtp_rtcp/include/rtp_receiver.h" #include "modules/rtp_rtcp/include/rtp_rtcp.h" #include "modules/rtp_rtcp/include/ulpfec_receiver.h" -#include "modules/rtp_rtcp/source/rtp_format.h" #include "modules/rtp_rtcp/source/rtp_header_extensions.h" #include "modules/rtp_rtcp/source/rtp_packet_received.h" -#include "modules/rtp_rtcp/source/rtp_rtcp_config.h" #include "modules/video_coding/frame_object.h" #include "modules/video_coding/h264_sprop_parameter_sets.h" #include "modules/video_coding/h264_sps_pps_tracker.h" @@ -100,6 +97,9 @@ RtpVideoStreamReceiver::RtpVideoStreamReceiver( process_thread_(process_thread), ntp_estimator_(clock_), rtp_header_extensions_(config_.rtp.extensions), + rtp_receiver_(RtpReceiver::CreateVideoReceiver(clock_, + this, + &rtp_payload_registry_)), rtp_receive_statistics_(rtp_receive_statistics), ulpfec_receiver_(UlpfecReceiver::Create(config->rtp.remote_ssrc, this)), receiving_(false), @@ -168,25 +168,26 @@ RtpVideoStreamReceiver::~RtpVideoStreamReceiver() { UpdateHistograms(); } -void RtpVideoStreamReceiver::AddReceiveCodec( +bool RtpVideoStreamReceiver::AddReceiveCodec( const VideoCodec& video_codec, const std::map& codec_params) { - pt_codec_type_.emplace(video_codec.plType, video_codec.codecType); - pt_codec_params_.emplace(video_codec.plType, codec_params); + pt_codec_params_.insert(make_pair(video_codec.plType, codec_params)); + return rtp_payload_registry_.RegisterReceivePayload(video_codec) == 0; } absl::optional RtpVideoStreamReceiver::GetSyncInfo() const { - if (!last_received_rtp_timestamp_ || !last_received_rtp_system_time_ms_) { + Syncable::Info info; + + if (!rtp_receiver_->GetLatestTimestamps( + &info.latest_received_capture_timestamp, + &info.latest_receive_time_ms)) { return absl::nullopt; } - Syncable::Info info; if (rtp_rtcp_->RemoteNTP(&info.capture_time_ntp_secs, &info.capture_time_ntp_frac, nullptr, nullptr, &info.capture_time_source_clock) != 0) { return absl::nullopt; } - info.latest_received_capture_timestamp = *last_received_rtp_timestamp_; - info.latest_receive_time_ms = *last_received_rtp_system_time_ms_; // Leaves info.current_delay_ms uninitialized. return info; @@ -243,20 +244,12 @@ void RtpVideoStreamReceiver::OnRecoveredPacket(const uint8_t* rtp_packet, RtpPacketReceived packet; if (!packet.Parse(rtp_packet, rtp_packet_length)) return; - if (packet.PayloadType() == config_.rtp.red_payload_type) { - RTC_LOG(LS_WARNING) << "Discarding recovered packet with RED encapsulation"; - return; - } - packet.IdentifyExtensions(rtp_header_extensions_); packet.set_payload_type_frequency(kVideoPayloadTypeFrequency); - // TODO(nisse): UlpfecReceiverImpl::ProcessReceivedFec passes both - // original (decapsulated) media packets and recovered packets to - // this callback. We need a way to distinguish, for setting - // packet.recovered() correctly. Ideally, move RED decapsulation out - // of the Ulpfec implementation. - ReceivePacket(packet); + RTPHeader header; + packet.GetHeader(&header); + ReceivePacket(rtp_packet, rtp_packet_length, header); } // This method handles both regular RTP packets and packets recovered @@ -270,9 +263,6 @@ void RtpVideoStreamReceiver::OnRtpPacket(const RtpPacketReceived& packet) { if (!packet.recovered()) { int64_t now_ms = clock_->TimeInMilliseconds(); - // TODO(nisse): Exclude out-of-order packets? - last_received_rtp_timestamp_ = packet.Timestamp(); - last_received_rtp_system_time_ms_ = now_ms; // Periodically log the RTP header of incoming packets. if (now_ms - last_packet_log_ms_ > kPacketLogIntervalMs) { @@ -295,14 +285,18 @@ void RtpVideoStreamReceiver::OnRtpPacket(const RtpPacketReceived& packet) { } } - ReceivePacket(packet); + // TODO(nisse): Delete use of GetHeader, but needs refactoring of + // ReceivePacket and IncomingPacket methods below. + RTPHeader header; + packet.GetHeader(&header); + header.payload_type_frequency = kVideoPayloadTypeFrequency; + + ReceivePacket(packet.data(), packet.size(), header); // Update receive statistics after ReceivePacket. // Receive statistics will be reset if the payload type changes (make sure // that the first packet is included in the stats). if (!packet.recovered()) { - RTPHeader header; - packet.GetHeader(&header); // TODO(nisse): We should pass a recovered flag to stats, to aid // fixing bug bugs.webrtc.org/6339. rtp_receive_statistics_->IncomingPacket(header, packet.size(), @@ -394,55 +388,22 @@ void RtpVideoStreamReceiver::RemoveSecondarySink( secondary_sinks_.erase(it); } -void RtpVideoStreamReceiver::ReceivePacket(const RtpPacketReceived& packet) { - if (packet.PayloadType() == config_.rtp.red_payload_type) { - RTPHeader header; - packet.GetHeader(&header); - ParseAndHandleEncapsulatingHeader(packet.data(), packet.size(), header); +void RtpVideoStreamReceiver::ReceivePacket(const uint8_t* packet, + size_t packet_length, + const RTPHeader& header) { + if (header.payloadType == config_.rtp.red_payload_type) { + ParseAndHandleEncapsulatingHeader(packet, packet_length, header); return; } - - const auto codec_type_it = pt_codec_type_.find(packet.PayloadType()); - if (codec_type_it == pt_codec_type_.end()) { - return; + const uint8_t* payload = packet + header.headerLength; + assert(packet_length >= header.headerLength); + size_t payload_length = packet_length - header.headerLength; + const auto pl = + rtp_payload_registry_.PayloadTypeToPayload(header.payloadType); + if (pl) { + rtp_receiver_->IncomingRtpPacket(header, payload, payload_length, + pl->typeSpecific); } - auto depacketizer = - absl::WrapUnique(RtpDepacketizer::Create(codec_type_it->second)); - - if (!depacketizer) { - RTC_LOG(LS_ERROR) << "Failed to create depacketizer."; - return; - } - RtpDepacketizer::ParsedPayload parsed_payload; - if (!depacketizer->Parse(&parsed_payload, packet.payload().data(), - packet.payload().size())) { - RTC_LOG(LS_WARNING) << "Failed parsing payload."; - return; - } - - WebRtcRTPHeader webrtc_rtp_header = {}; - packet.GetHeader(&webrtc_rtp_header.header); - - webrtc_rtp_header.frameType = parsed_payload.frame_type; - webrtc_rtp_header.video_header() = parsed_payload.video_header(); - webrtc_rtp_header.video_header().rotation = kVideoRotation_0; - webrtc_rtp_header.video_header().content_type = VideoContentType::UNSPECIFIED; - webrtc_rtp_header.video_header().video_timing.flags = - VideoSendTiming::kInvalid; - - // Retrieve the video rotation information. - packet.GetExtension( - &webrtc_rtp_header.video_header().rotation); - - packet.GetExtension( - &webrtc_rtp_header.video_header().content_type); - packet.GetExtension( - &webrtc_rtp_header.video_header().video_timing); - packet.GetExtension( - &webrtc_rtp_header.video_header().playout_delay); - - OnReceivedPayloadData(parsed_payload.payload, parsed_payload.payload_length, - &webrtc_rtp_header); } void RtpVideoStreamReceiver::ParseAndHandleEncapsulatingHeader( @@ -495,7 +456,7 @@ bool RtpVideoStreamReceiver::DeliverRtcp(const uint8_t* rtcp_packet, rtp_rtcp_->IncomingRtcpPacket(rtcp_packet, rtcp_packet_length); int64_t rtt = 0; - rtp_rtcp_->RTT(config_.rtp.remote_ssrc, &rtt, nullptr, nullptr, nullptr); + rtp_rtcp_->RTT(rtp_receiver_->SSRC(), &rtt, nullptr, nullptr, nullptr); if (rtt == 0) { // Waiting for valid rtt. return true; diff --git a/video/rtp_video_stream_receiver.h b/video/rtp_video_stream_receiver.h index f5e47e9e64..91b993a4b8 100644 --- a/video/rtp_video_stream_receiver.h +++ b/video/rtp_video_stream_receiver.h @@ -17,9 +17,6 @@ #include #include -#include "absl/types/optional.h" - -#include "api/video_codecs/video_codec.h" #include "call/rtp_packet_sink_interface.h" #include "call/syncable.h" #include "call/video_receive_stream.h" @@ -27,6 +24,7 @@ #include "modules/rtp_rtcp/include/receive_statistics.h" #include "modules/rtp_rtcp/include/remote_ntp_time_estimator.h" #include "modules/rtp_rtcp/include/rtp_header_extension_map.h" +#include "modules/rtp_rtcp/include/rtp_payload_registry.h" #include "modules/rtp_rtcp/include/rtp_rtcp.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "modules/video_coding/h264_sps_pps_tracker.h" @@ -76,7 +74,7 @@ class RtpVideoStreamReceiver : public RtpData, video_coding::OnCompleteFrameCallback* complete_frame_callback); ~RtpVideoStreamReceiver(); - void AddReceiveCodec(const VideoCodec& video_codec, + bool AddReceiveCodec(const VideoCodec& video_codec, const std::map& codec_params); void StartReceive(); @@ -140,10 +138,10 @@ class RtpVideoStreamReceiver : public RtpData, void RemoveSecondarySink(const RtpPacketSinkInterface* sink); private: - // Entry point doing non-stats work for a received packet. Called - // for the same packet both before and after RED decapsulation. - void ReceivePacket(const RtpPacketReceived& packet); - // Parses and handles RED headers. + void ReceivePacket(const uint8_t* packet, + size_t packet_length, + const RTPHeader& header); + // Parses and handles for instance RTX and RED headers. // This function assumes that it's being called from only one thread. void ParseAndHandleEncapsulatingHeader(const uint8_t* packet, size_t packet_length, @@ -162,8 +160,10 @@ class RtpVideoStreamReceiver : public RtpData, ProcessThread* const process_thread_; RemoteNtpTimeEstimator ntp_estimator_; + RTPPayloadRegistry rtp_payload_registry_; RtpHeaderExtensionMap rtp_header_extensions_; + const std::unique_ptr rtp_receiver_; ReceiveStatistics* const rtp_receive_statistics_; std::unique_ptr ulpfec_receiver_; @@ -183,10 +183,6 @@ class RtpVideoStreamReceiver : public RtpData, std::map last_seq_num_for_pic_id_ RTC_GUARDED_BY(last_seq_num_cs_); video_coding::H264SpsPpsTracker tracker_; - - absl::optional last_received_rtp_timestamp_; - absl::optional last_received_rtp_system_time_ms_; - std::map pt_codec_type_; // TODO(johan): Remove pt_codec_params_ once // https://bugs.chromium.org/p/webrtc/issues/detail?id=6883 is resolved. // Maps a payload type to a map of out-of-band supplied codec parameters. diff --git a/video/video_receive_stream.cc b/video/video_receive_stream.cc index 088c0eb096..6580baebfc 100644 --- a/video/video_receive_stream.cc +++ b/video/video_receive_stream.cc @@ -24,6 +24,7 @@ #include "common_video/h264/profile_level_id.h" #include "common_video/include/incoming_video_stream.h" #include "common_video/libyuv/include/webrtc_libyuv.h" +#include "modules/rtp_rtcp/include/rtp_receiver.h" #include "modules/rtp_rtcp/include/rtp_rtcp.h" #include "modules/utility/include/process_thread.h" #include "modules/video_coding/frame_object.h" @@ -202,7 +203,8 @@ void VideoReceiveStream::Start() { video_receiver_.RegisterExternalDecoder(decoder.decoder, decoder.payload_type); VideoCodec codec = CreateDecoderVideoCodec(decoder); - rtp_video_stream_receiver_.AddReceiveCodec(codec, decoder.codec_params); + RTC_CHECK(rtp_video_stream_receiver_.AddReceiveCodec(codec, + decoder.codec_params)); RTC_CHECK_EQ(VCM_OK, video_receiver_.RegisterReceiveCodec( &codec, num_cpu_cores_, false)); }