Skip to content

Commit

Permalink
Delete VCMSendStatisticsCallback and corresponding use of ProcessThread
Browse files Browse the repository at this point in the history
Bug: webrtc:8422
Change-Id: I5863266a0226d475c4fdd810f2f6f1acdf922df3
Reviewed-on: https://webrtc-review.googlesource.com/14880
Reviewed-by: Åsa Persson <[email protected]>
Commit-Queue: Niels Moller <[email protected]>
Cr-Commit-Position: refs/heads/master@{#20440}
  • Loading branch information
Niels Möller authored and Commit Bot committed Oct 26, 2017
1 parent 2729c16 commit a056599
Show file tree
Hide file tree
Showing 8 changed files with 9 additions and 78 deletions.
10 changes: 0 additions & 10 deletions modules/video_coding/include/video_coding_defines.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,16 +84,6 @@ class VCMReceiveCallback {
virtual ~VCMReceiveCallback() {}
};

// Callback class used for informing the user of the bit rate and frame rate,
// and the name of the encoder.
class VCMSendStatisticsCallback {
public:
virtual void SendStatistics(uint32_t bitRate, uint32_t frameRate) = 0;

protected:
virtual ~VCMSendStatisticsCallback() {}
};

// Callback class used for informing the user of the incoming bit rate and frame
// rate.
class VCMReceiveStatisticsCallback {
Expand Down
7 changes: 2 additions & 5 deletions modules/video_coding/video_coding_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ class VideoCodingModuleImpl : public VideoCodingModule {
KeyFrameRequestSender* keyframe_request_sender,
EncodedImageCallback* pre_decode_image_callback)
: VideoCodingModule(),
sender_(clock, &post_encode_callback_, nullptr),
sender_(clock, &post_encode_callback_),
timing_(new VCMTiming(clock)),
receiver_(clock,
event_factory,
Expand All @@ -95,15 +95,12 @@ class VideoCodingModuleImpl : public VideoCodingModule {
virtual ~VideoCodingModuleImpl() {}

int64_t TimeUntilNextProcess() override {
int64_t sender_time = sender_.TimeUntilNextProcess();
int64_t receiver_time = receiver_.TimeUntilNextProcess();
RTC_DCHECK_GE(sender_time, 0);
RTC_DCHECK_GE(receiver_time, 0);
return VCM_MIN(sender_time, receiver_time);
return receiver_time;
}

void Process() override {
sender_.Process();
receiver_.Process();
}

Expand Down
12 changes: 2 additions & 10 deletions modules/video_coding/video_coding_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,12 @@ class VCMProcessTimer {
int64_t _latestMs;
};

class VideoSender : public Module {
class VideoSender {
public:
typedef VideoCodingModule::SenderNackMode SenderNackMode;

VideoSender(Clock* clock,
EncodedImageCallback* post_encode_callback,
VCMSendStatisticsCallback* send_stats_callback);
EncodedImageCallback* post_encode_callback);

~VideoSender();

Expand Down Expand Up @@ -109,9 +108,6 @@ class VideoSender : public Module {
int32_t IntraFrameRequest(size_t stream_index);
int32_t EnableFrameDropper(bool enable);

int64_t TimeUntilNextProcess() override;
void Process() override;

private:
EncoderParameters UpdateEncoderParameters(
const EncoderParameters& params,
Expand All @@ -120,17 +116,13 @@ class VideoSender : public Module {
void SetEncoderParameters(EncoderParameters params, bool has_internal_source)
RTC_EXCLUSIVE_LOCKS_REQUIRED(encoder_crit_);

Clock* const clock_;

rtc::CriticalSection encoder_crit_;
VCMGenericEncoder* _encoder;
media_optimization::MediaOptimization _mediaOpt;
VCMEncodedFrameCallback _encodedFrameCallback RTC_GUARDED_BY(encoder_crit_);
EncodedImageCallback* const post_encode_callback_;
VCMSendStatisticsCallback* const send_stats_callback_;
VCMCodecDataBase _codecDataBase RTC_GUARDED_BY(encoder_crit_);
bool frame_dropper_enabled_ RTC_GUARDED_BY(encoder_crit_);
VCMProcessTimer _sendStatsTimer;

// Must be accessed on the construction thread of VideoSender.
VideoCodec current_codec_;
Expand Down
28 changes: 3 additions & 25 deletions modules/video_coding/video_sender.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,13 @@ namespace webrtc {
namespace vcm {

VideoSender::VideoSender(Clock* clock,
EncodedImageCallback* post_encode_callback,
VCMSendStatisticsCallback* send_stats_callback)
: clock_(clock),
_encoder(nullptr),
_mediaOpt(clock_),
EncodedImageCallback* post_encode_callback)
: _encoder(nullptr),
_mediaOpt(clock),
_encodedFrameCallback(post_encode_callback, &_mediaOpt),
post_encode_callback_(post_encode_callback),
send_stats_callback_(send_stats_callback),
_codecDataBase(&_encodedFrameCallback),
frame_dropper_enabled_(true),
_sendStatsTimer(VCMProcessTimer::kDefaultProcessIntervalMs, clock_),
current_codec_(),
encoder_params_({BitrateAllocation(), 0, 0, 0}),
encoder_has_internal_source_(false),
Expand All @@ -52,24 +48,6 @@ VideoSender::VideoSender(Clock* clock,

VideoSender::~VideoSender() {}

// TODO(asapersson): Remove _sendStatsTimer and send_stats_callback_.
void VideoSender::Process() {
if (_sendStatsTimer.TimeUntilProcess() == 0) {
// |_sendStatsTimer.Processed()| must be called. Otherwise
// VideoSender::Process() will be called in an infinite loop.
_sendStatsTimer.Processed();
if (send_stats_callback_) {
uint32_t bitRate = 0;
uint32_t frameRate = 0;
send_stats_callback_->SendStatistics(bitRate, frameRate);
}
}
}

int64_t VideoSender::TimeUntilNextProcess() {
return _sendStatsTimer.TimeUntilProcess();
}

// Register the send codec to be used.
int32_t VideoSender::RegisterSendCodec(const VideoCodec* sendCodec,
uint32_t numberOfCores,
Expand Down
2 changes: 1 addition & 1 deletion modules/video_coding/video_sender_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ class TestVideoSender : public ::testing::Test {
TestVideoSender() : clock_(1000), encoded_frame_callback_(&clock_) {}

void SetUp() override {
sender_.reset(new VideoSender(&clock_, &encoded_frame_callback_, nullptr));
sender_.reset(new VideoSender(&clock_, &encoded_frame_callback_));
}

void AddFrame() {
Expand Down
2 changes: 0 additions & 2 deletions video/video_send_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,6 @@ VideoSendStream::VideoSendStream(
// Only signal target bitrate for screenshare streams, for now.
video_stream_encoder_->SetBitrateObserver(send_stream_.get());
}
video_stream_encoder_->RegisterProcessThread(module_process_thread);

ReconfigureVideoEncoder(std::move(encoder_config));
}
Expand Down Expand Up @@ -620,7 +619,6 @@ void VideoSendStream::StopPermanentlyAndGetRtpStates(
VideoSendStream::RtpPayloadStateMap* payload_state_map) {
RTC_DCHECK_RUN_ON(&thread_checker_);
video_stream_encoder_->Stop();
video_stream_encoder_->DeRegisterProcessThread();
send_stream_->DeRegisterProcessThread();
worker_queue_->PostTask(
std::unique_ptr<rtc::QueuedTask>(new DestructAndGetRtpStateTask(
Expand Down
17 changes: 1 addition & 16 deletions video/video_stream_encoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ VideoStreamEncoder::VideoStreamEncoder(
sink_(nullptr),
settings_(settings),
codec_type_(PayloadStringToCodecType(settings.payload_name)),
video_sender_(Clock::GetRealTimeClock(), this, nullptr),
video_sender_(Clock::GetRealTimeClock(), this),
overuse_detector_(
overuse_detector.get()
? overuse_detector.release()
Expand All @@ -402,7 +402,6 @@ VideoStreamEncoder::VideoStreamEncoder(
stats_proxy)),
stats_proxy_(stats_proxy),
pre_encode_callback_(pre_encode_callback),
module_process_thread_(nullptr),
max_framerate_(-1),
pending_encoder_reconfiguration_(false),
encoder_start_bitrate_bps_(0),
Expand Down Expand Up @@ -468,20 +467,6 @@ void VideoStreamEncoder::Stop() {
shutdown_event_.Wait(rtc::Event::kForever);
}

void VideoStreamEncoder::RegisterProcessThread(
ProcessThread* module_process_thread) {
RTC_DCHECK_RUN_ON(&thread_checker_);
RTC_DCHECK(!module_process_thread_);
module_process_thread_ = module_process_thread;
module_process_thread_->RegisterModule(&video_sender_, RTC_FROM_HERE);
module_process_thread_checker_.DetachFromThread();
}

void VideoStreamEncoder::DeRegisterProcessThread() {
RTC_DCHECK_RUN_ON(&thread_checker_);
module_process_thread_->DeRegisterModule(&video_sender_);
}

void VideoStreamEncoder::SetBitrateObserver(
VideoBitrateAllocationObserver* bitrate_observer) {
RTC_DCHECK_RUN_ON(&thread_checker_);
Expand Down
9 changes: 0 additions & 9 deletions video/video_stream_encoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@

namespace webrtc {

class ProcessThread;
class SendStatisticsProxy;
class VideoBitrateAllocationObserver;

Expand Down Expand Up @@ -79,12 +78,6 @@ class VideoStreamEncoder : public rtc::VideoSinkInterface<VideoFrame>,
EncodedFrameObserver* encoder_timing,
std::unique_ptr<OveruseFrameDetector> overuse_detector);
~VideoStreamEncoder();
// RegisterProcessThread register |module_process_thread| with those objects
// that use it. Registration has to happen on the thread where
// |module_process_thread| was created (libjingle's worker thread).
// TODO(perkj): Replace the use of |module_process_thread| with a TaskQueue.
void RegisterProcessThread(ProcessThread* module_process_thread);
void DeRegisterProcessThread();

// Sets the source that will provide I420 video frames.
// |degradation_preference| control whether or not resolution or frame rate
Expand Down Expand Up @@ -236,8 +229,6 @@ class VideoStreamEncoder : public rtc::VideoSinkInterface<VideoFrame>,

SendStatisticsProxy* const stats_proxy_;
rtc::VideoSinkInterface<VideoFrame>* const pre_encode_callback_;
ProcessThread* module_process_thread_;
rtc::ThreadChecker module_process_thread_checker_;
// |thread_checker_| checks that public methods that are related to lifetime
// of VideoStreamEncoder are called on the same thread.
rtc::ThreadChecker thread_checker_;
Expand Down

0 comments on commit a056599

Please sign in to comment.