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

Movable support for VideoReceiveStream::Config and avoid copies.

Browse Source 
Instead of the default copy constructor, the Copy() method has to be used.  In this CL, the number of copies has been reduced significantly in production code. One case in the video engine remains, where we need to restart a video stream.  Even in that case, I'm sure we could avoid it, but for this particular CL, I decided against it to keep things simple (and it's also an edge case).  Most importantly, creating copies is made harder and the interface encourages ownership transfers.

R=mflodman@webrtc.org, pbos@webrtc.org

Review URL: https://codereview.webrtc.org/2042603002 .

Cr-Commit-Position: refs/heads/master@{#13102}
This commit is contained in:
Tommi 2016-06-10 17:58:01 +02:00
parent bd3380ff7e
commit 733b5478dd
19 changed files with 84 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
webrtc/call.h
View File

@ -114,7 +114,7 @@ class Call {
virtual void DestroyVideoSendStream(VideoSendStream* send_stream) = 0;
virtual VideoReceiveStream* CreateVideoReceiveStream(
const VideoReceiveStream::Config& config) = 0;
VideoReceiveStream::Config configuration) = 0;
virtual void DestroyVideoReceiveStream(
VideoReceiveStream* receive_stream) = 0;

2
webrtc/call/bitrate_estimator_tests.cc
View File

@ -205,7 +205,7 @@ class BitrateEstimatorTest : public test::CallTest {
test_->video_send_config_.rtp.ssrcs[0];
test_->receive_config_.rtp.local_ssrc++;
video_receive_stream_ = test_->receiver_call_->CreateVideoReceiveStream(
test_->receive_config_);
test_->receive_config_.Copy());
video_receive_stream_->Start();
}
is_sending_receiving_ = true;

10
webrtc/call/call.cc
View File

