Delete pre_encode_callback from VideoSendStream::Config
Bug: webrtc:9864
Change-Id: I7f0c897345c99765ea9de77bc70b43ba0e4af19b
Reviewed-on: https://webrtc-review.googlesource.com/c/115320
Commit-Queue: Niels Moller <nisse@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26110}diff --git a/api/video/video_stream_encoder_create.cc b/api/video/video_stream_encoder_create.cc
index b36c916..b72d455 100644
--- a/api/video/video_stream_encoder_create.cc
+++ b/api/video/video_stream_encoder_create.cc
@@ -18,11 +18,9 @@
std::unique_ptr<VideoStreamEncoderInterface> CreateVideoStreamEncoder(
uint32_t number_of_cores,
VideoStreamEncoderObserver* encoder_stats_observer,
- const VideoStreamEncoderSettings& settings,
- // Deprecated, used for tests only.
- rtc::VideoSinkInterface<VideoFrame>* pre_encode_callback) {
+ const VideoStreamEncoderSettings& settings) {
return absl::make_unique<VideoStreamEncoder>(
- number_of_cores, encoder_stats_observer, settings, pre_encode_callback,
+ number_of_cores, encoder_stats_observer, settings,
absl::make_unique<OveruseFrameDetector>(encoder_stats_observer));
}
} // namespace webrtc
diff --git a/api/video/video_stream_encoder_create.h b/api/video/video_stream_encoder_create.h
index 72a2719..38749da 100644
--- a/api/video/video_stream_encoder_create.h
+++ b/api/video/video_stream_encoder_create.h
@@ -25,17 +25,7 @@
std::unique_ptr<VideoStreamEncoderInterface> CreateVideoStreamEncoder(
uint32_t number_of_cores,
VideoStreamEncoderObserver* encoder_stats_observer,
- const VideoStreamEncoderSettings& settings,
- // Deprecated, used for tests only.
- rtc::VideoSinkInterface<VideoFrame>* pre_encode_callback);
-
-inline std::unique_ptr<VideoStreamEncoderInterface> CreateVideoStreamEncoder(
- uint32_t number_of_cores,
- VideoStreamEncoderObserver* encoder_stats_observer,
- const VideoStreamEncoderSettings& settings) {
- return CreateVideoStreamEncoder(number_of_cores, encoder_stats_observer,
- settings, nullptr);
-}
+ const VideoStreamEncoderSettings& settings);
} // namespace webrtc
diff --git a/call/video_send_stream.cc b/call/video_send_stream.cc
index c8ba308..868b2f3 100644
--- a/call/video_send_stream.cc
+++ b/call/video_send_stream.cc
@@ -80,8 +80,6 @@
<< (encoder_settings.experiment_cpu_load_estimator ? "on" : "off") << "}}";
ss << ", rtp: " << rtp.ToString();
ss << ", rtcp_report_interval_ms: " << rtcp_report_interval_ms;
- ss << ", pre_encode_callback: "
- << (pre_encode_callback ? "(VideoSinkInterface)" : "nullptr");
ss << ", render_delay_ms: " << render_delay_ms;
ss << ", target_delay_ms: " << target_delay_ms;
ss << ", suspend_below_min_bitrate: "
diff --git a/call/video_send_stream.h b/call/video_send_stream.h
index 4d3d9c0..74a1e8b 100644
--- a/call/video_send_stream.h
+++ b/call/video_send_stream.h
@@ -119,10 +119,6 @@
// Transport for outgoing packets.
Transport* send_transport = nullptr;
- // Called for each I420 frame before encoding the frame. Can be used for
- // effects, snapshots etc. 'nullptr' disables the callback.
- rtc::VideoSinkInterface<VideoFrame>* pre_encode_callback = nullptr;
-
// Expected delay needed by the renderer, i.e. the frame will be delivered
// this many milliseconds, if possible, earlier than expected render time.
// Only valid if |local_renderer| is set.
diff --git a/video/end_to_end_tests/stats_tests.cc b/video/end_to_end_tests/stats_tests.cc
index d6e92c8..c2b00d1 100644
--- a/video/end_to_end_tests/stats_tests.cc
+++ b/video/end_to_end_tests/stats_tests.cc
@@ -37,8 +37,7 @@
void OnFrame(const VideoFrame& video_frame) override {}
};
- class StatsObserver : public test::EndToEndTest,
- public rtc::VideoSinkInterface<VideoFrame> {
+ class StatsObserver : public test::EndToEndTest {
public:
StatsObserver()
: EndToEndTest(kLongTimeoutMs),
@@ -80,11 +79,6 @@
return SEND_PACKET;
}
- void OnFrame(const VideoFrame& video_frame) override {
- // Ensure that we have at least 5ms send side delay.
- SleepMs(5);
- }
-
bool CheckReceiveStats() {
for (size_t i = 0; i < receive_streams_.size(); ++i) {
VideoReceiveStream::Stats stats = receive_streams_[i]->GetStats();
@@ -250,7 +244,7 @@
// in ModifyVideoConfigs.
class VideoStreamFactory
: public VideoEncoderConfig::VideoStreamFactoryInterface {
- public:
+ public: // NOLINT(whitespace/blank_line)
VideoStreamFactory() {}
private:
@@ -276,7 +270,6 @@
VideoEncoderConfig* encoder_config) override {
encoder_config->video_stream_factory =
new rtc::RefCountedObject<VideoStreamFactory>();
- send_config->pre_encode_callback = this; // Used to inject delay.
expected_cname_ = send_config->rtp.c_name = "SomeCName";
send_config->rtp.nack.rtp_history_ms = kNackRtpHistoryMs;
diff --git a/video/video_quality_test.cc b/video/video_quality_test.cc
index 274388a..b3cf33b 100644
--- a/video/video_quality_test.cc
+++ b/video/video_quality_test.cc
@@ -1286,12 +1286,7 @@
if (params_.video[0].enabled) {
// Create video renderers.
- local_preview.reset(test::VideoRenderer::Create(
- "Local Preview", params_.video[0].width, params_.video[0].height));
-
SetupVideo(send_transport.get(), recv_transport.get());
- GetVideoSendConfig()->pre_encode_callback = local_preview.get();
-
size_t num_streams_processed = 0;
for (size_t video_idx = 0; video_idx < num_video_streams_; ++video_idx) {
const size_t selected_stream_id = params_.ss[video_idx].selected_stream;
@@ -1330,6 +1325,14 @@
CreateVideoStreams();
CreateCapturers();
+ if (params_.video[0].enabled) {
+ // Create local preview
+ local_preview.reset(test::VideoRenderer::Create(
+ "Local Preview", params_.video[0].width, params_.video[0].height));
+
+ video_sources_[0]->AddOrUpdateSink(local_preview.get(),
+ rtc::VideoSinkWants());
+ }
ConnectVideoSourcesToStreams();
}
diff --git a/video/video_send_stream.cc b/video/video_send_stream.cc
index b222fe42..2ec348b 100644
--- a/video/video_send_stream.cc
+++ b/video/video_send_stream.cc
@@ -88,8 +88,7 @@
RTC_DCHECK(config_.encoder_settings.bitrate_allocator_factory);
video_stream_encoder_ = CreateVideoStreamEncoder(num_cpu_cores, &stats_proxy_,
- config_.encoder_settings,
- config_.pre_encode_callback);
+ config_.encoder_settings);
// TODO(srte): Initialization should not be done posted on a task queue.
// Note that the posted task must not outlive this scope since the closure
// references local variables.
diff --git a/video/video_send_stream_tests.cc b/video/video_send_stream_tests.cc
index 00be2ca..37613e8 100644
--- a/video/video_send_stream_tests.cc
+++ b/video/video_send_stream_tests.cc
@@ -1219,12 +1219,32 @@
TEST_P(VideoSendStreamTest, SuspendBelowMinBitrate) {
static const int kSuspendTimeFrames = 60; // Suspend for 2 seconds @ 30 fps.
- class RembObserver : public test::SendTest,
- public rtc::VideoSinkInterface<VideoFrame> {
+ class RembObserver : public test::SendTest {
public:
+ class CaptureObserver : public rtc::VideoSinkInterface<VideoFrame> {
+ public:
+ explicit CaptureObserver(RembObserver* remb_observer)
+ : remb_observer_(remb_observer) {}
+
+ void OnFrame(const VideoFrame&) {
+ rtc::CritScope lock(&remb_observer_->crit_);
+ if (remb_observer_->test_state_ == kDuringSuspend &&
+ ++remb_observer_->suspended_frame_count_ > kSuspendTimeFrames) {
+ VideoSendStream::Stats stats = remb_observer_->stream_->GetStats();
+ EXPECT_TRUE(stats.suspended);
+ remb_observer_->SendRtcpFeedback(remb_observer_->high_remb_bps_);
+ remb_observer_->test_state_ = kWaitingForPacket;
+ }
+ }
+
+ private:
+ RembObserver* const remb_observer_;
+ };
+
RembObserver()
: SendTest(kDefaultTimeoutMs),
clock_(Clock::GetRealTimeClock()),
+ capture_observer_(this),
stream_(nullptr),
test_state_(kBeforeSuspend),
rtp_count_(0),
@@ -1271,19 +1291,6 @@
return SEND_PACKET;
}
- // This method implements the rtc::VideoSinkInterface. This is called when
- // a frame is provided to the VideoSendStream.
- void OnFrame(const VideoFrame& video_frame) override {
- rtc::CritScope lock(&crit_);
- if (test_state_ == kDuringSuspend &&
- ++suspended_frame_count_ > kSuspendTimeFrames) {
- VideoSendStream::Stats stats = stream_->GetStats();
- EXPECT_TRUE(stats.suspended);
- SendRtcpFeedback(high_remb_bps_);
- test_state_ = kWaitingForPacket;
- }
- }
-
void set_low_remb_bps(int value) {
rtc::CritScope lock(&crit_);
low_remb_bps_ = value;
@@ -1300,6 +1307,12 @@
stream_ = send_stream;
}
+ void OnFrameGeneratorCapturerCreated(
+ test::FrameGeneratorCapturer* frame_generator_capturer) override {
+ frame_generator_capturer->AddOrUpdateSink(&capture_observer_,
+ rtc::VideoSinkWants());
+ }
+
void ModifyVideoConfigs(
VideoSendStream::Config* send_config,
std::vector<VideoReceiveStream::Config>* receive_configs,
@@ -1309,7 +1322,6 @@
new internal::TransportAdapter(send_config->send_transport));
transport_adapter_->Enable();
send_config->rtp.nack.rtp_history_ms = kNackRtpHistoryMs;
- send_config->pre_encode_callback = this;
send_config->suspend_below_min_bitrate = true;
int min_bitrate_bps =
test::DefaultVideoStreamFactory::kDefaultMinBitratePerStream[0];
@@ -1349,6 +1361,7 @@
std::unique_ptr<internal::TransportAdapter> transport_adapter_;
Clock* const clock_;
+ CaptureObserver capture_observer_;
VideoSendStream* stream_;
rtc::CriticalSection crit_;
diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc
index 235e72b..dfacb21 100644
--- a/video/video_stream_encoder.cc
+++ b/video/video_stream_encoder.cc
@@ -347,7 +347,6 @@
uint32_t number_of_cores,
VideoStreamEncoderObserver* encoder_stats_observer,
const VideoStreamEncoderSettings& settings,
- rtc::VideoSinkInterface<VideoFrame>* pre_encode_callback,
std::unique_ptr<OveruseFrameDetector> overuse_detector)
: shutdown_event_(true /* manual_reset */, false),
number_of_cores_(number_of_cores),
@@ -361,7 +360,6 @@
video_sender_(Clock::GetRealTimeClock(), this),
overuse_detector_(std::move(overuse_detector)),
encoder_stats_observer_(encoder_stats_observer),
- pre_encode_callback_(pre_encode_callback),
max_framerate_(-1),
pending_encoder_reconfiguration_(false),
pending_encoder_creation_(false),
@@ -782,9 +780,6 @@
int64_t time_when_posted_us) {
RTC_DCHECK_RUN_ON(&encoder_queue_);
- if (pre_encode_callback_)
- pre_encode_callback_->OnFrame(video_frame);
-
if (!last_frame_info_ || video_frame.width() != last_frame_info_->width ||
video_frame.height() != last_frame_info_->height ||
video_frame.is_texture() != last_frame_info_->is_texture) {
diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h
index 8c5efbe..e5ef981 100644
--- a/video/video_stream_encoder.h
+++ b/video/video_stream_encoder.h
@@ -50,7 +50,6 @@
VideoStreamEncoder(uint32_t number_of_cores,
VideoStreamEncoderObserver* encoder_stats_observer,
const VideoStreamEncoderSettings& settings,
- rtc::VideoSinkInterface<VideoFrame>* pre_encode_callback,
std::unique_ptr<OveruseFrameDetector> overuse_detector);
~VideoStreamEncoder() override;
@@ -196,7 +195,6 @@
RTC_PT_GUARDED_BY(&encoder_queue_);
VideoStreamEncoderObserver* const encoder_stats_observer_;
- rtc::VideoSinkInterface<VideoFrame>* const pre_encode_callback_;
// |thread_checker_| checks that public methods that are related to lifetime
// of VideoStreamEncoder are called on the same thread.
rtc::ThreadChecker thread_checker_;
diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc
index 05a23c0..c45c65f 100644
--- a/video/video_stream_encoder_unittest.cc
+++ b/video/video_stream_encoder_unittest.cc
@@ -99,7 +99,6 @@
: VideoStreamEncoder(1 /* number_of_cores */,
stats_proxy,
settings,
- nullptr /* pre_encode_callback */,
std::unique_ptr<OveruseFrameDetector>(
overuse_detector_proxy_ =
new CpuOveruseDetectorProxy(stats_proxy))) {}
