From 054e3bbbe7921b8b84213ecbbb58f68cd5712f6d Mon Sep 17 00:00:00 2001 From: Chen Xing Date: Fri, 2 Aug 2019 10:29:26 +0000 Subject: [PATCH] Reland "Replace the implementation of `GetContributingSources()` on the audio side." MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 67008dfb366237469fe088a61b62c0cad852c024. Reason for revert: Tests in the Chromium repo have been changed to accomodate this CL: https://chromium-review.googlesource.com/c/chromium/src/+/1728565 Original change's description: > Revert "Replace the implementation of `GetContributingSources()` on the audio side." > > This reverts commit 8fa7151e4bbad40fec1f964fe0c003b8787bb78a. > > Reason for revert: Speculative revert to fix roll of webrtc into chrome. Right now tests related to RTCRtpReceiver failing and looks like it is main candidate, who can affect that behavior. > > Original change's description: > > Replace the implementation of `GetContributingSources()` on the audio side. > > > > This change replaces the `ContributingSources`-implementation of `GetContributingSources()` and `GetSynchronizationSources()` on the audio side with the spec-compliant `SourceTracker`-implementation. > > > > The most noticeable impact is that the per-frame dictionaries are now updated when frames are delivered to the RTCRtpReceiver's MediaStreamTrack rather than when RTP packets are received on the network. > > > > This change is almost identical to the previous video side change at: https://webrtc-review.googlesource.com/c/src/+/143177 > > > > Bug: webrtc:10545 > > Change-Id: Ife7f08ee8ca1346099b7466837a3756947085fc5 > > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/144422 > > Reviewed-by: Oskar Sundbom > > Commit-Queue: Chen Xing > > Cr-Commit-Position: refs/heads/master@{#28459} > > TBR=ossu@webrtc.org,chxg@google.com > > Change-Id: I5c631d4dcfb39601055ffce9b104f45eea871fd3 > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: webrtc:10545 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/144562 > Reviewed-by: Artem Titov > Commit-Queue: Artem Titov > Cr-Commit-Position: refs/heads/master@{#28478} TBR=ossu@webrtc.org,titovartem@webrtc.org,chxg@google.com # Not skipping CQ checks because original CL landed > 1 day ago. Bug: webrtc:10545 Change-Id: I609cca4f0ca4e1d31a156ba9eb44407518409f57 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/147865 Reviewed-by: Henrik Boström Reviewed-by: Chen Xing Commit-Queue: Chen Xing Cr-Commit-Position: refs/heads/master@{#28746} --- audio/audio_receive_stream.cc | 13 +++++++--- audio/audio_receive_stream.h | 2 ++ audio/channel_receive.cc | 49 +++++------------------------------ audio/channel_receive.h | 2 -- 4 files changed, 19 insertions(+), 47 deletions(-) diff --git a/audio/audio_receive_stream.cc b/audio/audio_receive_stream.cc index 0ff2b0c0e3..1a55adbe46 100644 --- a/audio/audio_receive_stream.cc +++ b/audio/audio_receive_stream.cc @@ -113,7 +113,9 @@ AudioReceiveStream::AudioReceiveStream( const rtc::scoped_refptr& audio_state, webrtc::RtcEventLog* event_log, std::unique_ptr channel_receive) - : audio_state_(audio_state), channel_receive_(std::move(channel_receive)) { + : audio_state_(audio_state), + channel_receive_(std::move(channel_receive)), + source_tracker_(clock) { RTC_LOG(LS_INFO) << "AudioReceiveStream: " << config.rtp.remote_ssrc; RTC_DCHECK(config.decoder_factory); RTC_DCHECK(config.rtcp_send_transport); @@ -267,13 +269,18 @@ int AudioReceiveStream::GetBaseMinimumPlayoutDelayMs() const { std::vector AudioReceiveStream::GetSources() const { RTC_DCHECK_RUN_ON(&worker_thread_checker_); - return channel_receive_->GetSources(); + return source_tracker_.GetSources(); } AudioMixer::Source::AudioFrameInfo AudioReceiveStream::GetAudioFrameWithInfo( int sample_rate_hz, AudioFrame* audio_frame) { - return channel_receive_->GetAudioFrameWithInfo(sample_rate_hz, audio_frame); + AudioMixer::Source::AudioFrameInfo audio_frame_info = + channel_receive_->GetAudioFrameWithInfo(sample_rate_hz, audio_frame); + if (audio_frame_info != AudioMixer::Source::AudioFrameInfo::kError) { + source_tracker_.OnFrameDelivered(audio_frame->packet_infos_); + } + return audio_frame_info; } int AudioReceiveStream::Ssrc() const { diff --git a/audio/audio_receive_stream.h b/audio/audio_receive_stream.h index 0924c03d5c..49969a2779 100644 --- a/audio/audio_receive_stream.h +++ b/audio/audio_receive_stream.h @@ -19,6 +19,7 @@ #include "audio/audio_state.h" #include "call/audio_receive_stream.h" #include "call/syncable.h" +#include "modules/rtp_rtcp/source/source_tracker.h" #include "rtc_base/constructor_magic.h" #include "rtc_base/thread_checker.h" #include "system_wrappers/include/clock.h" @@ -107,6 +108,7 @@ class AudioReceiveStream final : public webrtc::AudioReceiveStream, webrtc::AudioReceiveStream::Config config_; rtc::scoped_refptr audio_state_; const std::unique_ptr channel_receive_; + SourceTracker source_tracker_; AudioSendStream* associated_send_stream_ = nullptr; bool playing_ RTC_GUARDED_BY(worker_thread_checker_) = false; diff --git a/audio/channel_receive.cc b/audio/channel_receive.cc index 0f92cfb5bd..aa6043811a 100644 --- a/audio/channel_receive.cc +++ b/audio/channel_receive.cc @@ -30,7 +30,6 @@ #include "modules/rtp_rtcp/include/receive_statistics.h" #include "modules/rtp_rtcp/include/remote_ntp_time_estimator.h" #include "modules/rtp_rtcp/include/rtp_rtcp.h" -#include "modules/rtp_rtcp/source/contributing_sources.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" @@ -161,8 +160,6 @@ class ChannelReceive : public ChannelReceiveInterface, // Used for obtaining RTT for a receive-only channel. void SetAssociatedSendChannel(const ChannelSendInterface* channel) override; - std::vector GetSources() const override; - // TODO(sukhanov): Return const pointer. It requires making media transport // getters like GetLatestTargetTransferRate to be also const. MediaTransportInterface* media_transport() const { @@ -219,16 +216,13 @@ class ChannelReceive : public ChannelReceiveInterface, std::unique_ptr _rtpRtcpModule; const uint32_t remote_ssrc_; - // Info for GetSources and GetSyncInfo is updated on network or worker thread, - // queried on the worker thread. - rtc::CriticalSection rtp_sources_lock_; - ContributingSources contributing_sources_ RTC_GUARDED_BY(&rtp_sources_lock_); + // Info for GetSyncInfo is updated on network or worker thread, and queried on + // the worker thread. + rtc::CriticalSection sync_info_lock_; absl::optional last_received_rtp_timestamp_ - RTC_GUARDED_BY(&rtp_sources_lock_); + RTC_GUARDED_BY(&sync_info_lock_); absl::optional last_received_rtp_system_time_ms_ - RTC_GUARDED_BY(&rtp_sources_lock_); - absl::optional last_received_rtp_audio_level_ - RTC_GUARDED_BY(&rtp_sources_lock_); + RTC_GUARDED_BY(&sync_info_lock_); std::unique_ptr audio_coding_; AudioSinkInterface* audio_sink_ = nullptr; @@ -565,24 +559,6 @@ absl::optional> ChannelReceive::GetReceiveCodec() return audio_coding_->ReceiveCodec(); } -std::vector ChannelReceive::GetSources() const { - RTC_DCHECK(worker_thread_checker_.IsCurrent()); - int64_t now_ms = rtc::TimeMillis(); - std::vector sources; - { - rtc::CritScope cs(&rtp_sources_lock_); - sources = contributing_sources_.GetSources(now_ms); - if (last_received_rtp_system_time_ms_ >= - now_ms - ContributingSources::kHistoryMs) { - RTC_DCHECK(last_received_rtp_timestamp_.has_value()); - sources.emplace_back(*last_received_rtp_system_time_ms_, remote_ssrc_, - RtpSourceType::SSRC, last_received_rtp_audio_level_, - *last_received_rtp_timestamp_); - } - } - return sources; -} - void ChannelReceive::SetReceiveCodecs( const std::map& codecs) { RTC_DCHECK(worker_thread_checker_.IsCurrent()); @@ -596,22 +572,11 @@ void ChannelReceive::SetReceiveCodecs( // May be called on either worker thread or network thread. void ChannelReceive::OnRtpPacket(const RtpPacketReceived& packet) { int64_t now_ms = rtc::TimeMillis(); - uint8_t audio_level; - bool voice_activity; - bool has_audio_level = - packet.GetExtension<::webrtc::AudioLevel>(&voice_activity, &audio_level); { - rtc::CritScope cs(&rtp_sources_lock_); + rtc::CritScope cs(&sync_info_lock_); last_received_rtp_timestamp_ = packet.Timestamp(); last_received_rtp_system_time_ms_ = now_ms; - if (has_audio_level) - last_received_rtp_audio_level_ = audio_level; - std::vector csrcs = packet.Csrcs(); - contributing_sources_.Update( - now_ms, csrcs, - has_audio_level ? absl::optional(audio_level) : absl::nullopt, - packet.Timestamp()); } // Store playout timestamp for the received RTP packet @@ -887,7 +852,7 @@ absl::optional ChannelReceive::GetSyncInfo() const { return absl::nullopt; } { - rtc::CritScope cs(&rtp_sources_lock_); + rtc::CritScope cs(&sync_info_lock_); if (!last_received_rtp_timestamp_ || !last_received_rtp_system_time_ms_) { return absl::nullopt; } diff --git a/audio/channel_receive.h b/audio/channel_receive.h index 1e16304fa0..d0dea67f26 100644 --- a/audio/channel_receive.h +++ b/audio/channel_receive.h @@ -135,8 +135,6 @@ class ChannelReceiveInterface : public RtpPacketSinkInterface { // Used for obtaining RTT for a receive-only channel. virtual void SetAssociatedSendChannel( const ChannelSendInterface* channel) = 0; - - virtual std::vector GetSources() const = 0; }; std::unique_ptr CreateChannelReceive(