@ -77,7 +77,7 @@ class Call : public webrtc::Call,
void DestroyVideoSendStream(webrtc::VideoSendStream* send_stream) override;
webrtc::VideoReceiveStream* CreateVideoReceiveStream(
const webrtc::VideoReceiveStream::Config& config) override;
webrtc::VideoReceiveStream::Config configuration) override;
void DestroyVideoReceiveStream(
webrtc::VideoReceiveStream* receive_stream) override;
@ -467,12 +467,14 @@ void Call::DestroyVideoSendStream(webrtc::VideoSendStream* send_stream) {
}
webrtc::VideoReceiveStream* Call::CreateVideoReceiveStream(
const webrtc::VideoReceiveStream::Config& config) {
webrtc::VideoReceiveStream::Config configuration) {
TRACE_EVENT0("webrtc", "Call::CreateVideoReceiveStream");
RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
VideoReceiveStream* receive_stream = new VideoReceiveStream(
num_cpu_cores_, congestion_controller_.get(), config, voice_engine(),
module_process_thread_.get(), call_stats_.get(), &remb_);
num_cpu_cores_, congestion_controller_.get(), std::move(configuration),
voice_engine(), module_process_thread_.get(), call_stats_.get(), &remb_);
const webrtc::VideoReceiveStream::Config& config = receive_stream->config();
{
WriteLockScoped write_lock(*receive_crit_);
RTC_DCHECK(video_receive_ssrcs_.find(config.rtp.remote_ssrc) ==

12
webrtc/media/engine/fakewebrtccall.cc
View File

@ -208,11 +208,10 @@ void FakeVideoSendStream::Stop() {
}
FakeVideoReceiveStream::FakeVideoReceiveStream(
const webrtc::VideoReceiveStream::Config& config)
: config_(config), receiving_(false) {
}
webrtc::VideoReceiveStream::Config config)
: config_(std::move(config)), receiving_(false) {}
webrtc::VideoReceiveStream::Config FakeVideoReceiveStream::GetConfig() {
const webrtc::VideoReceiveStream::Config& FakeVideoReceiveStream::GetConfig() {
return config_;
}
@ -374,8 +373,9 @@ void FakeCall::DestroyVideoSendStream(webrtc::VideoSendStream* send_stream) {
}
webrtc::VideoReceiveStream* FakeCall::CreateVideoReceiveStream(
const webrtc::VideoReceiveStream::Config& config) {
video_receive_streams_.push_back(new FakeVideoReceiveStream(config));
webrtc::VideoReceiveStream::Config config) {
video_receive_streams_.push_back(
new FakeVideoReceiveStream(std::move(config)));
++num_created_receive_streams_;
return video_receive_streams_.back();
}

7
webrtc/media/engine/fakewebrtccall.h
View File

@ -140,10 +140,9 @@ class FakeVideoSendStream final : public webrtc::VideoSendStream,
class FakeVideoReceiveStream final : public webrtc::VideoReceiveStream {
public:
explicit FakeVideoReceiveStream(
const webrtc::VideoReceiveStream::Config& config);
explicit FakeVideoReceiveStream(webrtc::VideoReceiveStream::Config config);
webrtc::VideoReceiveStream::Config GetConfig();
const webrtc::VideoReceiveStream::Config& GetConfig();
bool IsReceiving() const;
@ -199,7 +198,7 @@ class FakeCall final : public webrtc::Call, public webrtc::PacketReceiver {
void DestroyVideoSendStream(webrtc::VideoSendStream* send_stream) override;
webrtc::VideoReceiveStream* CreateVideoReceiveStream(
const webrtc::VideoReceiveStream::Config& config) override;
webrtc::VideoReceiveStream::Config config) override;
void DestroyVideoReceiveStream(
webrtc::VideoReceiveStream* receive_stream) override;
webrtc::PacketReceiver* Receiver() override;

10
webrtc/media/engine/webrtcvideoengine2.cc
View File

@ -1230,7 +1230,7 @@ bool WebRtcVideoChannel2::AddRecvStream(const StreamParams& sp,
video_config_.disable_prerenderer_smoothing;
receive_streams_[ssrc] = new WebRtcVideoReceiveStream(
call_, sp, config, external_decoder_factory_, default_stream,
call_, sp, std::move(config), external_decoder_factory_, default_stream,
recv_codecs_, red_disabled_by_remote_side_);
return true;
@ -2200,7 +2200,7 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::RecreateWebRtcStream() {
WebRtcVideoChannel2::WebRtcVideoReceiveStream::WebRtcVideoReceiveStream(
webrtc::Call* call,
const StreamParams& sp,
const webrtc::VideoReceiveStream::Config& config,
webrtc::VideoReceiveStream::Config config,
WebRtcVideoDecoderFactory* external_decoder_factory,
bool default_stream,
const std::vector<VideoCodecSettings>& recv_codecs,
@ -2210,7 +2210,7 @@ WebRtcVideoChannel2::WebRtcVideoReceiveStream::WebRtcVideoReceiveStream(
ssrc_groups_(sp.ssrc_groups),
stream_(NULL),
default_stream_(default_stream),
config_(config),
config_(std::move(config)),
red_disabled_by_remote_side_(red_disabled_by_remote_side),
external_decoder_factory_(external_decoder_factory),
sink_(NULL),
@ -2387,13 +2387,13 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::RecreateWebRtcStream() {
if (stream_ != NULL) {
call_->DestroyVideoReceiveStream(stream_);
}
webrtc::VideoReceiveStream::Config config = config_;
webrtc::VideoReceiveStream::Config config = config_.Copy();
if (red_disabled_by_remote_side_) {
config.rtp.fec.red_payload_type = -1;
config.rtp.fec.ulpfec_payload_type = -1;
config.rtp.fec.red_rtx_payload_type = -1;
}
stream_ = call_->CreateVideoReceiveStream(config);
stream_ = call_->CreateVideoReceiveStream(std::move(config));
stream_->Start();
}

2
webrtc/media/engine/webrtcvideoengine2.h
View File

@ -417,7 +417,7 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport {
WebRtcVideoReceiveStream(
webrtc::Call* call,
const StreamParams& sp,
const webrtc::VideoReceiveStream::Config& config,
webrtc::VideoReceiveStream::Config config,
WebRtcVideoDecoderFactory* external_decoder_factory,
bool default_stream,
const std::vector<VideoCodecSettings>& recv_codecs,

21
webrtc/media/engine/webrtcvideoengine2_unittest.cc
View File

@ -2628,7 +2628,7 @@ TEST_F(WebRtcVideoChannel2Test, SetRecvCodecsAcceptDefaultCodecs) {
EXPECT_TRUE(channel_->SetRecvParameters(parameters));
FakeVideoReceiveStream* stream = AddRecvStream();
webrtc::VideoReceiveStream::Config config = stream->GetConfig();
const webrtc::VideoReceiveStream::Config& config = stream->GetConfig();
EXPECT_EQ(engine_.codecs()[0].name, config.decoders[0].payload_name);
EXPECT_EQ(engine_.codecs()[0].id, config.decoders[0].payload_type);
}
@ -2666,25 +2666,21 @@ TEST_F(WebRtcVideoChannel2Test, SetRecvCodecsWithoutFecDisablesFec) {
ASSERT_TRUE(channel_->SetSendParameters(send_parameters));
FakeVideoReceiveStream* stream = AddRecvStream();
webrtc::VideoReceiveStream::Config config = stream->GetConfig();
EXPECT_EQ(kUlpfecCodec.id, config.rtp.fec.ulpfec_payload_type);
EXPECT_EQ(kUlpfecCodec.id, stream->GetConfig().rtp.fec.ulpfec_payload_type);
cricket::VideoRecvParameters recv_parameters;
recv_parameters.codecs.push_back(kVp8Codec);
ASSERT_TRUE(channel_->SetRecvParameters(recv_parameters));
stream = fake_call_->GetVideoReceiveStreams()[0];
ASSERT_TRUE(stream != NULL);
config = stream->GetConfig();
EXPECT_EQ(-1, config.rtp.fec.ulpfec_payload_type)
EXPECT_EQ(-1, stream->GetConfig().rtp.fec.ulpfec_payload_type)
<< "SetSendCodec without FEC should disable current FEC.";
}
TEST_F(WebRtcVideoChannel2Test, SetSendParamsWithoutFecDisablesReceivingFec) {
FakeVideoReceiveStream* stream = AddRecvStream();
webrtc::VideoReceiveStream::Config config = stream->GetConfig();
EXPECT_EQ(kUlpfecCodec.id, config.rtp.fec.ulpfec_payload_type);
EXPECT_EQ(kUlpfecCodec.id, stream->GetConfig().rtp.fec.ulpfec_payload_type);
cricket::VideoRecvParameters recv_parameters;
recv_parameters.codecs.push_back(kVp8Codec);
@ -2693,16 +2689,14 @@ TEST_F(WebRtcVideoChannel2Test, SetSendParamsWithoutFecDisablesReceivingFec) {
ASSERT_TRUE(channel_->SetRecvParameters(recv_parameters));
stream = fake_call_->GetVideoReceiveStreams()[0];
ASSERT_TRUE(stream != NULL);
config = stream->GetConfig();
EXPECT_EQ(kUlpfecCodec.id, config.rtp.fec.ulpfec_payload_type)
EXPECT_EQ(kUlpfecCodec.id, stream->GetConfig().rtp.fec.ulpfec_payload_type)
<< "FEC should be enabled on the recieve stream.";
cricket::VideoSendParameters send_parameters;
send_parameters.codecs.push_back(kVp8Codec);
ASSERT_TRUE(channel_->SetSendParameters(send_parameters));
stream = fake_call_->GetVideoReceiveStreams()[0];
config = stream->GetConfig();
EXPECT_EQ(-1, config.rtp.fec.ulpfec_payload_type)
EXPECT_EQ(-1, stream->GetConfig().rtp.fec.ulpfec_payload_type)
<< "FEC should have been disabled when we know the other side won't do "
"FEC.";
@ -2710,8 +2704,7 @@ TEST_F(WebRtcVideoChannel2Test, SetSendParamsWithoutFecDisablesReceivingFec) {
send_parameters.codecs.push_back(kUlpfecCodec);
ASSERT_TRUE(channel_->SetSendParameters(send_parameters));
stream = fake_call_->GetVideoReceiveStreams()[0];
config = stream->GetConfig();
EXPECT_EQ(kUlpfecCodec.id, config.rtp.fec.ulpfec_payload_type)
EXPECT_EQ(kUlpfecCodec.id, stream->GetConfig().rtp.fec.ulpfec_payload_type)
<< "FEC should be enabled on the recieve stream.";
}

6
webrtc/test/call_test.cc
View File

@ -217,7 +217,7 @@ void CallTest::CreateMatchingReceiveConfigs(Transport* rtcp_send_transport) {
video_config.decoders.clear();
video_config.decoders.push_back(decoder);
video_config.rtp.remote_ssrc = video_send_config_.rtp.ssrcs[i];
video_receive_configs_.push_back(video_config);
video_receive_configs_.push_back(video_config.Copy());
}
}
@ -266,8 +266,8 @@ void CallTest::CreateVideoStreams() {
video_send_stream_ = sender_call_->CreateVideoSendStream(
video_send_config_, video_encoder_config_);
for (size_t i = 0; i < video_receive_configs_.size(); ++i) {
video_receive_streams_.push_back(
receiver_call_->CreateVideoReceiveStream(video_receive_configs_[i]));
video_receive_streams_.push_back(receiver_call_->CreateVideoReceiveStream(
video_receive_configs_[i].Copy()));
}
}

2
webrtc/video/end_to_end_tests.cc
View File

@ -1214,7 +1214,7 @@ class MultiStreamTest {
UpdateReceiveConfig(i, &receive_config);
receive_streams[i] =
receiver_call->CreateVideoReceiveStream(receive_config);
receiver_call->CreateVideoReceiveStream(std::move(receive_config));
receive_streams[i]->Start();
frame_generators[i] = test::FrameGeneratorCapturer::Create(

8
webrtc/video/receive_statistics_proxy.cc
View File

@ -20,17 +20,17 @@
namespace webrtc {
ReceiveStatisticsProxy::ReceiveStatisticsProxy(
const VideoReceiveStream::Config& config,
const VideoReceiveStream::Config* config,
Clock* clock)
: clock_(clock),
config_(config),
config_(*config),
// 1000ms window, scale 1000 for ms to s.
decode_fps_estimator_(1000, 1000),
renders_fps_estimator_(1000, 1000),
render_fps_tracker_(100, 10u),
render_pixel_tracker_(100, 10u) {
stats_.ssrc = config.rtp.remote_ssrc;
for (auto it : config.rtp.rtx)
stats_.ssrc = config_.rtp.remote_ssrc;
for (auto it : config_.rtp.rtx)
rtx_stats_[it.second.ssrc] = StreamDataCounters();
}

11
webrtc/video/receive_statistics_proxy.h
View File

@ -37,7 +37,7 @@ class ReceiveStatisticsProxy : public VCMReceiveStatisticsCallback,
public RtcpPacketTypeCounterObserver,
public StreamDataCountersCallback {
public:
ReceiveStatisticsProxy(const VideoReceiveStream::Config& config,
ReceiveStatisticsProxy(const VideoReceiveStream::Config* config,
Clock* clock);
virtual ~ReceiveStatisticsProxy();
@ -96,7 +96,14 @@ class ReceiveStatisticsProxy : public VCMReceiveStatisticsCallback,
void UpdateHistograms() EXCLUSIVE_LOCKS_REQUIRED(crit_);
Clock* const clock_;
const VideoReceiveStream::Config config_;
// Ownership of this object lies with the owner of the ReceiveStatisticsProxy
// instance. Lifetime is guaranteed to outlive |this|.
// TODO(tommi): In practice the config_ reference is only used for accessing
// config_.rtp.fec.ulpfec_payload_type. Instead of holding a pointer back,
// we could just store the value of ulpfec_payload_type and change the
// ReceiveStatisticsProxy() ctor to accept a const& of Config (since we'll
// then no longer store a pointer to the object).
const VideoReceiveStream::Config& config_;
rtc::CriticalSection crit_;
VideoReceiveStream::Stats stats_ GUARDED_BY(crit_);

2
webrtc/video/replay.cc
View File

@ -251,7 +251,7 @@ void RtpReplay() {
receive_config.decoders.push_back(decoder);
VideoReceiveStream* receive_stream =
call->CreateVideoReceiveStream(receive_config);
call->CreateVideoReceiveStream(std::move(receive_config));
std::unique_ptr<test::RtpFileReader> rtp_reader(test::RtpFileReader::Create(
test::RtpFileReader::kRtpDump, flags::InputFile()));

25
webrtc/video/rtp_stream_receiver.cc
View File

@ -77,11 +77,11 @@ RtpStreamReceiver::RtpStreamReceiver(
PacedSender* paced_sender,
PacketRouter* packet_router,
VieRemb* remb,
const VideoReceiveStream::Config& config,
const VideoReceiveStream::Config* config,
ReceiveStatisticsProxy* receive_stats_proxy,
ProcessThread* process_thread)
: clock_(Clock::GetRealTimeClock()),
config_(config),
config_(*config),
video_receiver_(video_receiver),
remote_bitrate_estimator_(remote_bitrate_estimator),
packet_router_(packet_router),
@ -110,7 +110,7 @@ RtpStreamReceiver::RtpStreamReceiver(
rtp_receive_statistics_->RegisterRtpStatisticsCallback(receive_stats_proxy);
rtp_receive_statistics_->RegisterRtcpStatisticsCallback(receive_stats_proxy);
RTC_DCHECK(config.rtp.rtcp_mode != RtcpMode::kOff)
RTC_DCHECK(config_.rtp.rtcp_mode != RtcpMode::kOff)
<< "A stream should not be configured with RTCP disabled. This value is "
"reserved for internal usage.";
RTC_DCHECK(config_.rtp.remote_ssrc != 0);
@ -118,22 +118,23 @@ RtpStreamReceiver::RtpStreamReceiver(
RTC_DCHECK(config_.rtp.local_ssrc != 0);
RTC_DCHECK(config_.rtp.remote_ssrc != config_.rtp.local_ssrc);
rtp_rtcp_->SetRTCPStatus(config.rtp.rtcp_mode);
rtp_rtcp_->SetSSRC(config.rtp.local_ssrc);
rtp_rtcp_->SetRTCPStatus(config_.rtp.rtcp_mode);
rtp_rtcp_->SetSSRC(config_.rtp.local_ssrc);
rtp_rtcp_->SetKeyFrameRequestMethod(kKeyFrameReqPliRtcp);
if (config.rtp.remb) {
if (config_.rtp.remb) {
rtp_rtcp_->SetREMBStatus(true);
remb_->AddReceiveChannel(rtp_rtcp_.get());
}
for (size_t i = 0; i < config.rtp.extensions.size(); ++i) {
EnableReceiveRtpHeaderExtension(config.rtp.extensions[i].uri,
config.rtp.extensions[i].id);
for (size_t i = 0; i < config_.rtp.extensions.size(); ++i) {
EnableReceiveRtpHeaderExtension(config_.rtp.extensions[i].uri,
config_.rtp.extensions[i].id);
}
static const int kMaxPacketAgeToNack = 450;
const int max_reordering_threshold = (config.rtp.nack.rtp_history_ms > 0)
? kMaxPacketAgeToNack : kDefaultMaxReorderingThreshold;
const int max_reordering_threshold = (config_.rtp.nack.rtp_history_ms > 0)
? kMaxPacketAgeToNack
: kDefaultMaxReorderingThreshold;
rtp_receive_statistics_->SetMaxReorderingThreshold(max_reordering_threshold);
// TODO(pbos): Support multiple RTX, per video payload.
@ -178,7 +179,7 @@ RtpStreamReceiver::RtpStreamReceiver(
config_.rtp.fec.ulpfec_payload_type);
}
if (config.rtp.rtcp_xr.receiver_reference_time_report)
if (config_.rtp.rtcp_xr.receiver_reference_time_report)
rtp_rtcp_->SetRtcpXrRrtrStatus(true);
// Stats callback for CNAME changes.

5
webrtc/video/rtp_stream_receiver.h
View File

@ -60,7 +60,7 @@ class RtpStreamReceiver : public RtpData, public RtpFeedback,
PacedSender* paced_sender,
PacketRouter* packet_router,
VieRemb* remb,
const VideoReceiveStream::Config& config,
const VideoReceiveStream::Config* config,
ReceiveStatisticsProxy* receive_stats_proxy,
ProcessThread* process_thread);
~RtpStreamReceiver();
@ -128,7 +128,8 @@ class RtpStreamReceiver : public RtpData, public RtpFeedback,
void EnableReceiveRtpHeaderExtension(const std::string& extension, int id);
Clock* const clock_;
const VideoReceiveStream::Config config_;
// Ownership of this object lies with VideoReceiveStream, which owns |this|.
const VideoReceiveStream::Config& config_;
vcm::VideoReceiver* const video_receiver_;
RemoteBitrateEstimator* const remote_bitrate_estimator_;
PacketRouter* const packet_router_;

2
webrtc/video/video_quality_test.cc
View File

@ -1140,7 +1140,7 @@ void VideoQualityTest::RunWithVideoRenderer(const Params& params) {
video_send_stream_ =
call->CreateVideoSendStream(video_send_config_, video_encoder_config_);
VideoReceiveStream* receive_stream =
call->CreateVideoReceiveStream(video_receive_configs_[stream_id]);
call->CreateVideoReceiveStream(video_receive_configs_[stream_id].Copy());
CreateCapturer(video_send_stream_->Input());
receive_stream->Start();

10
webrtc/video/video_receive_stream.cc
View File

@ -147,22 +147,22 @@ namespace internal {
VideoReceiveStream::VideoReceiveStream(
int num_cpu_cores,
CongestionController* congestion_controller,
const VideoReceiveStream::Config& config,
VideoReceiveStream::Config config,
webrtc::VoiceEngine* voice_engine,
ProcessThread* process_thread,
CallStats* call_stats,
VieRemb* remb)
: transport_adapter_(config.rtcp_send_transport),
encoded_frame_proxy_(config.pre_decode_callback),
config_(config),
config_(std::move(config)),
process_thread_(process_thread),
clock_(Clock::GetRealTimeClock()),
decode_thread_(DecodeThreadFunction, this, "DecodingThread"),
congestion_controller_(congestion_controller),
call_stats_(call_stats),
video_receiver_(clock_, nullptr, this, this, this),
incoming_video_stream_(config.disable_prerenderer_smoothing),
stats_proxy_(config_, clock_),
incoming_video_stream_(config_.disable_prerenderer_smoothing),
stats_proxy_(&config_, clock_),
rtp_stream_receiver_(&video_receiver_,
congestion_controller_->GetRemoteBitrateEstimator(
UseSendSideBwe(config_)),
@ -171,7 +171,7 @@ VideoReceiveStream::VideoReceiveStream(
congestion_controller_->pacer(),
congestion_controller_->packet_router(),
remb,
config,
&config_,
&stats_proxy_,
process_thread_),
video_stream_decoder_(&video_receiver_,

2
webrtc/video/video_receive_stream.h
View File

@ -45,7 +45,7 @@ class VideoReceiveStream : public webrtc::VideoReceiveStream,
public:
VideoReceiveStream(int num_cpu_cores,
CongestionController* congestion_controller,
const VideoReceiveStream::Config& config,
VideoReceiveStream::Config config,
webrtc::VoiceEngine* voice_engine,
ProcessThread* process_thread,
CallStats* call_stats,

13
webrtc/video_receive_stream.h
View File

@ -76,10 +76,23 @@ class VideoReceiveStream {
};
struct Config {
private:
// Access to the copy constructor is private to force use of the Copy()
// method for those exceptional cases where we do use it.
Config(const Config&) = default;
public:
Config() = delete;
Config(Config&&) = default;
explicit Config(Transport* rtcp_send_transport)
: rtcp_send_transport(rtcp_send_transport) {}
Config& operator=(Config&&) = default;
Config& operator=(const Config&) = delete;
// Mostly used by tests. Avoid creating copies if you can.
Config Copy() const { return Config(*this); }
std::string ToString() const;
// Decoders for every payload that we can receive.