diff --git a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc index eba59d0fdf..486968286d 100644 --- a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc +++ b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc @@ -410,6 +410,7 @@ EncodedImageCallback::Result SimulcastEncoderAdapter::OnEncodedImage( const CodecSpecificInfo* codecSpecificInfo, const RTPFragmentationHeader* fragmentation) { CodecSpecificInfo stream_codec_specific = *codecSpecificInfo; + stream_codec_specific.codec_name = implementation_name_.c_str(); CodecSpecificInfoVP8* vp8Info = &(stream_codec_specific.codecSpecific.VP8); vp8Info->simulcastIdx = stream_idx; diff --git a/webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc b/webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc index 179081ae33..dc32d3af41 100644 --- a/webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc +++ b/webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc @@ -960,6 +960,7 @@ void VP8EncoderImpl::PopulateCodecSpecific( bool only_predicting_from_key_frame) { assert(codec_specific != NULL); codec_specific->codecType = kVideoCodecVP8; + codec_specific->codec_name = ImplementationName(); CodecSpecificInfoVP8* vp8Info = &(codec_specific->codecSpecific.VP8); vp8Info->pictureId = picture_id_[stream_idx]; if (pkt.data.frame.flags & VPX_FRAME_IS_KEY) { diff --git a/webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc b/webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc index 884196ad7e..f6eaf0257b 100644 --- a/webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc +++ b/webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc @@ -560,6 +560,7 @@ void VP9EncoderImpl::PopulateCodecSpecific(CodecSpecificInfo* codec_specific, uint32_t timestamp) { assert(codec_specific != NULL); codec_specific->codecType = kVideoCodecVP9; + codec_specific->codec_name = ImplementationName(); CodecSpecificInfoVP9* vp9_info = &(codec_specific->codecSpecific.VP9); // TODO(asapersson): Set correct value. vp9_info->inter_pic_predicted = diff --git a/webrtc/modules/video_coding/generic_encoder.cc b/webrtc/modules/video_coding/generic_encoder.cc index 28eb10ada1..1e8362ef09 100644 --- a/webrtc/modules/video_coding/generic_encoder.cc +++ b/webrtc/modules/video_coding/generic_encoder.cc @@ -77,11 +77,6 @@ int32_t VCMGenericEncoder::Encode(const VideoFrame& frame, return result; } -const char* VCMGenericEncoder::ImplementationName() const { - RTC_DCHECK_RUNS_SERIALIZED(&race_checker_); - return encoder_->ImplementationName(); -} - void VCMGenericEncoder::SetEncoderParameters(const EncoderParameters& params) { RTC_DCHECK_RUNS_SERIALIZED(&race_checker_); bool channel_parameters_have_changed; diff --git a/webrtc/modules/video_coding/generic_encoder.h b/webrtc/modules/video_coding/generic_encoder.h index 9b5d2e62eb..bc1afae55f 100644 --- a/webrtc/modules/video_coding/generic_encoder.h +++ b/webrtc/modules/video_coding/generic_encoder.h @@ -71,8 +71,6 @@ class VCMGenericEncoder { const CodecSpecificInfo* codec_specific, const std::vector& frame_types); - const char* ImplementationName() const; - void SetEncoderParameters(const EncoderParameters& params); EncoderParameters GetEncoderParameters() const; diff --git a/webrtc/modules/video_coding/include/video_codec_interface.h b/webrtc/modules/video_coding/include/video_codec_interface.h index 19303c0d67..f0a11e7a53 100644 --- a/webrtc/modules/video_coding/include/video_codec_interface.h +++ b/webrtc/modules/video_coding/include/video_codec_interface.h @@ -90,7 +90,9 @@ union CodecSpecificInfoUnion { // must be fitted with a copy-constructor. This is because it is copied // in the copy-constructor of VCMEncodedFrame. struct CodecSpecificInfo { + CodecSpecificInfo() : codecType(kVideoCodecUnknown), codec_name(nullptr) {} VideoCodecType codecType; + const char* codec_name; CodecSpecificInfoUnion codecSpecific; }; diff --git a/webrtc/modules/video_coding/include/video_coding_defines.h b/webrtc/modules/video_coding/include/video_coding_defines.h index e27de47ee0..4657804896 100644 --- a/webrtc/modules/video_coding/include/video_coding_defines.h +++ b/webrtc/modules/video_coding/include/video_coding_defines.h @@ -77,9 +77,7 @@ class VCMReceiveCallback { // and the name of the encoder. class VCMSendStatisticsCallback { public: - virtual void SendStatistics(uint32_t bitRate, - uint32_t frameRate, - const std::string& encoder_name) = 0; + virtual void SendStatistics(uint32_t bitRate, uint32_t frameRate) = 0; protected: virtual ~VCMSendStatisticsCallback() {} diff --git a/webrtc/modules/video_coding/video_coding_impl.h b/webrtc/modules/video_coding/video_coding_impl.h index 40bccf2530..cb24654041 100644 --- a/webrtc/modules/video_coding/video_coding_impl.h +++ b/webrtc/modules/video_coding/video_coding_impl.h @@ -113,7 +113,6 @@ class VideoSender : public Module { 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 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 f3e2bfe1b6..0f2da583a0 100644 --- a/webrtc/modules/video_coding/video_sender.cc +++ b/webrtc/modules/video_coding/video_sender.cc @@ -56,13 +56,7 @@ void VideoSender::Process() { if (send_stats_callback_) { uint32_t bitRate = _mediaOpt.SentBitRate(); uint32_t frameRate = _mediaOpt.SentFrameRate(); - 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); + send_stats_callback_->SendStatistics(bitRate, frameRate); } } @@ -314,8 +308,6 @@ int32_t VideoSender::AddVideoFrame(const VideoFrame& videoFrame, { 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 diff --git a/webrtc/test/fake_encoder.cc b/webrtc/test/fake_encoder.cc index a3fb7686f1..0a3f34e85d 100644 --- a/webrtc/test/fake_encoder.cc +++ b/webrtc/test/fake_encoder.cc @@ -108,6 +108,7 @@ int32_t FakeEncoder::Encode(const VideoFrame& input_image, if (min_stream_bits > bits_available && i > 0) continue; assert(callback_ != NULL); + specifics.codec_name = ImplementationName(); if (callback_->Encoded(encoded, &specifics, NULL) != 0) return -1; bits_available -= std::min(encoded._length * 8, bits_available); diff --git a/webrtc/video/send_statistics_proxy.cc b/webrtc/video/send_statistics_proxy.cc index 9325b2f7dd..72417f2f5d 100644 --- a/webrtc/video/send_statistics_proxy.cc +++ b/webrtc/video/send_statistics_proxy.cc @@ -349,14 +349,11 @@ void SendStatisticsProxy::SetContentType( } } -void SendStatisticsProxy::OnEncoderStatsUpdate( - uint32_t framerate, - uint32_t bitrate, - const std::string& encoder_name) { +void SendStatisticsProxy::OnEncoderStatsUpdate(uint32_t framerate, + uint32_t bitrate) { rtc::CritScope lock(&crit_); stats_.encode_frame_rate = framerate; stats_.media_bitrate_bps = bitrate; - stats_.encoder_implementation_name = encoder_name; } void SendStatisticsProxy::OnEncodedFrameTimeMeasured( @@ -441,12 +438,16 @@ void SendStatisticsProxy::OnSendEncodedImage( const CodecSpecificInfo* codec_info) { size_t simulcast_idx = 0; + rtc::CritScope lock(&crit_); if (codec_info) { if (codec_info->codecType == kVideoCodecVP8) { simulcast_idx = codec_info->codecSpecific.VP8.simulcastIdx; } else if (codec_info->codecType == kVideoCodecGeneric) { simulcast_idx = codec_info->codecSpecific.generic.simulcast_idx; } + if (codec_info->codec_name) { + stats_.encoder_implementation_name = codec_info->codec_name; + } } if (simulcast_idx >= config_.rtp.ssrcs.size()) { @@ -456,7 +457,6 @@ void SendStatisticsProxy::OnSendEncodedImage( } uint32_t ssrc = config_.rtp.ssrcs[simulcast_idx]; - rtc::CritScope lock(&crit_); VideoSendStream::StreamStats* stats = GetStatsEntry(ssrc); if (!stats) return; diff --git a/webrtc/video/send_statistics_proxy.h b/webrtc/video/send_statistics_proxy.h index b47691ab39..5d9dbf0921 100644 --- a/webrtc/video/send_statistics_proxy.h +++ b/webrtc/video/send_statistics_proxy.h @@ -52,9 +52,7 @@ class SendStatisticsProxy : public CpuOveruseMetricsObserver, // Used to update incoming frame rate. void OnIncomingFrame(int width, int height); - void OnEncoderStatsUpdate(uint32_t framerate, - uint32_t bitrate, - const std::string& encoder_name); + void OnEncoderStatsUpdate(uint32_t framerate, uint32_t bitrate); 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 1e9287749e..593c93b8e1 100644 --- a/webrtc/video/send_statistics_proxy_unittest.cc +++ b/webrtc/video/send_statistics_proxy_unittest.cc @@ -146,13 +146,11 @@ TEST_F(SendStatisticsProxyTest, EncodedBitrateAndFramerate) { int media_bitrate_bps = 500; int encode_fps = 29; - statistics_proxy_->OnEncoderStatsUpdate(encode_fps, media_bitrate_bps, - "encoder name"); + statistics_proxy_->OnEncoderStatsUpdate(encode_fps, media_bitrate_bps); 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_encoder.cc b/webrtc/video/video_encoder.cc index 1534a97fdf..5d4867f8c0 100644 --- a/webrtc/video/video_encoder.cc +++ b/webrtc/video/video_encoder.cc @@ -186,10 +186,4 @@ bool VideoEncoderSoftwareFallbackWrapper::SupportsNativeHandle() const { return encoder_->SupportsNativeHandle(); } -const char* VideoEncoderSoftwareFallbackWrapper::ImplementationName() const { - if (fallback_encoder_) - return fallback_implementation_name_.c_str(); - return encoder_->ImplementationName(); -} - } // namespace webrtc diff --git a/webrtc/video/video_encoder_unittest.cc b/webrtc/video/video_encoder_unittest.cc index ac006bb872..84ac4fd466 100644 --- a/webrtc/video/video_encoder_unittest.cc +++ b/webrtc/video/video_encoder_unittest.cc @@ -11,6 +11,7 @@ #include "webrtc/video_encoder.h" #include "testing/gtest/include/gtest/gtest.h" +#include "webrtc/modules/video_coding/include/video_codec_interface.h" #include "webrtc/modules/video_coding/include/video_error_codes.h" namespace webrtc { @@ -36,6 +37,13 @@ class VideoEncoderSoftwareFallbackWrapperTest : public ::testing::Test { const CodecSpecificInfo* codec_specific_info, const std::vector* frame_types) override { ++encode_count_; + if (encode_complete_callback_ && + encode_return_code_ == WEBRTC_VIDEO_CODEC_OK) { + CodecSpecificInfo info; + info.codec_name = ImplementationName(); + encode_complete_callback_->OnEncodedImage(EncodedImage(), &info, + nullptr); + } return encode_return_code_; } @@ -90,14 +98,19 @@ class VideoEncoderSoftwareFallbackWrapperTest : public ::testing::Test { const CodecSpecificInfo* codec_specific_info, const RTPFragmentationHeader* fragmentation) override { ++callback_count_; + last_codec_name_ = codec_specific_info->codec_name; return Result(Result::OK, callback_count_); } int callback_count_ = 0; + std::string last_codec_name_; }; void UtilizeFallbackEncoder(); void FallbackFromEncodeRequest(); void EncodeFrame(); + void CheckLastEncoderName(const char* expected_name) { + EXPECT_STREQ(expected_name, callback_.last_codec_name_.c_str()); + } FakeEncodedImageCallback callback_; CountingFakeEncoder fake_encoder_; @@ -265,13 +278,20 @@ TEST_F(VideoEncoderSoftwareFallbackWrapperTest, EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, fallback_wrapper_.Release()); } +TEST_F(VideoEncoderSoftwareFallbackWrapperTest, ReportsImplementationName) { + VideoCodec codec = {}; + fallback_wrapper_.RegisterEncodeCompleteCallback(&callback_); + fallback_wrapper_.InitEncode(&codec, 2, kMaxPayloadSize); + EncodeFrame(); + CheckLastEncoderName("fake-encoder"); +} + TEST_F(VideoEncoderSoftwareFallbackWrapperTest, ReportsFallbackImplementationName) { UtilizeFallbackEncoder(); // Hard coded expected value since libvpx is the software implementation name // for VP8. Change accordingly if the underlying implementation does. - EXPECT_STREQ("libvpx (fallback from: fake-encoder)", - fallback_wrapper_.ImplementationName()); + CheckLastEncoderName("libvpx"); } } // namespace webrtc diff --git a/webrtc/video/vie_encoder.cc b/webrtc/video/vie_encoder.cc index 956fd776d4..1afbed00f3 100644 --- a/webrtc/video/vie_encoder.cc +++ b/webrtc/video/vie_encoder.cc @@ -212,11 +212,9 @@ EncodedImageCallback::Result ViEEncoder::OnEncodedImage( return result; } -void ViEEncoder::SendStatistics(uint32_t bit_rate, - uint32_t frame_rate, - const std::string& encoder_name) { +void ViEEncoder::SendStatistics(uint32_t bit_rate, uint32_t frame_rate) { if (stats_proxy_) - stats_proxy_->OnEncoderStatsUpdate(frame_rate, bit_rate, encoder_name); + stats_proxy_->OnEncoderStatsUpdate(frame_rate, bit_rate); } void ViEEncoder::OnReceivedSLI(uint8_t picture_id) { diff --git a/webrtc/video/vie_encoder.h b/webrtc/video/vie_encoder.h index f3f8340c28..b302f9e918 100644 --- a/webrtc/video/vie_encoder.h +++ b/webrtc/video/vie_encoder.h @@ -88,9 +88,7 @@ class ViEEncoder : public EncodedImageCallback, const RTPFragmentationHeader* fragmentation) override; // Implements VideoSendStatisticsCallback. - void SendStatistics(uint32_t bit_rate, - uint32_t frame_rate, - const std::string& encoder_name) override; + void SendStatistics(uint32_t bit_rate, uint32_t frame_rate) override; // virtual to test EncoderStateFeedback with mocks. virtual void OnReceivedIntraFrameRequest(size_t stream_index); diff --git a/webrtc/video_encoder.h b/webrtc/video_encoder.h index 2584568533..9bfec55eb7 100644 --- a/webrtc/video_encoder.h +++ b/webrtc/video_encoder.h @@ -189,7 +189,6 @@ class VideoEncoderSoftwareFallbackWrapper : public VideoEncoder { int32_t SetRates(uint32_t bitrate, uint32_t framerate) override; void OnDroppedFrame() override; bool SupportsNativeHandle() const override; - const char* ImplementationName() const override; private: bool InitFallbackEncoder();