diff --git a/talk/app/webrtc/java/jni/peerconnection_jni.cc b/talk/app/webrtc/java/jni/peerconnection_jni.cc index 63ba8c5bac..d2b163974d 100644 --- a/talk/app/webrtc/java/jni/peerconnection_jni.cc +++ b/talk/app/webrtc/java/jni/peerconnection_jni.cc @@ -1916,8 +1916,6 @@ class MediaCodecVideoEncoderFactory // WebRtcVideoEncoderFactory implementation. virtual webrtc::VideoEncoder* CreateVideoEncoder(webrtc::VideoCodecType type) OVERRIDE; - virtual void AddObserver(Observer* observer) OVERRIDE; - virtual void RemoveObserver(Observer* observer) OVERRIDE; virtual const std::vector& codecs() const OVERRIDE; virtual void DestroyVideoEncoder(webrtc::VideoEncoder* encoder) OVERRIDE; @@ -1953,11 +1951,6 @@ webrtc::VideoEncoder* MediaCodecVideoEncoderFactory::CreateVideoEncoder( return new MediaCodecVideoEncoder(AttachCurrentThreadIfNeeded()); } -// Since the available codec list is never going to change, we ignore the -// Observer-related interface here. -void MediaCodecVideoEncoderFactory::AddObserver(Observer* observer) {} -void MediaCodecVideoEncoderFactory::RemoveObserver(Observer* observer) {} - const std::vector& MediaCodecVideoEncoderFactory::codecs() const { return supported_codecs_; diff --git a/talk/media/webrtc/fakewebrtcvideoengine.h b/talk/media/webrtc/fakewebrtcvideoengine.h index ec4b9c6e1f..e6de35bc61 100644 --- a/talk/media/webrtc/fakewebrtcvideoengine.h +++ b/talk/media/webrtc/fakewebrtcvideoengine.h @@ -206,16 +206,6 @@ class FakeWebRtcVideoEncoderFactory : public WebRtcVideoEncoderFactory { delete encoder; } - virtual void AddObserver(WebRtcVideoEncoderFactory::Observer* observer) { - bool inserted = observers_.insert(observer).second; - EXPECT_TRUE(inserted); - } - - virtual void RemoveObserver(WebRtcVideoEncoderFactory::Observer* observer) { - size_t erased = observers_.erase(observer); - EXPECT_EQ(erased, 1UL); - } - virtual const std::vector& codecs() const { return codecs_; @@ -228,12 +218,6 @@ class FakeWebRtcVideoEncoderFactory : public WebRtcVideoEncoderFactory { WebRtcVideoEncoderFactory::VideoCodec(type, name, 1280, 720, 30)); } - void NotifyCodecsAvailable() { - std::set::iterator it; - for (it = observers_.begin(); it != observers_.end(); ++it) - (*it)->OnCodecsAvailable(); - } - int GetNumCreatedEncoders() { return num_created_encoders_; } @@ -246,7 +230,6 @@ class FakeWebRtcVideoEncoderFactory : public WebRtcVideoEncoderFactory { std::set supported_codec_types_; std::vector codecs_; std::vector encoders_; - std::set observers_; int num_created_encoders_; }; diff --git a/talk/media/webrtc/webrtcvideoencoderfactory.h b/talk/media/webrtc/webrtcvideoencoderfactory.h index aa05d04ab5..e47541cc59 100644 --- a/talk/media/webrtc/webrtcvideoencoderfactory.h +++ b/talk/media/webrtc/webrtcvideoencoderfactory.h @@ -53,32 +53,14 @@ class WebRtcVideoEncoderFactory { } }; - class Observer { - public: - // Invoked when the list of supported codecs becomes available. - // This will not be invoked if the list of codecs is already available when - // the factory is installed. Otherwise this will be invoked only once if the - // list of codecs is not yet available when the factory is installed. - virtual void OnCodecsAvailable() = 0; - - protected: - virtual ~Observer() {} - }; + virtual ~WebRtcVideoEncoderFactory() {} // Caller takes the ownership of the returned object and it should be released // by calling DestroyVideoEncoder(). virtual webrtc::VideoEncoder* CreateVideoEncoder( webrtc::VideoCodecType type) = 0; - virtual ~WebRtcVideoEncoderFactory() {} - - // Adds/removes observer to receive OnCodecsChanged notifications. - // Factory must outlive Observer. Observer is responsible for removing itself - // from the Factory by the time its dtor is done. - virtual void AddObserver(Observer* observer) {} - virtual void RemoveObserver(Observer* observer) {} // Returns a list of supported codecs in order of preference. - // The list is empty if the list of codecs is not yet available. virtual const std::vector& codecs() const = 0; virtual void DestroyVideoEncoder(webrtc::VideoEncoder* encoder) = 0; diff --git a/talk/media/webrtc/webrtcvideoengine.cc b/talk/media/webrtc/webrtcvideoengine.cc index 5125062852..8cc5fed66b 100644 --- a/talk/media/webrtc/webrtcvideoengine.cc +++ b/talk/media/webrtc/webrtcvideoengine.cc @@ -988,9 +988,6 @@ WebRtcVideoEngine::~WebRtcVideoEngine() { if (initialized_) { Terminate(); } - if (encoder_factory_) { - encoder_factory_->RemoveObserver(this); - } tracing_->SetTraceCallback(NULL); // Test to see if the media processor was deregistered properly. ASSERT(SignalMediaFrame.is_empty()); @@ -1539,21 +1536,8 @@ void WebRtcVideoEngine::SetExternalEncoderFactory( if (encoder_factory_ == encoder_factory) return; - if (encoder_factory_) { - encoder_factory_->RemoveObserver(this); - } encoder_factory_ = encoder_factory; - if (encoder_factory_) { - encoder_factory_->AddObserver(this); - } - // Invoke OnCodecAvailable() here in case the list of codecs is already - // available when the encoder factory is installed. If not the encoder - // factory will invoke the callback later when the codecs become available. - OnCodecsAvailable(); -} - -void WebRtcVideoEngine::OnCodecsAvailable() { // Rebuild codec list while reapplying the current default codec format. VideoCodec max_codec(kVideoCodecPrefs[0].payload_type, kVideoCodecPrefs[0].name, diff --git a/talk/media/webrtc/webrtcvideoengine.h b/talk/media/webrtc/webrtcvideoengine.h index 68eea71c32..9dd8dc8f16 100644 --- a/talk/media/webrtc/webrtcvideoengine.h +++ b/talk/media/webrtc/webrtcvideoengine.h @@ -86,8 +86,7 @@ struct CapturedFrame; struct Device; class WebRtcVideoEngine : public sigslot::has_slots<>, - public webrtc::TraceCallback, - public WebRtcVideoEncoderFactory::Observer { + public webrtc::TraceCallback { public: // Creates the WebRtcVideoEngine with internal VideoCaptureModule. WebRtcVideoEngine(); @@ -210,9 +209,6 @@ class WebRtcVideoEngine : public sigslot::has_slots<>, const char* trace, int length) OVERRIDE; - // WebRtcVideoEncoderFactory::Observer implementation. - virtual void OnCodecsAvailable() OVERRIDE; - rtc::Thread* worker_thread_; rtc::scoped_ptr vie_wrapper_; bool vie_wrapper_base_initialized_; diff --git a/talk/media/webrtc/webrtcvideoengine2.cc b/talk/media/webrtc/webrtcvideoengine2.cc index befe38ef12..d8b5616649 100644 --- a/talk/media/webrtc/webrtcvideoengine2.cc +++ b/talk/media/webrtc/webrtcvideoengine2.cc @@ -405,21 +405,7 @@ void WebRtcVideoEngine2::SetExternalDecoderFactory( void WebRtcVideoEngine2::SetExternalEncoderFactory( WebRtcVideoEncoderFactory* encoder_factory) { - if (external_encoder_factory_ == encoder_factory) { - return; - } - if (external_encoder_factory_) { - external_encoder_factory_->RemoveObserver(this); - } external_encoder_factory_ = encoder_factory; - if (external_encoder_factory_) { - external_encoder_factory_->AddObserver(this); - } - - // Invoke OnCodecAvailable() here in case the list of codecs is already - // available when the encoder factory is installed. If not the encoder - // factory will invoke the callback later when the codecs become available. - OnCodecsAvailable(); } bool WebRtcVideoEngine2::EnableTimedRender() { @@ -509,9 +495,6 @@ WebRtcVideoEncoderFactory2* WebRtcVideoEngine2::GetVideoEncoderFactory() { return &default_video_encoder_factory_; } -void WebRtcVideoEngine2::OnCodecsAvailable() { - // TODO(pbos): Implement. -} // Thin map between VideoFrame and an existing webrtc::I420VideoFrame // to avoid having to copy the rendered VideoFrame prematurely. // This implementation is only safe to use in a const context and should never diff --git a/talk/media/webrtc/webrtcvideoengine2.h b/talk/media/webrtc/webrtcvideoengine2.h index e7308ec06f..de591f0566 100644 --- a/talk/media/webrtc/webrtcvideoengine2.h +++ b/talk/media/webrtc/webrtcvideoengine2.h @@ -138,8 +138,7 @@ class WebRtcCallFactory { }; // WebRtcVideoEngine2 is used for the new native WebRTC Video API (webrtc:1667). -class WebRtcVideoEngine2 : public sigslot::has_slots<>, - public WebRtcVideoEncoderFactory::Observer { +class WebRtcVideoEngine2 : public sigslot::has_slots<> { public: // Creates the WebRtcVideoEngine2 with internal VideoCaptureModule. WebRtcVideoEngine2(); @@ -193,8 +192,6 @@ class WebRtcVideoEngine2 : public sigslot::has_slots<>, virtual WebRtcVideoEncoderFactory2* GetVideoEncoderFactory(); private: - virtual void OnCodecsAvailable() OVERRIDE; - rtc::Thread* worker_thread_; WebRtcVoiceEngine* voice_engine_; std::vector video_codecs_; diff --git a/talk/media/webrtc/webrtcvideoengine_unittest.cc b/talk/media/webrtc/webrtcvideoengine_unittest.cc index 8fc1920d1e..c65e2cafd7 100644 --- a/talk/media/webrtc/webrtcvideoengine_unittest.cc +++ b/talk/media/webrtc/webrtcvideoengine_unittest.cc @@ -2006,7 +2006,6 @@ TEST_F(WebRtcVideoEngineTestFake, ExternalCodecFeedbackParams) { encoder_factory_.AddSupportedVideoCodecType(webrtc::kVideoCodecGeneric, "GENERIC"); engine_.SetExternalEncoderFactory(&encoder_factory_); - encoder_factory_.NotifyCodecsAvailable(); EXPECT_TRUE(SetupEngine()); std::vector codecs(engine_.codecs()); @@ -2016,18 +2015,16 @@ TEST_F(WebRtcVideoEngineTestFake, ExternalCodecFeedbackParams) { VerifyCodecFeedbackParams(codecs[pos]); } -// Test external codec with be added to the end of the supported codec list. +// Test external codec will be added to the end of the supported codec list. TEST_F(WebRtcVideoEngineTestFake, ExternalCodecAddedToTheEnd) { + encoder_factory_.AddSupportedVideoCodecType(webrtc::kVideoCodecGeneric, + "GENERIC"); + engine_.SetExternalEncoderFactory(&encoder_factory_); EXPECT_TRUE(SetupEngine()); std::vector codecs(engine_.codecs()); EXPECT_EQ("VP8", codecs[0].name); - encoder_factory_.AddSupportedVideoCodecType(webrtc::kVideoCodecGeneric, - "GENERIC"); - engine_.SetExternalEncoderFactory(&encoder_factory_); - encoder_factory_.NotifyCodecsAvailable(); - codecs = engine_.codecs(); cricket::VideoCodec internal_codec = codecs[0]; cricket::VideoCodec external_codec = codecs[codecs.size() - 1]; @@ -2037,18 +2034,16 @@ TEST_F(WebRtcVideoEngineTestFake, ExternalCodecAddedToTheEnd) { EXPECT_GE(internal_codec.preference, external_codec.preference); } -// Test that external codec with be ignored if it has the same name as one of -// the internal codecs. +// Test that external codecs that we support internally are not added as +// duplicate entries to the codecs list. TEST_F(WebRtcVideoEngineTestFake, ExternalCodecIgnored) { + encoder_factory_.AddSupportedVideoCodecType(webrtc::kVideoCodecVP8, "VP8"); + engine_.SetExternalEncoderFactory(&encoder_factory_); EXPECT_TRUE(SetupEngine()); std::vector internal_codecs(engine_.codecs()); EXPECT_EQ("VP8", internal_codecs[0].name); - encoder_factory_.AddSupportedVideoCodecType(webrtc::kVideoCodecVP8, "VP8"); - engine_.SetExternalEncoderFactory(&encoder_factory_); - encoder_factory_.NotifyCodecsAvailable(); - std::vector codecs = engine_.codecs(); EXPECT_EQ("VP8", codecs[0].name); EXPECT_EQ(internal_codecs[0].height, codecs[0].height); @@ -2057,27 +2052,6 @@ TEST_F(WebRtcVideoEngineTestFake, ExternalCodecIgnored) { EXPECT_NE("VP8", codecs[codecs.size() - 1].name); } -TEST_F(WebRtcVideoEngineTestFake, UpdateEncoderCodecsAfterSetFactory) { - engine_.SetExternalEncoderFactory(&encoder_factory_); - EXPECT_TRUE(SetupEngine()); - int channel_num = vie_.GetLastChannel(); - - encoder_factory_.AddSupportedVideoCodecType(webrtc::kVideoCodecVP8, "VP8"); - encoder_factory_.NotifyCodecsAvailable(); - std::vector codecs; - codecs.push_back(kVP8Codec); - EXPECT_TRUE(channel_->SetSendCodecs(codecs)); - - EXPECT_TRUE(channel_->AddSendStream( - cricket::StreamParams::CreateLegacy(kSsrc))); - - EXPECT_TRUE(vie_.ExternalEncoderRegistered(channel_num, 100)); - EXPECT_EQ(1, vie_.GetNumExternalEncoderRegistered(channel_num)); - - // Remove stream previously added to free the external encoder instance. - EXPECT_TRUE(channel_->RemoveSendStream(kSsrc)); -} - #ifdef USE_WEBRTC_DEV_BRANCH TEST_F(WebRtcVideoEngineTestFake, SetSendCodecsWithExternalH264) { encoder_factory_.AddSupportedVideoCodecType(webrtc::kVideoCodecH264, "H264");