From a5dec16b42b1f9c36c10ebd10e5609e6752c67a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20Bostr=C3=B6m?= Date: Wed, 20 Jan 2016 15:53:55 +0100 Subject: [PATCH] Name SimulcastEncoderApdater on InitEncode. Provides a better string (provides names of all implementations), but also fixes a crash when accessing the ImplementationName() of SimulcastEncoderAdapter where InitEncode has failed. BUG=chromium:577932, webrtc:4897 R=stefan@webrtc.org Review URL: https://codereview.webrtc.org/1599353003 . Cr-Commit-Position: refs/heads/master@{#11321} --- .../codecs/vp8/simulcast_encoder_adapter.cc | 15 ++++++++---- .../codecs/vp8/simulcast_encoder_adapter.h | 2 ++ .../vp8/simulcast_encoder_adapter_unittest.cc | 24 +++++++++++++++++++ 3 files changed, 36 insertions(+), 5 deletions(-) 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 3e03cda21a..0a17e743f7 100644 --- a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc +++ b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc @@ -136,7 +136,9 @@ class AdapterEncodedImageCallback : public webrtc::EncodedImageCallback { namespace webrtc { SimulcastEncoderAdapter::SimulcastEncoderAdapter(VideoEncoderFactory* factory) - : factory_(factory), encoded_complete_callback_(NULL) { + : factory_(factory), + encoded_complete_callback_(NULL), + implementation_name_("SimulcastEncoderAdapter") { memset(&codec_, 0, sizeof(webrtc::VideoCodec)); } @@ -192,6 +194,7 @@ int SimulcastEncoderAdapter::InitEncode(const VideoCodec* inst, codec_.codecSpecific.VP8.tl_factory = screensharing_tl_factory_.get(); } + std::string implementation_name; // Create |number_of_streams| of encoder instances and init them. for (int i = 0; i < number_of_streams; ++i) { VideoCodec stream_codec; @@ -221,7 +224,12 @@ int SimulcastEncoderAdapter::InitEncode(const VideoCodec* inst, encoder->RegisterEncodeCompleteCallback(callback); streaminfos_.push_back(StreamInfo(encoder, callback, stream_codec.width, stream_codec.height, send_stream)); + if (i != 0) + implementation_name += ", "; + implementation_name += streaminfos_[i].encoder->ImplementationName(); } + implementation_name_ = + "SimulcastEncoderAdapter (" + implementation_name + ")"; return WEBRTC_VIDEO_CODEC_OK; } @@ -494,10 +502,7 @@ bool SimulcastEncoderAdapter::SupportsNativeHandle() const { } const char* SimulcastEncoderAdapter::ImplementationName() const { - // We should not be calling this method before streaminfos_ are configured. - RTC_DCHECK(!streaminfos_.empty()); - // TODO(pbos): Support multiple implementation names for different encoders. - return streaminfos_[0].encoder->ImplementationName(); + return implementation_name_.c_str(); } } // namespace webrtc diff --git a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.h b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.h index c7bd656659..f75a158802 100644 --- a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.h +++ b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.h @@ -12,6 +12,7 @@ #ifndef WEBRTC_MODULES_VIDEO_CODING_CODECS_VP8_SIMULCAST_ENCODER_ADAPTER_H_ #define WEBRTC_MODULES_VIDEO_CODING_CODECS_VP8_SIMULCAST_ENCODER_ADAPTER_H_ +#include #include #include "webrtc/base/scoped_ptr.h" @@ -114,6 +115,7 @@ class SimulcastEncoderAdapter : public VP8Encoder { VideoCodec codec_; std::vector streaminfos_; EncodedImageCallback* encoded_complete_callback_; + std::string implementation_name_; }; } // namespace webrtc diff --git a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter_unittest.cc b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter_unittest.cc index 1340af3e6a..684e214c1a 100644 --- a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter_unittest.cc +++ b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter_unittest.cc @@ -152,6 +152,8 @@ class MockVideoEncoder : public VideoEncoder { supports_native_handle_ = enabled; } + MOCK_CONST_METHOD0(ImplementationName, const char*()); + private: bool supports_native_handle_ = false; VideoCodec codec_; @@ -162,6 +164,10 @@ class MockVideoEncoderFactory : public VideoEncoderFactory { public: VideoEncoder* Create() override { MockVideoEncoder* encoder = new MockVideoEncoder(); + const char* encoder_name = encoder_names_.empty() + ? "codec_implementation_name" + : encoder_names_[encoders_.size()]; + ON_CALL(*encoder, ImplementationName()).WillByDefault(Return(encoder_name)); encoders_.push_back(encoder); return encoder; } @@ -171,9 +177,13 @@ class MockVideoEncoderFactory : public VideoEncoderFactory { virtual ~MockVideoEncoderFactory() {} const std::vector& encoders() const { return encoders_; } + void SetEncoderNames(const std::vector& encoder_names) { + encoder_names_ = encoder_names; + } private: std::vector encoders_; + std::vector encoder_names_; }; class TestSimulcastEncoderAdapterFakeHelper { @@ -397,6 +407,20 @@ TEST_F(TestSimulcastEncoderAdapterFake, SupportsNativeHandleForSingleStreams) { EXPECT_FALSE(adapter_->SupportsNativeHandle()); } +TEST_F(TestSimulcastEncoderAdapterFake, SupportsImplementationName) { + EXPECT_STREQ("SimulcastEncoderAdapter", adapter_->ImplementationName()); + TestVp8Simulcast::DefaultSettings( + &codec_, static_cast(kTestTemporalLayerProfile)); + std::vector encoder_names; + encoder_names.push_back("codec1"); + encoder_names.push_back("codec2"); + encoder_names.push_back("codec3"); + helper_->factory()->SetEncoderNames(encoder_names); + EXPECT_EQ(0, adapter_->InitEncode(&codec_, 1, 1200)); + EXPECT_STREQ("SimulcastEncoderAdapter (codec1, codec2, codec3)", + adapter_->ImplementationName()); +} + TEST_F(TestSimulcastEncoderAdapterFake, SupportsNativeHandleDisabledForMultipleStreams) { // TODO(pbos): Implement actual test (verify that it works) when implemented