Remove VideoCodingModule::VCMPacketizationCallback
And move encoder name cb to VCMSendStatisticsCallback.
BUG=webrtc:5687
Review-Url: https://codereview.webrtc.org/1900193004
Cr-Commit-Position: refs/heads/master@{#12596}
diff --git a/webrtc/modules/video_coding/generic_encoder.cc b/webrtc/modules/video_coding/generic_encoder.cc
index 1704a92..abc6369 100644
--- a/webrtc/modules/video_coding/generic_encoder.cc
+++ b/webrtc/modules/video_coding/generic_encoder.cc
@@ -72,11 +72,6 @@
int32_t result = encoder_->Encode(frame, codec_specific, &frame_types);
- if (vcm_encoded_frame_callback_) {
- vcm_encoded_frame_callback_->SignalLastEncoderImplementationUsed(
- encoder_->ImplementationName());
- }
-
if (is_screenshare_ &&
result == WEBRTC_VIDEO_CODEC_TARGET_BITRATE_OVERSHOOT) {
// Target bitrate exceeded, encoder state has been reset - try again.
@@ -86,6 +81,10 @@
return result;
}
+const char* VCMGenericEncoder::ImplementationName() const {
+ return encoder_->ImplementationName();
+}
+
void VCMGenericEncoder::SetEncoderParameters(const EncoderParameters& params) {
bool channel_parameters_have_changed;
bool rates_have_changed;
@@ -139,20 +138,14 @@
}
VCMEncodedFrameCallback::VCMEncodedFrameCallback(
- EncodedImageCallback* post_encode_callback)
- : send_callback_(),
- media_opt_(nullptr),
- internal_source_(false),
- post_encode_callback_(post_encode_callback) {}
+ EncodedImageCallback* post_encode_callback,
+ media_optimization::MediaOptimization* media_opt)
+ : internal_source_(false),
+ post_encode_callback_(post_encode_callback),
+ media_opt_(media_opt) {}
VCMEncodedFrameCallback::~VCMEncodedFrameCallback() {}
-int32_t VCMEncodedFrameCallback::SetTransportCallback(
- VCMPacketizationCallback* transport) {
- send_callback_ = transport;
- return VCM_OK;
-}
-
int32_t VCMEncodedFrameCallback::Encoded(
const EncodedImage& encoded_image,
const CodecSpecificInfo* codec_specific,
@@ -172,14 +165,4 @@
return VCM_OK;
}
-void VCMEncodedFrameCallback::SetMediaOpt(
- media_optimization::MediaOptimization* mediaOpt) {
- media_opt_ = mediaOpt;
-}
-
-void VCMEncodedFrameCallback::SignalLastEncoderImplementationUsed(
- const char* implementation_name) {
- if (send_callback_)
- send_callback_->OnEncoderImplementationName(implementation_name);
-}
} // namespace webrtc
diff --git a/webrtc/modules/video_coding/generic_encoder.h b/webrtc/modules/video_coding/generic_encoder.h
index 0309d8d..469f04d 100644
--- a/webrtc/modules/video_coding/generic_encoder.h
+++ b/webrtc/modules/video_coding/generic_encoder.h
@@ -35,27 +35,22 @@
class VCMEncodedFrameCallback : public EncodedImageCallback {
public:
- explicit VCMEncodedFrameCallback(EncodedImageCallback* post_encode_callback);
+ VCMEncodedFrameCallback(EncodedImageCallback* post_encode_callback,
+ media_optimization::MediaOptimization* media_opt);
virtual ~VCMEncodedFrameCallback();
// Implements EncodedImageCallback.
int32_t Encoded(const EncodedImage& encoded_image,
const CodecSpecificInfo* codec_specific,
const RTPFragmentationHeader* fragmentation_header) override;
- int32_t SetTransportCallback(VCMPacketizationCallback* transport);
- void SetMediaOpt(media_optimization::MediaOptimization* media_opt);
void SetInternalSource(bool internal_source) {
internal_source_ = internal_source;
}
- void SignalLastEncoderImplementationUsed(
- const char* encoder_implementation_name);
private:
- VCMPacketizationCallback* send_callback_;
- media_optimization::MediaOptimization* media_opt_;
bool internal_source_;
-
- EncodedImageCallback* post_encode_callback_;
+ EncodedImageCallback* const post_encode_callback_;
+ media_optimization::MediaOptimization* const media_opt_;
};
class VCMGenericEncoder {
@@ -75,6 +70,8 @@
const CodecSpecificInfo* codec_specific,
const std::vector<FrameType>& frame_types);
+ const char* ImplementationName() const;
+
void SetEncoderParameters(const EncoderParameters& params);
EncoderParameters GetEncoderParameters() const;
diff --git a/webrtc/modules/video_coding/include/video_coding.h b/webrtc/modules/video_coding/include/video_coding.h
index a51065e..295911d 100644
--- a/webrtc/modules/video_coding/include/video_coding.h
+++ b/webrtc/modules/video_coding/include/video_coding.h
@@ -184,32 +184,6 @@
// < 0, on error.
virtual int32_t SetReceiveChannelParameters(int64_t rtt) = 0;
- // Register a transport callback which will be called to deliver the encoded
- // data and
- // side information.
- //
- // Input:
- // - transport : The callback object to register.
- //
- // Return value : VCM_OK, on success.
- // < 0, on error.
- virtual int32_t RegisterTransportCallback(
- VCMPacketizationCallback* transport) = 0;
-
- // Register video output information callback which will be called to deliver
- // information
- // about the video stream produced by the encoder, for instance the average
- // frame rate and
- // bit rate.
- //
- // Input:
- // - outputInformation : The callback object to register.
- //
- // Return value : VCM_OK, on success.
- // < 0, on error.
- virtual int32_t RegisterSendStatisticsCallback(
- VCMSendStatisticsCallback* sendStats) = 0;
-
// Register a video protection callback which will be called to deliver
// the requested FEC rate and NACK status (on/off).
//
diff --git a/webrtc/modules/video_coding/include/video_coding_defines.h b/webrtc/modules/video_coding/include/video_coding_defines.h
index 083c477..7c5d00b 100644
--- a/webrtc/modules/video_coding/include/video_coding_defines.h
+++ b/webrtc/modules/video_coding/include/video_coding_defines.h
@@ -11,6 +11,7 @@
#ifndef WEBRTC_MODULES_VIDEO_CODING_INCLUDE_VIDEO_CODING_DEFINES_H_
#define WEBRTC_MODULES_VIDEO_CODING_INCLUDE_VIDEO_CODING_DEFINES_H_
+#include <string>
#include <vector>
#include "webrtc/modules/include/module_common_types.h"
@@ -56,18 +57,6 @@
uint32_t numDeltaFrames;
};
-// Callback class used for sending data ready to be packetized
-// Deprecated.
-// TODO(perkj): Remove once OnEncoderImplementationName is not used.
-class VCMPacketizationCallback {
- public:
- // TODO(perkj): Refactor this. It does not belong in VCMPacketizationCallback.
- virtual void OnEncoderImplementationName(const char* implementation_name) {}
-
- protected:
- virtual ~VCMPacketizationCallback() {}
-};
-
// Callback class used for passing decoded frames which are ready to be
// rendered.
class VCMReceiveCallback {
@@ -84,13 +73,13 @@
virtual ~VCMReceiveCallback() {}
};
-// Callback class used for informing the user of the bit rate and frame rate
-// produced by the
-// encoder.
+// Callback class used for informing the user of the bit rate and frame rate,
+// and the name of the encoder.
class VCMSendStatisticsCallback {
public:
- virtual int32_t SendStatistics(const uint32_t bitRate,
- const uint32_t frameRate) = 0;
+ virtual void SendStatistics(uint32_t bitRate,
+ uint32_t frameRate,
+ const std::string& encoder_name) = 0;
protected:
virtual ~VCMSendStatisticsCallback() {}
diff --git a/webrtc/modules/video_coding/video_coding_impl.cc b/webrtc/modules/video_coding/video_coding_impl.cc
index 1128e04..219f668 100644
--- a/webrtc/modules/video_coding/video_coding_impl.cc
+++ b/webrtc/modules/video_coding/video_coding_impl.cc
@@ -82,7 +82,8 @@
sender_(clock,
&post_encode_callback_,
encoder_rate_observer,
- qm_settings_callback),
+ qm_settings_callback,
+ nullptr),
receiver_(clock,
event_factory,
pre_decode_image_callback,
@@ -132,16 +133,6 @@
return sender_.SetChannelParameters(target_bitrate, lossRate, rtt);
}
- int32_t RegisterTransportCallback(
- VCMPacketizationCallback* transport) override {
- return sender_.RegisterTransportCallback(transport);
- }
-
- int32_t RegisterSendStatisticsCallback(
- VCMSendStatisticsCallback* sendStats) override {
- return sender_.RegisterSendStatisticsCallback(sendStats);
- }
-
int32_t RegisterProtectionCallback(
VCMProtectionCallback* protection) override {
return sender_.RegisterProtectionCallback(protection);
diff --git a/webrtc/modules/video_coding/video_coding_impl.h b/webrtc/modules/video_coding/video_coding_impl.h
index 2a0c4b8..a4f74eb 100644
--- a/webrtc/modules/video_coding/video_coding_impl.h
+++ b/webrtc/modules/video_coding/video_coding_impl.h
@@ -14,6 +14,7 @@
#include "webrtc/modules/video_coding/include/video_coding.h"
#include <memory>
+#include <string>
#include <vector>
#include "webrtc/base/onetimeevent.h"
@@ -58,7 +59,8 @@
VideoSender(Clock* clock,
EncodedImageCallback* post_encode_callback,
VideoEncoderRateObserver* encoder_rate_observer,
- VCMQMSettingsCallback* qm_settings_callback);
+ VCMQMSettingsCallback* qm_settings_callback,
+ VCMSendStatisticsCallback* send_stats_callback);
~VideoSender();
@@ -79,10 +81,6 @@
uint8_t lossRate,
int64_t rtt);
- // Deprecated. Use |post_encode_callback| instead.
- // TODO(perkj): Remove once |OnEncoderImplementationName| is not used.
- int32_t RegisterTransportCallback(VCMPacketizationCallback* transport);
- int32_t RegisterSendStatisticsCallback(VCMSendStatisticsCallback* sendStats);
int32_t RegisterProtectionCallback(VCMProtectionCallback* protection);
void SetVideoProtection(VCMVideoProtection videoProtection);
@@ -105,12 +103,11 @@
Clock* const clock_;
- rtc::CriticalSection process_crit_;
rtc::CriticalSection encoder_crit_;
VCMGenericEncoder* _encoder;
- VCMEncodedFrameCallback _encodedFrameCallback GUARDED_BY(encoder_crit_);
media_optimization::MediaOptimization _mediaOpt;
- VCMSendStatisticsCallback* _sendStatsCallback GUARDED_BY(process_crit_);
+ VCMEncodedFrameCallback _encodedFrameCallback GUARDED_BY(encoder_crit_);
+ VCMSendStatisticsCallback* const send_stats_callback_;
VCMCodecDataBase _codecDataBase GUARDED_BY(encoder_crit_);
bool frame_dropper_enabled_ GUARDED_BY(encoder_crit_);
VCMProcessTimer _sendStatsTimer;
@@ -125,6 +122,7 @@
rtc::CriticalSection params_crit_;
EncoderParameters encoder_params_ GUARDED_BY(params_crit_);
bool encoder_has_internal_source_ GUARDED_BY(params_crit_);
+ std::string encoder_name_ GUARDED_BY(params_crit_);
std::vector<FrameType> next_frame_types_ GUARDED_BY(params_crit_);
};
diff --git a/webrtc/modules/video_coding/video_sender.cc b/webrtc/modules/video_coding/video_sender.cc
index f47c420..a407c9e 100644
--- a/webrtc/modules/video_coding/video_sender.cc
+++ b/webrtc/modules/video_coding/video_sender.cc
@@ -27,12 +27,13 @@
VideoSender::VideoSender(Clock* clock,
EncodedImageCallback* post_encode_callback,
VideoEncoderRateObserver* encoder_rate_observer,
- VCMQMSettingsCallback* qm_settings_callback)
+ VCMQMSettingsCallback* qm_settings_callback,
+ VCMSendStatisticsCallback* send_stats_callback)
: clock_(clock),
_encoder(nullptr),
- _encodedFrameCallback(post_encode_callback),
_mediaOpt(clock_),
- _sendStatsCallback(nullptr),
+ _encodedFrameCallback(post_encode_callback, &_mediaOpt),
+ send_stats_callback_(send_stats_callback),
_codecDataBase(encoder_rate_observer, &_encodedFrameCallback),
frame_dropper_enabled_(true),
_sendStatsTimer(1000, clock_),
@@ -54,12 +55,19 @@
void VideoSender::Process() {
if (_sendStatsTimer.TimeUntilProcess() == 0) {
+ // |_sendStatsTimer.Processed()| must be called. Otherwise
+ // VideoSender::Process() will be called in an infinite loop.
_sendStatsTimer.Processed();
- rtc::CritScope cs(&process_crit_);
- if (_sendStatsCallback != nullptr) {
+ if (send_stats_callback_) {
uint32_t bitRate = _mediaOpt.SentBitRate();
uint32_t frameRate = _mediaOpt.SentFrameRate();
- _sendStatsCallback->SendStatistics(bitRate, frameRate);
+ std::string encoder_name;
+ {
+ rtc::CritScope cs(¶ms_crit_);
+ // Copy the string here so that we don't hold |params_crit_| in the CB.
+ encoder_name = encoder_name_;
+ }
+ send_stats_callback_->SendStatistics(bitRate, frameRate, encoder_name);
}
}
@@ -235,24 +243,6 @@
_encoder->SetEncoderParameters(params);
}
-int32_t VideoSender::RegisterTransportCallback(
- VCMPacketizationCallback* transport) {
- rtc::CritScope lock(&encoder_crit_);
- _encodedFrameCallback.SetMediaOpt(&_mediaOpt);
- _encodedFrameCallback.SetTransportCallback(transport);
- return VCM_OK;
-}
-
-// Register video output information callback which will be called to deliver
-// information about the video stream produced by the encoder, for instance the
-// average frame rate and bit rate.
-int32_t VideoSender::RegisterSendStatisticsCallback(
- VCMSendStatisticsCallback* sendStats) {
- rtc::CritScope cs(&process_crit_);
- _sendStatsCallback = sendStats;
- return VCM_OK;
-}
-
// Register a video protection callback which will be called to deliver the
// requested FEC rate and NACK status (on/off).
// Note: this callback is assumed to only be registered once and before it is
@@ -329,9 +319,12 @@
LOG(LS_ERROR) << "Failed to encode frame. Error code: " << ret;
return ret;
}
+
{
- // Change all keyframe requests to encode delta frames the next time.
rtc::CritScope lock(¶ms_crit_);
+ encoder_name_ = _encoder->ImplementationName();
+
+ // Change all keyframe requests to encode delta frames the next time.
for (size_t i = 0; i < next_frame_types_.size(); ++i) {
// Check for equality (same requested as before encoding) to not
// accidentally drop a keyframe request while encoding.
diff --git a/webrtc/modules/video_coding/video_sender_unittest.cc b/webrtc/modules/video_coding/video_sender_unittest.cc
index 50283fc..7d4d066 100644
--- a/webrtc/modules/video_coding/video_sender_unittest.cc
+++ b/webrtc/modules/video_coding/video_sender_unittest.cc
@@ -180,8 +180,8 @@
TestVideoSender() : clock_(1000), encoded_frame_callback_(&clock_) {}
void SetUp() override {
- sender_.reset(
- new VideoSender(&clock_, &encoded_frame_callback_, nullptr, nullptr));
+ sender_.reset(new VideoSender(&clock_, &encoded_frame_callback_, nullptr,
+ nullptr, nullptr));
}
void AddFrame() {
diff --git a/webrtc/video/send_statistics_proxy.cc b/webrtc/video/send_statistics_proxy.cc
index cf3332c..b8fdef2 100644
--- a/webrtc/video/send_statistics_proxy.cc
+++ b/webrtc/video/send_statistics_proxy.cc
@@ -342,16 +342,14 @@
}
}
-void SendStatisticsProxy::OnEncoderImplementationName(
- const char* implementation_name) {
- rtc::CritScope lock(&crit_);
- stats_.encoder_implementation_name = implementation_name;
-}
-
-void SendStatisticsProxy::OnOutgoingRate(uint32_t framerate, uint32_t bitrate) {
+void SendStatisticsProxy::OnEncoderStatsUpdate(
+ uint32_t framerate,
+ uint32_t bitrate,
+ const std::string& encoder_name) {
rtc::CritScope lock(&crit_);
stats_.encode_frame_rate = framerate;
stats_.media_bitrate_bps = bitrate;
+ stats_.encoder_implementation_name = encoder_name;
}
void SendStatisticsProxy::OnEncodedFrameTimeMeasured(
diff --git a/webrtc/video/send_statistics_proxy.h b/webrtc/video/send_statistics_proxy.h
index ce4c05e..74d5261 100644
--- a/webrtc/video/send_statistics_proxy.h
+++ b/webrtc/video/send_statistics_proxy.h
@@ -53,8 +53,9 @@
// Used to update incoming frame rate.
void OnIncomingFrame(int width, int height);
- void OnEncoderImplementationName(const char* implementation_name);
- void OnOutgoingRate(uint32_t framerate, uint32_t bitrate);
+ void OnEncoderStatsUpdate(uint32_t framerate,
+ uint32_t bitrate,
+ const std::string& encoder_name);
void OnSuspendChange(bool is_suspended);
void OnInactiveSsrc(uint32_t ssrc);
diff --git a/webrtc/video/send_statistics_proxy_unittest.cc b/webrtc/video/send_statistics_proxy_unittest.cc
index 5f8fa1c..d7f4a62 100644
--- a/webrtc/video/send_statistics_proxy_unittest.cc
+++ b/webrtc/video/send_statistics_proxy_unittest.cc
@@ -146,11 +146,13 @@
int media_bitrate_bps = 500;
int encode_fps = 29;
- statistics_proxy_->OnOutgoingRate(encode_fps, media_bitrate_bps);
+ statistics_proxy_->OnEncoderStatsUpdate(encode_fps, media_bitrate_bps,
+ "encoder name");
VideoSendStream::Stats stats = statistics_proxy_->GetStats();
EXPECT_EQ(media_bitrate_bps, stats.media_bitrate_bps);
EXPECT_EQ(encode_fps, stats.encode_frame_rate);
+ EXPECT_EQ("encoder name", stats.encoder_implementation_name);
}
TEST_F(SendStatisticsProxyTest, Suspended) {
diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc
index 9e38384..c42c6d0 100644
--- a/webrtc/video/video_send_stream.cc
+++ b/webrtc/video/video_send_stream.cc
@@ -405,7 +405,6 @@
RTC_DCHECK(congestion_controller_);
RTC_DCHECK(remb_);
- RTC_CHECK(vie_encoder_.Init());
encoder_feedback_.Init(config_.rtp.ssrcs, &vie_encoder_);
// RTP/RTCP initialization.
diff --git a/webrtc/video/vie_encoder.cc b/webrtc/video/vie_encoder.cc
index 89cbb52..79b79d9 100644
--- a/webrtc/video/vie_encoder.cc
+++ b/webrtc/video/vie_encoder.cc
@@ -64,7 +64,11 @@
ssrcs_(ssrcs),
vp_(VideoProcessing::Create()),
qm_callback_(new QMVideoSettingsCallback(vp_.get())),
- video_sender_(Clock::GetRealTimeClock(), this, this, qm_callback_.get()),
+ video_sender_(Clock::GetRealTimeClock(),
+ this,
+ this,
+ qm_callback_.get(),
+ this),
stats_proxy_(stats_proxy),
pre_encode_callback_(pre_encode_callback),
overuse_detector_(overuse_detector),
@@ -84,23 +88,10 @@
picture_id_rpsi_(0),
video_suspended_(false) {
module_process_thread_->RegisterModule(&video_sender_);
-}
-
-bool ViEEncoder::Init() {
vp_->EnableTemporalDecimation(true);
// Enable/disable content analysis: off by default for now.
vp_->EnableContentAnalysis(false);
-
- // TODO(perkj): Remove |RegisterTransportCallback| as soon as we don't use
- // VCMPacketizationCallback::OnEncoderImplementationName.
- if (video_sender_.RegisterTransportCallback(this) != 0) {
- return false;
- }
- if (video_sender_.RegisterSendStatisticsCallback(this) != 0) {
- return false;
- }
- return true;
}
vcm::VideoSender* ViEEncoder::video_sender() {
@@ -353,11 +344,6 @@
stats_proxy_->OnSetRates(bitrate_bps, framerate);
}
-void ViEEncoder::OnEncoderImplementationName(const char* implementation_name) {
- if (stats_proxy_)
- stats_proxy_->OnEncoderImplementationName(implementation_name);
-}
-
int32_t ViEEncoder::Encoded(const EncodedImage& encoded_image,
const CodecSpecificInfo* codec_specific_info,
const RTPFragmentationHeader* fragmentation) {
@@ -403,11 +389,11 @@
return success;
}
-int32_t ViEEncoder::SendStatistics(const uint32_t bit_rate,
- const uint32_t frame_rate) {
+void ViEEncoder::SendStatistics(uint32_t bit_rate,
+ uint32_t frame_rate,
+ const std::string& encoder_name) {
if (stats_proxy_)
- stats_proxy_->OnOutgoingRate(frame_rate, bit_rate);
- return 0;
+ stats_proxy_->OnEncoderStatsUpdate(frame_rate, bit_rate, encoder_name);
}
void ViEEncoder::OnReceivedSLI(uint32_t /*ssrc*/,
diff --git a/webrtc/video/vie_encoder.h b/webrtc/video/vie_encoder.h
index 3d05a15..9369f9f 100644
--- a/webrtc/video/vie_encoder.h
+++ b/webrtc/video/vie_encoder.h
@@ -12,6 +12,7 @@
#define WEBRTC_VIDEO_VIE_ENCODER_H_
#include <memory>
+#include <string>
#include <vector>
#include "webrtc/base/criticalsection.h"
@@ -52,7 +53,6 @@
// 6. For each available raw video frame call EncodeVideoFrame.
class ViEEncoder : public VideoEncoderRateObserver,
public EncodedImageCallback,
- public VCMPacketizationCallback,
public VCMSendStatisticsCallback {
public:
friend class ViEBitrateObserver;
@@ -67,8 +67,6 @@
PacedSender* pacer);
~ViEEncoder();
- bool Init();
-
vcm::VideoSender* video_sender();
void SetNetworkTransmissionState(bool is_transmitting);
@@ -102,17 +100,15 @@
// Implements VideoEncoderRateObserver.
void OnSetRates(uint32_t bitrate_bps, int framerate) override;
- // Implements VCMPacketizationCallback.
- void OnEncoderImplementationName(const char* implementation_name) override;
-
// Implements EncodedImageCallback.
int32_t Encoded(const EncodedImage& encoded_image,
const CodecSpecificInfo* codec_specific_info,
const RTPFragmentationHeader* fragmentation) override;
// Implements VideoSendStatisticsCallback.
- int32_t SendStatistics(const uint32_t bit_rate,
- const uint32_t frame_rate) override;
+ void SendStatistics(uint32_t bit_rate,
+ uint32_t frame_rate,
+ const std::string& encoder_name) override;
// virtual to test EncoderStateFeedback with mocks.
virtual void OnReceivedIntraFrameRequest(uint32_t ssrc);
