From 83fb2a9e5fdfd7cc90928a4fd1a280eff3963d09 Mon Sep 17 00:00:00 2001 From: "xhwang@chromium.org" Date: Wed, 16 Jul 2014 23:36:41 +0000 Subject: [PATCH] Fold {Audio|Video}Decoder::Stop() into the dtor. The Stop() process is already synchronous. This CL folds the Stop() call into the dtor which simplifies a lot of code. TBR=bbudge@chromium.org BUG=349211 TEST=Current unittests pass. Review URL: https://codereview.chromium.org/395703002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@283578 0039d316-1c4b-4281-b951-d872f2087c98 --- content/renderer/pepper/video_decoder_shim.cc | 8 +-- media/base/audio_decoder.h | 15 ++--- media/base/mock_filters.h | 2 - media/base/video_decoder.h | 17 +++-- media/filters/audio_decoder_selector_unittest.cc | 18 ------ media/filters/audio_decoder_unittest.cc | 22 +------ media/filters/audio_renderer_impl_unittest.cc | 3 - media/filters/decoder_selector.cc | 2 +- media/filters/decoder_stream.cc | 5 +- media/filters/decrypting_audio_decoder.cc | 21 +++---- media/filters/decrypting_audio_decoder.h | 4 +- media/filters/decrypting_audio_decoder_unittest.cc | 6 +- media/filters/decrypting_video_decoder.cc | 23 ++----- media/filters/decrypting_video_decoder.h | 5 -- media/filters/decrypting_video_decoder_unittest.cc | 72 ++++++++++------------ media/filters/fake_video_decoder.cc | 36 +++++------ media/filters/fake_video_decoder.h | 1 - media/filters/fake_video_decoder_unittest.cc | 26 ++++---- media/filters/ffmpeg_audio_decoder.cc | 20 ++---- media/filters/ffmpeg_audio_decoder.h | 1 - media/filters/ffmpeg_video_decoder.cc | 15 +---- media/filters/ffmpeg_video_decoder.h | 1 - media/filters/ffmpeg_video_decoder_unittest.cc | 24 ++++---- media/filters/gpu_video_decoder.cc | 19 +++--- media/filters/gpu_video_decoder.h | 1 - media/filters/opus_audio_decoder.cc | 6 +- media/filters/opus_audio_decoder.h | 1 - media/filters/video_decoder_selector_unittest.cc | 18 ------ media/filters/video_renderer_impl_unittest.cc | 11 ---- media/filters/vpx_video_decoder.cc | 8 +-- media/filters/vpx_video_decoder.h | 1 - 31 files changed, 133 insertions(+), 279 deletions(-) diff --git a/content/renderer/pepper/video_decoder_shim.cc b/content/renderer/pepper/video_decoder_shim.cc index 50a63989278f..dab44cce5007 100644 --- a/content/renderer/pepper/video_decoder_shim.cc +++ b/content/renderer/pepper/video_decoder_shim.cc @@ -135,9 +135,9 @@ void VideoDecoderShim::DecoderImpl::Initialize( decoder_ = ffmpeg_video_decoder.Pass(); } max_decodes_at_decoder_ = decoder_->GetMaxDecodeRequests(); - // We can use base::Unretained() safely in decoder callbacks because we call - // VideoDecoder::Stop() before deletion. Stop() guarantees there will be no - // outstanding callbacks after it returns. + // We can use base::Unretained() safely in decoder callbacks because + // |decoder_| is owned by DecoderImpl. During Stop(), the |decoder_| will be + // destroyed and all outstanding callbacks will be fired. decoder_->Initialize( config, true /* low_delay */, @@ -178,7 +178,7 @@ void VideoDecoderShim::DecoderImpl::Stop() { // again. while (!pending_decodes_.empty()) pending_decodes_.pop(); - decoder_->Stop(); + decoder_.reset(); // This instance is deleted once we exit this scope. } diff --git a/media/base/audio_decoder.h b/media/base/audio_decoder.h index 0118b5e4fdd5..779e5ef50265 100644 --- a/media/base/audio_decoder.h +++ b/media/base/audio_decoder.h @@ -25,7 +25,7 @@ class MEDIA_EXPORT AudioDecoder { // match, break them into a decoder_status.h. enum Status { kOk, // We're all good. - kAborted, // We aborted as a result of Stop() or Reset(). + kAborted, // We aborted as a result of Reset() or destruction. kDecodeError, // A decoding error occurred. kDecryptError // Decrypting error happened. }; @@ -40,6 +40,11 @@ class MEDIA_EXPORT AudioDecoder { typedef base::Callback DecodeCB; AudioDecoder(); + + // Fires any pending callbacks, stops and destroys the decoder. + // Note: Since this is a destructor, |this| will be destroyed after this call. + // Make sure the callbacks fired from this call doesn't post any task that + // depends on |this|. virtual ~AudioDecoder(); // Initializes an AudioDecoder with the given DemuxerStream, executing the @@ -68,14 +73,6 @@ class MEDIA_EXPORT AudioDecoder { // aborted before |closure| is called. virtual void Reset(const base::Closure& closure) = 0; - // Stops decoder, fires any pending callbacks and sets the decoder to an - // uninitialized state. An AudioDecoder cannot be re-initialized after it has - // been stopped. DecodeCB and OutputCB may still be called for older buffers - // if they were scheduled before this method is called. - // Note that if Initialize() is pending or has finished successfully, Stop() - // must be called before destructing the decoder. - virtual void Stop() = 0; - private: DISALLOW_COPY_AND_ASSIGN(AudioDecoder); }; diff --git a/media/base/mock_filters.h b/media/base/mock_filters.h index 7afa62501678..28d4d090b4db 100644 --- a/media/base/mock_filters.h +++ b/media/base/mock_filters.h @@ -84,7 +84,6 @@ class MockVideoDecoder : public VideoDecoder { MOCK_METHOD2(Decode, void(const scoped_refptr& buffer, const DecodeCB&)); MOCK_METHOD1(Reset, void(const base::Closure&)); - MOCK_METHOD0(Stop, void()); MOCK_CONST_METHOD0(HasAlpha, bool()); private: @@ -105,7 +104,6 @@ class MockAudioDecoder : public AudioDecoder { void(const scoped_refptr& buffer, const DecodeCB&)); MOCK_METHOD1(Reset, void(const base::Closure&)); - MOCK_METHOD0(Stop, void()); private: DISALLOW_COPY_AND_ASSIGN(MockAudioDecoder); diff --git a/media/base/video_decoder.h b/media/base/video_decoder.h index e7d7ad628785..edca238e784a 100644 --- a/media/base/video_decoder.h +++ b/media/base/video_decoder.h @@ -39,6 +39,11 @@ class MEDIA_EXPORT VideoDecoder { typedef base::Callback DecodeCB; VideoDecoder(); + + // Fires any pending callbacks, stops and destroys the decoder. + // Note: Since this is a destructor, |this| will be destroyed after this call. + // Make sure the callbacks fired from this call doesn't post any task that + // depends on |this|. virtual ~VideoDecoder(); // Initializes a VideoDecoder with the given |config|, executing the @@ -48,9 +53,8 @@ class MEDIA_EXPORT VideoDecoder { // Note: // 1) The VideoDecoder will be reinitialized if it was initialized before. // Upon reinitialization, all internal buffered frames will be dropped. - // 2) This method should not be called during pending decode, reset or stop. - // 3) No VideoDecoder calls except for Stop() should be made before - // |status_cb| is executed. + // 2) This method should not be called during pending decode or reset. + // 3) No VideoDecoder calls should be made before |status_cb| is executed. virtual void Initialize(const VideoDecoderConfig& config, bool low_delay, const PipelineStatusCB& status_cb, @@ -83,13 +87,6 @@ class MEDIA_EXPORT VideoDecoder { // Note: No VideoDecoder calls should be made before |closure| is executed. virtual void Reset(const base::Closure& closure) = 0; - // Stops decoder, fires any pending callbacks and sets the decoder to an - // uninitialized state. A VideoDecoder cannot be re-initialized after it has - // been stopped. - // Note that if Initialize() is pending or has finished successfully, Stop() - // must be called before destructing the decoder. - virtual void Stop() = 0; - // Returns true if the decoder needs bitstream conversion before decoding. virtual bool NeedsBitstreamConversion() const; diff --git a/media/filters/audio_decoder_selector_unittest.cc b/media/filters/audio_decoder_selector_unittest.cc index 56005822b10b..2b4cce54675c 100644 --- a/media/filters/audio_decoder_selector_unittest.cc +++ b/media/filters/audio_decoder_selector_unittest.cc @@ -45,9 +45,6 @@ class AudioDecoderSelectorTest : public ::testing::Test { } ~AudioDecoderSelectorTest() { - if (selected_decoder_) - selected_decoder_->Stop(); - message_loop_.RunUntilIdle(); } @@ -151,11 +148,6 @@ class AudioDecoderSelectorTest : public ::testing::Test { DISALLOW_COPY_AND_ASSIGN(AudioDecoderSelectorTest); }; -// Note: -// In all the tests, Stop() is expected to be called on a decoder if a decoder: -// - is pending initialization and DecoderSelector::Abort() is called, or -// - has been successfully initialized. - // The stream is not encrypted but we have no clear decoder. No decoder can be // selected. TEST_F(AudioDecoderSelectorTest, ClearStream_NoDecryptor_NoClearDecoder) { @@ -176,7 +168,6 @@ TEST_F(AudioDecoderSelectorTest, ClearStream_NoDecryptor_OneClearDecoder) { EXPECT_CALL(*decoder_1_, Initialize(_, _, _)) .WillOnce(RunCallback<1>(PIPELINE_OK)); EXPECT_CALL(*this, OnDecoderSelected(decoder_1_, IsNull())); - EXPECT_CALL(*decoder_1_, Stop()); SelectDecoder(); } @@ -187,7 +178,6 @@ TEST_F(AudioDecoderSelectorTest, InitializeDecoderSelector(kNoDecryptor, 1); EXPECT_CALL(*decoder_1_, Initialize(_, _, _)); - EXPECT_CALL(*decoder_1_, Stop()); SelectDecoderAndAbort(); } @@ -203,7 +193,6 @@ TEST_F(AudioDecoderSelectorTest, ClearStream_NoDecryptor_MultipleClearDecoder) { EXPECT_CALL(*decoder_2_, Initialize(_, _, _)) .WillOnce(RunCallback<1>(PIPELINE_OK)); EXPECT_CALL(*this, OnDecoderSelected(decoder_2_, IsNull())); - EXPECT_CALL(*decoder_2_, Stop()); SelectDecoder(); } @@ -216,7 +205,6 @@ TEST_F(AudioDecoderSelectorTest, EXPECT_CALL(*decoder_1_, Initialize(_, _, _)) .WillOnce(RunCallback<1>(DECODER_ERROR_NOT_SUPPORTED)); EXPECT_CALL(*decoder_2_, Initialize(_, _, _)); - EXPECT_CALL(*decoder_2_, Stop()); SelectDecoderAndAbort(); } @@ -230,7 +218,6 @@ TEST_F(AudioDecoderSelectorTest, ClearStream_HasDecryptor) { EXPECT_CALL(*decoder_1_, Initialize(_, _, _)) .WillOnce(RunCallback<1>(PIPELINE_OK)); EXPECT_CALL(*this, OnDecoderSelected(decoder_1_, IsNull())); - EXPECT_CALL(*decoder_1_, Stop()); SelectDecoder(); } @@ -240,7 +227,6 @@ TEST_F(AudioDecoderSelectorTest, Abort_ClearStream_HasDecryptor) { InitializeDecoderSelector(kDecryptOnly, 1); EXPECT_CALL(*decoder_1_, Initialize(_, _, _)); - EXPECT_CALL(*decoder_1_, Stop()); SelectDecoderAndAbort(); } @@ -283,7 +269,6 @@ TEST_F(AudioDecoderSelectorTest, EncryptedStream_DecryptOnly_OneClearDecoder) { EXPECT_CALL(*decoder_1_, Initialize(_, _, _)) .WillOnce(RunCallback<1>(PIPELINE_OK)); EXPECT_CALL(*this, OnDecoderSelected(decoder_1_, NotNull())); - EXPECT_CALL(*decoder_1_, Stop()); SelectDecoder(); } @@ -294,7 +279,6 @@ TEST_F(AudioDecoderSelectorTest, InitializeDecoderSelector(kDecryptOnly, 1); EXPECT_CALL(*decoder_1_, Initialize(_, _, _)); - EXPECT_CALL(*decoder_1_, Stop()); SelectDecoderAndAbort(); } @@ -312,7 +296,6 @@ TEST_F(AudioDecoderSelectorTest, EXPECT_CALL(*decoder_2_, Initialize(_, _, _)) .WillOnce(RunCallback<1>(PIPELINE_OK)); EXPECT_CALL(*this, OnDecoderSelected(decoder_2_, NotNull())); - EXPECT_CALL(*decoder_2_, Stop()); SelectDecoder(); } @@ -325,7 +308,6 @@ TEST_F(AudioDecoderSelectorTest, EXPECT_CALL(*decoder_1_, Initialize(_, _, _)) .WillOnce(RunCallback<1>(DECODER_ERROR_NOT_SUPPORTED)); EXPECT_CALL(*decoder_2_, Initialize(_, _, _)); - EXPECT_CALL(*decoder_2_, Stop()); SelectDecoderAndAbort(); } diff --git a/media/filters/audio_decoder_unittest.cc b/media/filters/audio_decoder_unittest.cc index a3aeb2565adc..23e21ef7d896 100644 --- a/media/filters/audio_decoder_unittest.cc +++ b/media/filters/audio_decoder_unittest.cc @@ -110,9 +110,6 @@ class AudioDecoderTest : public testing::TestWithParam { } virtual ~AudioDecoderTest() { - // Always issue a Stop() even if it's already been sent to avoid assertion - // failures causing crashes. - Stop(); EXPECT_FALSE(pending_decode_); EXPECT_FALSE(pending_reset_); } @@ -209,10 +206,6 @@ class AudioDecoderTest : public testing::TestWithParam { ASSERT_FALSE(pending_reset_); } - void Stop() { - decoder_->Stop(); - } - void Seek(base::TimeDelta seek_time) { Reset(); decoded_audio_.clear(); @@ -319,10 +312,9 @@ class FFmpegAudioDecoderBehavioralTest : public AudioDecoderTest {}; TEST_P(AudioDecoderTest, Initialize) { ASSERT_NO_FATAL_FAILURE(Initialize()); - Stop(); } -// Verifies decode audio as well as the Decode() -> Reset() -> Stop() sequence. +// Verifies decode audio as well as the Decode() -> Reset() sequence. TEST_P(AudioDecoderTest, ProduceAudioSamples) { ASSERT_NO_FATAL_FAILURE(Initialize()); @@ -357,21 +349,17 @@ TEST_P(AudioDecoderTest, ProduceAudioSamples) { // Seek back to the beginning. Calls Reset() on the decoder. Seek(start_timestamp()); } - - Stop(); } -TEST_P(AudioDecoderTest, DecodeStop) { +TEST_P(AudioDecoderTest, Decode) { ASSERT_NO_FATAL_FAILURE(Initialize()); Decode(); EXPECT_EQ(AudioDecoder::kOk, last_decode_status()); - Stop(); } -TEST_P(AudioDecoderTest, ResetStop) { +TEST_P(AudioDecoderTest, Reset) { ASSERT_NO_FATAL_FAILURE(Initialize()); Reset(); - Stop(); } TEST_P(AudioDecoderTest, NoTimestamp) { @@ -380,7 +368,6 @@ TEST_P(AudioDecoderTest, NoTimestamp) { buffer->set_timestamp(kNoTimestamp()); DecodeBuffer(buffer); EXPECT_EQ(AudioDecoder::kDecodeError, last_decode_status()); - Stop(); } TEST_P(OpusAudioDecoderBehavioralTest, InitializeWithNoCodecDelay) { @@ -397,7 +384,6 @@ TEST_P(OpusAudioDecoderBehavioralTest, InitializeWithNoCodecDelay) { base::TimeDelta::FromMilliseconds(80), 0); InitializeDecoder(decoder_config); - Stop(); } TEST_P(OpusAudioDecoderBehavioralTest, InitializeWithBadCodecDelay) { @@ -416,7 +402,6 @@ TEST_P(OpusAudioDecoderBehavioralTest, InitializeWithBadCodecDelay) { // Use a different codec delay than in the extradata. 100); InitializeDecoderWithStatus(decoder_config, DECODER_ERROR_NOT_SUPPORTED); - Stop(); } TEST_P(FFmpegAudioDecoderBehavioralTest, InitializeWithBadConfig) { @@ -429,7 +414,6 @@ TEST_P(FFmpegAudioDecoderBehavioralTest, InitializeWithBadConfig) { 0, false); InitializeDecoderWithStatus(decoder_config, DECODER_ERROR_NOT_SUPPORTED); - Stop(); } const DecodedBufferExpectations kSfxOpusExpectations[] = { diff --git a/media/filters/audio_renderer_impl_unittest.cc b/media/filters/audio_renderer_impl_unittest.cc index 830b12ce42dd..c6cb9523cf7d 100644 --- a/media/filters/audio_renderer_impl_unittest.cc +++ b/media/filters/audio_renderer_impl_unittest.cc @@ -130,7 +130,6 @@ class AudioRendererImplTest : public ::testing::Test { EXPECT_CALL(*decoder_, Initialize(_, _, _)) .WillOnce(DoAll(SaveArg<2>(&output_cb_), RunCallback<1>(PIPELINE_OK))); - EXPECT_CALL(*decoder_, Stop()); InitializeWithStatus(PIPELINE_OK); next_timestamp_.reset(new AudioTimestampHelper( @@ -152,7 +151,6 @@ class AudioRendererImplTest : public ::testing::Test { EXPECT_CALL(*decoder_, Initialize(_, _, _)) .WillOnce(DoAll(SaveArg<2>(&output_cb_), RunCallback<1>(PIPELINE_OK))); - EXPECT_CALL(*decoder_, Stop()); WaitableMessageLoopEvent event; InitializeRenderer(event.GetPipelineStatusCB()); @@ -169,7 +167,6 @@ class AudioRendererImplTest : public ::testing::Test { EXPECT_CALL(*decoder_, Initialize(_, _, _)) .WillOnce(DoAll(SaveArg<2>(&output_cb_), EnterPendingDecoderInitStateAction(this))); - EXPECT_CALL(*decoder_, Stop()); WaitableMessageLoopEvent event; InitializeRenderer(event.GetPipelineStatusCB()); diff --git a/media/filters/decoder_selector.cc b/media/filters/decoder_selector.cc index fbd6d01741c8..077aed0d80b0 100644 --- a/media/filters/decoder_selector.cc +++ b/media/filters/decoder_selector.cc @@ -130,7 +130,7 @@ void DecoderSelector::Abort() { if (decoder_) { // |decrypted_stream_| is either NULL or already initialized. We don't // need to Stop() |decrypted_stream_| in either case. - decoder_->Stop(); + decoder_.reset(); ReturnNullDecoder(); return; } diff --git a/media/filters/decoder_stream.cc b/media/filters/decoder_stream.cc index 8a17c593edc6..f8714f25fa0c 100644 --- a/media/filters/decoder_stream.cc +++ b/media/filters/decoder_stream.cc @@ -189,13 +189,12 @@ void DecoderStream::Stop(const base::Closure& closure) { if (decrypting_demuxer_stream_) decrypting_demuxer_stream_->Stop(); - if (decoder_) - decoder_->Stop(); - state_ = STATE_STOPPED; stream_ = NULL; decoder_.reset(); decrypting_demuxer_stream_.reset(); + + state_ = STATE_STOPPED; task_runner_->PostTask(FROM_HERE, closure); } diff --git a/media/filters/decrypting_audio_decoder.cc b/media/filters/decrypting_audio_decoder.cc index 136e171d7383..79d82093302c 100644 --- a/media/filters/decrypting_audio_decoder.cc +++ b/media/filters/decrypting_audio_decoder.cc @@ -144,13 +144,12 @@ void DecryptingAudioDecoder::Reset(const base::Closure& closure) { DoReset(); } -void DecryptingAudioDecoder::Stop() { - DVLOG(2) << "Stop() - state: " << state_; +DecryptingAudioDecoder::~DecryptingAudioDecoder() { + DVLOG(2) << __FUNCTION__; DCHECK(task_runner_->BelongsToCurrentThread()); - // Invalidate all weak pointers so that pending callbacks won't be fired into - // this object. - weak_factory_.InvalidateWeakPtrs(); + if (state_ == kUninitialized) + return; if (decryptor_) { decryptor_->DeinitializeDecoder(Decryptor::kAudio); @@ -165,12 +164,6 @@ void DecryptingAudioDecoder::Stop() { base::ResetAndReturn(&decode_cb_).Run(kAborted); if (!reset_cb_.is_null()) base::ResetAndReturn(&reset_cb_).Run(); - - state_ = kStopped; -} - -DecryptingAudioDecoder::~DecryptingAudioDecoder() { - DCHECK(state_ == kUninitialized || state_ == kStopped) << state_; } void DecryptingAudioDecoder::SetDecryptor(Decryptor* decryptor) { @@ -184,8 +177,7 @@ void DecryptingAudioDecoder::SetDecryptor(Decryptor* decryptor) { if (!decryptor) { base::ResetAndReturn(&init_cb_).Run(DECODER_ERROR_NOT_SUPPORTED); - // TODO(xhwang): Add kError state. See http://crbug.com/251503 - state_ = kStopped; + state_ = kError; return; } @@ -212,7 +204,8 @@ void DecryptingAudioDecoder::FinishInitialization(bool success) { if (!success) { base::ResetAndReturn(&init_cb_).Run(DECODER_ERROR_NOT_SUPPORTED); - state_ = kStopped; + decryptor_ = NULL; + state_ = kError; return; } diff --git a/media/filters/decrypting_audio_decoder.h b/media/filters/decrypting_audio_decoder.h index 6d1df7c3c936..70cbc5632268 100644 --- a/media/filters/decrypting_audio_decoder.h +++ b/media/filters/decrypting_audio_decoder.h @@ -49,7 +49,6 @@ class MEDIA_EXPORT DecryptingAudioDecoder : public AudioDecoder { virtual void Decode(const scoped_refptr& buffer, const DecodeCB& decode_cb) OVERRIDE; virtual void Reset(const base::Closure& closure) OVERRIDE; - virtual void Stop() OVERRIDE; private: // For a detailed state diagram please see this link: http://goo.gl/8jAok @@ -64,7 +63,7 @@ class MEDIA_EXPORT DecryptingAudioDecoder : public AudioDecoder { kPendingDecode, kWaitingForKey, kDecodeFinished, - kStopped, + kError }; // Callback for DecryptorHost::RequestDecryptor(). @@ -101,7 +100,6 @@ class MEDIA_EXPORT DecryptingAudioDecoder : public AudioDecoder { OutputCB output_cb_; DecodeCB decode_cb_; base::Closure reset_cb_; - base::Closure stop_cb_; // The current decoder configuration. AudioDecoderConfig config_; diff --git a/media/filters/decrypting_audio_decoder_unittest.cc b/media/filters/decrypting_audio_decoder_unittest.cc index 8f187e1ae75a..83d7f5f36bdc 100644 --- a/media/filters/decrypting_audio_decoder_unittest.cc +++ b/media/filters/decrypting_audio_decoder_unittest.cc @@ -79,7 +79,7 @@ class DecryptingAudioDecoderTest : public testing::Test { virtual ~DecryptingAudioDecoderTest() { EXPECT_CALL(*this, RequestDecryptorNotification(_)) .Times(testing::AnyNumber()); - Stop(); + Destroy(); } void InitializeAndExpectStatus(const AudioDecoderConfig& config, @@ -234,12 +234,12 @@ class DecryptingAudioDecoderTest : public testing::Test { message_loop_.RunUntilIdle(); } - void Stop() { + void Destroy() { EXPECT_CALL(*decryptor_, DeinitializeDecoder(Decryptor::kAudio)) .WillRepeatedly(InvokeWithoutArgs( this, &DecryptingAudioDecoderTest::AbortAllPendingCBs)); - decoder_->Stop(); + decoder_.reset(); message_loop_.RunUntilIdle(); } diff --git a/media/filters/decrypting_video_decoder.cc b/media/filters/decrypting_video_decoder.cc index eb40625f8a37..2651c5da7404 100644 --- a/media/filters/decrypting_video_decoder.cc +++ b/media/filters/decrypting_video_decoder.cc @@ -125,18 +125,12 @@ void DecryptingVideoDecoder::Reset(const base::Closure& closure) { DoReset(); } -void DecryptingVideoDecoder::Stop() { +DecryptingVideoDecoder::~DecryptingVideoDecoder() { DCHECK(task_runner_->BelongsToCurrentThread()); - DVLOG(2) << "Stop() - state: " << state_; - // Invalidate all weak pointers so that pending callbacks won't be fired into - // this object. - weak_factory_.InvalidateWeakPtrs(); + if (state_ == kUninitialized) + return; - // At this point the render thread is likely paused (in WebMediaPlayerImpl's - // Destroy()), so running |closure| can't wait for anything that requires the - // render thread to be processing messages to complete (such as PPAPI - // callbacks). if (decryptor_) { decryptor_->DeinitializeDecoder(Decryptor::kVideo); decryptor_ = NULL; @@ -150,12 +144,6 @@ void DecryptingVideoDecoder::Stop() { base::ResetAndReturn(&decode_cb_).Run(kAborted); if (!reset_cb_.is_null()) base::ResetAndReturn(&reset_cb_).Run(); - - state_ = kStopped; -} - -DecryptingVideoDecoder::~DecryptingVideoDecoder() { - DCHECK(state_ == kUninitialized || state_ == kStopped) << state_; } void DecryptingVideoDecoder::SetDecryptor(Decryptor* decryptor) { @@ -168,7 +156,7 @@ void DecryptingVideoDecoder::SetDecryptor(Decryptor* decryptor) { if (!decryptor) { base::ResetAndReturn(&init_cb_).Run(DECODER_ERROR_NOT_SUPPORTED); - state_ = kStopped; + state_ = kError; return; } @@ -191,7 +179,8 @@ void DecryptingVideoDecoder::FinishInitialization(bool success) { if (!success) { base::ResetAndReturn(&init_cb_).Run(DECODER_ERROR_NOT_SUPPORTED); - state_ = kStopped; + decryptor_ = NULL; + state_ = kError; return; } diff --git a/media/filters/decrypting_video_decoder.h b/media/filters/decrypting_video_decoder.h index ac4caf869511..a1f43edfeac2 100644 --- a/media/filters/decrypting_video_decoder.h +++ b/media/filters/decrypting_video_decoder.h @@ -39,7 +39,6 @@ class MEDIA_EXPORT DecryptingVideoDecoder : public VideoDecoder { virtual void Decode(const scoped_refptr& buffer, const DecodeCB& decode_cb) OVERRIDE; virtual void Reset(const base::Closure& closure) OVERRIDE; - virtual void Stop() OVERRIDE; private: // For a detailed state diagram please see this link: http://goo.gl/8jAok @@ -53,7 +52,6 @@ class MEDIA_EXPORT DecryptingVideoDecoder : public VideoDecoder { kPendingDecode, kWaitingForKey, kDecodeFinished, - kStopped, kError }; @@ -77,9 +75,6 @@ class MEDIA_EXPORT DecryptingVideoDecoder : public VideoDecoder { // Reset decoder and call |reset_cb_|. void DoReset(); - // Free decoder resources and call |stop_cb_|. - void DoStop(); - scoped_refptr task_runner_; State state_; diff --git a/media/filters/decrypting_video_decoder_unittest.cc b/media/filters/decrypting_video_decoder_unittest.cc index 738753b518bf..ed956ea1d51e 100644 --- a/media/filters/decrypting_video_decoder_unittest.cc +++ b/media/filters/decrypting_video_decoder_unittest.cc @@ -75,7 +75,7 @@ class DecryptingVideoDecoderTest : public testing::Test { } virtual ~DecryptingVideoDecoderTest() { - Stop(); + Destroy(); } // Initializes the |decoder_| and expects |status|. Note the initialization @@ -209,12 +209,12 @@ class DecryptingVideoDecoderTest : public testing::Test { message_loop_.RunUntilIdle(); } - void Stop() { + void Destroy() { EXPECT_CALL(*decryptor_, DeinitializeDecoder(Decryptor::kVideo)) .WillRepeatedly(InvokeWithoutArgs( this, &DecryptingVideoDecoderTest::AbortAllPendingCBs)); - decoder_->Stop(); + decoder_.reset(); message_loop_.RunUntilIdle(); } @@ -399,8 +399,8 @@ TEST_F(DecryptingVideoDecoderTest, Reset_AfterReset) { Reset(); } -// Test stopping when the decoder is in kDecryptorRequested state. -TEST_F(DecryptingVideoDecoderTest, Stop_DuringDecryptorRequested) { +// Test destruction when the decoder is in kDecryptorRequested state. +TEST_F(DecryptingVideoDecoderTest, Destroy_DuringDecryptorRequested) { DecryptorReadyCB decryptor_ready_cb; EXPECT_CALL(*this, RequestDecryptorNotification(_)) .WillOnce(SaveArg<0>(&decryptor_ready_cb)); @@ -413,16 +413,16 @@ TEST_F(DecryptingVideoDecoderTest, Stop_DuringDecryptorRequested) { // |decryptor_ready_cb| is saved but not called here. EXPECT_FALSE(decryptor_ready_cb.is_null()); - // During stop, RequestDecryptorNotification() should be called with a NULL - // callback to cancel the |decryptor_ready_cb|. + // During destruction, RequestDecryptorNotification() should be called with a + // NULL callback to cancel the |decryptor_ready_cb|. EXPECT_CALL(*this, RequestDecryptorNotification(IsNullCallback())) .WillOnce(ResetAndRunCallback(&decryptor_ready_cb, reinterpret_cast(NULL))); - Stop(); + Destroy(); } -// Test stopping when the decoder is in kPendingDecoderInit state. -TEST_F(DecryptingVideoDecoderTest, Stop_DuringPendingDecoderInit) { +// Test destruction when the decoder is in kPendingDecoderInit state. +TEST_F(DecryptingVideoDecoderTest, Destroy_DuringPendingDecoderInit) { EXPECT_CALL(*decryptor_, InitializeVideoDecoder(_, _)) .WillOnce(SaveArg<1>(&pending_init_cb_)); @@ -430,57 +430,57 @@ TEST_F(DecryptingVideoDecoderTest, Stop_DuringPendingDecoderInit) { DECODER_ERROR_NOT_SUPPORTED); EXPECT_FALSE(pending_init_cb_.is_null()); - Stop(); + Destroy(); } -// Test stopping when the decoder is in kIdle state but has not decoded any +// Test destruction when the decoder is in kIdle state but has not decoded any // frame. -TEST_F(DecryptingVideoDecoderTest, Stop_DuringIdleAfterInitialization) { +TEST_F(DecryptingVideoDecoderTest, Destroy_DuringIdleAfterInitialization) { Initialize(); - Stop(); + Destroy(); } -// Test stopping when the decoder is in kIdle state after it has decoded one +// Test destruction when the decoder is in kIdle state after it has decoded one // frame. -TEST_F(DecryptingVideoDecoderTest, Stop_DuringIdleAfterDecodedOneFrame) { +TEST_F(DecryptingVideoDecoderTest, Destroy_DuringIdleAfterDecodedOneFrame) { Initialize(); EnterNormalDecodingState(); - Stop(); + Destroy(); } -// Test stopping when the decoder is in kPendingDecode state. -TEST_F(DecryptingVideoDecoderTest, Stop_DuringPendingDecode) { +// Test destruction when the decoder is in kPendingDecode state. +TEST_F(DecryptingVideoDecoderTest, Destroy_DuringPendingDecode) { Initialize(); EnterPendingDecodeState(); EXPECT_CALL(*this, DecodeDone(VideoDecoder::kAborted)); - Stop(); + Destroy(); } -// Test stopping when the decoder is in kWaitingForKey state. -TEST_F(DecryptingVideoDecoderTest, Stop_DuringWaitingForKey) { +// Test destruction when the decoder is in kWaitingForKey state. +TEST_F(DecryptingVideoDecoderTest, Destroy_DuringWaitingForKey) { Initialize(); EnterWaitingForKeyState(); EXPECT_CALL(*this, DecodeDone(VideoDecoder::kAborted)); - Stop(); + Destroy(); } -// Test stopping when the decoder has hit end of stream and is in +// Test destruction when the decoder has hit end of stream and is in // kDecodeFinished state. -TEST_F(DecryptingVideoDecoderTest, Stop_AfterDecodeFinished) { +TEST_F(DecryptingVideoDecoderTest, Destroy_AfterDecodeFinished) { Initialize(); EnterNormalDecodingState(); EnterEndOfStreamState(); - Stop(); + Destroy(); } -// Test stopping when there is a pending reset on the decoder. +// Test destruction when there is a pending reset on the decoder. // Reset is pending because it cannot complete when the video decode callback // is pending. -TEST_F(DecryptingVideoDecoderTest, Stop_DuringPendingReset) { +TEST_F(DecryptingVideoDecoderTest, Destroy_DuringPendingReset) { Initialize(); EnterPendingDecodeState(); @@ -488,23 +488,15 @@ TEST_F(DecryptingVideoDecoderTest, Stop_DuringPendingReset) { EXPECT_CALL(*this, DecodeDone(VideoDecoder::kAborted)); decoder_->Reset(NewExpectedClosure()); - Stop(); + Destroy(); } -// Test stopping after the decoder has been reset. -TEST_F(DecryptingVideoDecoderTest, Stop_AfterReset) { +// Test destruction after the decoder has been reset. +TEST_F(DecryptingVideoDecoderTest, Destroy_AfterReset) { Initialize(); EnterNormalDecodingState(); Reset(); - Stop(); -} - -// Test stopping after the decoder has been stopped. -TEST_F(DecryptingVideoDecoderTest, Stop_AfterStop) { - Initialize(); - EnterNormalDecodingState(); - Stop(); - Stop(); + Destroy(); } } // namespace media diff --git a/media/filters/fake_video_decoder.cc b/media/filters/fake_video_decoder.cc index 1df718227a57..05dc410f870b 100644 --- a/media/filters/fake_video_decoder.cc +++ b/media/filters/fake_video_decoder.cc @@ -25,7 +25,19 @@ FakeVideoDecoder::FakeVideoDecoder(int decoding_delay, } FakeVideoDecoder::~FakeVideoDecoder() { - DCHECK_EQ(state_, STATE_UNINITIALIZED); + DCHECK(thread_checker_.CalledOnValidThread()); + + if (state_ == STATE_UNINITIALIZED) + return; + + if (!init_cb_.IsNull()) + SatisfyInit(); + if (!held_decode_callbacks_.empty()) + SatisfyDecode(); + if (!reset_cb_.IsNull()) + SatisfyReset(); + + decoded_frames_.clear(); } void FakeVideoDecoder::Initialize(const VideoDecoderConfig& config, @@ -64,10 +76,10 @@ void FakeVideoDecoder::Decode(const scoped_refptr& buffer, max_parallel_decoding_requests_); int buffer_size = buffer->end_of_stream() ? 0 : buffer->data_size(); - DecodeCB wrapped_decode_cb = - BindToCurrentLoop(base::Bind(&FakeVideoDecoder::OnFrameDecoded, - weak_factory_.GetWeakPtr(), - buffer_size, decode_cb)); + DecodeCB wrapped_decode_cb = base::Bind(&FakeVideoDecoder::OnFrameDecoded, + weak_factory_.GetWeakPtr(), + buffer_size, + BindToCurrentLoop(decode_cb)); if (state_ == STATE_ERROR) { wrapped_decode_cb.Run(kDecodeError); @@ -100,20 +112,6 @@ void FakeVideoDecoder::Reset(const base::Closure& closure) { DoReset(); } -void FakeVideoDecoder::Stop() { - DCHECK(thread_checker_.CalledOnValidThread()); - - if (!init_cb_.IsNull()) - SatisfyInit(); - if (!held_decode_callbacks_.empty()) - SatisfyDecode(); - if (!reset_cb_.IsNull()) - SatisfyReset(); - - decoded_frames_.clear(); - state_ = STATE_UNINITIALIZED; -} - void FakeVideoDecoder::HoldNextInit() { DCHECK(thread_checker_.CalledOnValidThread()); init_cb_.HoldCallback(); diff --git a/media/filters/fake_video_decoder.h b/media/filters/fake_video_decoder.h index 21cb2a1f7962..5e476d8faeb8 100644 --- a/media/filters/fake_video_decoder.h +++ b/media/filters/fake_video_decoder.h @@ -43,7 +43,6 @@ class FakeVideoDecoder : public VideoDecoder { virtual void Decode(const scoped_refptr& buffer, const DecodeCB& decode_cb) OVERRIDE; virtual void Reset(const base::Closure& closure) OVERRIDE; - virtual void Stop() OVERRIDE; virtual int GetMaxDecodeRequests() const OVERRIDE; // Holds the next init/decode/reset callback from firing. diff --git a/media/filters/fake_video_decoder_unittest.cc b/media/filters/fake_video_decoder_unittest.cc index 2772b54ffbfc..3598a7a48eda 100644 --- a/media/filters/fake_video_decoder_unittest.cc +++ b/media/filters/fake_video_decoder_unittest.cc @@ -39,7 +39,7 @@ class FakeVideoDecoderTest is_reset_pending_(false) {} virtual ~FakeVideoDecoderTest() { - Stop(); + Destroy(); } void InitializeWithConfig(const VideoDecoderConfig& config) { @@ -197,8 +197,8 @@ class FakeVideoDecoderTest ExpectResetResult(OK); } - void Stop() { - decoder_->Stop(); + void Destroy() { + decoder_.reset(); message_loop_.RunUntilIdle(); // All pending callbacks must have been fired. @@ -365,35 +365,35 @@ TEST_P(FakeVideoDecoderTest, Reset_PendingDuringPendingRead) { SatisfyReset(); } -TEST_P(FakeVideoDecoderTest, Stop) { +TEST_P(FakeVideoDecoderTest, Destroy) { Initialize(); ReadOneFrame(); ExpectReadResult(OK); - Stop(); + Destroy(); } -TEST_P(FakeVideoDecoderTest, Stop_DuringPendingInitialization) { +TEST_P(FakeVideoDecoderTest, Destroy_DuringPendingInitialization) { EnterPendingInitState(); - Stop(); + Destroy(); } -TEST_P(FakeVideoDecoderTest, Stop_DuringPendingRead) { +TEST_P(FakeVideoDecoderTest, Destroy_DuringPendingRead) { Initialize(); EnterPendingReadState(); - Stop(); + Destroy(); } -TEST_P(FakeVideoDecoderTest, Stop_DuringPendingReset) { +TEST_P(FakeVideoDecoderTest, Destroy_DuringPendingReset) { Initialize(); EnterPendingResetState(); - Stop(); + Destroy(); } -TEST_P(FakeVideoDecoderTest, Stop_DuringPendingReadAndPendingReset) { +TEST_P(FakeVideoDecoderTest, Destroy_DuringPendingReadAndPendingReset) { Initialize(); EnterPendingReadState(); EnterPendingResetState(); - Stop(); + Destroy(); } } // namespace media diff --git a/media/filters/ffmpeg_audio_decoder.cc b/media/filters/ffmpeg_audio_decoder.cc index 28347acb52e6..b45b9401b56d 100644 --- a/media/filters/ffmpeg_audio_decoder.cc +++ b/media/filters/ffmpeg_audio_decoder.cc @@ -135,9 +135,12 @@ FFmpegAudioDecoder::FFmpegAudioDecoder( } FFmpegAudioDecoder::~FFmpegAudioDecoder() { - DCHECK_EQ(state_, kUninitialized); - DCHECK(!codec_context_); - DCHECK(!av_frame_); + DCHECK(task_runner_->BelongsToCurrentThread()); + + if (state_ != kUninitialized) { + ReleaseFFmpegResources(); + ResetTimestampState(); + } } void FFmpegAudioDecoder::Initialize(const AudioDecoderConfig& config, @@ -192,17 +195,6 @@ void FFmpegAudioDecoder::Reset(const base::Closure& closure) { task_runner_->PostTask(FROM_HERE, closure); } -void FFmpegAudioDecoder::Stop() { - DCHECK(task_runner_->BelongsToCurrentThread()); - - if (state_ == kUninitialized) - return; - - ReleaseFFmpegResources(); - ResetTimestampState(); - state_ = kUninitialized; -} - void FFmpegAudioDecoder::DecodeBuffer( const scoped_refptr& buffer, const DecodeCB& decode_cb) { diff --git a/media/filters/ffmpeg_audio_decoder.h b/media/filters/ffmpeg_audio_decoder.h index 39a408973dcd..680128c3fdb4 100644 --- a/media/filters/ffmpeg_audio_decoder.h +++ b/media/filters/ffmpeg_audio_decoder.h @@ -42,7 +42,6 @@ class MEDIA_EXPORT FFmpegAudioDecoder : public AudioDecoder { virtual void Decode(const scoped_refptr& buffer, const DecodeCB& decode_cb) OVERRIDE; virtual void Reset(const base::Closure& closure) OVERRIDE; - virtual void Stop() OVERRIDE; private: // There are four states the decoder can be in: diff --git a/media/filters/ffmpeg_video_decoder.cc b/media/filters/ffmpeg_video_decoder.cc index 3436aa9955dd..b7da7f7a462d 100644 --- a/media/filters/ffmpeg_video_decoder.cc +++ b/media/filters/ffmpeg_video_decoder.cc @@ -241,20 +241,11 @@ void FFmpegVideoDecoder::Reset(const base::Closure& closure) { task_runner_->PostTask(FROM_HERE, closure); } -void FFmpegVideoDecoder::Stop() { +FFmpegVideoDecoder::~FFmpegVideoDecoder() { DCHECK(task_runner_->BelongsToCurrentThread()); - if (state_ == kUninitialized) - return; - - ReleaseFFmpegResources(); - state_ = kUninitialized; -} - -FFmpegVideoDecoder::~FFmpegVideoDecoder() { - DCHECK_EQ(kUninitialized, state_); - DCHECK(!codec_context_); - DCHECK(!av_frame_); + if (state_ != kUninitialized) + ReleaseFFmpegResources(); } bool FFmpegVideoDecoder::FFmpegDecode( diff --git a/media/filters/ffmpeg_video_decoder.h b/media/filters/ffmpeg_video_decoder.h index d7b35f1d37b6..529d69e70b79 100644 --- a/media/filters/ffmpeg_video_decoder.h +++ b/media/filters/ffmpeg_video_decoder.h @@ -43,7 +43,6 @@ class MEDIA_EXPORT FFmpegVideoDecoder : public VideoDecoder { virtual void Decode(const scoped_refptr& buffer, const DecodeCB& decode_cb) OVERRIDE; virtual void Reset(const base::Closure& closure) OVERRIDE; - virtual void Stop() OVERRIDE; // Callback called from within FFmpeg to allocate a buffer based on // the dimensions of |codec_context|. See AVCodecContext.get_buffer2 diff --git a/media/filters/ffmpeg_video_decoder_unittest.cc b/media/filters/ffmpeg_video_decoder_unittest.cc index 9ccdbc2865f9..c3fde2700bdf 100644 --- a/media/filters/ffmpeg_video_decoder_unittest.cc +++ b/media/filters/ffmpeg_video_decoder_unittest.cc @@ -61,7 +61,7 @@ class FFmpegVideoDecoderTest : public testing::Test { } virtual ~FFmpegVideoDecoderTest() { - Stop(); + Destroy(); } void Initialize() { @@ -89,8 +89,8 @@ class FFmpegVideoDecoderTest : public testing::Test { message_loop_.RunUntilIdle(); } - void Stop() { - decoder_->Stop(); + void Destroy() { + decoder_.reset(); message_loop_.RunUntilIdle(); } @@ -421,25 +421,25 @@ TEST_F(FFmpegVideoDecoderTest, Reset_EndOfStream) { Reset(); } -// Test stopping when decoder has initialized but not decoded. -TEST_F(FFmpegVideoDecoderTest, Stop_Initialized) { +// Test destruction when decoder has initialized but not decoded. +TEST_F(FFmpegVideoDecoderTest, Destroy_Initialized) { Initialize(); - Stop(); + Destroy(); } -// Test stopping when decoder has decoded single frame. -TEST_F(FFmpegVideoDecoderTest, Stop_Decoding) { +// Test destruction when decoder has decoded single frame. +TEST_F(FFmpegVideoDecoderTest, Destroy_Decoding) { Initialize(); EnterDecodingState(); - Stop(); + Destroy(); } -// Test stopping when decoder has hit end of stream. -TEST_F(FFmpegVideoDecoderTest, Stop_EndOfStream) { +// Test destruction when decoder has hit end of stream. +TEST_F(FFmpegVideoDecoderTest, Destroy_EndOfStream) { Initialize(); EnterDecodingState(); EnterEndOfStreamState(); - Stop(); + Destroy(); } } // namespace media diff --git a/media/filters/gpu_video_decoder.cc b/media/filters/gpu_video_decoder.cc index bc545b7d652c..efabc95eca0b 100644 --- a/media/filters/gpu_video_decoder.cc +++ b/media/filters/gpu_video_decoder.cc @@ -98,15 +98,6 @@ void GpuVideoDecoder::Reset(const base::Closure& closure) { vda_->Reset(); } -void GpuVideoDecoder::Stop() { - DCheckGpuVideoAcceleratorFactoriesTaskRunnerIsCurrent(); - if (vda_) - DestroyVDA(); - DCHECK(bitstream_buffers_in_decoder_.empty()); - if (!pending_reset_cb_.is_null()) - base::ResetAndReturn(&pending_reset_cb_).Run(); -} - static bool IsCodedSizeSupported(const gfx::Size& coded_size) { #if defined(OS_WIN) // Windows Media Foundation H.264 decoding does not support decoding videos @@ -560,14 +551,20 @@ void GpuVideoDecoder::NotifyEndOfBitstreamBuffer(int32 id) { GpuVideoDecoder::~GpuVideoDecoder() { DCheckGpuVideoAcceleratorFactoriesTaskRunnerIsCurrent(); - // Stop should have been already called. - DCHECK(!vda_.get() && assigned_picture_buffers_.empty()); + if (vda_) + DestroyVDA(); DCHECK(bitstream_buffers_in_decoder_.empty()); + DCHECK(assigned_picture_buffers_.empty()); + + if (!pending_reset_cb_.is_null()) + base::ResetAndReturn(&pending_reset_cb_).Run(); + for (size_t i = 0; i < available_shm_segments_.size(); ++i) { available_shm_segments_[i]->shm->Close(); delete available_shm_segments_[i]; } available_shm_segments_.clear(); + for (std::map::iterator it = bitstream_buffers_in_decoder_.begin(); it != bitstream_buffers_in_decoder_.end(); ++it) { diff --git a/media/filters/gpu_video_decoder.h b/media/filters/gpu_video_decoder.h index e15200830b91..bae5566d1e30 100644 --- a/media/filters/gpu_video_decoder.h +++ b/media/filters/gpu_video_decoder.h @@ -49,7 +49,6 @@ class MEDIA_EXPORT GpuVideoDecoder virtual void Decode(const scoped_refptr& buffer, const DecodeCB& decode_cb) OVERRIDE; virtual void Reset(const base::Closure& closure) OVERRIDE; - virtual void Stop() OVERRIDE; virtual bool NeedsBitstreamConversion() const OVERRIDE; virtual bool CanReadWithoutStalling() const OVERRIDE; virtual int GetMaxDecodeRequests() const OVERRIDE; diff --git a/media/filters/opus_audio_decoder.cc b/media/filters/opus_audio_decoder.cc index 84b3ac286fef..62784ddf7103 100644 --- a/media/filters/opus_audio_decoder.cc +++ b/media/filters/opus_audio_decoder.cc @@ -282,7 +282,7 @@ void OpusAudioDecoder::Reset(const base::Closure& closure) { task_runner_->PostTask(FROM_HERE, closure); } -void OpusAudioDecoder::Stop() { +OpusAudioDecoder::~OpusAudioDecoder() { DCHECK(task_runner_->BelongsToCurrentThread()); if (!opus_decoder_) @@ -293,10 +293,6 @@ void OpusAudioDecoder::Stop() { CloseDecoder(); } -OpusAudioDecoder::~OpusAudioDecoder() { - DCHECK(!opus_decoder_); -} - void OpusAudioDecoder::DecodeBuffer( const scoped_refptr& input, const DecodeCB& decode_cb) { diff --git a/media/filters/opus_audio_decoder.h b/media/filters/opus_audio_decoder.h index 504701a52f88..19ef04d0dbcf 100644 --- a/media/filters/opus_audio_decoder.h +++ b/media/filters/opus_audio_decoder.h @@ -37,7 +37,6 @@ class MEDIA_EXPORT OpusAudioDecoder : public AudioDecoder { virtual void Decode(const scoped_refptr& buffer, const DecodeCB& decode_cb) OVERRIDE; virtual void Reset(const base::Closure& closure) OVERRIDE; - virtual void Stop() OVERRIDE; private: // Reads from the demuxer stream with corresponding callback method. diff --git a/media/filters/video_decoder_selector_unittest.cc b/media/filters/video_decoder_selector_unittest.cc index 62b1a421456b..7a9580e6f0c3 100644 --- a/media/filters/video_decoder_selector_unittest.cc +++ b/media/filters/video_decoder_selector_unittest.cc @@ -45,9 +45,6 @@ class VideoDecoderSelectorTest : public ::testing::Test { } ~VideoDecoderSelectorTest() { - if (selected_decoder_) - selected_decoder_->Stop(); - message_loop_.RunUntilIdle(); } @@ -149,11 +146,6 @@ class VideoDecoderSelectorTest : public ::testing::Test { DISALLOW_COPY_AND_ASSIGN(VideoDecoderSelectorTest); }; -// Note: -// In all the tests, Stop() is expected to be called on a decoder if a decoder: -// - is pending initialization and DecoderSelector::Abort() is called, or -// - has been successfully initialized. - // The stream is not encrypted but we have no clear decoder. No decoder can be // selected. TEST_F(VideoDecoderSelectorTest, ClearStream_NoDecryptor_NoClearDecoder) { @@ -174,7 +166,6 @@ TEST_F(VideoDecoderSelectorTest, ClearStream_NoDecryptor_OneClearDecoder) { EXPECT_CALL(*decoder_1_, Initialize(_, _, _, _)) .WillOnce(RunCallback<2>(PIPELINE_OK)); EXPECT_CALL(*this, OnDecoderSelected(decoder_1_, IsNull())); - EXPECT_CALL(*decoder_1_, Stop()); SelectDecoder(); } @@ -185,7 +176,6 @@ TEST_F(VideoDecoderSelectorTest, InitializeDecoderSelector(kNoDecryptor, 1); EXPECT_CALL(*decoder_1_, Initialize(_, _, _, _)); - EXPECT_CALL(*decoder_1_, Stop()); SelectDecoderAndAbort(); } @@ -201,7 +191,6 @@ TEST_F(VideoDecoderSelectorTest, ClearStream_NoDecryptor_MultipleClearDecoder) { EXPECT_CALL(*decoder_2_, Initialize(_, _, _, _)) .WillOnce(RunCallback<2>(PIPELINE_OK)); EXPECT_CALL(*this, OnDecoderSelected(decoder_2_, IsNull())); - EXPECT_CALL(*decoder_2_, Stop()); SelectDecoder(); } @@ -214,7 +203,6 @@ TEST_F(VideoDecoderSelectorTest, EXPECT_CALL(*decoder_1_, Initialize(_, _, _, _)) .WillOnce(RunCallback<2>(DECODER_ERROR_NOT_SUPPORTED)); EXPECT_CALL(*decoder_2_, Initialize(_, _, _, _)); - EXPECT_CALL(*decoder_2_, Stop()); SelectDecoderAndAbort(); } @@ -228,7 +216,6 @@ TEST_F(VideoDecoderSelectorTest, ClearStream_HasDecryptor) { EXPECT_CALL(*decoder_1_, Initialize(_, _, _, _)) .WillOnce(RunCallback<2>(PIPELINE_OK)); EXPECT_CALL(*this, OnDecoderSelected(decoder_1_, IsNull())); - EXPECT_CALL(*decoder_1_, Stop()); SelectDecoder(); } @@ -238,7 +225,6 @@ TEST_F(VideoDecoderSelectorTest, Abort_ClearStream_HasDecryptor) { InitializeDecoderSelector(kDecryptOnly, 1); EXPECT_CALL(*decoder_1_, Initialize(_, _, _, _)); - EXPECT_CALL(*decoder_1_, Stop()); SelectDecoderAndAbort(); } @@ -281,7 +267,6 @@ TEST_F(VideoDecoderSelectorTest, EncryptedStream_DecryptOnly_OneClearDecoder) { EXPECT_CALL(*decoder_1_, Initialize(_, _, _, _)) .WillOnce(RunCallback<2>(PIPELINE_OK)); EXPECT_CALL(*this, OnDecoderSelected(decoder_1_, NotNull())); - EXPECT_CALL(*decoder_1_, Stop()); SelectDecoder(); } @@ -292,7 +277,6 @@ TEST_F(VideoDecoderSelectorTest, InitializeDecoderSelector(kDecryptOnly, 1); EXPECT_CALL(*decoder_1_, Initialize(_, _, _, _)); - EXPECT_CALL(*decoder_1_, Stop()); SelectDecoderAndAbort(); } @@ -310,7 +294,6 @@ TEST_F(VideoDecoderSelectorTest, EXPECT_CALL(*decoder_2_, Initialize(_, _, _, _)) .WillOnce(RunCallback<2>(PIPELINE_OK)); EXPECT_CALL(*this, OnDecoderSelected(decoder_2_, NotNull())); - EXPECT_CALL(*decoder_2_, Stop()); SelectDecoder(); } @@ -323,7 +306,6 @@ TEST_F(VideoDecoderSelectorTest, EXPECT_CALL(*decoder_1_, Initialize(_, _, _, _)) .WillOnce(RunCallback<2>(DECODER_ERROR_NOT_SUPPORTED)); EXPECT_CALL(*decoder_2_, Initialize(_, _, _, _)); - EXPECT_CALL(*decoder_2_, Stop()); SelectDecoderAndAbort(); } diff --git a/media/filters/video_renderer_impl_unittest.cc b/media/filters/video_renderer_impl_unittest.cc index 56898926cc63..9d48ee42d046 100644 --- a/media/filters/video_renderer_impl_unittest.cc +++ b/media/filters/video_renderer_impl_unittest.cc @@ -69,8 +69,6 @@ class VideoRendererImplTest : public ::testing::Test { EXPECT_CALL(demuxer_stream_, Read(_)).WillRepeatedly( RunCallback<0>(DemuxerStream::kOk, scoped_refptr(new DecoderBuffer(0)))); - EXPECT_CALL(*decoder_, Stop()) - .WillRepeatedly(Invoke(this, &VideoRendererImplTest::StopRequested)); EXPECT_CALL(statistics_cb_object_, OnStatistics(_)) .Times(AnyNumber()); EXPECT_CALL(*this, OnTimeUpdate(_)) @@ -320,15 +318,6 @@ class VideoRendererImplTest : public ::testing::Test { message_loop_.PostTask(FROM_HERE, callback); } - void StopRequested() { - DCHECK_EQ(&message_loop_, base::MessageLoop::current()); - decode_results_.clear(); - if (!decode_cb_.is_null()) { - QueueFrames("abort"); - SatisfyPendingRead(); - } - } - base::MessageLoop message_loop_; // Used to protect |time_|. diff --git a/media/filters/vpx_video_decoder.cc b/media/filters/vpx_video_decoder.cc index 20416f148077..9cadbca36c33 100644 --- a/media/filters/vpx_video_decoder.cc +++ b/media/filters/vpx_video_decoder.cc @@ -209,7 +209,7 @@ VpxVideoDecoder::VpxVideoDecoder( vpx_codec_alpha_(NULL) {} VpxVideoDecoder::~VpxVideoDecoder() { - DCHECK_EQ(kUninitialized, state_); + DCHECK(task_runner_->BelongsToCurrentThread()); CloseDecoder(); } @@ -338,12 +338,6 @@ void VpxVideoDecoder::Reset(const base::Closure& closure) { task_runner_->PostTask(FROM_HERE, closure); } -void VpxVideoDecoder::Stop() { - DCHECK(task_runner_->BelongsToCurrentThread()); - - state_ = kUninitialized; -} - void VpxVideoDecoder::DecodeBuffer(const scoped_refptr& buffer) { DCHECK(task_runner_->BelongsToCurrentThread()); DCHECK_NE(state_, kUninitialized); diff --git a/media/filters/vpx_video_decoder.h b/media/filters/vpx_video_decoder.h index 22d119bea7e6..0e1a941632fa 100644 --- a/media/filters/vpx_video_decoder.h +++ b/media/filters/vpx_video_decoder.h @@ -39,7 +39,6 @@ class MEDIA_EXPORT VpxVideoDecoder : public VideoDecoder { virtual void Decode(const scoped_refptr& buffer, const DecodeCB& decode_cb) OVERRIDE; virtual void Reset(const base::Closure& closure) OVERRIDE; - virtual void Stop() OVERRIDE; private: enum DecoderState { -- 2.11.4.GIT