Remove the VideoReceiveStream2::rtp() accessor.
Instead offer accessors for the specific config values from the struct
that are needed at different times. The remote_ssrc and rtx_ssrc
properties maybe accessed from any thread, other properties have
stricter requiremets.
Bug: webrtc:11993
Change-Id: I3ff8527b13452c773fae1b2574f1e3fd2583b481
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/261319
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36823}
diff --git a/call/call.cc b/call/call.cc
index 4c7bebc..a89aed3 100644
--- a/call/call.cc
+++ b/call/call.cc
@@ -1160,15 +1160,14 @@
// thread.
receive_stream->RegisterWithTransport(&video_receiver_controller_);
- const webrtc::VideoReceiveStream::Config::Rtp& rtp = receive_stream->rtp();
- if (rtp.rtx_ssrc) {
+ if (receive_stream->rtx_ssrc()) {
// We record identical config for the rtx stream as for the main
// stream. Since the transport_send_cc negotiation is per payload
// type, we may get an incorrect value for the rtx stream, but
// that is unlikely to matter in practice.
- RegisterReceiveStream(rtp.rtx_ssrc, receive_stream);
+ RegisterReceiveStream(receive_stream->rtx_ssrc(), receive_stream);
}
- RegisterReceiveStream(rtp.remote_ssrc, receive_stream);
+ RegisterReceiveStream(receive_stream->remote_ssrc(), receive_stream);
video_receive_streams_.insert(receive_stream);
ConfigureSync(receive_stream->sync_group());
@@ -1188,22 +1187,19 @@
// TODO(bugs.webrtc.org/11993): Unregister on the network thread.
receive_stream_impl->UnregisterFromTransport();
- // TODO(tommi): Remove `rtp()` accessor.
- const webrtc::VideoReceiveStream::Config::Rtp& rtp =
- receive_stream_impl->rtp();
-
// Remove all ssrcs pointing to a receive stream. As RTX retransmits on a
// separate SSRC there can be either one or two.
- UnregisterReceiveStream(rtp.remote_ssrc);
- if (rtp.rtx_ssrc) {
- UnregisterReceiveStream(rtp.rtx_ssrc);
+ UnregisterReceiveStream(receive_stream_impl->remote_ssrc());
+
+ if (receive_stream_impl->rtx_ssrc()) {
+ UnregisterReceiveStream(receive_stream_impl->rtx_ssrc());
}
video_receive_streams_.erase(receive_stream_impl);
ConfigureSync(receive_stream_impl->sync_group());
receive_side_cc_
.GetRemoteBitrateEstimator(UseSendSideBwe(receive_stream_impl))
- ->RemoveStream(rtp.remote_ssrc);
+ ->RemoveStream(receive_stream_impl->remote_ssrc());
UpdateAggregateNetworkState();
delete receive_stream_impl;
diff --git a/video/video_receive_stream2.cc b/video/video_receive_stream2.cc
index dcc2f80..5574b35 100644
--- a/video/video_receive_stream2.cc
+++ b/video/video_receive_stream2.cc
@@ -220,7 +220,7 @@
clock_(clock),
call_stats_(call_stats),
source_tracker_(clock_),
- stats_proxy_(config_.rtp.remote_ssrc,
+ stats_proxy_(remote_ssrc(),
clock_,
call->worker_thread(),
call->trials()),
@@ -276,13 +276,12 @@
&decode_queue_, this, max_wait_for_keyframe_, max_wait_for_frame_,
decode_sync_, call_->trials());
- if (config_.rtp.rtx_ssrc) {
+ if (rtx_ssrc()) {
rtx_receive_stream_ = std::make_unique<RtxReceiveStream>(
&rtp_video_stream_receiver_, config_.rtp.rtx_associated_payload_types,
- config_.rtp.remote_ssrc, rtp_receive_statistics_.get());
+ remote_ssrc(), rtp_receive_statistics_.get());
} else {
- rtp_receive_statistics_->EnableRetransmitDetection(config_.rtp.remote_ssrc,
- true);
+ rtp_receive_statistics_->EnableRetransmitDetection(remote_ssrc(), true);
}
ParseFieldTrial(
@@ -308,11 +307,11 @@
// Register with RtpStreamReceiverController.
media_receiver_ = receiver_controller->CreateReceiver(
- config_.rtp.remote_ssrc, &rtp_video_stream_receiver_);
- if (config_.rtp.rtx_ssrc) {
+ remote_ssrc(), &rtp_video_stream_receiver_);
+ if (rtx_ssrc()) {
RTC_DCHECK(rtx_receive_stream_);
rtx_receiver_ = receiver_controller->CreateReceiver(
- config_.rtp.rtx_ssrc, rtx_receive_stream_.get());
+ rtx_ssrc(), rtx_receive_stream_.get());
}
}
@@ -322,11 +321,6 @@
rtx_receiver_.reset();
}
-const VideoReceiveStream2::Config::Rtp& VideoReceiveStream2::rtp() const {
- RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
- return config_.rtp;
-}
-
const std::string& VideoReceiveStream2::sync_group() const {
RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
return config_.sync_group;
@@ -519,8 +513,8 @@
if (!decoded_output_file.empty()) {
char filename_buffer[256];
rtc::SimpleStringBuilder ssb(filename_buffer);
- ssb << decoded_output_file << "/webrtc_receive_stream_"
- << config_.rtp.remote_ssrc << "-" << rtc::TimeMicros() << ".ivf";
+ ssb << decoded_output_file << "/webrtc_receive_stream_" << remote_ssrc()
+ << "-" << rtc::TimeMicros() << ".ivf";
video_decoder = CreateFrameDumpingDecoderWrapper(
std::move(video_decoder), FileWrapper::OpenWriteOnly(ssb.str()));
}
@@ -540,9 +534,9 @@
stats.rtp_stats = statistician->GetStats();
stats.total_bitrate_bps = statistician->BitrateReceived();
}
- if (config_.rtp.rtx_ssrc) {
+ if (rtx_ssrc()) {
StreamStatistician* rtx_statistician =
- rtp_receive_statistics_->GetStatistician(config_.rtp.rtx_ssrc);
+ rtp_receive_statistics_->GetStatistician(rtx_ssrc());
if (rtx_statistician)
stats.total_bitrate_bps += rtx_statistician->BitrateReceived();
}
@@ -554,14 +548,14 @@
absl::optional<int> fraction_lost;
StreamDataCounters rtp_stats;
StreamStatistician* statistician =
- rtp_receive_statistics_->GetStatistician(config_.rtp.remote_ssrc);
+ rtp_receive_statistics_->GetStatistician(remote_ssrc());
if (statistician) {
fraction_lost = statistician->GetFractionLostInPercent();
rtp_stats = statistician->GetReceiveStreamDataCounters();
}
- if (config_.rtp.rtx_ssrc) {
+ if (rtx_ssrc()) {
StreamStatistician* rtx_statistician =
- rtp_receive_statistics_->GetStatistician(config_.rtp.rtx_ssrc);
+ rtp_receive_statistics_->GetStatistician(rtx_ssrc());
if (rtx_statistician) {
StreamDataCounters rtx_stats =
rtx_statistician->GetReceiveStreamDataCounters();
@@ -707,7 +701,7 @@
uint32_t VideoReceiveStream2::id() const {
RTC_DCHECK_RUN_ON(&worker_sequence_checker_);
- return config_.rtp.remote_ssrc;
+ return remote_ssrc();
}
absl::optional<Syncable::Info> VideoReceiveStream2::GetInfo() const {
diff --git a/video/video_receive_stream2.h b/video/video_receive_stream2.h
index 9513626..9caa7f6 100644
--- a/video/video_receive_stream2.h
+++ b/video/video_receive_stream2.h
@@ -117,14 +117,15 @@
// network thread.
void UnregisterFromTransport();
- // Convenience getters for parts of the receive stream's config.
- // The accessors must be called on the packet delivery thread in accordance
- // to documentation for RtpConfig (see receive_stream.h), the returned
- // values should not be cached and should just be used within the calling
- // context as some values might change.
- const Config::Rtp& rtp() const;
+ // Accessor for the a/v sync group. This value may change and the caller
+ // must be on the packet delivery thread.
const std::string& sync_group() const;
+ // Getters for const remote SSRC values that won't change throughout the
+ // object's lifetime.
+ uint32_t remote_ssrc() const { return config_.rtp.remote_ssrc; }
+ uint32_t rtx_ssrc() const { return config_.rtp.rtx_ssrc; }
+
void SignalNetworkState(NetworkState state);
bool DeliverRtcp(const uint8_t* packet, size_t length);
@@ -136,7 +137,7 @@
void SetRtpExtensions(std::vector<RtpExtension> extensions) override;
const std::vector<RtpExtension>& GetRtpExtensions() const override;
- bool transport_cc() const override { return rtp().transport_cc; }
+ bool transport_cc() const override { return config_.rtp.transport_cc; }
webrtc::VideoReceiveStream::Stats GetStats() const override;
