Remove callback_cs_ in ViEEncoder.
Instead make callbacks const and set on construction.
BUG=webrtc:1695
R=philipel@webrtc.org
Review URL: https://codereview.webrtc.org/1354143004 .
Cr-Commit-Position: refs/heads/master@{#10017}
diff --git a/webrtc/video/send_statistics_proxy.cc b/webrtc/video/send_statistics_proxy.cc
index 505dc07..a95ade5 100644
--- a/webrtc/video/send_statistics_proxy.cc
+++ b/webrtc/video/send_statistics_proxy.cc
@@ -66,9 +66,7 @@
RTC_HISTOGRAM_COUNTS_1000("WebRTC.Video.EncodeTimeInMs", encode_ms);
}
-void SendStatisticsProxy::OutgoingRate(const int video_channel,
- const unsigned int framerate,
- const unsigned int bitrate) {
+void SendStatisticsProxy::OnOutgoingRate(uint32_t framerate, uint32_t bitrate) {
rtc::CritScope lock(&crit_);
stats_.encode_frame_rate = framerate;
stats_.media_bitrate_bps = bitrate;
@@ -82,7 +80,7 @@
stats_.encode_usage_percent = metrics.encode_usage_percent;
}
-void SendStatisticsProxy::SuspendChange(int video_channel, bool is_suspended) {
+void SendStatisticsProxy::OnSuspendChange(bool is_suspended) {
rtc::CritScope lock(&crit_);
stats_.suspended = is_suspended;
}
diff --git a/webrtc/video/send_statistics_proxy.h b/webrtc/video/send_statistics_proxy.h
index 6c6a0ba..3510ded 100644
--- a/webrtc/video/send_statistics_proxy.h
+++ b/webrtc/video/send_statistics_proxy.h
@@ -33,7 +33,6 @@
public StreamDataCountersCallback,
public BitrateStatisticsObserver,
public FrameCountObserver,
- public ViEEncoderObserver,
public VideoEncoderRateObserver,
public SendSideDelayObserver {
public:
@@ -55,6 +54,8 @@
// From VideoEncoderRateObserver.
void OnSetRates(uint32_t bitrate_bps, int framerate) override;
+ void OnOutgoingRate(uint32_t framerate, uint32_t bitrate);
+ void OnSuspendChange(bool is_suspended);
void OnInactiveSsrc(uint32_t ssrc);
protected:
@@ -81,13 +82,6 @@
void FrameCountUpdated(const FrameCounts& frame_counts,
uint32_t ssrc) override;
- // From ViEEncoderObserver.
- void OutgoingRate(const int video_channel,
- const unsigned int framerate,
- const unsigned int bitrate) override;
-
- void SuspendChange(int video_channel, bool is_suspended) override;
-
void SendSideDelayUpdated(int avg_delay_ms,
int max_delay_ms,
uint32_t ssrc) override;
diff --git a/webrtc/video/send_statistics_proxy_unittest.cc b/webrtc/video/send_statistics_proxy_unittest.cc
index ae05842..8e6b7bc 100644
--- a/webrtc/video/send_statistics_proxy_unittest.cc
+++ b/webrtc/video/send_statistics_proxy_unittest.cc
@@ -129,11 +129,10 @@
}
TEST_F(SendStatisticsProxyTest, EncodedBitrateAndFramerate) {
- const int media_bitrate_bps = 500;
- const int encode_fps = 29;
+ int media_bitrate_bps = 500;
+ int encode_fps = 29;
- ViEEncoderObserver* encoder_observer = statistics_proxy_.get();
- encoder_observer->OutgoingRate(0, encode_fps, media_bitrate_bps);
+ statistics_proxy_->OnOutgoingRate(encode_fps, media_bitrate_bps);
VideoSendStream::Stats stats = statistics_proxy_->GetStats();
EXPECT_EQ(media_bitrate_bps, stats.media_bitrate_bps);
@@ -145,12 +144,11 @@
EXPECT_FALSE(statistics_proxy_->GetStats().suspended);
// Verify that we can set it to true.
- ViEEncoderObserver* encoder_observer = statistics_proxy_.get();
- encoder_observer->SuspendChange(0, true);
+ statistics_proxy_->OnSuspendChange(true);
EXPECT_TRUE(statistics_proxy_->GetStats().suspended);
// Verify that we can set it back to false again.
- encoder_observer->SuspendChange(0, false);
+ statistics_proxy_->OnSuspendChange(false);
EXPECT_FALSE(statistics_proxy_->GetStats().suspended);
}
diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc
index c9cdfa8..6703448 100644
--- a/webrtc/video/video_send_stream.cc
+++ b/webrtc/video/video_send_stream.cc
@@ -118,8 +118,9 @@
use_config_bitrate_(true),
stats_proxy_(Clock::GetRealTimeClock(), config) {
RTC_DCHECK(!config_.rtp.ssrcs.empty());
- RTC_CHECK(channel_group->CreateSendChannel(channel_id_, &transport_adapter_,
- num_cpu_cores, config_));
+ RTC_CHECK(channel_group->CreateSendChannel(
+ channel_id_, &transport_adapter_, &stats_proxy_,
+ config.pre_encode_callback, num_cpu_cores, config_));
vie_channel_ = channel_group_->GetChannel(channel_id_);
vie_encoder_ = channel_group_->GetEncoder(channel_id_);
@@ -178,9 +179,7 @@
RTC_CHECK(ReconfigureVideoEncoder(encoder_config));
vie_channel_->RegisterSendSideDelayObserver(&stats_proxy_);
- vie_encoder_->RegisterSendStatisticsProxy(&stats_proxy_);
- vie_encoder_->RegisterPreEncodeCallback(config_.pre_encode_callback);
if (config_.post_encode_callback)
vie_encoder_->RegisterPostEncodeImageCallback(&encoded_frame_proxy_);
@@ -198,22 +197,15 @@
vie_channel_->RegisterRtcpPacketTypeCounterObserver(&stats_proxy_);
vie_channel_->RegisterSendBitrateObserver(&stats_proxy_);
vie_channel_->RegisterSendFrameCountObserver(&stats_proxy_);
-
- vie_encoder_->RegisterCodecObserver(&stats_proxy_);
}
VideoSendStream::~VideoSendStream() {
- vie_encoder_->RegisterCodecObserver(nullptr);
-
vie_channel_->RegisterSendFrameCountObserver(nullptr);
vie_channel_->RegisterSendBitrateObserver(nullptr);
vie_channel_->RegisterRtcpPacketTypeCounterObserver(nullptr);
vie_channel_->RegisterSendChannelRtpStatisticsCallback(nullptr);
vie_channel_->RegisterSendChannelRtcpStatisticsCallback(nullptr);
- vie_encoder_->RegisterPreEncodeCallback(nullptr);
- vie_encoder_->RegisterPostEncodeImageCallback(nullptr);
-
// Remove capture input (thread) so that it's not running after the current
// channel is deleted.
input_.reset();
diff --git a/webrtc/video_engine/encoder_state_feedback_unittest.cc b/webrtc/video_engine/encoder_state_feedback_unittest.cc
index dc93266..b61fc56 100644
--- a/webrtc/video_engine/encoder_state_feedback_unittest.cc
+++ b/webrtc/video_engine/encoder_state_feedback_unittest.cc
@@ -32,7 +32,7 @@
class MockVieEncoder : public ViEEncoder {
public:
explicit MockVieEncoder(ProcessThread* process_thread, PacedSender* pacer)
- : ViEEncoder(1, 1, *process_thread, pacer, NULL) {}
+ : ViEEncoder(1, 1, process_thread, nullptr, nullptr, pacer, nullptr) {}
~MockVieEncoder() {}
MOCK_METHOD1(OnReceivedIntraFrameRequest,
diff --git a/webrtc/video_engine/vie_channel_group.cc b/webrtc/video_engine/vie_channel_group.cc
index ce04795..6553ce2 100644
--- a/webrtc/video_engine/vie_channel_group.cc
+++ b/webrtc/video_engine/vie_channel_group.cc
@@ -194,6 +194,8 @@
bool ChannelGroup::CreateSendChannel(int channel_id,
Transport* transport,
+ SendStatisticsProxy* stats_proxy,
+ I420FrameCallback* pre_encode_callback,
int number_of_cores,
const VideoSendStream::Config& config) {
TransportFeedbackObserver* transport_feedback_observer = nullptr;
@@ -220,9 +222,9 @@
const std::vector<uint32_t>& ssrcs = config.rtp.ssrcs;
RTC_DCHECK(!ssrcs.empty());
- rtc::scoped_ptr<ViEEncoder> vie_encoder(
- new ViEEncoder(channel_id, number_of_cores, *process_thread_,
- pacer_.get(), bitrate_allocator_.get()));
+ rtc::scoped_ptr<ViEEncoder> vie_encoder(new ViEEncoder(
+ channel_id, number_of_cores, process_thread_, stats_proxy,
+ pre_encode_callback, pacer_.get(), bitrate_allocator_.get()));
if (!vie_encoder->Init()) {
return false;
}
diff --git a/webrtc/video_engine/vie_channel_group.h b/webrtc/video_engine/vie_channel_group.h
index 1c4c732..e133d1a 100644
--- a/webrtc/video_engine/vie_channel_group.h
+++ b/webrtc/video_engine/vie_channel_group.h
@@ -28,11 +28,13 @@
class CallStats;
class Config;
class EncoderStateFeedback;
+class I420FrameCallback;
class PacedSender;
class PacketRouter;
class ProcessThread;
class RemoteBitrateEstimator;
class RemoteEstimatorProxy;
+class SendStatisticsProxy;
class TransportFeedbackAdapter;
class ViEChannel;
class ViEEncoder;
@@ -49,6 +51,8 @@
~ChannelGroup();
bool CreateSendChannel(int channel_id,
Transport* transport,
+ SendStatisticsProxy* stats_proxy,
+ I420FrameCallback* pre_encode_callback,
int number_of_cores,
const VideoSendStream::Config& config);
bool CreateReceiveChannel(int channel_id,
diff --git a/webrtc/video_engine/vie_encoder.cc b/webrtc/video_engine/vie_encoder.cc
index f73f060..cc4783f 100644
--- a/webrtc/video_engine/vie_encoder.cc
+++ b/webrtc/video_engine/vie_encoder.cc
@@ -104,7 +104,9 @@
ViEEncoder::ViEEncoder(int32_t channel_id,
uint32_t number_of_cores,
- ProcessThread& module_process_thread,
+ ProcessThread* module_process_thread,
+ SendStatisticsProxy* stats_proxy,
+ I420FrameCallback* pre_encode_callback,
PacedSender* pacer,
BitrateAllocator* bitrate_allocator)
: channel_id_(channel_id),
@@ -115,8 +117,9 @@
this,
qm_callback_.get())),
send_payload_router_(NULL),
- callback_cs_(CriticalSectionWrapper::CreateCriticalSection()),
data_cs_(CriticalSectionWrapper::CreateCriticalSection()),
+ stats_proxy_(stats_proxy),
+ pre_encode_callback_(pre_encode_callback),
pacer_(pacer),
bitrate_allocator_(bitrate_allocator),
time_of_last_frame_activity_ms_(0),
@@ -129,16 +132,13 @@
encoder_paused_and_dropped_frame_(false),
fec_enabled_(false),
nack_enabled_(false),
- codec_observer_(NULL),
module_process_thread_(module_process_thread),
has_received_sli_(false),
picture_id_sli_(0),
has_received_rpsi_(false),
picture_id_rpsi_(0),
video_suspended_(false),
- pre_encode_callback_(NULL),
- start_ms_(Clock::GetRealTimeClock()->TimeInMilliseconds()),
- send_statistics_proxy_(NULL) {
+ start_ms_(Clock::GetRealTimeClock()->TimeInMilliseconds()) {
bitrate_observer_.reset(new ViEBitrateObserver(this));
}
@@ -164,14 +164,14 @@
send_payload_router_ = send_payload_router;
vcm_->RegisterProtectionCallback(vcm_protection_callback);
- module_process_thread_.RegisterModule(vcm_.get());
+ module_process_thread_->RegisterModule(vcm_.get());
}
void ViEEncoder::StopThreadsAndRemoveSharedMembers() {
if (bitrate_allocator_)
bitrate_allocator_->RemoveBitrateObserver(bitrate_observer_.get());
- module_process_thread_.DeRegisterModule(vcm_.get());
- module_process_thread_.DeRegisterModule(vpm_.get());
+ module_process_thread_->DeRegisterModule(vcm_.get());
+ module_process_thread_->DeRegisterModule(vpm_.get());
}
ViEEncoder::~ViEEncoder() {
@@ -449,16 +449,13 @@
// If we haven't resampled the frame and we have a FrameCallback, we need to
// make a deep copy of |video_frame|.
VideoFrame copied_frame;
- {
- CriticalSectionScoped cs(callback_cs_.get());
- if (pre_encode_callback_) {
- // If the frame was not resampled or scaled => use copy of original.
- if (decimated_frame == NULL) {
- copied_frame.CopyFrame(video_frame);
- decimated_frame = &copied_frame;
- }
- pre_encode_callback_->FrameCallback(decimated_frame);
+ if (pre_encode_callback_) {
+ // If the frame was not resampled or scaled => use copy of original.
+ if (decimated_frame == NULL) {
+ copied_frame.CopyFrame(video_frame);
+ decimated_frame = &copied_frame;
}
+ pre_encode_callback_->FrameCallback(decimated_frame);
}
// If the frame was not resampled, scaled, or touched by FrameCallback => use
@@ -577,9 +574,8 @@
}
void ViEEncoder::OnSetRates(uint32_t bitrate_bps, int framerate) {
- CriticalSectionScoped cs(callback_cs_.get());
- if (send_statistics_proxy_ != nullptr)
- send_statistics_proxy_->OnSetRates(bitrate_bps, framerate);
+ if (stats_proxy_)
+ stats_proxy_->OnSetRates(bitrate_bps, framerate);
}
int32_t ViEEncoder::SendData(
@@ -594,11 +590,8 @@
time_of_last_frame_activity_ms_ = TickTime::MillisecondTimestamp();
}
- {
- CriticalSectionScoped cs(callback_cs_.get());
- if (send_statistics_proxy_ != NULL)
- send_statistics_proxy_->OnSendEncodedImage(encoded_image, rtp_video_hdr);
- }
+ if (stats_proxy_ != NULL)
+ stats_proxy_->OnSendEncodedImage(encoded_image, rtp_video_hdr);
return send_payload_router_->RoutePayload(
VCMEncodedFrame::ConvertFrameType(encoded_image._frameType), payload_type,
@@ -609,20 +602,8 @@
int32_t ViEEncoder::SendStatistics(const uint32_t bit_rate,
const uint32_t frame_rate) {
- CriticalSectionScoped cs(callback_cs_.get());
- if (codec_observer_) {
- codec_observer_->OutgoingRate(channel_id_, frame_rate, bit_rate);
- }
- return 0;
-}
-
-int32_t ViEEncoder::RegisterCodecObserver(ViEEncoderObserver* observer) {
- CriticalSectionScoped cs(callback_cs_.get());
- if (observer && codec_observer_) {
- LOG_F(LS_ERROR) << "Observer already set.";
- return -1;
- }
- codec_observer_ = observer;
+ if (stats_proxy_)
+ stats_proxy_->OnOutgoingRate(frame_rate, bit_rate);
return 0;
}
@@ -743,14 +724,12 @@
if (video_suspended_ == video_is_suspended)
return;
video_suspended_ = video_is_suspended;
- }
- // Video suspend-state changed, inform codec observer.
- CriticalSectionScoped crit(callback_cs_.get());
- if (codec_observer_) {
LOG(LS_INFO) << "Video suspended " << video_is_suspended
<< " for channel " << channel_id_;
- codec_observer_->SuspendChange(channel_id_, video_is_suspended);
}
+ // Video suspend-state changed, inform codec observer.
+ if (stats_proxy_)
+ stats_proxy_->OnSuspendChange(video_is_suspended);
}
void ViEEncoder::SuspendBelowMinBitrate() {
@@ -758,23 +737,11 @@
bitrate_allocator_->EnforceMinBitrate(false);
}
-void ViEEncoder::RegisterPreEncodeCallback(
- I420FrameCallback* pre_encode_callback) {
- CriticalSectionScoped cs(callback_cs_.get());
- pre_encode_callback_ = pre_encode_callback;
-}
-
void ViEEncoder::RegisterPostEncodeImageCallback(
EncodedImageCallback* post_encode_callback) {
vcm_->RegisterPostEncodeImageCallback(post_encode_callback);
}
-void ViEEncoder::RegisterSendStatisticsProxy(
- SendStatisticsProxy* send_statistics_proxy) {
- CriticalSectionScoped cs(callback_cs_.get());
- send_statistics_proxy_ = send_statistics_proxy;
-}
-
QMVideoSettingsCallback::QMVideoSettingsCallback(VideoProcessingModule* vpm)
: vpm_(vpm) {
}
diff --git a/webrtc/video_engine/vie_encoder.h b/webrtc/video_engine/vie_encoder.h
index 424a2a2..2221f07 100644
--- a/webrtc/video_engine/vie_encoder.h
+++ b/webrtc/video_engine/vie_encoder.h
@@ -39,29 +39,8 @@
class SendStatisticsProxy;
class ViEBitrateObserver;
class ViEEffectFilter;
-class ViEEncoderObserver;
class VideoCodingModule;
-// This class declares an abstract interface for a user defined observer. It is
-// up to the VideoEngine user to implement a derived class which implements the
-// observer class. The observer is registered using RegisterEncoderObserver()
-// and deregistered using DeregisterEncoderObserver().
-class ViEEncoderObserver {
- public:
- // This method is called once per second with the current encoded frame rate
- // and bit rate.
- virtual void OutgoingRate(const int video_channel,
- const unsigned int framerate,
- const unsigned int bitrate) = 0;
-
- // This method is called whenever the state of the SuspendBelowMinBitrate
- // changes, i.e., when |is_suspended| toggles.
- virtual void SuspendChange(int video_channel, bool is_suspended) = 0;
-
- protected:
- virtual ~ViEEncoderObserver() {}
-};
-
class ViEEncoder : public RtcpIntraFrameObserver,
public VideoEncoderRateObserver,
public VCMPacketizationCallback,
@@ -72,7 +51,9 @@
ViEEncoder(int32_t channel_id,
uint32_t number_of_cores,
- ProcessThread& module_process_thread,
+ ProcessThread* module_process_thread,
+ SendStatisticsProxy* stats_proxy,
+ I420FrameCallback* pre_encode_callback,
PacedSender* pacer,
BitrateAllocator* bitrate_allocator);
~ViEEncoder();
@@ -146,8 +127,6 @@
int32_t SendStatistics(const uint32_t bit_rate,
const uint32_t frame_rate) override;
- int32_t RegisterCodecObserver(ViEEncoderObserver* observer);
-
// Implements RtcpIntraFrameObserver.
void OnReceivedIntraFrameRequest(uint32_t ssrc) override;
void OnReceivedSLI(uint32_t ssrc, uint8_t picture_id) override;
@@ -165,12 +144,9 @@
void SuspendBelowMinBitrate();
// New-style callbacks, used by VideoSendStream.
- void RegisterPreEncodeCallback(I420FrameCallback* pre_encode_callback);
void RegisterPostEncodeImageCallback(
EncodedImageCallback* post_encode_callback);
- void RegisterSendStatisticsProxy(SendStatisticsProxy* send_statistics_proxy);
-
int channel_id() const { return channel_id_; }
int GetPaddingNeededBps() const;
@@ -196,10 +172,11 @@
const rtc::scoped_ptr<VideoCodingModule> vcm_;
rtc::scoped_refptr<PayloadRouter> send_payload_router_;
- rtc::scoped_ptr<CriticalSectionWrapper> callback_cs_;
rtc::scoped_ptr<CriticalSectionWrapper> data_cs_;
rtc::scoped_ptr<BitrateObserver> bitrate_observer_;
+ SendStatisticsProxy* const stats_proxy_;
+ I420FrameCallback* const pre_encode_callback_;
PacedSender* const pacer_;
BitrateAllocator* const bitrate_allocator_;
@@ -220,8 +197,7 @@
bool fec_enabled_;
bool nack_enabled_;
- ViEEncoderObserver* codec_observer_ GUARDED_BY(callback_cs_);
- ProcessThread& module_process_thread_;
+ ProcessThread* module_process_thread_;
bool has_received_sli_ GUARDED_BY(data_cs_);
uint8_t picture_id_sli_ GUARDED_BY(data_cs_);
@@ -230,10 +206,7 @@
std::map<uint32_t, int> ssrc_streams_ GUARDED_BY(data_cs_);
bool video_suspended_ GUARDED_BY(data_cs_);
- I420FrameCallback* pre_encode_callback_ GUARDED_BY(callback_cs_);
const int64_t start_ms_;
-
- SendStatisticsProxy* send_statistics_proxy_ GUARDED_BY(callback_cs_);
};
} // namespace webrtc
