admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove MediaTransport from Call.

Browse Source 
There aren't any tests for this and the code isn't currently
active except for the fact that it adds complexity to the Call
class, synchronization into the active code path and makes future
improvements to the class more complex or impossible.

Bug: webrtc:9719
Change-Id: Ia41af0b2186b8a36ca70a07858990b6af7f3a5c3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/148078
Commit-Queue: Tommi <tommi@webrtc.org>
Reviewed-by: Magnus Flodman <mflodman@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28807}
This commit is contained in:
Tommi 2019-08-08 12:27:53 +02:00 committed by Commit Bot
parent 44327c33ed
commit 78a7138600
8 changed files with 39 additions and 195 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

161
call/call.cc
View File

@ -231,15 +231,6 @@ class Call final : public webrtc::Call,
uint32_t max_padding_bitrate_bps,
uint32_t total_bitrate_bps) override;
// This method is invoked when the media transport is created and when the
// media transport is being destructed.
// We only allow one media transport per connection.
//
// It should be called with non-null argument at most once, and if it was
// called with non-null argument, it has to be called with a null argument
// at least once after that.
void MediaTransportChange(MediaTransportInterface* media_transport) override;
void SetClientBitratePreferences(const BitrateSettings& preferences) override;
private:
@ -262,29 +253,24 @@ class Call final : public webrtc::Call,
void UpdateHistograms();
void UpdateAggregateNetworkState();
// If |media_transport| is not null, it registers the rate observer for the
// media transport.
void RegisterRateObserver() RTC_LOCKS_EXCLUDED(target_observer_crit_);
// Intended for DCHECKs, to avoid locking in production builds.
MediaTransportInterface* media_transport()
RTC_LOCKS_EXCLUDED(target_observer_crit_);
void RegisterRateObserver();
Clock* const clock_;
TaskQueueFactory* const task_queue_factory_;
// Caching the last SetBitrate for media transport.
absl::optional<MediaTransportTargetRateConstraints> last_set_bitrate_
RTC_GUARDED_BY(&target_observer_crit_);
const int num_cpu_cores_;
const std::unique_ptr<ProcessThread> module_process_thread_;
const std::unique_ptr<CallStats> call_stats_;
const std::unique_ptr<BitrateAllocator> bitrate_allocator_;
Call::Config config_;
SequenceChecker configuration_sequence_checker_;
SequenceChecker worker_sequence_checker_;
NetworkState audio_network_state_;
NetworkState video_network_state_;
// TODO(tommi): Once tests have been fixed to not call GetStats() on the wrong
// thread, remove this lock and protect aggregate_network_up_crit_ with the
// configuration_sequence_checker_.
rtc::CriticalSection aggregate_network_up_crit_;
bool aggregate_network_up_ RTC_GUARDED_BY(aggregate_network_up_crit_);
@ -370,7 +356,8 @@ class Call final : public webrtc::Call,
// TODO(holmer): Remove this lock once BitrateController no longer calls
// OnNetworkChanged from multiple threads.
rtc::CriticalSection bitrate_crit_;
uint32_t min_allocated_send_bitrate_bps_ RTC_GUARDED_BY(&bitrate_crit_);
uint32_t min_allocated_send_bitrate_bps_
RTC_GUARDED_BY(&worker_sequence_checker_);
uint32_t configured_max_padding_bitrate_bps_ RTC_GUARDED_BY(&bitrate_crit_);
AvgCounter estimated_send_bitrate_kbps_counter_
RTC_GUARDED_BY(&bitrate_crit_);
@ -387,18 +374,13 @@ class Call final : public webrtc::Call,
// Note that this is declared before transport_send_ to ensure that it is not
// invalidated until no more tasks can be running on the transport_send_ task
// queue.
RtpTransportControllerSendInterface* transport_send_ptr_;
RtpTransportControllerSendInterface* const transport_send_ptr_;
// Declared last since it will issue callbacks from a task queue. Declaring it
// last ensures that it is destroyed first and any running tasks are finished.
std::unique_ptr<RtpTransportControllerSendInterface> transport_send_;
// This is a precaution, since |MediaTransportChange| is not guaranteed to be
// invoked on a particular thread.
rtc::CriticalSection target_observer_crit_;
bool is_target_rate_observer_registered_
RTC_GUARDED_BY(&target_observer_crit_) = false;
MediaTransportInterface* media_transport_
RTC_GUARDED_BY(&target_observer_crit_) = nullptr;
RTC_GUARDED_BY(&configuration_sequence_checker_) = false;
RTC_DISALLOW_COPY_AND_ASSIGN(Call);
};
@ -479,10 +461,11 @@ Call::Call(Clock* clock,
receive_side_cc_(clock_, transport_send->packet_router()),
receive_time_calculator_(ReceiveTimeCalculator::CreateFromFieldTrial()),
video_send_delay_stats_(new SendDelayStats(clock_)),
start_ms_(clock_->TimeInMilliseconds()) {
start_ms_(clock_->TimeInMilliseconds()),
transport_send_ptr_(transport_send.get()),
transport_send_(std::move(transport_send)) {
RTC_DCHECK(config.event_log != nullptr);
transport_send_ = std::move(transport_send);
transport_send_ptr_ = transport_send_.get();
worker_sequence_checker_.Detach();
}
Call::~Call() {
@ -494,14 +477,12 @@ Call::~Call() {
RTC_CHECK(audio_receive_streams_.empty());
RTC_CHECK(video_receive_streams_.empty());
if (!media_transport_) {
module_process_thread_->DeRegisterModule(
receive_side_cc_.GetRemoteBitrateEstimator(true));
module_process_thread_->DeRegisterModule(&receive_side_cc_);
module_process_thread_->DeRegisterModule(call_stats_.get());
module_process_thread_->Stop();
call_stats_->DeregisterStatsObserver(&receive_side_cc_);
}
absl::optional<Timestamp> first_sent_packet_ms =
transport_send_->GetFirstPacketTime();
@ -515,21 +496,19 @@ Call::~Call() {
UpdateHistograms();
}
// TODO(tommi): Most of this work could be done when Call gets created.
// Starting the process thread itself could be done on demand when streams
// are created and in that case, calling Start() multiple times is harmless
// so holding an extra state variable, |is_target_rate_observer_registered_|
// also shouldn't be necessary.
void Call::RegisterRateObserver() {
rtc::CritScope lock(&target_observer_crit_);
RTC_DCHECK_RUN_ON(&configuration_sequence_checker_);
if (is_target_rate_observer_registered_) {
if (is_target_rate_observer_registered_)
return;
}
is_target_rate_observer_registered_ = true;
if (media_transport_) {
// TODO(bugs.webrtc.org/9719): We should report call_stats_ from
// media transport (at least Rtt). We should extend media transport
// interface to include "receive_side bwe" if needed.
media_transport_->AddTargetTransferRateObserver(this);
} else {
transport_send_ptr_->RegisterTargetTransferRateObserver(this);
call_stats_->RegisterStatsObserver(&receive_side_cc_);
@ -539,78 +518,11 @@ void Call::RegisterRateObserver() {
module_process_thread_->RegisterModule(call_stats_.get(), RTC_FROM_HERE);
module_process_thread_->RegisterModule(&receive_side_cc_, RTC_FROM_HERE);
module_process_thread_->Start();
}
}
MediaTransportInterface* Call::media_transport() {
rtc::CritScope lock(&target_observer_crit_);
return media_transport_;
}
void Call::MediaTransportChange(MediaTransportInterface* media_transport) {
rtc::CritScope lock(&target_observer_crit_);
if (is_target_rate_observer_registered_) {
// Only used to unregister rate observer from media transport. Registration
// happens when the stream is created.
if (!media_transport && media_transport_) {
media_transport_->RemoveTargetTransferRateObserver(this);
media_transport_ = nullptr;
is_target_rate_observer_registered_ = false;
}
} else if (media_transport) {
RTC_DCHECK(media_transport_ == nullptr ||
media_transport_ == media_transport)
<< "media_transport_=" << (media_transport_ != nullptr)
<< ", (media_transport_==media_transport)="
<< (media_transport_ == media_transport);
media_transport_ = media_transport;
MediaTransportTargetRateConstraints constraints;
if (config_.bitrate_config.start_bitrate_bps > 0) {
constraints.starting_bitrate =
DataRate::bps(config_.bitrate_config.start_bitrate_bps);
}
if (config_.bitrate_config.max_bitrate_bps > 0) {
constraints.max_bitrate =
DataRate::bps(config_.bitrate_config.max_bitrate_bps);
}
if (config_.bitrate_config.min_bitrate_bps > 0) {
constraints.min_bitrate =
DataRate::bps(config_.bitrate_config.min_bitrate_bps);
}
// User called ::SetBitrate on peer connection before
// media transport was created.
if (last_set_bitrate_) {
media_transport_->SetTargetBitrateLimits(*last_set_bitrate_);
} else {
media_transport_->SetTargetBitrateLimits(constraints);
}
}
}
void Call::SetClientBitratePreferences(const BitrateSettings& preferences) {
RTC_DCHECK_RUN_ON(&configuration_sequence_checker_);
GetTransportControllerSend()->SetClientBitratePreferences(preferences);
// Can the client code invoke 'SetBitrate' before media transport is created?
// It's probably possible :/
MediaTransportTargetRateConstraints constraints;
if (preferences.start_bitrate_bps.has_value()) {
constraints.starting_bitrate =
webrtc::DataRate::bps(*preferences.start_bitrate_bps);
}
if (preferences.max_bitrate_bps.has_value()) {
constraints.max_bitrate =
webrtc::DataRate::bps(*preferences.max_bitrate_bps);
}
if (preferences.min_bitrate_bps.has_value()) {
constraints.min_bitrate =
webrtc::DataRate::bps(*preferences.min_bitrate_bps);
}
rtc::CritScope lock(&target_observer_crit_);
last_set_bitrate_ = constraints;
if (media_transport_) {
media_transport_->SetTargetBitrateLimits(constraints);
}
}
void Call::UpdateHistograms() {
@ -699,9 +611,6 @@ webrtc::AudioSendStream* Call::CreateAudioSendStream(
TRACE_EVENT0("webrtc", "Call::CreateAudioSendStream");
RTC_DCHECK_RUN_ON(&configuration_sequence_checker_);
RTC_DCHECK_EQ(media_transport(),
config.media_transport_config.media_transport);
RegisterRateObserver();
// Stream config is logged in AudioSendStream::ConfigureStream, as it may
@ -831,8 +740,6 @@ webrtc::VideoSendStream* Call::CreateVideoSendStream(
TRACE_EVENT0("webrtc", "Call::CreateVideoSendStream");
RTC_DCHECK_RUN_ON(&configuration_sequence_checker_);
RTC_DCHECK(media_transport() == config.media_transport);
RegisterRateObserver();
video_send_delay_stats_->AddSsrcs(config);
@ -1164,17 +1071,8 @@ void Call::OnStartRateUpdate(DataRate start_rate) {
}
void Call::OnTargetTransferRate(TargetTransferRate msg) {
// TODO(bugs.webrtc.org/9719)
// Call::OnTargetTransferRate requires that on target transfer rate is invoked
// from the worker queue (because bitrate_allocator_ requires it). Media
// transport does not guarantee the callback on the worker queue.
// When the threading model for MediaTransportInterface is update, reconsider
// changing this implementation.
if (!transport_send_ptr_->GetWorkerQueue()->IsCurrent()) {
transport_send_ptr_->GetWorkerQueue()->PostTask(
[this, msg] { this->OnTargetTransferRate(msg); });
return;
}
RTC_DCHECK(transport_send_ptr_->GetWorkerQueue()->IsCurrent());
RTC_DCHECK_RUN_ON(&worker_sequence_checker_);
uint32_t target_bitrate_bps = msg.target_rate.bps();
int loss_ratio_255 = msg.network_estimate.loss_rate_ratio * 255;
@ -1224,22 +1122,13 @@ void Call::OnTargetTransferRate(TargetTransferRate msg) {
void Call::OnAllocationLimitsChanged(uint32_t min_send_bitrate_bps,
uint32_t max_padding_bitrate_bps,
uint32_t total_bitrate_bps) {
RTC_DCHECK_RUN_ON(&worker_sequence_checker_);
transport_send_ptr_->SetAllocatedSendBitrateLimits(
min_send_bitrate_bps, max_padding_bitrate_bps, total_bitrate_bps);
{
rtc::CritScope lock(&target_observer_crit_);
if (media_transport_) {
MediaTransportAllocatedBitrateLimits limits;
limits.min_pacing_rate = DataRate::bps(min_send_bitrate_bps);
limits.max_padding_bitrate = DataRate::bps(max_padding_bitrate_bps);
limits.max_total_allocated_bitrate = DataRate::bps(total_bitrate_bps);
media_transport_->SetAllocatedBitrateLimits(limits);
}
}
min_allocated_send_bitrate_bps_ = min_send_bitrate_bps;
rtc::CritScope lock(&bitrate_crit_);
min_allocated_send_bitrate_bps_ = min_send_bitrate_bps;
configured_max_padding_bitrate_bps_ = max_padding_bitrate_bps;
}

4
call/call.h
View File

@ -57,10 +57,6 @@ class Call {
virtual AudioSendStream* CreateAudioSendStream(
const AudioSendStream::Config& config) = 0;
// Gets called when media transport is created or removed.
virtual void MediaTransportChange(
MediaTransportInterface* media_transport_interface) = 0;
virtual void DestroyAudioSendStream(AudioSendStream* send_stream) = 0;
virtual AudioReceiveStream* CreateAudioReceiveStream(

26
call/call_unittest.cc
View File

@ -295,30 +295,4 @@ TEST(CallTest, RecreatingAudioStreamWithSameSsrcReusesRtpState) {
EXPECT_EQ(rtp_state1.media_has_been_sent, rtp_state2.media_has_been_sent);
}
TEST(CallTest, RegisterMediaTransportBitrateCallbacksInCreateStream) {
CallHelper call;
MediaTransportSettings settings;
webrtc::FakeMediaTransport fake_media_transport(settings);
EXPECT_EQ(0, fake_media_transport.target_rate_observers_size());
// TODO(solenberg): This test shouldn't require a Transport, but currently
// RTCPSender requires one.
MockTransport send_transport;
AudioSendStream::Config config(&send_transport,
MediaTransportConfig(&fake_media_transport));
call->MediaTransportChange(&fake_media_transport);
AudioSendStream* stream = call->CreateAudioSendStream(config);
// We get 2 subscribers: one subscriber from call.cc, and one from
// ChannelSend.
EXPECT_EQ(2, fake_media_transport.target_rate_observers_size());
call->DestroyAudioSendStream(stream);
EXPECT_EQ(1, fake_media_transport.target_rate_observers_size());
call->MediaTransportChange(nullptr);
EXPECT_EQ(0, fake_media_transport.target_rate_observers_size());
}
} // namespace webrtc

6
call/degraded_call.cc
View File

@ -242,10 +242,4 @@ PacketReceiver::DeliveryStatus DegradedCall::DeliverPacket(
return status;
}
void DegradedCall::MediaTransportChange(
MediaTransportInterface* media_transport) {
// TODO(bugs.webrtc.org/9719) We should add support for media transport here
// at some point.
}
} // namespace webrtc

1
call/degraded_call.h
View File

@ -129,7 +129,6 @@ class DegradedCall : public Call, private Transport, private PacketReceiver {
const std::unique_ptr<Call> call_;
TaskQueueFactory* const task_queue_factory_;
void MediaTransportChange(MediaTransportInterface* media_transport) override;
void SetClientBitratePreferences(
const webrtc::BitrateSettings& preferences) override {}
const absl::optional<BuiltInNetworkBehaviorConfig> send_config_;

3
media/engine/fake_webrtc_call.cc
View File

@ -644,7 +644,4 @@ void FakeCall::OnSentPacket(const rtc::SentPacket& sent_packet) {
}
}
void FakeCall::MediaTransportChange(
webrtc::MediaTransportInterface* media_transport_interface) {}
} // namespace cricket

3
media/engine/fake_webrtc_call.h
View File

@ -303,9 +303,6 @@ class FakeCall final : public webrtc::Call, public webrtc::PacketReceiver {
int GetNumCreatedReceiveStreams() const;
void SetStats(const webrtc::Call::Stats& stats);
void MediaTransportChange(
webrtc::MediaTransportInterface* media_transport_interface) override;
void SetClientBitratePreferences(
const webrtc::BitrateSettings& preferences) override {}

4
pc/peer_connection.cc
View File

@ -7305,9 +7305,7 @@ bool PeerConnection::OnTransportChanged(
}
if (use_media_transport_) {
// Only pass media transport to call object if media transport is used
// for media (and not data channel).
call_ptr_->MediaTransportChange(media_transport);
RTC_LOG(LS_ERROR) << "Media transport isn't supported.";
}
return ret;