diff options
author | kinaba@chromium.org <kinaba@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-07-10 06:58:16 +0000 |
---|---|---|
committer | kinaba@chromium.org <kinaba@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-07-10 06:58:16 +0000 |
commit | 3bd47372646b389eec3d4a90ba47b3a7be24f2b1 (patch) | |
tree | dfe06182fc9b69c057ec7a629e2bcfb3ae1fa158 /media | |
parent | 90ecddb83af2b46b0322a2c1db2a484f46b28a10 (diff) | |
download | chromium_src-3bd47372646b389eec3d4a90ba47b3a7be24f2b1.zip chromium_src-3bd47372646b389eec3d4a90ba47b3a7be24f2b1.tar.gz chromium_src-3bd47372646b389eec3d4a90ba47b3a7be24f2b1.tar.bz2 |
Revert 210741 "Separate DemuxerStream and VideoDecoder."
The following test became flaky after this CL.
> [ RUN ] VideoRendererBaseTest.DecodeError_Playing
> [20332:20336:0709/234037:1674250:FATAL:video_frame_stream.cc(376)] Check failed: read_cb_.is_null().
> Separate DemuxerStream and VideoDecoder.
>
> - Change VideoDecoder::Read() to VideoDecoder::Decode.
> - VideoFrameStream handles DemuxerStream::Read() and VideoDecoder::Decode.
>
> BUG=141788
> TEST=none
>
> Review URL: https://chromiumcodereview.appspot.com/16274005
TBR=xhwang@chromium.org
Review URL: https://codereview.chromium.org/18305007
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@210765 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'media')
26 files changed, 1010 insertions, 694 deletions
diff --git a/media/base/mock_filters.h b/media/base/mock_filters.h index e335477..5522681 100644 --- a/media/base/mock_filters.h +++ b/media/base/mock_filters.h @@ -44,7 +44,7 @@ class MockDemuxer : public Demuxer { class MockDemuxerStream : public DemuxerStream { public: - explicit MockDemuxerStream(DemuxerStream::Type type); + MockDemuxerStream(DemuxerStream::Type type); virtual ~MockDemuxerStream(); // DemuxerStream implementation. @@ -71,11 +71,10 @@ class MockVideoDecoder : public VideoDecoder { virtual ~MockVideoDecoder(); // VideoDecoder implementation. - MOCK_METHOD3(Initialize, void(const VideoDecoderConfig& config, + MOCK_METHOD3(Initialize, void(DemuxerStream*, const PipelineStatusCB&, const StatisticsCB&)); - MOCK_METHOD2(Decode, void(const scoped_refptr<DecoderBuffer>& buffer, - const ReadCB&)); + MOCK_METHOD1(Read, void(const ReadCB&)); MOCK_METHOD1(Reset, void(const base::Closure&)); MOCK_METHOD1(Stop, void(const base::Closure&)); MOCK_CONST_METHOD0(HasAlpha, bool()); diff --git a/media/base/test_helpers.cc b/media/base/test_helpers.cc index 333624b..5f7604b 100644 --- a/media/base/test_helpers.cc +++ b/media/base/test_helpers.cc @@ -7,13 +7,11 @@ #include "base/bind.h" #include "base/logging.h" #include "base/message_loop.h" -#include "base/pickle.h" #include "base/test/test_timeouts.h" #include "base/time/time.h" #include "base/timer/timer.h" #include "media/base/audio_buffer.h" #include "media/base/bind_to_loop.h" -#include "media/base/decoder_buffer.h" #include "ui/gfx/rect.h" using ::testing::_; @@ -244,40 +242,4 @@ DEFINE_INTERLEAVED_INSTANCE(float); DEFINE_PLANAR_INSTANCE(int16); DEFINE_PLANAR_INSTANCE(float); -static const char kFakeVideoBufferHeader[] = "FakeVideoBufferForTest"; - -scoped_refptr<DecoderBuffer> CreateFakeVideoBufferForTest( - const VideoDecoderConfig& config, - base::TimeDelta timestamp, base::TimeDelta duration) { - Pickle pickle; - pickle.WriteString(kFakeVideoBufferHeader); - pickle.WriteInt(config.coded_size().width()); - pickle.WriteInt(config.coded_size().height()); - pickle.WriteInt64(timestamp.InMilliseconds()); - - scoped_refptr<DecoderBuffer> buffer = DecoderBuffer::CopyFrom( - static_cast<const uint8*>(pickle.data()), - static_cast<int>(pickle.size())); - buffer->SetTimestamp(timestamp); - buffer->SetDuration(duration); - - return buffer; -} - -bool VerifyFakeVideoBufferForTest( - const scoped_refptr<DecoderBuffer>& buffer, - const VideoDecoderConfig& config) { - // Check if the input |buffer| matches the |config|. - PickleIterator pickle(Pickle(reinterpret_cast<const char*>(buffer->GetData()), - buffer->GetDataSize())); - std::string header; - int width = 0; - int height = 0; - bool success = pickle.ReadString(&header) && pickle.ReadInt(&width) && - pickle.ReadInt(&height); - return (success && header == kFakeVideoBufferHeader && - width == config.coded_size().width() && - height == config.coded_size().height()); -} - } // namespace media diff --git a/media/base/test_helpers.h b/media/base/test_helpers.h index 872d08d..a7eb8f5 100644 --- a/media/base/test_helpers.h +++ b/media/base/test_helpers.h @@ -21,7 +21,6 @@ class TimeDelta; namespace media { class AudioBuffer; -class DecoderBuffer; // Return a callback that expects to be run once. base::Closure NewExpectedClosure(); @@ -131,18 +130,6 @@ scoped_refptr<AudioBuffer> MakePlanarAudioBuffer( base::TimeDelta start_time, base::TimeDelta duration); -// Create a fake video DecoderBuffer for testing purpose. The buffer contains -// part of video decoder config info embedded so that the testing code can do -// some sanity check. -scoped_refptr<DecoderBuffer> CreateFakeVideoBufferForTest( - const VideoDecoderConfig& config, - base::TimeDelta timestamp, - base::TimeDelta duration); - -// Verify if a fake video DecoderBuffer is valid. -bool VerifyFakeVideoBufferForTest(const scoped_refptr<DecoderBuffer>& buffer, - const VideoDecoderConfig& config); - } // namespace media #endif // MEDIA_BASE_TEST_HELPERS_H_ diff --git a/media/base/video_decoder.h b/media/base/video_decoder.h index 1c45a28..11d8a7f 100644 --- a/media/base/video_decoder.h +++ b/media/base/video_decoder.h @@ -13,16 +13,14 @@ namespace media { -class DecoderBuffer; -class VideoDecoderConfig; +class DemuxerStream; class VideoFrame; class MEDIA_EXPORT VideoDecoder { public: - // Status codes for decode operations on VideoDecoder. + // Status codes for read operations on VideoDecoder. enum Status { kOk, // Everything went as planned. - kNotEnoughData, // Not enough data to produce a video frame. kDecodeError, // Decoding error happened. kDecryptError // Decrypting error happened. }; @@ -30,38 +28,40 @@ class MEDIA_EXPORT VideoDecoder { VideoDecoder(); virtual ~VideoDecoder(); - // Initializes a VideoDecoder with the given |config|, executing the + // Initializes a VideoDecoder with the given DemuxerStream, executing the // |status_cb| upon completion. // |statistics_cb| is used to update the global pipeline statistics. // // 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. + // 2) This method should not be called during any pending read, reset or stop. // 3) No VideoDecoder calls except for Stop() should be made before // |status_cb| is executed. - virtual void Initialize(const VideoDecoderConfig& config, + // 4) DemuxerStream should not be accessed after the VideoDecoder is stopped. + // + // TODO(xhwang): Make all VideoDecoder implementations reinitializable. + // See http://crbug.com/233608 + virtual void Initialize(DemuxerStream* stream, const PipelineStatusCB& status_cb, const StatisticsCB& statistics_cb) = 0; - // Requests a |buffer| to be decoded. The status of the decoder and decoded - // frame are returned via the provided callback. Only one decode may be in - // flight at any given time. + // Requests a frame to be decoded. The status of the decoder and decoded frame + // are returned via the provided callback. Only one read may be in flight at + // any given time. // // Implementations guarantee that the callback will not be called from within // this method. // - // If the returned status is kOk: - // - Non-EOS (end of stream) frame contains decoded video data. - // - EOS frame indicates the end of the stream. - // - NULL frame indicates an aborted decode. This can happen if Reset() or - // Stop() is called during the decoding process. - // Otherwise the returned frame must be NULL. + // If the returned status is not kOk, some error has occurred in the video + // decoder. In this case, the returned frame should always be NULL. // - // TODO(xhwang): Rename this to DecodeCB. + // Otherwise, the video decoder is in good shape. In this case, Non-NULL + // frames contain decoded video data or may indicate the end of the stream. + // NULL video frames indicate an aborted read. This can happen if the + // DemuxerStream gets flushed and doesn't have any more data to return. typedef base::Callback<void(Status, const scoped_refptr<VideoFrame>&)> ReadCB; - virtual void Decode(const scoped_refptr<DecoderBuffer>& buffer, - const ReadCB& read_cb) = 0; + virtual void Read(const ReadCB& read_cb) = 0; // Resets decoder state, fulfilling all pending ReadCB and dropping extra // queued decoded data. After this call, the decoder is back to an initialized diff --git a/media/filters/decrypting_demuxer_stream.h b/media/filters/decrypting_demuxer_stream.h index bd75d66..27ed736 100644 --- a/media/filters/decrypting_demuxer_stream.h +++ b/media/filters/decrypting_demuxer_stream.h @@ -37,6 +37,12 @@ class MEDIA_EXPORT DecryptingDemuxerStream : public DemuxerStream { const PipelineStatusCB& status_cb); void Reset(const base::Closure& closure); + // Creates and initializes either |audio_config_| or |video_config_| based on + // |demuxer_stream_|. + // TODO(xhwang): Make this private after the hack in VideoFrameStream is + // removed. + void InitializeDecoderConfig(); + // DemuxerStream implementation. virtual void Read(const ReadCB& read_cb) OVERRIDE; virtual AudioDecoderConfig audio_decoder_config() OVERRIDE; @@ -81,10 +87,6 @@ class MEDIA_EXPORT DecryptingDemuxerStream : public DemuxerStream { // Returns Decryptor::StreamType converted from |stream_type_|. Decryptor::StreamType GetDecryptorStreamType() const; - // Creates and initializes either |audio_config_| or |video_config_| based on - // |demuxer_stream_|. - void InitializeDecoderConfig(); - scoped_refptr<base::MessageLoopProxy> message_loop_; base::WeakPtrFactory<DecryptingDemuxerStream> weak_factory_; base::WeakPtr<DecryptingDemuxerStream> weak_this_; diff --git a/media/filters/decrypting_video_decoder.cc b/media/filters/decrypting_video_decoder.cc index 72f8d48..3c39e33 100644 --- a/media/filters/decrypting_video_decoder.cc +++ b/media/filters/decrypting_video_decoder.cc @@ -13,6 +13,7 @@ #include "media/base/bind_to_loop.h" #include "media/base/decoder_buffer.h" #include "media/base/decryptor.h" +#include "media/base/demuxer_stream.h" #include "media/base/pipeline.h" #include "media/base/video_decoder_config.h" #include "media/base/video_frame.h" @@ -25,6 +26,7 @@ DecryptingVideoDecoder::DecryptingVideoDecoder( : message_loop_(message_loop), weak_factory_(this), state_(kUninitialized), + demuxer_stream_(NULL), set_decryptor_ready_cb_(set_decryptor_ready_cb), decryptor_(NULL), key_added_while_decode_pending_(false), @@ -32,7 +34,7 @@ DecryptingVideoDecoder::DecryptingVideoDecoder( } void DecryptingVideoDecoder::Initialize( - const VideoDecoderConfig& config, + DemuxerStream* stream, const PipelineStatusCB& status_cb, const StatisticsCB& statistics_cb) { DVLOG(2) << "Initialize()"; @@ -42,14 +44,17 @@ void DecryptingVideoDecoder::Initialize( state_ == kDecodeFinished) << state_; DCHECK(read_cb_.is_null()); DCHECK(reset_cb_.is_null()); - DCHECK(config.IsValidConfig()); - DCHECK(config.is_encrypted()); + DCHECK(stream); init_cb_ = BindToCurrentLoop(status_cb); weak_this_ = weak_factory_.GetWeakPtr(); - config_ = config; + demuxer_stream_ = stream; statistics_cb_ = statistics_cb; + const VideoDecoderConfig& config = demuxer_stream_->video_decoder_config(); + DCHECK(config.IsValidConfig()); + DCHECK(config.is_encrypted()); + if (state_ == kUninitialized) { state_ = kDecryptorRequested; set_decryptor_ready_cb_.Run(BindToCurrentLoop(base::Bind( @@ -64,16 +69,14 @@ void DecryptingVideoDecoder::Initialize( &DecryptingVideoDecoder::FinishInitialization, weak_this_))); } -void DecryptingVideoDecoder::Decode(const scoped_refptr<DecoderBuffer>& buffer, - const ReadCB& read_cb) { - DVLOG(3) << "Decode()"; +void DecryptingVideoDecoder::Read(const ReadCB& read_cb) { + DVLOG(3) << "Read()"; DCHECK(message_loop_->BelongsToCurrentThread()); DCHECK(state_ == kIdle || state_ == kDecodeFinished || state_ == kError) << state_; DCHECK(!read_cb.is_null()); CHECK(read_cb_.is_null()) << "Overlapping decodes are not supported."; - read_cb_ = BindToCurrentLoop(read_cb); if (state_ == kError) { @@ -87,15 +90,15 @@ void DecryptingVideoDecoder::Decode(const scoped_refptr<DecoderBuffer>& buffer, return; } - pending_buffer_to_decode_ = buffer; - state_ = kPendingDecode; - DecodePendingBuffer(); + state_ = kPendingDemuxerRead; + ReadFromDemuxerStream(); } void DecryptingVideoDecoder::Reset(const base::Closure& closure) { DVLOG(2) << "Reset() - state: " << state_; DCHECK(message_loop_->BelongsToCurrentThread()); DCHECK(state_ == kIdle || + state_ == kPendingDemuxerRead || state_ == kPendingDecode || state_ == kWaitingForKey || state_ == kDecodeFinished || @@ -111,7 +114,7 @@ void DecryptingVideoDecoder::Reset(const base::Closure& closure) { // Defer the resetting process in this case. The |reset_cb_| will be fired // after the read callback is fired - see DecryptAndDecodeBuffer() and // DeliverFrame(). - if (state_ == kPendingDecode) { + if (state_ == kPendingDemuxerRead || state_ == kPendingDecode) { DCHECK(!read_cb_.is_null()); return; } @@ -178,8 +181,7 @@ void DecryptingVideoDecoder::SetDecryptor(Decryptor* decryptor) { state_ = kPendingDecoderInit; decryptor_->InitializeVideoDecoder( - config_, - BindToCurrentLoop(base::Bind( + demuxer_stream_->video_decoder_config(), BindToCurrentLoop(base::Bind( &DecryptingVideoDecoder::FinishInitialization, weak_this_))); } @@ -209,6 +211,47 @@ void DecryptingVideoDecoder::FinishInitialization(bool success) { base::ResetAndReturn(&init_cb_).Run(PIPELINE_OK); } +void DecryptingVideoDecoder::ReadFromDemuxerStream() { + DCHECK(message_loop_->BelongsToCurrentThread()); + DCHECK_EQ(state_, kPendingDemuxerRead) << state_; + DCHECK(!read_cb_.is_null()); + + demuxer_stream_->Read( + base::Bind(&DecryptingVideoDecoder::DecryptAndDecodeBuffer, weak_this_)); +} + +void DecryptingVideoDecoder::DecryptAndDecodeBuffer( + DemuxerStream::Status status, + const scoped_refptr<DecoderBuffer>& buffer) { + DVLOG(3) << "DecryptAndDecodeBuffer()"; + DCHECK(message_loop_->BelongsToCurrentThread()); + + if (state_ == kStopped) + return; + + DCHECK_EQ(state_, kPendingDemuxerRead) << state_; + DCHECK(!read_cb_.is_null()); + DCHECK_EQ(buffer.get() != NULL, status == DemuxerStream::kOk) << status; + + if (!reset_cb_.is_null()) { + base::ResetAndReturn(&read_cb_).Run(kOk, NULL); + DoReset(); + return; + } + + if (status == DemuxerStream::kAborted) { + DVLOG(2) << "DecryptAndDecodeBuffer() - kAborted"; + state_ = kIdle; + base::ResetAndReturn(&read_cb_).Run(kOk, NULL); + return; + } + + // VideoFrameStream ensures no kConfigChanged is passed to VideoDecoders. + DCHECK_EQ(status, DemuxerStream::kOk) << status; + pending_buffer_to_decode_ = buffer; + state_ = kPendingDecode; + DecodePendingBuffer(); +} void DecryptingVideoDecoder::DecodePendingBuffer() { DCHECK(message_loop_->BelongsToCurrentThread()); @@ -296,8 +339,8 @@ void DecryptingVideoDecoder::DeliverFrame( return; } - state_ = kIdle; - base::ResetAndReturn(&read_cb_).Run(kNotEnoughData, NULL); + state_ = kPendingDemuxerRead; + ReadFromDemuxerStream(); return; } diff --git a/media/filters/decrypting_video_decoder.h b/media/filters/decrypting_video_decoder.h index af9099f..6cbf9bc 100644 --- a/media/filters/decrypting_video_decoder.h +++ b/media/filters/decrypting_video_decoder.h @@ -8,8 +8,8 @@ #include "base/callback.h" #include "base/memory/weak_ptr.h" #include "media/base/decryptor.h" +#include "media/base/demuxer_stream.h" #include "media/base/video_decoder.h" -#include "media/base/video_decoder_config.h" namespace base { class MessageLoopProxy; @@ -32,11 +32,10 @@ class MEDIA_EXPORT DecryptingVideoDecoder : public VideoDecoder { virtual ~DecryptingVideoDecoder(); // VideoDecoder implementation. - virtual void Initialize(const VideoDecoderConfig& config, + virtual void Initialize(DemuxerStream* stream, const PipelineStatusCB& status_cb, const StatisticsCB& statistics_cb) OVERRIDE; - virtual void Decode(const scoped_refptr<DecoderBuffer>& buffer, - const ReadCB& read_cb) OVERRIDE; + virtual void Read(const ReadCB& read_cb) OVERRIDE; virtual void Reset(const base::Closure& closure) OVERRIDE; virtual void Stop(const base::Closure& closure) OVERRIDE; @@ -49,6 +48,7 @@ class MEDIA_EXPORT DecryptingVideoDecoder : public VideoDecoder { kDecryptorRequested, kPendingDecoderInit, kIdle, + kPendingDemuxerRead, kPendingDecode, kWaitingForKey, kDecodeFinished, @@ -62,6 +62,12 @@ class MEDIA_EXPORT DecryptingVideoDecoder : public VideoDecoder { // Callback for Decryptor::InitializeVideoDecoder() during initialization. void FinishInitialization(bool success); + void ReadFromDemuxerStream(); + + // Callback for DemuxerStream::Read(). + void DecryptAndDecodeBuffer(DemuxerStream::Status status, + const scoped_refptr<DecoderBuffer>& buffer); + void DecodePendingBuffer(); // Callback for Decryptor::DecryptAndDecodeVideo(). @@ -90,14 +96,15 @@ class MEDIA_EXPORT DecryptingVideoDecoder : public VideoDecoder { ReadCB read_cb_; base::Closure reset_cb_; - VideoDecoderConfig config_; + // Pointer to the demuxer stream that will feed us compressed buffers. + DemuxerStream* demuxer_stream_; // Callback to request/cancel decryptor creation notification. SetDecryptorReadyCB set_decryptor_ready_cb_; Decryptor* decryptor_; - // The buffer that needs decrypting/decoding. + // The buffer returned by the demuxer that needs decrypting/decoding. scoped_refptr<media::DecoderBuffer> pending_buffer_to_decode_; // Indicates the situation where new key is added during pending decode diff --git a/media/filters/decrypting_video_decoder_unittest.cc b/media/filters/decrypting_video_decoder_unittest.cc index 9d86b4d..27d4e53 100644 --- a/media/filters/decrypting_video_decoder_unittest.cc +++ b/media/filters/decrypting_video_decoder_unittest.cc @@ -46,6 +46,10 @@ static scoped_refptr<DecoderBuffer> CreateFakeEncryptedBuffer() { // times across multiple test files. Sadly we can't use static for them. namespace { +ACTION_P(ReturnBuffer, buffer) { + arg0.Run(buffer.get() ? DemuxerStream::kOk : DemuxerStream::kAborted, buffer); +} + ACTION_P(RunCallbackIfNotNull, param) { if (!arg0.is_null()) arg0.Run(param); @@ -70,6 +74,7 @@ class DecryptingVideoDecoderTest : public testing::Test { &DecryptingVideoDecoderTest::RequestDecryptorNotification, base::Unretained(this)))), decryptor_(new StrictMock<MockDecryptor>()), + demuxer_(new StrictMock<MockDemuxerStream>(DemuxerStream::VIDEO)), encrypted_buffer_(CreateFakeEncryptedBuffer()), decoded_video_frame_(VideoFrame::CreateBlackFrame( TestVideoConfig::NormalCodedSize())), @@ -85,7 +90,8 @@ class DecryptingVideoDecoderTest : public testing::Test { void InitializeAndExpectStatus(const VideoDecoderConfig& config, PipelineStatus status) { - decoder_->Initialize(config, NewExpectedStatusCB(status), + demuxer_->set_video_decoder_config(config); + decoder_->Initialize(demuxer_.get(), NewExpectedStatusCB(status), base::Bind(&MockStatisticsCB::OnStatistics, base::Unretained(&statistics_cb_))); message_loop_.RunUntilIdle(); @@ -106,7 +112,6 @@ class DecryptingVideoDecoderTest : public testing::Test { } void ReadAndExpectFrameReadyWith( - const scoped_refptr<DecoderBuffer>& buffer, VideoDecoder::Status status, const scoped_refptr<VideoFrame>& video_frame) { if (status != VideoDecoder::kOk) @@ -116,55 +121,68 @@ class DecryptingVideoDecoderTest : public testing::Test { else EXPECT_CALL(*this, FrameReady(status, video_frame)); - decoder_->Decode(buffer, - base::Bind(&DecryptingVideoDecoderTest::FrameReady, - base::Unretained(this))); + decoder_->Read(base::Bind(&DecryptingVideoDecoderTest::FrameReady, + base::Unretained(this))); message_loop_.RunUntilIdle(); } // Sets up expectations and actions to put DecryptingVideoDecoder in an // active normal decoding state. void EnterNormalDecodingState() { + EXPECT_CALL(*demuxer_, Read(_)) + .WillOnce(ReturnBuffer(encrypted_buffer_)) + .WillRepeatedly(ReturnBuffer(DecoderBuffer::CreateEOSBuffer())); EXPECT_CALL(*decryptor_, DecryptAndDecodeVideo(_, _)) .WillOnce(RunCallback<1>(Decryptor::kSuccess, decoded_video_frame_)) .WillRepeatedly(RunCallback<1>(Decryptor::kNeedMoreData, scoped_refptr<VideoFrame>())); EXPECT_CALL(statistics_cb_, OnStatistics(_)); - ReadAndExpectFrameReadyWith( - encrypted_buffer_, VideoDecoder::kOk, decoded_video_frame_); + ReadAndExpectFrameReadyWith(VideoDecoder::kOk, decoded_video_frame_); } // Sets up expectations and actions to put DecryptingVideoDecoder in an end // of stream state. This function must be called after // EnterNormalDecodingState() to work. void EnterEndOfStreamState() { - ReadAndExpectFrameReadyWith(DecoderBuffer::CreateEOSBuffer(), - VideoDecoder::kOk, - end_of_stream_video_frame_); + ReadAndExpectFrameReadyWith(VideoDecoder::kOk, end_of_stream_video_frame_); + } + + // Make the read callback pending by saving and not firing it. + void EnterPendingReadState() { + EXPECT_TRUE(pending_demuxer_read_cb_.is_null()); + EXPECT_CALL(*demuxer_, Read(_)) + .WillOnce(SaveArg<0>(&pending_demuxer_read_cb_)); + decoder_->Read(base::Bind(&DecryptingVideoDecoderTest::FrameReady, + base::Unretained(this))); + message_loop_.RunUntilIdle(); + // Make sure the Read() on the decoder triggers a Read() on the demuxer. + EXPECT_FALSE(pending_demuxer_read_cb_.is_null()); } // Make the video decode callback pending by saving and not firing it. void EnterPendingDecodeState() { EXPECT_TRUE(pending_video_decode_cb_.is_null()); + EXPECT_CALL(*demuxer_, Read(_)) + .WillRepeatedly(ReturnBuffer(encrypted_buffer_)); EXPECT_CALL(*decryptor_, DecryptAndDecodeVideo(encrypted_buffer_, _)) .WillOnce(SaveArg<1>(&pending_video_decode_cb_)); - decoder_->Decode(encrypted_buffer_, - base::Bind(&DecryptingVideoDecoderTest::FrameReady, - base::Unretained(this))); + decoder_->Read(base::Bind(&DecryptingVideoDecoderTest::FrameReady, + base::Unretained(this))); message_loop_.RunUntilIdle(); - // Make sure the Decode() on the decoder triggers a DecryptAndDecode() on - // the decryptor. + // Make sure the Read() on the decoder triggers a DecryptAndDecode() on the + // decryptor. EXPECT_FALSE(pending_video_decode_cb_.is_null()); } void EnterWaitingForKeyState() { + EXPECT_CALL(*demuxer_, Read(_)) + .WillRepeatedly(ReturnBuffer(encrypted_buffer_)); EXPECT_CALL(*decryptor_, DecryptAndDecodeVideo(_, _)) .WillRepeatedly(RunCallback<1>(Decryptor::kNoKey, null_video_frame_)); - decoder_->Decode(encrypted_buffer_, - base::Bind(&DecryptingVideoDecoderTest::FrameReady, - base::Unretained(this))); + decoder_->Read(base::Bind(&DecryptingVideoDecoderTest::FrameReady, + base::Unretained(this))); message_loop_.RunUntilIdle(); } @@ -214,13 +232,15 @@ class DecryptingVideoDecoderTest : public testing::Test { base::MessageLoop message_loop_; scoped_ptr<DecryptingVideoDecoder> decoder_; scoped_ptr<StrictMock<MockDecryptor> > decryptor_; + scoped_ptr<StrictMock<MockDemuxerStream> > demuxer_; MockStatisticsCB statistics_cb_; + DemuxerStream::ReadCB pending_demuxer_read_cb_; Decryptor::DecoderInitCB pending_init_cb_; Decryptor::NewKeyCB key_added_cb_; Decryptor::VideoDecodeCB pending_video_decode_cb_; - // Constant buffer/frames. + // Constant buffer/frames to be returned by the |demuxer_| and |decryptor_|. scoped_refptr<DecoderBuffer> encrypted_buffer_; scoped_refptr<VideoFrame> decoded_video_frame_; scoped_refptr<VideoFrame> null_video_frame_; @@ -280,16 +300,16 @@ TEST_F(DecryptingVideoDecoderTest, DecryptAndDecode_Normal) { TEST_F(DecryptingVideoDecoderTest, DecryptAndDecode_DecodeError) { Initialize(); + EXPECT_CALL(*demuxer_, Read(_)) + .WillRepeatedly(ReturnBuffer(encrypted_buffer_)); EXPECT_CALL(*decryptor_, DecryptAndDecodeVideo(_, _)) .WillRepeatedly(RunCallback<1>(Decryptor::kError, - scoped_refptr<VideoFrame>(NULL))); + scoped_refptr<VideoFrame>(NULL))); - ReadAndExpectFrameReadyWith( - encrypted_buffer_, VideoDecoder::kDecodeError, null_video_frame_); + ReadAndExpectFrameReadyWith(VideoDecoder::kDecodeError, null_video_frame_); // After a decode error occurred, all following read returns kDecodeError. - ReadAndExpectFrameReadyWith( - encrypted_buffer_, VideoDecoder::kDecodeError, null_video_frame_); + ReadAndExpectFrameReadyWith(VideoDecoder::kDecodeError, null_video_frame_); } // Test the case where the decryptor returns kNeedMoreData to ask for more @@ -297,6 +317,9 @@ TEST_F(DecryptingVideoDecoderTest, DecryptAndDecode_DecodeError) { TEST_F(DecryptingVideoDecoderTest, DecryptAndDecode_NeedMoreData) { Initialize(); + EXPECT_CALL(*demuxer_, Read(_)) + .Times(2) + .WillRepeatedly(ReturnBuffer(encrypted_buffer_)); EXPECT_CALL(*decryptor_, DecryptAndDecodeVideo(_, _)) .WillOnce(RunCallback<1>(Decryptor::kNeedMoreData, scoped_refptr<VideoFrame>())) @@ -305,10 +328,7 @@ TEST_F(DecryptingVideoDecoderTest, DecryptAndDecode_NeedMoreData) { EXPECT_CALL(statistics_cb_, OnStatistics(_)) .Times(2); - ReadAndExpectFrameReadyWith( - encrypted_buffer_, VideoDecoder::kNotEnoughData, decoded_video_frame_); - ReadAndExpectFrameReadyWith( - encrypted_buffer_, VideoDecoder::kOk, decoded_video_frame_); + ReadAndExpectFrameReadyWith(VideoDecoder::kOk, decoded_video_frame_); } // Test the case where the decryptor receives end-of-stream buffer. @@ -318,6 +338,17 @@ TEST_F(DecryptingVideoDecoderTest, DecryptAndDecode_EndOfStream) { EnterEndOfStreamState(); } +// Test aborted read on the demuxer stream. +TEST_F(DecryptingVideoDecoderTest, DemuxerRead_Aborted) { + Initialize(); + + // ReturnBuffer() with NULL triggers aborted demuxer read. + EXPECT_CALL(*demuxer_, Read(_)) + .WillOnce(ReturnBuffer(scoped_refptr<DecoderBuffer>())); + + ReadAndExpectFrameReadyWith(VideoDecoder::kOk, null_video_frame_); +} + // Test the case where the a key is added when the decryptor is in // kWaitingForKey state. TEST_F(DecryptingVideoDecoderTest, KeyAdded_DuringWaitingForKey) { @@ -367,6 +398,35 @@ TEST_F(DecryptingVideoDecoderTest, Reset_DuringIdleAfterDecodedOneFrame) { Reset(); } +// Test resetting when the decoder is in kPendingDemuxerRead state and the read +// callback is returned with kOk. +TEST_F(DecryptingVideoDecoderTest, Reset_DuringDemuxerRead_Ok) { + Initialize(); + EnterPendingReadState(); + + EXPECT_CALL(*this, FrameReady(VideoDecoder::kOk, IsNull())); + + Reset(); + base::ResetAndReturn(&pending_demuxer_read_cb_).Run(DemuxerStream::kOk, + encrypted_buffer_); + message_loop_.RunUntilIdle(); +} + +// Test resetting when the decoder is in kPendingDemuxerRead state and the read +// callback is returned with kAborted. +TEST_F(DecryptingVideoDecoderTest, Reset_DuringDemuxerRead_Aborted) { + Initialize(); + EnterPendingReadState(); + + // Make sure we get a NULL video frame returned. + EXPECT_CALL(*this, FrameReady(VideoDecoder::kOk, IsNull())); + + Reset(); + base::ResetAndReturn(&pending_demuxer_read_cb_).Run(DemuxerStream::kAborted, + NULL); + message_loop_.RunUntilIdle(); +} + // Test resetting when the decoder is in kPendingDecode state. TEST_F(DecryptingVideoDecoderTest, Reset_DuringPendingDecode) { Initialize(); @@ -406,10 +466,11 @@ TEST_F(DecryptingVideoDecoderTest, Reset_AfterReset) { // Test stopping when the decoder is in kDecryptorRequested state. TEST_F(DecryptingVideoDecoderTest, Stop_DuringDecryptorRequested) { + demuxer_->set_video_decoder_config(TestVideoConfig::NormalEncrypted()); DecryptorReadyCB decryptor_ready_cb; EXPECT_CALL(*this, RequestDecryptorNotification(_)) .WillOnce(SaveArg<0>(&decryptor_ready_cb)); - decoder_->Initialize(TestVideoConfig::NormalEncrypted(), + decoder_->Initialize(demuxer_.get(), NewExpectedStatusCB(DECODER_ERROR_NOT_SUPPORTED), base::Bind(&MockStatisticsCB::OnStatistics, base::Unretained(&statistics_cb_))); @@ -452,6 +513,19 @@ TEST_F(DecryptingVideoDecoderTest, Stop_DuringIdleAfterDecodedOneFrame) { Stop(); } +// Test stopping when the decoder is in kPendingDemuxerRead state. +TEST_F(DecryptingVideoDecoderTest, Stop_DuringPendingDemuxerRead) { + Initialize(); + EnterPendingReadState(); + + EXPECT_CALL(*this, FrameReady(VideoDecoder::kOk, IsNull())); + + Stop(); + base::ResetAndReturn(&pending_demuxer_read_cb_).Run(DemuxerStream::kOk, + encrypted_buffer_); + message_loop_.RunUntilIdle(); +} + // Test stopping when the decoder is in kPendingDecode state. TEST_F(DecryptingVideoDecoderTest, Stop_DuringPendingDecode) { Initialize(); diff --git a/media/filters/fake_demuxer_stream.cc b/media/filters/fake_demuxer_stream.cc index 8cab3a3..1d5915c 100644 --- a/media/filters/fake_demuxer_stream.cc +++ b/media/filters/fake_demuxer_stream.cc @@ -9,9 +9,9 @@ #include "base/location.h" #include "base/logging.h" #include "base/message_loop.h" +#include "base/pickle.h" #include "media/base/bind_to_loop.h" #include "media/base/decoder_buffer.h" -#include "media/base/test_helpers.h" #include "media/base/video_frame.h" #include "ui/gfx/rect.h" #include "ui/gfx/size.h" @@ -24,6 +24,7 @@ static const int kStartWidth = 320; static const int kStartHeight = 240; static const int kWidthDelta = 4; static const int kHeightDelta = 3; +static const char kFakeBufferHeader[] = "Fake Buffer"; FakeDemuxerStream::FakeDemuxerStream(int num_configs, int num_buffers_in_one_config, @@ -138,10 +139,19 @@ void FakeDemuxerStream::DoRead() { return; } - scoped_refptr<DecoderBuffer> buffer = CreateFakeVideoBufferForTest( - video_decoder_config_, current_timestamp_, duration_); + // Prepare the next buffer. + Pickle pickle; + pickle.WriteString(kFakeBufferHeader); + pickle.WriteInt(video_decoder_config_.coded_size().width()); + pickle.WriteInt(video_decoder_config_.coded_size().height()); + pickle.WriteInt64(current_timestamp_.InMilliseconds()); + + scoped_refptr<DecoderBuffer> buffer = DecoderBuffer::CopyFrom( + static_cast<const uint8*>(pickle.data()), pickle.size()); // TODO(xhwang): Output out-of-order buffers if needed. + buffer->SetTimestamp(current_timestamp_); + buffer->SetDuration(duration_); current_timestamp_ += duration_; num_buffers_left_in_current_config_--; diff --git a/media/filters/fake_demuxer_stream.h b/media/filters/fake_demuxer_stream.h index 4ed4796..2a82f37 100644 --- a/media/filters/fake_demuxer_stream.h +++ b/media/filters/fake_demuxer_stream.h @@ -9,6 +9,7 @@ #include "base/memory/ref_counted.h" #include "media/base/audio_decoder_config.h" #include "media/base/demuxer_stream.h" +#include "media/base/media_export.h" #include "media/base/video_decoder_config.h" namespace base { @@ -17,7 +18,7 @@ class MessageLoopProxy; namespace media { -class FakeDemuxerStream : public DemuxerStream { +class MEDIA_EXPORT FakeDemuxerStream : public DemuxerStream { public: // Constructs an object that outputs |num_configs| different configs in // sequence with |num_frames_in_one_config| buffers for each config. The diff --git a/media/filters/fake_video_decoder.cc b/media/filters/fake_video_decoder.cc index 6e0577c..9a59d1c 100644 --- a/media/filters/fake_video_decoder.cc +++ b/media/filters/fake_video_decoder.cc @@ -9,7 +9,7 @@ #include "base/location.h" #include "base/message_loop/message_loop_proxy.h" #include "media/base/bind_to_loop.h" -#include "media/base/test_helpers.h" +#include "media/base/demuxer_stream.h" namespace media { @@ -17,7 +17,8 @@ FakeVideoDecoder::FakeVideoDecoder(int decoding_delay) : message_loop_(base::MessageLoopProxy::current()), weak_factory_(this), decoding_delay_(decoding_delay), - state_(UNINITIALIZED) { + state_(UNINITIALIZED), + demuxer_stream_(NULL) { DCHECK_GE(decoding_delay, 0); } @@ -25,18 +26,20 @@ FakeVideoDecoder::~FakeVideoDecoder() { DCHECK_EQ(state_, UNINITIALIZED); } -void FakeVideoDecoder::Initialize(const VideoDecoderConfig& config, +void FakeVideoDecoder::Initialize(DemuxerStream* stream, const PipelineStatusCB& status_cb, const StatisticsCB& statistics_cb) { DCHECK(message_loop_->BelongsToCurrentThread()); - DCHECK(config.IsValidConfig()); + DCHECK(stream); + DCHECK(stream->video_decoder_config().IsValidConfig()); DCHECK(read_cb_.IsNull()) << "No reinitialization during pending read."; DCHECK(reset_cb_.IsNull()) << "No reinitialization during pending reset."; weak_this_ = weak_factory_.GetWeakPtr(); + demuxer_stream_ = stream; statistics_cb_ = statistics_cb; - current_config_ = config; + current_config_ = stream->video_decoder_config(); init_cb_.SetCallback(BindToCurrentLoop(status_cb)); if (!decoded_frames_.empty()) { @@ -48,35 +51,14 @@ void FakeVideoDecoder::Initialize(const VideoDecoderConfig& config, init_cb_.RunOrHold(PIPELINE_OK); } -void FakeVideoDecoder::Decode(const scoped_refptr<DecoderBuffer>& buffer, - const ReadCB& read_cb) { +void FakeVideoDecoder::Read(const ReadCB& read_cb) { DCHECK(message_loop_->BelongsToCurrentThread()); DCHECK(read_cb_.IsNull()) << "Overlapping decodes are not supported."; DCHECK(reset_cb_.IsNull()); DCHECK_LE(decoded_frames_.size(), static_cast<size_t>(decoding_delay_)); read_cb_.SetCallback(BindToCurrentLoop(read_cb)); - - if (buffer->IsEndOfStream() && decoded_frames_.empty()) { - read_cb_.RunOrHold(kOk, VideoFrame::CreateEmptyFrame()); - return; - } - - if (!buffer->IsEndOfStream()) { - DCHECK(VerifyFakeVideoBufferForTest(buffer, current_config_)); - scoped_refptr<VideoFrame> video_frame = VideoFrame::CreateColorFrame( - current_config_.coded_size(), 0, 0, 0, buffer->GetTimestamp()); - decoded_frames_.push_back(video_frame); - - if (decoded_frames_.size() <= static_cast<size_t>(decoding_delay_)) { - read_cb_.RunOrHold(kNotEnoughData, scoped_refptr<VideoFrame>()); - return; - } - } - - scoped_refptr<VideoFrame> frame = decoded_frames_.front(); - decoded_frames_.pop_front(); - read_cb_.RunOrHold(kOk, frame); + ReadFromDemuxerStream(); } void FakeVideoDecoder::Reset(const base::Closure& closure) { @@ -160,6 +142,79 @@ void FakeVideoDecoder::SatisfyStop() { stop_cb_.RunHeldCallback(); } +void FakeVideoDecoder::ReadFromDemuxerStream() { + DCHECK_EQ(state_, NORMAL); + DCHECK(!read_cb_.IsNull()); + demuxer_stream_->Read(base::Bind(&FakeVideoDecoder::BufferReady, weak_this_)); +} + +void FakeVideoDecoder::BufferReady(DemuxerStream::Status status, + const scoped_refptr<DecoderBuffer>& buffer) { + DCHECK(message_loop_->BelongsToCurrentThread()); + DCHECK_EQ(state_, NORMAL); + DCHECK(!read_cb_.IsNull()); + DCHECK_EQ(status != DemuxerStream::kOk, !buffer.get()) << status; + + if (!stop_cb_.IsNull()) { + read_cb_.RunOrHold(kOk, scoped_refptr<VideoFrame>()); + if (!reset_cb_.IsNull()) { + DoReset(); + } + DoStop(); + return; + } + + if (status == DemuxerStream::kConfigChanged) { + DCHECK(demuxer_stream_->video_decoder_config().IsValidConfig()); + current_config_ = demuxer_stream_->video_decoder_config(); + + if (reset_cb_.IsNull()) { + ReadFromDemuxerStream(); + return; + } + } + + if (!reset_cb_.IsNull()) { + read_cb_.RunOrHold(kOk, scoped_refptr<VideoFrame>()); + DoReset(); + return; + } + + if (status == DemuxerStream::kAborted) { + read_cb_.RunOrHold(kOk, scoped_refptr<VideoFrame>()); + return; + } + + DCHECK_EQ(status, DemuxerStream::kOk); + + if (buffer->IsEndOfStream() && decoded_frames_.empty()) { + read_cb_.RunOrHold(kOk, VideoFrame::CreateEmptyFrame()); + return; + } + + if (!buffer->IsEndOfStream()) { + // Make sure the decoder is always configured with the latest config. + DCHECK(current_config_.Matches(demuxer_stream_->video_decoder_config())) + << "Decoder's Current Config: " + << current_config_.AsHumanReadableString() + << "DemuxerStream's Current Config: " + << demuxer_stream_->video_decoder_config().AsHumanReadableString(); + + scoped_refptr<VideoFrame> video_frame = VideoFrame::CreateColorFrame( + current_config_.coded_size(), 0, 0, 0, buffer->GetTimestamp()); + decoded_frames_.push_back(video_frame); + + if (decoded_frames_.size() <= static_cast<size_t>(decoding_delay_)) { + ReadFromDemuxerStream(); + return; + } + } + + scoped_refptr<VideoFrame> frame = decoded_frames_.front(); + decoded_frames_.pop_front(); + read_cb_.RunOrHold(kOk, frame); +} + void FakeVideoDecoder::DoReset() { DCHECK(message_loop_->BelongsToCurrentThread()); DCHECK(read_cb_.IsNull()); @@ -176,6 +231,7 @@ void FakeVideoDecoder::DoStop() { DCHECK(!stop_cb_.IsNull()); state_ = UNINITIALIZED; + demuxer_stream_ = NULL; decoded_frames_.clear(); stop_cb_.RunOrHold(); } diff --git a/media/filters/fake_video_decoder.h b/media/filters/fake_video_decoder.h index c20e44d..c82679c 100644 --- a/media/filters/fake_video_decoder.h +++ b/media/filters/fake_video_decoder.h @@ -13,6 +13,8 @@ #include "base/memory/weak_ptr.h" #include "media/base/callback_holder.h" #include "media/base/decoder_buffer.h" +#include "media/base/demuxer_stream.h" +#include "media/base/media_export.h" #include "media/base/pipeline_status.h" #include "media/base/video_decoder.h" #include "media/base/video_decoder_config.h" @@ -27,18 +29,17 @@ class MessageLoopProxy; namespace media { -class FakeVideoDecoder : public VideoDecoder { +class MEDIA_EXPORT FakeVideoDecoder : public VideoDecoder { public: // Constructs an object with a decoding delay of |decoding_delay| frames. explicit FakeVideoDecoder(int decoding_delay); virtual ~FakeVideoDecoder(); // VideoDecoder implementation. - virtual void Initialize(const VideoDecoderConfig& config, + virtual void Initialize(DemuxerStream* stream, const PipelineStatusCB& status_cb, const StatisticsCB& statistics_cb) OVERRIDE; - virtual void Decode(const scoped_refptr<DecoderBuffer>& buffer, - const ReadCB& read_cb) OVERRIDE; + virtual void Read(const ReadCB& read_cb) OVERRIDE; virtual void Reset(const base::Closure& closure) OVERRIDE; virtual void Stop(const base::Closure& closure) OVERRIDE; @@ -61,6 +62,12 @@ class FakeVideoDecoder : public VideoDecoder { NORMAL }; + void ReadFromDemuxerStream(); + + // Callback for DemuxerStream::Read(). + void BufferReady(DemuxerStream::Status status, + const scoped_refptr<DecoderBuffer>& buffer); + void DoReset(); void DoStop(); @@ -79,6 +86,9 @@ class FakeVideoDecoder : public VideoDecoder { CallbackHolder<base::Closure> reset_cb_; CallbackHolder<base::Closure> stop_cb_; + // Pointer to the demuxer stream that will feed us compressed buffers. + DemuxerStream* demuxer_stream_; + VideoDecoderConfig current_config_; std::list<scoped_refptr<VideoFrame> > decoded_frames_; diff --git a/media/filters/fake_video_decoder_unittest.cc b/media/filters/fake_video_decoder_unittest.cc index 9bd62b1..fc1cb14 100644 --- a/media/filters/fake_video_decoder_unittest.cc +++ b/media/filters/fake_video_decoder_unittest.cc @@ -9,23 +9,25 @@ #include "media/base/mock_filters.h" #include "media/base/test_helpers.h" #include "media/base/video_frame.h" +#include "media/filters/fake_demuxer_stream.h" #include "media/filters/fake_video_decoder.h" #include "testing/gtest/include/gtest/gtest.h" namespace media { static const int kDecodingDelay = 9; -static const int kTotalBuffers = 12; -static const int kDurationMs = 30; +static const int kNumConfigs = 3; +static const int kNumBuffersInOneConfig = 9; +static const int kNumInputBuffers = kNumConfigs * kNumBuffersInOneConfig; class FakeVideoDecoderTest : public testing::Test { public: FakeVideoDecoderTest() : decoder_(new FakeVideoDecoder(kDecodingDelay)), - num_input_buffers_(0), + demuxer_stream_( + new FakeDemuxerStream(kNumConfigs, kNumBuffersInOneConfig, false)), num_decoded_frames_(0), - decode_status_(VideoDecoder::kNotEnoughData), - is_decode_pending_(false), + is_read_pending_(false), is_reset_pending_(false), is_stop_pending_(false) {} @@ -33,17 +35,12 @@ class FakeVideoDecoderTest : public testing::Test { StopAndExpect(OK); } - void InitializeWithConfig(const VideoDecoderConfig& config) { - decoder_->Initialize(config, + void Initialize() { + decoder_->Initialize(demuxer_stream_.get(), NewExpectedStatusCB(PIPELINE_OK), base::Bind(&MockStatisticsCB::OnStatistics, base::Unretained(&statistics_cb_))); message_loop_.RunUntilIdle(); - current_config_ = config; - } - - void Initialize() { - InitializeWithConfig(TestVideoConfig::Normal()); } void EnterPendingInitState() { @@ -59,21 +56,19 @@ class FakeVideoDecoderTest : public testing::Test { // Callback for VideoDecoder::Read(). void FrameReady(VideoDecoder::Status status, const scoped_refptr<VideoFrame>& frame) { - DCHECK(is_decode_pending_); - ASSERT_TRUE(status == VideoDecoder::kOk || - status == VideoDecoder::kNotEnoughData); - is_decode_pending_ = false; - decode_status_ = status; - frame_decoded_ = frame; - - if (frame && !frame->IsEndOfStream()) + DCHECK(is_read_pending_); + ASSERT_EQ(VideoDecoder::kOk, status); + + is_read_pending_ = false; + frame_read_ = frame; + + if (frame.get() && !frame->IsEndOfStream()) num_decoded_frames_++; } enum CallbackResult { PENDING, OK, - NOT_ENOUGH_DATA, ABROTED, EOS }; @@ -81,86 +76,50 @@ class FakeVideoDecoderTest : public testing::Test { void ExpectReadResult(CallbackResult result) { switch (result) { case PENDING: - EXPECT_TRUE(is_decode_pending_); - ASSERT_FALSE(frame_decoded_); + EXPECT_TRUE(is_read_pending_); + ASSERT_FALSE(frame_read_.get()); break; case OK: - EXPECT_FALSE(is_decode_pending_); - ASSERT_EQ(VideoDecoder::kOk, decode_status_); - ASSERT_TRUE(frame_decoded_); - EXPECT_FALSE(frame_decoded_->IsEndOfStream()); - break; - case NOT_ENOUGH_DATA: - EXPECT_FALSE(is_decode_pending_); - ASSERT_EQ(VideoDecoder::kNotEnoughData, decode_status_); - ASSERT_FALSE(frame_decoded_); + EXPECT_FALSE(is_read_pending_); + ASSERT_TRUE(frame_read_.get()); + EXPECT_FALSE(frame_read_->IsEndOfStream()); break; case ABROTED: - EXPECT_FALSE(is_decode_pending_); - ASSERT_EQ(VideoDecoder::kOk, decode_status_); - EXPECT_FALSE(frame_decoded_); + EXPECT_FALSE(is_read_pending_); + EXPECT_FALSE(frame_read_.get()); break; case EOS: - EXPECT_FALSE(is_decode_pending_); - ASSERT_EQ(VideoDecoder::kOk, decode_status_); - ASSERT_TRUE(frame_decoded_); - EXPECT_TRUE(frame_decoded_->IsEndOfStream()); + EXPECT_FALSE(is_read_pending_); + ASSERT_TRUE(frame_read_.get()); + EXPECT_TRUE(frame_read_->IsEndOfStream()); break; } } - void Decode() { - scoped_refptr<DecoderBuffer> buffer; - - if (num_input_buffers_ < kTotalBuffers) { - buffer = CreateFakeVideoBufferForTest( - current_config_, - base::TimeDelta::FromMilliseconds(kDurationMs * num_input_buffers_), - base::TimeDelta::FromMilliseconds(kDurationMs)); - num_input_buffers_++; - } else { - buffer = DecoderBuffer::CreateEOSBuffer(); - } - - decode_status_ = VideoDecoder::kDecodeError; - frame_decoded_ = NULL; - is_decode_pending_ = true; - - decoder_->Decode( - buffer, + void ReadOneFrame() { + frame_read_ = NULL; + is_read_pending_ = true; + decoder_->Read( base::Bind(&FakeVideoDecoderTest::FrameReady, base::Unretained(this))); message_loop_.RunUntilIdle(); } - void ReadOneFrame() { - do { - Decode(); - } while (decode_status_ == VideoDecoder::kNotEnoughData && - !is_decode_pending_); - } - void ReadUntilEOS() { do { ReadOneFrame(); - } while (frame_decoded_ && !frame_decoded_->IsEndOfStream()); + } while (frame_read_.get() && !frame_read_->IsEndOfStream()); } void EnterPendingReadState() { - // Pass the initial NOT_ENOUGH_DATA stage. - ReadOneFrame(); decoder_->HoldNextRead(); ReadOneFrame(); ExpectReadResult(PENDING); } - void SatisfyReadAndExpect(CallbackResult result) { + void SatisfyRead() { decoder_->SatisfyRead(); message_loop_.RunUntilIdle(); - ExpectReadResult(result); - } - - void SatisfyRead() { - SatisfyReadAndExpect(OK); + ExpectReadResult(OK); } // Callback for VideoDecoder::Reset(). @@ -239,19 +198,49 @@ class FakeVideoDecoderTest : public testing::Test { ExpectStopResult(OK); } - base::MessageLoop message_loop_; - VideoDecoderConfig current_config_; + // Callback for DemuxerStream::Read so that we can skip frames to trigger a + // config change. + void BufferReady(bool* config_changed, + DemuxerStream::Status status, + const scoped_refptr<DecoderBuffer>& buffer) { + if (status == DemuxerStream::kConfigChanged) + *config_changed = true; + } + + void ChangeConfig() { + bool config_changed = false; + while (!config_changed) { + demuxer_stream_->Read(base::Bind(&FakeVideoDecoderTest::BufferReady, + base::Unretained(this), + &config_changed)); + message_loop_.RunUntilIdle(); + } + } + + void EnterPendingDemuxerReadState() { + demuxer_stream_->HoldNextRead(); + ReadOneFrame(); + } + + void SatisfyDemuxerRead() { + demuxer_stream_->SatisfyRead(); + message_loop_.RunUntilIdle(); + } + + void AbortDemuxerRead() { + demuxer_stream_->Reset(); + message_loop_.RunUntilIdle(); + } + base::MessageLoop message_loop_; scoped_ptr<FakeVideoDecoder> decoder_; + scoped_ptr<FakeDemuxerStream> demuxer_stream_; MockStatisticsCB statistics_cb_; - - int num_input_buffers_; int num_decoded_frames_; // Callback result/status. - VideoDecoder::Status decode_status_; - scoped_refptr<VideoFrame> frame_decoded_; - bool is_decode_pending_; + scoped_refptr<VideoFrame> frame_read_; + bool is_read_pending_; bool is_reset_pending_; bool is_stop_pending_; @@ -266,15 +255,24 @@ TEST_F(FakeVideoDecoderTest, Initialize) { TEST_F(FakeVideoDecoderTest, Read_AllFrames) { Initialize(); ReadUntilEOS(); - EXPECT_EQ(kTotalBuffers, num_decoded_frames_); + EXPECT_EQ(kNumInputBuffers, num_decoded_frames_); +} + +TEST_F(FakeVideoDecoderTest, Read_AbortedDemuxerRead) { + Initialize(); + demuxer_stream_->HoldNextRead(); + ReadOneFrame(); + AbortDemuxerRead(); + ExpectReadResult(ABROTED); } TEST_F(FakeVideoDecoderTest, Read_DecodingDelay) { Initialize(); - while (num_input_buffers_ < kTotalBuffers) { + while (demuxer_stream_->num_buffers_returned() < kNumInputBuffers) { ReadOneFrame(); - EXPECT_EQ(num_input_buffers_, num_decoded_frames_ + kDecodingDelay); + EXPECT_EQ(demuxer_stream_->num_buffers_returned(), + num_decoded_frames_ + kDecodingDelay); } } @@ -282,32 +280,25 @@ TEST_F(FakeVideoDecoderTest, Read_ZeroDelay) { decoder_.reset(new FakeVideoDecoder(0)); Initialize(); - while (num_input_buffers_ < kTotalBuffers) { + while (demuxer_stream_->num_buffers_returned() < kNumInputBuffers) { ReadOneFrame(); - EXPECT_EQ(num_input_buffers_, num_decoded_frames_); + EXPECT_EQ(demuxer_stream_->num_buffers_returned(), num_decoded_frames_); } } -TEST_F(FakeVideoDecoderTest, Read_Pending_NotEnoughData) { +TEST_F(FakeVideoDecoderTest, Read_Pending) { Initialize(); - decoder_->HoldNextRead(); - ReadOneFrame(); - ExpectReadResult(PENDING); - SatisfyReadAndExpect(NOT_ENOUGH_DATA); -} - -TEST_F(FakeVideoDecoderTest, Read_Pending_OK) { - Initialize(); - ReadOneFrame(); EnterPendingReadState(); - SatisfyReadAndExpect(OK); + SatisfyRead(); } TEST_F(FakeVideoDecoderTest, Reinitialize) { Initialize(); - ReadOneFrame(); - InitializeWithConfig(TestVideoConfig::Large()); - ReadOneFrame(); + VideoDecoderConfig old_config = demuxer_stream_->video_decoder_config(); + ChangeConfig(); + VideoDecoderConfig new_config = demuxer_stream_->video_decoder_config(); + EXPECT_FALSE(new_config.Matches(old_config)); + Initialize(); } // Reinitializing the decoder during the middle of the decoding process can @@ -317,7 +308,7 @@ TEST_F(FakeVideoDecoderTest, Reinitialize_FrameDropped) { ReadOneFrame(); Initialize(); ReadUntilEOS(); - EXPECT_LT(num_decoded_frames_, kTotalBuffers); + EXPECT_LT(num_decoded_frames_, kNumInputBuffers); } TEST_F(FakeVideoDecoderTest, Reset) { @@ -326,6 +317,22 @@ TEST_F(FakeVideoDecoderTest, Reset) { ResetAndExpect(OK); } +TEST_F(FakeVideoDecoderTest, Reset_DuringPendingDemuxerRead) { + Initialize(); + EnterPendingDemuxerReadState(); + ResetAndExpect(PENDING); + SatisfyDemuxerRead(); + ExpectReadResult(ABROTED); +} + +TEST_F(FakeVideoDecoderTest, Reset_DuringPendingDemuxerRead_Aborted) { + Initialize(); + EnterPendingDemuxerReadState(); + ResetAndExpect(PENDING); + AbortDemuxerRead(); + ExpectReadResult(ABROTED); +} + TEST_F(FakeVideoDecoderTest, Reset_DuringPendingRead) { Initialize(); EnterPendingReadState(); @@ -361,6 +368,25 @@ TEST_F(FakeVideoDecoderTest, Stop_DuringPendingInitialization) { SatisfyStop(); } +TEST_F(FakeVideoDecoderTest, Stop_DuringPendingDemuxerRead) { + Initialize(); + EnterPendingDemuxerReadState(); + StopAndExpect(PENDING); + SatisfyDemuxerRead(); + ExpectReadResult(ABROTED); +} + +TEST_F(FakeVideoDecoderTest, Stop_DuringPendingDemuxerRead_Aborted) { + Initialize(); + EnterPendingDemuxerReadState(); + ResetAndExpect(PENDING); + StopAndExpect(PENDING); + SatisfyDemuxerRead(); + ExpectReadResult(ABROTED); + ExpectResetResult(OK); + ExpectStopResult(OK); +} + TEST_F(FakeVideoDecoderTest, Stop_DuringPendingRead) { Initialize(); EnterPendingReadState(); diff --git a/media/filters/ffmpeg_video_decoder.cc b/media/filters/ffmpeg_video_decoder.cc index 0911d51..8131d07 100644 --- a/media/filters/ffmpeg_video_decoder.cc +++ b/media/filters/ffmpeg_video_decoder.cc @@ -15,6 +15,7 @@ #include "base/strings/string_number_conversions.h" #include "media/base/bind_to_loop.h" #include "media/base/decoder_buffer.h" +#include "media/base/demuxer_stream.h" #include "media/base/limits.h" #include "media/base/media_switches.h" #include "media/base/pipeline.h" @@ -60,7 +61,8 @@ FFmpegVideoDecoder::FFmpegVideoDecoder( weak_factory_(this), state_(kUninitialized), codec_context_(NULL), - av_frame_(NULL) { + av_frame_(NULL), + demuxer_stream_(NULL) { } int FFmpegVideoDecoder::GetVideoBuffer(AVCodecContext* codec_context, @@ -86,7 +88,7 @@ int FFmpegVideoDecoder::GetVideoBuffer(AVCodecContext* codec_context, codec_context->sample_aspect_ratio.num, codec_context->sample_aspect_ratio.den); } else { - natural_size = config_.natural_size(); + natural_size = demuxer_stream_->video_decoder_config().natural_size(); } if (!VideoFrame::IsValidConfig(format, size, gfx::Rect(size), natural_size)) @@ -129,22 +131,22 @@ static void ReleaseVideoBufferImpl(AVCodecContext* s, AVFrame* frame) { frame->opaque = NULL; } -void FFmpegVideoDecoder::Initialize(const VideoDecoderConfig& config, +void FFmpegVideoDecoder::Initialize(DemuxerStream* stream, const PipelineStatusCB& status_cb, const StatisticsCB& statistics_cb) { DCHECK(message_loop_->BelongsToCurrentThread()); + DCHECK(stream); DCHECK(read_cb_.is_null()); DCHECK(reset_cb_.is_null()); - DCHECK(!config.is_encrypted()); FFmpegGlue::InitializeFFmpeg(); weak_this_ = weak_factory_.GetWeakPtr(); - config_ = config; + demuxer_stream_ = stream; statistics_cb_ = statistics_cb; PipelineStatusCB initialize_cb = BindToCurrentLoop(status_cb); - if (!config.IsValidConfig() || !ConfigureDecoder()) { + if (!ConfigureDecoder()) { initialize_cb.Run(DECODER_ERROR_NOT_SUPPORTED); return; } @@ -154,8 +156,7 @@ void FFmpegVideoDecoder::Initialize(const VideoDecoderConfig& config, initialize_cb.Run(PIPELINE_OK); } -void FFmpegVideoDecoder::Decode(const scoped_refptr<DecoderBuffer>& buffer, - const ReadCB& read_cb) { +void FFmpegVideoDecoder::Read(const ReadCB& read_cb) { DCHECK(message_loop_->BelongsToCurrentThread()); DCHECK(!read_cb.is_null()); CHECK_NE(state_, kUninitialized); @@ -173,7 +174,7 @@ void FFmpegVideoDecoder::Decode(const scoped_refptr<DecoderBuffer>& buffer, return; } - DecodeBuffer(buffer); + ReadFromDemuxerStream(); } void FFmpegVideoDecoder::Reset(const base::Closure& closure) { @@ -220,6 +221,45 @@ FFmpegVideoDecoder::~FFmpegVideoDecoder() { DCHECK(!av_frame_); } +void FFmpegVideoDecoder::ReadFromDemuxerStream() { + DCHECK_NE(state_, kUninitialized); + DCHECK_NE(state_, kDecodeFinished); + DCHECK_NE(state_, kError); + DCHECK(!read_cb_.is_null()); + + demuxer_stream_->Read(base::Bind( + &FFmpegVideoDecoder::BufferReady, weak_this_)); +} + +void FFmpegVideoDecoder::BufferReady( + DemuxerStream::Status status, + const scoped_refptr<DecoderBuffer>& buffer) { + DCHECK(message_loop_->BelongsToCurrentThread()); + DCHECK_NE(state_, kDecodeFinished); + DCHECK_NE(state_, kError); + DCHECK_EQ(status != DemuxerStream::kOk, !buffer.get()) << status; + + if (state_ == kUninitialized) + return; + + DCHECK(!read_cb_.is_null()); + + if (!reset_cb_.is_null()) { + base::ResetAndReturn(&read_cb_).Run(kOk, NULL); + DoReset(); + return; + } + + if (status == DemuxerStream::kAborted) { + base::ResetAndReturn(&read_cb_).Run(kOk, NULL); + return; + } + + // VideoFrameStream ensures no kConfigChanged is passed to VideoDecoders. + DCHECK_EQ(status, DemuxerStream::kOk) << status; + DecodeBuffer(buffer); +} + void FFmpegVideoDecoder::DecodeBuffer( const scoped_refptr<DecoderBuffer>& buffer) { DCHECK(message_loop_->BelongsToCurrentThread()); @@ -228,7 +268,7 @@ void FFmpegVideoDecoder::DecodeBuffer( DCHECK_NE(state_, kError); DCHECK(reset_cb_.is_null()); DCHECK(!read_cb_.is_null()); - DCHECK(buffer); + DCHECK(buffer.get()); // During decode, because reads are issued asynchronously, it is possible to // receive multiple end of stream buffers since each read is acked. When the @@ -264,7 +304,7 @@ void FFmpegVideoDecoder::DecodeBuffer( } scoped_refptr<VideoFrame> video_frame; - if (!FFmpegDecode(buffer, &video_frame)) { + if (!Decode(buffer, &video_frame)) { state_ = kError; base::ResetAndReturn(&read_cb_).Run(kDecodeError, NULL); return; @@ -285,14 +325,14 @@ void FFmpegVideoDecoder::DecodeBuffer( return; } - base::ResetAndReturn(&read_cb_).Run(kNotEnoughData, NULL); + ReadFromDemuxerStream(); return; } base::ResetAndReturn(&read_cb_).Run(kOk, video_frame); } -bool FFmpegVideoDecoder::FFmpegDecode( +bool FFmpegVideoDecoder::Decode( const scoped_refptr<DecoderBuffer>& buffer, scoped_refptr<VideoFrame>* video_frame) { DCHECK(video_frame); @@ -377,12 +417,24 @@ void FFmpegVideoDecoder::ReleaseFFmpegResources() { } bool FFmpegVideoDecoder::ConfigureDecoder() { + const VideoDecoderConfig& config = demuxer_stream_->video_decoder_config(); + + if (!config.IsValidConfig()) { + DLOG(ERROR) << "Invalid video stream - " << config.AsHumanReadableString(); + return false; + } + + if (config.is_encrypted()) { + DLOG(ERROR) << "Encrypted video stream not supported."; + return false; + } + // Release existing decoder resources if necessary. ReleaseFFmpegResources(); // Initialize AVCodecContext structure. codec_context_ = avcodec_alloc_context3(NULL); - VideoDecoderConfigToAVCodecContext(config_, codec_context_); + VideoDecoderConfigToAVCodecContext(config, codec_context_); // Enable motion vector search (potentially slow), strong deblocking filter // for damaged macroblocks, and set our error detection sensitivity. diff --git a/media/filters/ffmpeg_video_decoder.h b/media/filters/ffmpeg_video_decoder.h index c2b0898..2db72a2 100644 --- a/media/filters/ffmpeg_video_decoder.h +++ b/media/filters/ffmpeg_video_decoder.h @@ -9,8 +9,8 @@ #include "base/callback.h" #include "base/memory/weak_ptr.h" +#include "media/base/demuxer_stream.h" #include "media/base/video_decoder.h" -#include "media/base/video_decoder_config.h" struct AVCodecContext; struct AVFrame; @@ -30,11 +30,10 @@ class MEDIA_EXPORT FFmpegVideoDecoder : public VideoDecoder { virtual ~FFmpegVideoDecoder(); // VideoDecoder implementation. - virtual void Initialize(const VideoDecoderConfig& config, + virtual void Initialize(DemuxerStream* stream, const PipelineStatusCB& status_cb, const StatisticsCB& statistics_cb) OVERRIDE; - virtual void Decode(const scoped_refptr<DecoderBuffer>& buffer, - const ReadCB& read_cb) OVERRIDE; + virtual void Read(const ReadCB& read_cb) OVERRIDE; virtual void Reset(const base::Closure& closure) OVERRIDE; virtual void Stop(const base::Closure& closure) OVERRIDE; @@ -52,10 +51,15 @@ class MEDIA_EXPORT FFmpegVideoDecoder : public VideoDecoder { kError }; + // Reads from the demuxer stream and corresponding read callback. + void ReadFromDemuxerStream(); + void BufferReady(DemuxerStream::Status status, + const scoped_refptr<DecoderBuffer>& buffer); + // Handles decoding an unencrypted encoded buffer. void DecodeBuffer(const scoped_refptr<DecoderBuffer>& buffer); - bool FFmpegDecode(const scoped_refptr<DecoderBuffer>& buffer, - scoped_refptr<VideoFrame>* video_frame); + bool Decode(const scoped_refptr<DecoderBuffer>& buffer, + scoped_refptr<VideoFrame>* video_frame); // Handles (re-)initializing the decoder with a (new) config. // Returns true if initialization was successful. @@ -83,7 +87,8 @@ class MEDIA_EXPORT FFmpegVideoDecoder : public VideoDecoder { AVCodecContext* codec_context_; AVFrame* av_frame_; - VideoDecoderConfig config_; + // Pointer to the demuxer stream that will feed us compressed buffers. + DemuxerStream* demuxer_stream_; DISALLOW_COPY_AND_ASSIGN(FFmpegVideoDecoder); }; diff --git a/media/filters/ffmpeg_video_decoder_unittest.cc b/media/filters/ffmpeg_video_decoder_unittest.cc index c3f5fd5..f6ac974 100644 --- a/media/filters/ffmpeg_video_decoder_unittest.cc +++ b/media/filters/ffmpeg_video_decoder_unittest.cc @@ -3,7 +3,6 @@ // found in the LICENSE file. #include <string> -#include <vector> #include "base/bind.h" #include "base/callback_helpers.h" @@ -47,6 +46,7 @@ class FFmpegVideoDecoderTest : public testing::Test { public: FFmpegVideoDecoderTest() : decoder_(new FFmpegVideoDecoder(message_loop_.message_loop_proxy())), + demuxer_(new StrictMock<MockDemuxerStream>(DemuxerStream::VIDEO)), read_cb_(base::Bind(&FFmpegVideoDecoderTest::FrameReady, base::Unretained(this))) { FFmpegGlue::InitializeFFmpeg(); @@ -68,9 +68,11 @@ class FFmpegVideoDecoderTest : public testing::Test { void InitializeWithConfigAndStatus(const VideoDecoderConfig& config, PipelineStatus status) { - decoder_->Initialize(config, NewExpectedStatusCB(status), + demuxer_->set_video_decoder_config(config); + decoder_->Initialize(demuxer_.get(), NewExpectedStatusCB(status), base::Bind(&MockStatisticsCB::OnStatistics, base::Unretained(&statistics_cb_))); + message_loop_.RunUntilIdle(); } @@ -107,58 +109,14 @@ class FFmpegVideoDecoderTest : public testing::Test { // Sets up expectations and actions to put FFmpegVideoDecoder in an end // of stream state. void EnterEndOfStreamState() { - VideoDecoder::Status status; scoped_refptr<VideoFrame> video_frame; - DecodeSingleFrame(end_of_stream_buffer_, &status, &video_frame); + VideoDecoder::Status status; + Read(&status, &video_frame); EXPECT_EQ(VideoDecoder::kOk, status); ASSERT_TRUE(video_frame.get()); EXPECT_TRUE(video_frame->IsEndOfStream()); } - typedef std::vector<scoped_refptr<DecoderBuffer> > InputBuffers; - typedef std::vector<scoped_refptr<VideoFrame> > OutputFrames; - - // Decodes all buffers in |input_buffers| and push all successfully decoded - // output frames (excluding EOS frames) into |output_frames|. - // Returns the last decode status returned by the decoder. - VideoDecoder::Status DecodeMultipleFrames(const InputBuffers& input_buffers, - OutputFrames* output_frames) { - InputBuffers::const_iterator input_iter = input_buffers.begin(); - - for (;;) { - // Prepare input buffer. - scoped_refptr<DecoderBuffer> buffer; - if (input_iter != input_buffers.end()) { - buffer = *input_iter; - ++input_iter; - } else { - buffer = end_of_stream_buffer_; - } - - VideoDecoder::Status status; - scoped_refptr<VideoFrame> frame; - Decode(buffer, &status, &frame); - - switch (status) { - case VideoDecoder::kOk: - DCHECK(frame); - if (!frame->IsEndOfStream()) { - output_frames->push_back(frame); - continue; - } else { // EOS - return status; - } - case VideoDecoder::kNotEnoughData: - DCHECK(!frame); - continue; - case VideoDecoder::kDecodeError: - case VideoDecoder::kDecryptError: - DCHECK(!frame); - return status; - } - } - } - // Decodes the single compressed frame in |buffer| and writes the // uncompressed output to |video_frame|. This method works with single // and multithreaded decoders. End of stream buffers are used to trigger @@ -166,23 +124,13 @@ class FFmpegVideoDecoderTest : public testing::Test { void DecodeSingleFrame(const scoped_refptr<DecoderBuffer>& buffer, VideoDecoder::Status* status, scoped_refptr<VideoFrame>* video_frame) { - InputBuffers input_buffers; - input_buffers.push_back(buffer); - - if (!buffer->IsEndOfStream()) - EXPECT_CALL(statistics_cb_, OnStatistics(_)); + EXPECT_CALL(*demuxer_, Read(_)) + .WillOnce(ReturnBuffer(buffer)) + .WillRepeatedly(ReturnBuffer(end_of_stream_buffer_)); - OutputFrames output_frames; - *status = DecodeMultipleFrames(input_buffers, &output_frames); + EXPECT_CALL(statistics_cb_, OnStatistics(_)); - if (*status != VideoDecoder::kOk) - return; - - ASSERT_LE(output_frames.size(), 1U); - if (output_frames.size() == 1U) - *video_frame = output_frames[0]; - else - *video_frame = VideoFrame::CreateEmptyFrame(); + Read(status, video_frame); } // Decodes |i_frame_buffer_| and then decodes the data contained in @@ -192,40 +140,44 @@ class FFmpegVideoDecoderTest : public testing::Test { int expected_width, int expected_height) { Initialize(); + + VideoDecoder::Status status_a; + VideoDecoder::Status status_b; + scoped_refptr<VideoFrame> video_frame_a; + scoped_refptr<VideoFrame> video_frame_b; + scoped_refptr<DecoderBuffer> buffer = ReadTestDataFile(test_file_name); - InputBuffers input_buffers; - input_buffers.push_back(i_frame_buffer_); - input_buffers.push_back(buffer); + EXPECT_CALL(*demuxer_, Read(_)) + .WillOnce(ReturnBuffer(i_frame_buffer_)) + .WillOnce(ReturnBuffer(buffer)) + .WillRepeatedly(ReturnBuffer(end_of_stream_buffer_)); EXPECT_CALL(statistics_cb_, OnStatistics(_)) .Times(2); - OutputFrames output_frames; - VideoDecoder::Status status = - DecodeMultipleFrames(input_buffers, &output_frames); - - EXPECT_EQ(VideoDecoder::kOk, status); - ASSERT_EQ(2U, output_frames.size()); + Read(&status_a, &video_frame_a); + Read(&status_b, &video_frame_b); gfx::Size original_size = kVisibleRect.size(); + EXPECT_EQ(VideoDecoder::kOk, status_a); + EXPECT_EQ(VideoDecoder::kOk, status_b); + ASSERT_TRUE(video_frame_a.get()); + ASSERT_TRUE(video_frame_b.get()); EXPECT_EQ(original_size.width(), - output_frames[0]->visible_rect().size().width()); + video_frame_a->visible_rect().size().width()); EXPECT_EQ(original_size.height(), - output_frames[0]->visible_rect().size().height()); - EXPECT_EQ(expected_width, - output_frames[1]->visible_rect().size().width()); - EXPECT_EQ(expected_height, - output_frames[1]->visible_rect().size().height()); + video_frame_a->visible_rect().size().height()); + EXPECT_EQ(expected_width, video_frame_b->visible_rect().size().width()); + EXPECT_EQ(expected_height, video_frame_b->visible_rect().size().height()); } - void Decode(const scoped_refptr<DecoderBuffer>& buffer, - VideoDecoder::Status* status, - scoped_refptr<VideoFrame>* video_frame) { + void Read(VideoDecoder::Status* status, + scoped_refptr<VideoFrame>* video_frame) { EXPECT_CALL(*this, FrameReady(_, _)) .WillOnce(DoAll(SaveArg<0>(status), SaveArg<1>(video_frame))); - decoder_->Decode(buffer, read_cb_); + decoder_->Read(read_cb_); message_loop_.RunUntilIdle(); } @@ -235,6 +187,7 @@ class FFmpegVideoDecoderTest : public testing::Test { base::MessageLoop message_loop_; scoped_ptr<FFmpegVideoDecoder> decoder_; + scoped_ptr<StrictMock<MockDemuxerStream> > demuxer_; MockStatisticsCB statistics_cb_; VideoDecoder::ReadCB read_cb_; @@ -260,7 +213,7 @@ TEST_F(FFmpegVideoDecoderTest, Initialize_UnsupportedDecoder) { } TEST_F(FFmpegVideoDecoderTest, Initialize_UnsupportedPixelFormat) { - // Ensure decoder handles unsupported pixel formats without crashing. + // Ensure decoder handles unsupport pixel formats without crashing. VideoDecoderConfig config(kCodecVP8, VIDEO_CODEC_PROFILE_UNKNOWN, VideoFrame::INVALID, kCodedSize, kVisibleRect, kNaturalSize, @@ -378,46 +331,63 @@ TEST_F(FFmpegVideoDecoderTest, DecodeFrame_0ByteFrame) { scoped_refptr<DecoderBuffer> zero_byte_buffer = new DecoderBuffer(0); - InputBuffers input_buffers; - input_buffers.push_back(i_frame_buffer_); - input_buffers.push_back(zero_byte_buffer); - input_buffers.push_back(i_frame_buffer_); + VideoDecoder::Status status_a; + VideoDecoder::Status status_b; + VideoDecoder::Status status_c; + scoped_refptr<VideoFrame> video_frame_a; + scoped_refptr<VideoFrame> video_frame_b; + scoped_refptr<VideoFrame> video_frame_c; + + EXPECT_CALL(*demuxer_, Read(_)) + .WillOnce(ReturnBuffer(i_frame_buffer_)) + .WillOnce(ReturnBuffer(zero_byte_buffer)) + .WillOnce(ReturnBuffer(i_frame_buffer_)) + .WillRepeatedly(ReturnBuffer(end_of_stream_buffer_)); EXPECT_CALL(statistics_cb_, OnStatistics(_)) .Times(2); - OutputFrames output_frames; - VideoDecoder::Status status = - DecodeMultipleFrames(input_buffers, &output_frames); + Read(&status_a, &video_frame_a); + Read(&status_b, &video_frame_b); + Read(&status_c, &video_frame_c); - EXPECT_EQ(VideoDecoder::kOk, status); - ASSERT_EQ(2U, output_frames.size()); + EXPECT_EQ(VideoDecoder::kOk, status_a); + EXPECT_EQ(VideoDecoder::kOk, status_b); + EXPECT_EQ(VideoDecoder::kOk, status_c); + + ASSERT_TRUE(video_frame_a.get()); + ASSERT_TRUE(video_frame_b.get()); + ASSERT_TRUE(video_frame_c.get()); - EXPECT_FALSE(output_frames[0]->IsEndOfStream()); - EXPECT_FALSE(output_frames[1]->IsEndOfStream()); + EXPECT_FALSE(video_frame_a->IsEndOfStream()); + EXPECT_FALSE(video_frame_b->IsEndOfStream()); + EXPECT_TRUE(video_frame_c->IsEndOfStream()); } TEST_F(FFmpegVideoDecoderTest, DecodeFrame_DecodeError) { Initialize(); - InputBuffers input_buffers; - input_buffers.push_back(corrupt_i_frame_buffer_); - input_buffers.push_back(i_frame_buffer_); - input_buffers.push_back(i_frame_buffer_); + EXPECT_CALL(*demuxer_, Read(_)) + .WillOnce(ReturnBuffer(corrupt_i_frame_buffer_)) + .WillRepeatedly(ReturnBuffer(i_frame_buffer_)); // The error is only raised on the second decode attempt, so we expect at // least one successful decode but we don't expect FrameReady() to be // executed as an error is raised instead. EXPECT_CALL(statistics_cb_, OnStatistics(_)); - OutputFrames output_frames; - VideoDecoder::Status status = - DecodeMultipleFrames(input_buffers, &output_frames); + // Our read should still get satisfied with end of stream frame during an + // error. + VideoDecoder::Status status; + scoped_refptr<VideoFrame> video_frame; + Read(&status, &video_frame); + EXPECT_EQ(VideoDecoder::kDecodeError, status); + EXPECT_FALSE(video_frame.get()); + // After a decode error occurred, all following read will return kDecodeError. + Read(&status, &video_frame); EXPECT_EQ(VideoDecoder::kDecodeError, status); - // After a decode error occurred, all following decodes will return - // kDecodeError. Therefore, no |output_frames| will be available. - ASSERT_TRUE(output_frames.empty()); + EXPECT_FALSE(video_frame.get()); } // Multi-threaded decoders have different behavior than single-threaded @@ -482,6 +452,25 @@ TEST_F(FFmpegVideoDecoderTest, Reset_EndOfStream) { Reset(); } +// Test resetting when there is a pending read on the demuxer. +TEST_F(FFmpegVideoDecoderTest, Reset_DuringPendingRead) { + Initialize(); + + // Request a read on the decoder and ensure the demuxer has been called. + DemuxerStream::ReadCB read_cb; + EXPECT_CALL(*demuxer_, Read(_)) + .WillOnce(SaveArg<0>(&read_cb)); + decoder_->Read(read_cb_); + ASSERT_FALSE(read_cb.is_null()); + + // Reset the decoder. + Reset(); + + EXPECT_CALL(*this, FrameReady(VideoDecoder::kOk, IsNull())); + + read_cb.Run(DemuxerStream::kOk, i_frame_buffer_); +} + // Test stopping when decoder has initialized but not decoded. TEST_F(FFmpegVideoDecoderTest, Stop_Initialized) { Initialize(); @@ -503,4 +492,77 @@ TEST_F(FFmpegVideoDecoderTest, Stop_EndOfStream) { Stop(); } +// Test stopping when there is a pending read on the demuxer. +TEST_F(FFmpegVideoDecoderTest, Stop_DuringPendingRead) { + Initialize(); + + // Request a read on the decoder and ensure the demuxer has been called. + DemuxerStream::ReadCB read_cb; + EXPECT_CALL(*demuxer_, Read(_)) + .WillOnce(SaveArg<0>(&read_cb)); + decoder_->Read(read_cb_); + ASSERT_FALSE(read_cb.is_null()); + + EXPECT_CALL(*this, FrameReady(VideoDecoder::kOk, IsNull())); + + Stop(); + + read_cb.Run(DemuxerStream::kOk, i_frame_buffer_); +} + +// Test stopping when there is a pending read on the demuxer. +TEST_F(FFmpegVideoDecoderTest, Stop_DuringPendingReadAndReset) { + Initialize(); + + // Request a read on the decoder and ensure the demuxer has been called. + DemuxerStream::ReadCB read_cb; + EXPECT_CALL(*demuxer_, Read(_)) + .WillOnce(SaveArg<0>(&read_cb)); + decoder_->Read(read_cb_); + ASSERT_FALSE(read_cb.is_null()); + + Reset(); + + EXPECT_CALL(*this, FrameReady(VideoDecoder::kOk, IsNull())); + Stop(); + + read_cb.Run(DemuxerStream::kOk, i_frame_buffer_); +} + +// Test aborted read on the demuxer stream. +TEST_F(FFmpegVideoDecoderTest, DemuxerRead_Aborted) { + Initialize(); + + EXPECT_CALL(*demuxer_, Read(_)) + .WillOnce(ReturnBuffer(scoped_refptr<DecoderBuffer>())); + + VideoDecoder::Status status; + scoped_refptr<VideoFrame> video_frame; + + Read(&status, &video_frame); + + EXPECT_EQ(VideoDecoder::kOk, status); + EXPECT_FALSE(video_frame.get()); +} + +// Test aborted read on the demuxer stream during pending reset. +TEST_F(FFmpegVideoDecoderTest, DemuxerRead_AbortedDuringReset) { + Initialize(); + + // Request a read on the decoder and ensure the demuxer has been called. + DemuxerStream::ReadCB read_cb; + EXPECT_CALL(*demuxer_, Read(_)) + .WillOnce(SaveArg<0>(&read_cb)); + decoder_->Read(read_cb_); + ASSERT_FALSE(read_cb.is_null()); + + // Reset while there is still an outstanding read on the demuxer. + Reset(); + + // Signal an aborted demuxer read: a NULL video frame should be returned. + EXPECT_CALL(*this, FrameReady(VideoDecoder::kOk, IsNull())); + read_cb.Run(DemuxerStream::kAborted, NULL); + message_loop_.RunUntilIdle(); +} + } // namespace media diff --git a/media/filters/gpu_video_decoder.cc b/media/filters/gpu_video_decoder.cc index fcb837d..8cf51ad 100644 --- a/media/filters/gpu_video_decoder.cc +++ b/media/filters/gpu_video_decoder.cc @@ -4,8 +4,6 @@ #include "media/filters/gpu_video_decoder.h" -#include <algorithm> - #include "base/bind.h" #include "base/callback_helpers.h" #include "base/cpu.h" @@ -14,6 +12,7 @@ #include "base/task_runner_util.h" #include "media/base/bind_to_loop.h" #include "media/base/decoder_buffer.h" +#include "media/base/demuxer_stream.h" #include "media/base/pipeline.h" #include "media/base/pipeline_status.h" #include "media/base/video_decoder_config.h" @@ -161,12 +160,14 @@ GpuVideoDecoder::BufferData::~BufferData() {} GpuVideoDecoder::GpuVideoDecoder( const scoped_refptr<base::MessageLoopProxy>& message_loop, const scoped_refptr<Factories>& factories) - : needs_bitstream_conversion_(false), + : demuxer_stream_(NULL), + needs_bitstream_conversion_(false), gvd_loop_proxy_(message_loop), weak_factory_(this), vda_loop_proxy_(factories->GetMessageLoop()), factories_(factories), state_(kNormal), + demuxer_read_in_progress_(false), decoder_texture_target_(0), next_picture_buffer_id_(0), next_bitstream_buffer_id_(0), @@ -175,7 +176,6 @@ GpuVideoDecoder::GpuVideoDecoder( } void GpuVideoDecoder::Reset(const base::Closure& closure) { - DVLOG(3) << "Reset()"; DCHECK(gvd_loop_proxy_->BelongsToCurrentThread()); if (state_ == kDrainingDecoder && !factories_->IsAborted()) { @@ -214,6 +214,7 @@ void GpuVideoDecoder::Stop(const base::Closure& closure) { EnqueueFrameAndTriggerFrameDelivery(VideoFrame::CreateEmptyFrame()); if (!pending_reset_cb_.is_null()) base::ResetAndReturn(&pending_reset_cb_).Run(); + demuxer_stream_ = NULL; BindToCurrentLoop(closure).Run(); } @@ -233,13 +234,11 @@ static bool IsCodedSizeSupported(const gfx::Size& coded_size) { return os_large_video_support && hw_large_video_support; } -void GpuVideoDecoder::Initialize(const VideoDecoderConfig& config, +void GpuVideoDecoder::Initialize(DemuxerStream* stream, const PipelineStatusCB& orig_status_cb, const StatisticsCB& statistics_cb) { - DVLOG(3) << "Initialize()"; DCHECK(gvd_loop_proxy_->BelongsToCurrentThread()); - DCHECK(config.IsValidConfig()); - DCHECK(!config.is_encrypted()); + DCHECK(stream); weak_this_ = weak_factory_.GetWeakPtr(); @@ -247,7 +246,7 @@ void GpuVideoDecoder::Initialize(const VideoDecoderConfig& config, "Media.GpuVideoDecoderInitializeStatus", BindToCurrentLoop(orig_status_cb)); - if (config_.IsValidConfig()) { + if (demuxer_stream_) { // TODO(xhwang): Make GpuVideoDecoder reinitializable. // See http://crbug.com/233608 DVLOG(1) << "GpuVideoDecoder reinitialization not supported."; @@ -255,6 +254,10 @@ void GpuVideoDecoder::Initialize(const VideoDecoderConfig& config, return; } + const VideoDecoderConfig& config = stream->video_decoder_config(); + DCHECK(config.IsValidConfig()); + DCHECK(!config.is_encrypted()); + if (!IsCodedSizeSupported(config.coded_size())) { status_cb.Run(DECODER_ERROR_NOT_SUPPORTED); return; @@ -268,11 +271,11 @@ void GpuVideoDecoder::Initialize(const VideoDecoderConfig& config, return; } - config_ = config; + demuxer_stream_ = stream; statistics_cb_ = statistics_cb; needs_bitstream_conversion_ = (config.codec() == kCodecH264); - DVLOG(3) << "GpuVideoDecoder::Initialize() succeeded."; + DVLOG(1) << "GpuVideoDecoder::Initialize() succeeded."; PostTaskAndReplyWithResult( vda_loop_proxy_.get(), FROM_HERE, @@ -330,37 +333,72 @@ void GpuVideoDecoder::DestroyVDA() { DestroyTextures(); } -void GpuVideoDecoder::Decode(const scoped_refptr<DecoderBuffer>& buffer, - const ReadCB& read_cb) { +void GpuVideoDecoder::Read(const ReadCB& read_cb) { DCHECK(gvd_loop_proxy_->BelongsToCurrentThread()); DCHECK(pending_reset_cb_.is_null()); DCHECK(pending_read_cb_.is_null()); - pending_read_cb_ = BindToCurrentLoop(read_cb); - if (state_ == kError || !vda_) { + if (state_ == kError) { base::ResetAndReturn(&pending_read_cb_).Run(kDecodeError, NULL); return; } + // TODO(xhwang): It's odd that we return kOk after VDA has been released. + // Fix this and simplify cases. + if (!vda_) { + base::ResetAndReturn(&pending_read_cb_).Run( + kOk, VideoFrame::CreateEmptyFrame()); + return; + } + + if (!ready_video_frames_.empty()) { + EnqueueFrameAndTriggerFrameDelivery(NULL); + return; + } + switch (state_) { case kDecoderDrained: - if (!ready_video_frames_.empty()) { - EnqueueFrameAndTriggerFrameDelivery(NULL); - return; - } state_ = kNormal; // Fall-through. case kNormal: + EnsureDemuxOrDecode(); break; case kDrainingDecoder: - DCHECK(buffer->IsEndOfStream()); // Do nothing. Will be satisfied either by a PictureReady or // NotifyFlushDone below. - return; + break; case kError: NOTREACHED(); + break; + } +} + +bool GpuVideoDecoder::CanMoreDecodeWorkBeDone() { + return bitstream_buffers_in_decoder_.size() < kMaxInFlightDecodes; +} + +void GpuVideoDecoder::RequestBufferDecode( + DemuxerStream::Status status, + const scoped_refptr<DecoderBuffer>& buffer) { + DCHECK(gvd_loop_proxy_->BelongsToCurrentThread()); + DCHECK_EQ(status != DemuxerStream::kOk, !buffer.get()) << status; + + demuxer_read_in_progress_ = false; + + if (status == DemuxerStream::kAborted) { + if (pending_read_cb_.is_null()) return; + base::ResetAndReturn(&pending_read_cb_).Run(kOk, NULL); + return; + } + + // VideoFrameStream ensures no kConfigChanged is passed to VideoDecoders. + DCHECK_EQ(status, DemuxerStream::kOk) << status; + + if (!vda_) { + EnqueueFrameAndTriggerFrameDelivery(VideoFrame::CreateEmptyFrame()); + return; } if (buffer->IsEndOfStream()) { @@ -372,12 +410,13 @@ void GpuVideoDecoder::Decode(const scoped_refptr<DecoderBuffer>& buffer, return; } + if (!pending_reset_cb_.is_null()) + return; + size_t size = buffer->GetDataSize(); SHMBuffer* shm_buffer = GetSHM(size); - if (!shm_buffer) { - base::ResetAndReturn(&pending_read_cb_).Run(kDecodeError, NULL); + if (!shm_buffer) return; - } memcpy(shm_buffer->shm->memory(), buffer->GetData(), size); BitstreamBuffer bitstream_buffer( @@ -392,25 +431,19 @@ void GpuVideoDecoder::Decode(const scoped_refptr<DecoderBuffer>& buffer, vda_loop_proxy_->PostTask(FROM_HERE, base::Bind( &VideoDecodeAccelerator::Decode, weak_vda_, bitstream_buffer)); - if (!ready_video_frames_.empty()) { - EnqueueFrameAndTriggerFrameDelivery(NULL); - return; + if (CanMoreDecodeWorkBeDone()) { + // Force post here to prevent reentrancy into DemuxerStream. + gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind( + &GpuVideoDecoder::EnsureDemuxOrDecode, weak_this_)); } - - if (CanMoreDecodeWorkBeDone()) - base::ResetAndReturn(&pending_read_cb_).Run(kNotEnoughData, NULL); -} - -bool GpuVideoDecoder::CanMoreDecodeWorkBeDone() { - return bitstream_buffers_in_decoder_.size() < kMaxInFlightDecodes; } -void GpuVideoDecoder::RecordBufferData(const BitstreamBuffer& bitstream_buffer, - const DecoderBuffer& buffer) { - input_buffer_data_.push_front(BufferData(bitstream_buffer.id(), - buffer.GetTimestamp(), - config_.visible_rect(), - config_.natural_size())); +void GpuVideoDecoder::RecordBufferData( + const BitstreamBuffer& bitstream_buffer, const DecoderBuffer& buffer) { + input_buffer_data_.push_front(BufferData( + bitstream_buffer.id(), buffer.GetTimestamp(), + demuxer_stream_->video_decoder_config().visible_rect(), + demuxer_stream_->video_decoder_config().natural_size())); // Why this value? Because why not. avformat.h:MAX_REORDER_DELAY is 16, but // that's too small for some pathological B-frame test videos. The cost of // using too-high a value is low (192 bits per extra slot). @@ -459,8 +492,6 @@ void GpuVideoDecoder::NotifyInitializeDone() { void GpuVideoDecoder::ProvidePictureBuffers(uint32 count, const gfx::Size& size, uint32 texture_target) { - DVLOG(3) << "ProvidePictureBuffers(" << count << ", " - << size.width() << "x" << size.height() << ")"; DCHECK(gvd_loop_proxy_->BelongsToCurrentThread()); std::vector<uint32> texture_ids; @@ -499,7 +530,6 @@ void GpuVideoDecoder::ProvidePictureBuffers(uint32 count, } void GpuVideoDecoder::DismissPictureBuffer(int32 id) { - DVLOG(3) << "DismissPictureBuffer(" << id << ")"; DCHECK(gvd_loop_proxy_->BelongsToCurrentThread()); std::map<int32, PictureBuffer>::iterator it = @@ -529,7 +559,6 @@ void GpuVideoDecoder::DismissPictureBuffer(int32 id) { } void GpuVideoDecoder::PictureReady(const media::Picture& picture) { - DVLOG(3) << "PictureReady()"; DCHECK(gvd_loop_proxy_->BelongsToCurrentThread()); std::map<int32, PictureBuffer>::iterator it = @@ -596,7 +625,6 @@ void GpuVideoDecoder::EnqueueFrameAndTriggerFrameDelivery( void GpuVideoDecoder::ReusePictureBuffer(int64 picture_buffer_id, uint32 sync_point) { - DVLOG(3) << "ReusePictureBuffer(" << picture_buffer_id << ")"; DCHECK(gvd_loop_proxy_->BelongsToCurrentThread()); if (!vda_) @@ -620,6 +648,7 @@ void GpuVideoDecoder::ReusePictureBuffer(int64 picture_buffer_id, } factories_->WaitSyncPoint(sync_point); + ++available_pictures_; vda_loop_proxy_->PostTask(FROM_HERE, base::Bind( @@ -649,7 +678,6 @@ void GpuVideoDecoder::PutSHM(SHMBuffer* shm_buffer) { } void GpuVideoDecoder::NotifyEndOfBitstreamBuffer(int32 id) { - DVLOG(3) << "NotifyEndOfBitstreamBuffer(" << id << ")"; DCHECK(gvd_loop_proxy_->BelongsToCurrentThread()); std::map<int32, BufferPair>::iterator it = @@ -670,8 +698,8 @@ void GpuVideoDecoder::NotifyEndOfBitstreamBuffer(int32 id) { bitstream_buffers_in_decoder_.erase(it); if (pending_reset_cb_.is_null() && state_ != kDrainingDecoder && - CanMoreDecodeWorkBeDone() && !pending_read_cb_.is_null()) { - base::ResetAndReturn(&pending_read_cb_).Run(kNotEnoughData, NULL); + CanMoreDecodeWorkBeDone()) { + EnsureDemuxOrDecode(); } } @@ -693,8 +721,22 @@ GpuVideoDecoder::~GpuVideoDecoder() { DestroyTextures(); } +void GpuVideoDecoder::EnsureDemuxOrDecode() { + DCHECK(gvd_loop_proxy_->BelongsToCurrentThread()); + + // The second condition can happen during the tear-down process. + // GpuVideoDecoder::Stop() returns the |pending_read_cb_| immediately without + // waiting for the demuxer read to be returned. Therefore, this function could + // be called even after the decoder has been stopped. + if (demuxer_read_in_progress_ || !demuxer_stream_) + return; + + demuxer_read_in_progress_ = true; + demuxer_stream_->Read(base::Bind( + &GpuVideoDecoder::RequestBufferDecode, weak_this_)); +} + void GpuVideoDecoder::NotifyFlushDone() { - DVLOG(3) << "NotifyFlushDone()"; DCHECK(gvd_loop_proxy_->BelongsToCurrentThread()); DCHECK_EQ(state_, kDrainingDecoder); state_ = kDecoderDrained; @@ -702,7 +744,6 @@ void GpuVideoDecoder::NotifyFlushDone() { } void GpuVideoDecoder::NotifyResetDone() { - DVLOG(3) << "NotifyResetDone()"; DCHECK(gvd_loop_proxy_->BelongsToCurrentThread()); DCHECK(ready_video_frames_.empty()); diff --git a/media/filters/gpu_video_decoder.h b/media/filters/gpu_video_decoder.h index 90f336d..225e19d 100644 --- a/media/filters/gpu_video_decoder.h +++ b/media/filters/gpu_video_decoder.h @@ -13,6 +13,7 @@ #include "base/memory/weak_ptr.h" #include "media/base/pipeline_status.h" +#include "media/base/demuxer_stream.h" #include "media/base/video_decoder.h" #include "media/video/video_decode_accelerator.h" @@ -81,11 +82,10 @@ class MEDIA_EXPORT GpuVideoDecoder const scoped_refptr<Factories>& factories); // VideoDecoder implementation. - virtual void Initialize(const VideoDecoderConfig& config, + virtual void Initialize(DemuxerStream* stream, const PipelineStatusCB& status_cb, const StatisticsCB& statistics_cb) OVERRIDE; - virtual void Decode(const scoped_refptr<DecoderBuffer>& buffer, - const ReadCB& read_cb) OVERRIDE; + virtual void Read(const ReadCB& read_cb) OVERRIDE; virtual void Reset(const base::Closure& closure) OVERRIDE; virtual void Stop(const base::Closure& closure) OVERRIDE; virtual bool HasAlpha() const OVERRIDE; @@ -115,9 +115,17 @@ class MEDIA_EXPORT GpuVideoDecoder kError }; + // If no demuxer read is in flight and no bitstream buffers are in the + // decoder, kick some off demuxing/decoding. + void EnsureDemuxOrDecode(); + // Return true if more decode work can be piled on to the VDA. bool CanMoreDecodeWorkBeDone(); + // Callback to pass to demuxer_stream_->Read() for receiving encoded bits. + void RequestBufferDecode(DemuxerStream::Status status, + const scoped_refptr<DecoderBuffer>& buffer); + // Enqueue a frame for later delivery (or drop it on the floor if a // vda->Reset() is in progress) and trigger out-of-line delivery of the oldest // ready frame to the client if there is a pending read. A NULL |frame| @@ -162,6 +170,8 @@ class MEDIA_EXPORT GpuVideoDecoder StatisticsCB statistics_cb_; + // Pointer to the demuxer stream that will feed us compressed buffers. + DemuxerStream* demuxer_stream_; bool needs_bitstream_conversion_; @@ -195,8 +205,6 @@ class MEDIA_EXPORT GpuVideoDecoder State state_; - VideoDecoderConfig config_; - // Is a demuxer read in flight? bool demuxer_read_in_progress_; diff --git a/media/filters/video_decoder_selector.cc b/media/filters/video_decoder_selector.cc index 1c222f5..d62b295 100644 --- a/media/filters/video_decoder_selector.cc +++ b/media/filters/video_decoder_selector.cc @@ -68,7 +68,7 @@ void VideoDecoderSelector::SelectVideoDecoder( message_loop_, set_decryptor_ready_cb_)); video_decoder_->Initialize( - input_stream_->video_decoder_config(), + input_stream_, BindToCurrentLoop(base::Bind( &VideoDecoderSelector::DecryptingVideoDecoderInitDone, weak_ptr_factory_.GetWeakPtr())), @@ -122,7 +122,7 @@ void VideoDecoderSelector::InitializeDecoder( } (*iter)->Initialize( - input_stream_->video_decoder_config(), + input_stream_, BindToCurrentLoop(base::Bind( &VideoDecoderSelector::DecoderInitDone, weak_ptr_factory_.GetWeakPtr(), diff --git a/media/filters/video_frame_stream.cc b/media/filters/video_frame_stream.cc index a302827..28aaac6 100644 --- a/media/filters/video_frame_stream.cc +++ b/media/filters/video_frame_stream.cc @@ -49,8 +49,7 @@ void VideoFrameStream::Initialize(DemuxerStream* stream, stream_ = stream; state_ = STATE_INITIALIZING; - // TODO(xhwang): VideoDecoderSelector only needs a config to select a decoder. - decoder_selector_->SelectVideoDecoder(stream, statistics_cb, base::Bind( + decoder_selector_->SelectVideoDecoder(this, statistics_cb, base::Bind( &VideoFrameStream::OnDecoderSelected, weak_this_)); } @@ -64,22 +63,24 @@ void VideoFrameStream::ReadFrame(const VideoDecoder::ReadCB& read_cb) { DCHECK(reset_cb_.is_null()); DCHECK(stop_cb_.is_null()); - read_cb_ = read_cb; - if (state_ == STATE_ERROR) { message_loop_->PostTask(FROM_HERE, base::Bind( read_cb, VideoDecoder::kDecodeError, scoped_refptr<VideoFrame>())); return; } - if (state_ == STATE_FLUSHING_DECODER) { - FlushDecoder(); - return; - } - - ReadFromDemuxerStream(); + read_cb_ = read_cb; + decoder_->Read(base::Bind(&VideoFrameStream::OnFrameReady, weak_this_)); } +// VideoDecoder API guarantees that if VideoDecoder::Reset() is called during +// a pending read, the read callback must be fired before the reset callback is +// fired. Therefore, we can call VideoDecoder::Reset() regardless of if we have +// a pending read and always satisfy the reset callback when the decoder reset +// is finished. The only exception is when Reset() is called during decoder +// reinitialization. In this case we cannot and don't need to reset the decoder. +// We should just wait for the reinitialization to finish to satisfy the reset +// callback. void VideoFrameStream::Reset(const base::Closure& closure) { DCHECK(message_loop_->BelongsToCurrentThread()); DCHECK(state_ != STATE_UNINITIALIZED && state_ != STATE_STOPPED) << state_; @@ -88,21 +89,14 @@ void VideoFrameStream::Reset(const base::Closure& closure) { reset_cb_ = closure; - // During decoder reinitialization, VideoDecoder does not need to be and - // cannot be Reset(). |decrypting_demuxer_stream_| was reset before decoder + // VideoDecoder does not need to be and cannot be Reset() during + // reinitialization. |decrypting_demuxer_stream_| was reset before decoder // reinitialization. - // During pending demuxer read, VideoDecoder will be reset after demuxer read - // is returned (in OnBufferReady()). - if (state_ == STATE_REINITIALIZING_DECODER || - state_ == STATE_PENDING_DEMUXER_READ) { + if (state_ == STATE_REINITIALIZING_DECODER) return; - } - // VideoDecoder API guarantees that if VideoDecoder::Reset() is called during - // a pending decode, the decode callback must be fired before the reset - // callback is fired. Therefore, we can call VideoDecoder::Reset() regardless - // of if we have a pending decode and always satisfy the reset callback when - // the decoder reset is finished. + // We may or may not have pending read, but we'll start to reset everything + // regardless. if (decrypting_demuxer_stream_) { decrypting_demuxer_stream_->Reset(base::Bind( &VideoFrameStream::ResetDecoder, weak_this_)); @@ -119,24 +113,22 @@ void VideoFrameStream::Stop(const base::Closure& closure) { stop_cb_ = closure; - // The stopping process will continue after the pending operation is finished. + // The stopping will continue after all of the following pending callbacks + // (if they are not null) are satisfied. // TODO(xhwang): Now we cannot stop the initialization process through // VideoDecoderSelector. Fix this. See: http://crbug.com/222054 - if (state_ == STATE_INITIALIZING || state_ == STATE_PENDING_DEMUXER_READ) + if (state_ == STATE_INITIALIZING) return; - // VideoDecoder API guarantees that if VideoDecoder::Stop() is called during - // a pending reset or a pending decode, the callbacks are always fired in the - // decode -> reset -> stop order. Therefore, we can call VideoDecoder::Stop() - // regardless of if we have a pending decode or reset and always satisfy the - // stop callback when the decoder decode/reset is finished. + // We may or may not have pending read and/or pending reset, but we'll start + // to stop everything regardless. + if (decrypting_demuxer_stream_) { decrypting_demuxer_stream_->Reset(base::Bind( &VideoFrameStream::StopDecoder, weak_this_)); return; } - // We may not have a |decoder_| if Stop() was called during initialization. if (decoder_) { StopDecoder(); return; @@ -154,15 +146,46 @@ bool VideoFrameStream::CanReadWithoutStalling() const { return decoder_->CanReadWithoutStalling(); } +void VideoFrameStream::Read(const DemuxerStream::ReadCB& demuxer_read_cb) { + DCHECK(message_loop_->BelongsToCurrentThread()); + + if (state_ == STATE_FLUSHING_DECODER) { + message_loop_->PostTask(FROM_HERE, base::Bind( + demuxer_read_cb, DemuxerStream::kOk, DecoderBuffer::CreateEOSBuffer())); + return; + } + + stream_->Read(base::Bind( + &VideoFrameStream::OnBufferReady, weak_this_, demuxer_read_cb)); +} + +AudioDecoderConfig VideoFrameStream::audio_decoder_config() { + DCHECK(message_loop_->BelongsToCurrentThread()); + LOG(FATAL) << "Method audio_decoder_config() called on VideoFrameStream"; + return stream_->audio_decoder_config(); +} + +VideoDecoderConfig VideoFrameStream::video_decoder_config() { + DCHECK(message_loop_->BelongsToCurrentThread()); + return stream_->video_decoder_config(); +} + +DemuxerStream::Type VideoFrameStream::type() { + DCHECK(message_loop_->BelongsToCurrentThread()); + return VIDEO; +} + +void VideoFrameStream::EnableBitstreamConverter() { + DCHECK(message_loop_->BelongsToCurrentThread()); + NOTREACHED(); +} + void VideoFrameStream::OnDecoderSelected( scoped_ptr<VideoDecoder> selected_decoder, scoped_ptr<DecryptingDemuxerStream> decrypting_demuxer_stream) { DCHECK(message_loop_->BelongsToCurrentThread()); DCHECK_EQ(state_, STATE_INITIALIZING) << state_; DCHECK(!init_cb_.is_null()); - DCHECK(read_cb_.is_null()); - DCHECK(reset_cb_.is_null()); - decoder_selector_.reset(); if (!selected_decoder) { @@ -170,12 +193,11 @@ void VideoFrameStream::OnDecoderSelected( base::ResetAndReturn(&init_cb_).Run(false, false); } else { state_ = STATE_NORMAL; - decrypting_demuxer_stream_ = decrypting_demuxer_stream.Pass(); - if (decrypting_demuxer_stream_) - stream_ = decrypting_demuxer_stream_.get(); decoder_ = selected_decoder.Pass(); - if (decoder_->NeedsBitstreamConversion()) + decrypting_demuxer_stream_ = decrypting_demuxer_stream.Pass(); + if (decoder_->NeedsBitstreamConversion()) { stream_->EnableBitstreamConverter(); + } // TODO(xhwang): We assume |decoder_->HasAlpha()| does not change after // reinitialization. Check this condition. base::ResetAndReturn(&init_cb_).Run(true, decoder_->HasAlpha()); @@ -188,136 +210,75 @@ void VideoFrameStream::OnDecoderSelected( } } -void VideoFrameStream::SatisfyRead(VideoDecoder::Status status, - const scoped_refptr<VideoFrame>& frame) { - DCHECK(!read_cb_.is_null()); - base::ResetAndReturn(&read_cb_).Run(status, frame); -} - -void VideoFrameStream::AbortRead() { - SatisfyRead(VideoDecoder::kOk, NULL); -} - -void VideoFrameStream::Decode(const scoped_refptr<DecoderBuffer>& buffer) { - DCHECK(state_ == STATE_NORMAL || state_ == STATE_FLUSHING_DECODER) << state_; - DCHECK(!read_cb_.is_null()); - DCHECK(reset_cb_.is_null()); - DCHECK(stop_cb_.is_null()); - DCHECK(buffer); - - decoder_->Decode(buffer, base::Bind(&VideoFrameStream::OnFrameReady, - weak_this_)); -} - -void VideoFrameStream::FlushDecoder() { - Decode(DecoderBuffer::CreateEOSBuffer()); -} - void VideoFrameStream::OnFrameReady(const VideoDecoder::Status status, const scoped_refptr<VideoFrame>& frame) { DCHECK(state_ == STATE_NORMAL || state_ == STATE_FLUSHING_DECODER) << state_; DCHECK(!read_cb_.is_null()); - if (status == VideoDecoder::kDecodeError || - status == VideoDecoder::kDecryptError) { + if (status != VideoDecoder::kOk) { DCHECK(!frame.get()); state_ = STATE_ERROR; - SatisfyRead(status, NULL); + base::ResetAndReturn(&read_cb_).Run(status, NULL); return; } - // Drop decoding result if Reset()/Stop() was called during decoding. // The stopping/resetting process will be handled when the decoder is // stopped/reset. if (!stop_cb_.is_null() || !reset_cb_.is_null()) { - AbortRead(); + base::ResetAndReturn(&read_cb_).Run(VideoDecoder::kOk, NULL); return; } - // Decoder flushed. Reinitialize the video decoder. + // Decoder flush finished. Reinitialize the video decoder. if (state_ == STATE_FLUSHING_DECODER && status == VideoDecoder::kOk && frame->IsEndOfStream()) { ReinitializeDecoder(); return; } - if (status == VideoDecoder::kNotEnoughData) { - if (state_ == STATE_NORMAL) - ReadFromDemuxerStream(); - else if (state_ == STATE_FLUSHING_DECODER) - FlushDecoder(); - return; - } - - SatisfyRead(status, frame); -} - -void VideoFrameStream::ReadFromDemuxerStream() { - DCHECK_EQ(state_, STATE_NORMAL) << state_; - DCHECK(!read_cb_.is_null()); - DCHECK(reset_cb_.is_null()); - DCHECK(stop_cb_.is_null()); - - state_ = STATE_PENDING_DEMUXER_READ; - stream_->Read(base::Bind(&VideoFrameStream::OnBufferReady, weak_this_)); + base::ResetAndReturn(&read_cb_).Run(status, frame); } void VideoFrameStream::OnBufferReady( + const DemuxerStream::ReadCB& demuxer_read_cb, DemuxerStream::Status status, const scoped_refptr<DecoderBuffer>& buffer) { DCHECK(message_loop_->BelongsToCurrentThread()); - DCHECK_EQ(state_, STATE_PENDING_DEMUXER_READ) << state_; + // VideoFrameStream reads from demuxer stream only when in NORMAL state. + DCHECK_EQ(state_, STATE_NORMAL) << state_; DCHECK_EQ(buffer.get() != NULL, status == DemuxerStream::kOk) << status; - DCHECK(!read_cb_.is_null()); - - state_ = STATE_NORMAL; - - // Reset()/Stop() was postponed during STATE_PENDING_DEMUXER_READ state. - // We need to handle them in this function. - - if (!stop_cb_.is_null()) { - AbortRead(); - if (!reset_cb_.is_null()) - Reset(base::ResetAndReturn(&reset_cb_)); - Stop(base::ResetAndReturn(&stop_cb_)); - return; - } if (status == DemuxerStream::kConfigChanged) { + DVLOG(2) << "OnBufferReady() - kConfigChanged"; state_ = STATE_FLUSHING_DECODER; - if (!reset_cb_.is_null()) { - AbortRead(); - Reset(base::ResetAndReturn(&reset_cb_)); - // Reinitialization will continue after Reset() is done. - } else { - FlushDecoder(); - } - return; - } - - if (!reset_cb_.is_null()) { - AbortRead(); - Reset(base::ResetAndReturn(&reset_cb_)); - return; - } - - if (status == DemuxerStream::kAborted) { - AbortRead(); + demuxer_read_cb.Run(DemuxerStream::kOk, DecoderBuffer::CreateEOSBuffer()); return; } - DCHECK(status == DemuxerStream::kOk) << status; - Decode(buffer); + DCHECK(status == DemuxerStream::kOk || status == DemuxerStream::kAborted); + demuxer_read_cb.Run(status, buffer); } void VideoFrameStream::ReinitializeDecoder() { DCHECK(message_loop_->BelongsToCurrentThread()); DCHECK_EQ(state_, STATE_FLUSHING_DECODER) << state_; - DCHECK(stream_->video_decoder_config().IsValidConfig()); + DemuxerStream* stream = this; + if (decrypting_demuxer_stream_) { + // TODO(xhwang): Remove this hack! Since VideoFrameStream handles + // kConfigChange internally and hides it from downstream filters. The + // DecryptingDemuxerStream never receives kConfigChanged to reset it's + // internal VideoDecoderConfig. Call InitializeDecoderConfig() here + // explicitly to solve this. This will be removed when we separate the + // DemuxerStream from the VideoDecoder. + decrypting_demuxer_stream_->InitializeDecoderConfig(); + stream = decrypting_demuxer_stream_.get(); + } + + DCHECK(stream->video_decoder_config().IsValidConfig()); state_ = STATE_REINITIALIZING_DECODER; decoder_->Initialize( - stream_->video_decoder_config(), + stream, base::Bind(&VideoFrameStream::OnDecoderReinitialized, weak_this_), statistics_cb_); } @@ -326,24 +287,16 @@ void VideoFrameStream::OnDecoderReinitialized(PipelineStatus status) { DCHECK(message_loop_->BelongsToCurrentThread()); DCHECK_EQ(state_, STATE_REINITIALIZING_DECODER) << state_; - // ReinitializeDecoder() can be called in two cases: - // 1, Flushing decoder finished (see OnFrameReady()). - // 2, Reset() was called during flushing decoder (see OnDecoderReset()). - // Also, Reset()/Stop() can be called during pending ReinitializeDecoder(). - // This function needs to handle them all! - state_ = (status == PIPELINE_OK) ? STATE_NORMAL : STATE_ERROR; - if (!read_cb_.is_null() && (!stop_cb_.is_null() || !reset_cb_.is_null())) - AbortRead(); - - if (!reset_cb_.is_null()) + if (!reset_cb_.is_null()) { + if (!read_cb_.is_null()) + base::ResetAndReturn(&read_cb_).Run(VideoDecoder::kOk, NULL); base::ResetAndReturn(&reset_cb_).Run(); - - // If !stop_cb_.is_null(), it will be handled in OnDecoderStopped(). - - if (read_cb_.is_null()) return; + } + + DCHECK(!read_cb_.is_null()); if (!stop_cb_.is_null()) { base::ResetAndReturn(&read_cb_).Run(VideoDecoder::kOk, NULL); @@ -351,16 +304,17 @@ void VideoFrameStream::OnDecoderReinitialized(PipelineStatus status) { } if (state_ == STATE_ERROR) { - SatisfyRead(VideoDecoder::kDecodeError, NULL); + base::ResetAndReturn(&read_cb_).Run(VideoDecoder::kDecodeError, NULL); return; } - ReadFromDemuxerStream(); + decoder_->Read(base::Bind(&VideoFrameStream::OnFrameReady, weak_this_)); } void VideoFrameStream::ResetDecoder() { DCHECK(message_loop_->BelongsToCurrentThread()); - DCHECK(state_ == STATE_NORMAL || state_ == STATE_FLUSHING_DECODER || + DCHECK(state_ == STATE_NORMAL || + state_ == STATE_FLUSHING_DECODER || state_ == STATE_ERROR) << state_; DCHECK(!reset_cb_.is_null()); @@ -369,7 +323,8 @@ void VideoFrameStream::ResetDecoder() { void VideoFrameStream::OnDecoderReset() { DCHECK(message_loop_->BelongsToCurrentThread()); - DCHECK(state_ == STATE_NORMAL || state_ == STATE_FLUSHING_DECODER || + DCHECK(state_ == STATE_NORMAL || + state_ == STATE_FLUSHING_DECODER || state_ == STATE_ERROR) << state_; // If Reset() was called during pending read, read callback should be fired // before the reset callback is fired. diff --git a/media/filters/video_frame_stream.h b/media/filters/video_frame_stream.h index 5673cff..c384bca 100644 --- a/media/filters/video_frame_stream.h +++ b/media/filters/video_frame_stream.h @@ -5,6 +5,8 @@ #ifndef MEDIA_FILTERS_VIDEO_FRAME_STREAM_H_ #define MEDIA_FILTERS_VIDEO_FRAME_STREAM_H_ +#include <list> + #include "base/basictypes.h" #include "base/callback.h" #include "base/compiler_specific.h" @@ -28,7 +30,7 @@ class VideoDecoderSelector; // Wraps a DemuxerStream and a list of VideoDecoders and provides decoded // VideoFrames to its client (e.g. VideoRendererBase). -class MEDIA_EXPORT VideoFrameStream { +class MEDIA_EXPORT VideoFrameStream : public DemuxerStream { public: // Indicates completion of VideoFrameStream initialization. typedef base::Callback<void(bool success, bool has_alpha)> InitCB; @@ -48,7 +50,6 @@ class MEDIA_EXPORT VideoFrameStream { // |read_cb| is always called asynchronously. This method should only be // called after initialization has succeeded and must not be called during // any pending Reset() and/or Stop(). - // TODO(xhwang): Rename this back to Read(). void ReadFrame(const VideoDecoder::ReadCB& read_cb); // Resets the decoder, flushes all decoded frames and/or internal buffers, @@ -69,13 +70,19 @@ class MEDIA_EXPORT VideoFrameStream { // a VideoFrame. bool CanReadWithoutStalling() const; + // DemuxerStream implementation. + virtual void Read(const ReadCB& read_cb) OVERRIDE; + virtual AudioDecoderConfig audio_decoder_config() OVERRIDE; + virtual VideoDecoderConfig video_decoder_config() OVERRIDE; + virtual Type type() OVERRIDE; + virtual void EnableBitstreamConverter() OVERRIDE; + private: enum State { STATE_UNINITIALIZED, STATE_INITIALIZING, - STATE_NORMAL, // Includes idle, pending decoder decode/reset/stop. + STATE_NORMAL, STATE_FLUSHING_DECODER, - STATE_PENDING_DEMUXER_READ, STATE_REINITIALIZING_DECODER, STATE_STOPPED, STATE_ERROR @@ -88,29 +95,13 @@ class MEDIA_EXPORT VideoFrameStream { scoped_ptr<VideoDecoder> selected_decoder, scoped_ptr<DecryptingDemuxerStream> decrypting_demuxer_stream); - // Satisfy pending |read_cb_| with |status| and |frame|. - void SatisfyRead(VideoDecoder::Status status, - const scoped_refptr<VideoFrame>& frame); - - // Abort pending |read_cb_|. - void AbortRead(); - - // Decodes |buffer| and returns the result via OnFrameReady(). - void Decode(const scoped_refptr<DecoderBuffer>& buffer); - - // Flushes the decoder with an EOS buffer to retrieve internally buffered - // video frames. - void FlushDecoder(); - - // Callback for VideoDecoder::Decode(). + // Callback for VideoDecoder::Read(). void OnFrameReady(const VideoDecoder::Status status, const scoped_refptr<VideoFrame>& frame); - // Reads a buffer from |stream_| and returns the result via OnBufferReady(). - void ReadFromDemuxerStream(); - // Callback for DemuxerStream::Read(). - void OnBufferReady(DemuxerStream::Status status, + void OnBufferReady(const DemuxerStream::ReadCB& demuxer_read_cb, + DemuxerStream::Status status, const scoped_refptr<DecoderBuffer>& buffer); void ReinitializeDecoder(); diff --git a/media/filters/video_frame_stream_unittest.cc b/media/filters/video_frame_stream_unittest.cc index eb6ec92..332785cd 100644 --- a/media/filters/video_frame_stream_unittest.cc +++ b/media/filters/video_frame_stream_unittest.cc @@ -279,8 +279,6 @@ TEST_P(VideoFrameStreamTest, Read_AfterReset) { ReadFrame(); } -// No Reset() before initialization is successfully completed. - TEST_P(VideoFrameStreamTest, Reset_AfterInitialization) { Initialize(); Reset(); @@ -295,18 +293,9 @@ TEST_P(VideoFrameStreamTest, Reset_DuringReinitialization) { video_frame_stream_->Reset( base::Bind(&VideoFrameStreamTest::OnReset, base::Unretained(this))); SatisfyPendingCallback(DECODER_REINIT); - ReadFrame(); } -TEST_P(VideoFrameStreamTest, Reset_AfterReinitialization) { - Initialize(); - EnterPendingState(DECODER_REINIT); - SatisfyPendingCallback(DECODER_REINIT); - Reset(); - ReadFrame(); -} - -TEST_P(VideoFrameStreamTest, Reset_DuringDemuxerRead_Normal) { +TEST_P(VideoFrameStreamTest, Reset_DuringNormalDemuxerRead) { Initialize(); EnterPendingState(DEMUXER_READ_NORMAL); EnterPendingState(DECODER_RESET); @@ -315,7 +304,7 @@ TEST_P(VideoFrameStreamTest, Reset_DuringDemuxerRead_Normal) { ReadFrame(); } -TEST_P(VideoFrameStreamTest, Reset_DuringDemuxerRead_ConfigChange) { +TEST_P(VideoFrameStreamTest, Reset_DuringConfigChangeDemuxerRead) { Initialize(); EnterPendingState(DEMUXER_READ_CONFIG_CHANGE); EnterPendingState(DECODER_RESET); @@ -340,7 +329,7 @@ TEST_P(VideoFrameStreamTest, Reset_AfterNormalRead) { ReadFrame(); } -TEST_P(VideoFrameStreamTest, Reset_AfterDemuxerRead_ConfigChange) { +TEST_P(VideoFrameStreamTest, Reset_AfterConfigChangeRead) { Initialize(); EnterPendingState(DEMUXER_READ_CONFIG_CHANGE); SatisfyPendingCallback(DEMUXER_READ_CONFIG_CHANGE); @@ -375,14 +364,7 @@ TEST_P(VideoFrameStreamTest, Stop_DuringReinitialization) { SatisfyPendingCallback(DECODER_STOP); } -TEST_P(VideoFrameStreamTest, Stop_AfterReinitialization) { - Initialize(); - EnterPendingState(DECODER_REINIT); - SatisfyPendingCallback(DECODER_REINIT); - Stop(); -} - -TEST_P(VideoFrameStreamTest, Stop_DuringDemuxerRead_Normal) { +TEST_P(VideoFrameStreamTest, Stop_DuringNormalDemuxerRead) { Initialize(); EnterPendingState(DEMUXER_READ_NORMAL); EnterPendingState(DECODER_STOP); @@ -390,7 +372,7 @@ TEST_P(VideoFrameStreamTest, Stop_DuringDemuxerRead_Normal) { SatisfyPendingCallback(DECODER_STOP); } -TEST_P(VideoFrameStreamTest, Stop_DuringDemuxerRead_ConfigChange) { +TEST_P(VideoFrameStreamTest, Stop_DuringConfigChangeDemuxerRead) { Initialize(); EnterPendingState(DEMUXER_READ_CONFIG_CHANGE); EnterPendingState(DECODER_STOP); diff --git a/media/filters/video_renderer_base_unittest.cc b/media/filters/video_renderer_base_unittest.cc index 8e6dfb7..8044a11 100644 --- a/media/filters/video_renderer_base_unittest.cc +++ b/media/filters/video_renderer_base_unittest.cc @@ -2,8 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include <utility> - #include "base/bind.h" #include "base/callback.h" #include "base/callback_helpers.h" @@ -26,7 +24,6 @@ using ::testing::_; using ::testing::AnyNumber; using ::testing::InSequence; using ::testing::Invoke; -using ::testing::NiceMock; using ::testing::NotNull; using ::testing::Return; using ::testing::StrictMock; @@ -55,9 +52,6 @@ class VideoRendererBaseTest : public ::testing::Test { demuxer_stream_.set_video_decoder_config(TestVideoConfig::Normal()); // We expect these to be called but we don't care how/when. - EXPECT_CALL(demuxer_stream_, Read(_)) - .WillRepeatedly(RunCallback<0>(DemuxerStream::kOk, - DecoderBuffer::CreateEOSBuffer())); EXPECT_CALL(*decoder_, Stop(_)) .WillRepeatedly(Invoke(this, &VideoRendererBaseTest::StopRequested)); EXPECT_CALL(statistics_cb_object_, OnStatistics(_)) @@ -84,8 +78,8 @@ class VideoRendererBaseTest : public ::testing::Test { void InitializeWithDuration(int duration_ms) { duration_ = base::TimeDelta::FromMilliseconds(duration_ms); - // Monitor decodes from the decoder. - EXPECT_CALL(*decoder_, Decode(_, _)) + // Monitor reads from the decoder. + EXPECT_CALL(*decoder_, Read(_)) .WillRepeatedly(Invoke(this, &VideoRendererBaseTest::FrameRequested)); EXPECT_CALL(*decoder_, Reset(_)) @@ -295,7 +289,7 @@ class VideoRendererBaseTest : public ::testing::Test { // Fixture members. scoped_ptr<VideoRendererBase> renderer_; MockVideoDecoder* decoder_; // Owned by |renderer_|. - NiceMock<MockDemuxerStream> demuxer_stream_; + MockDemuxerStream demuxer_stream_; MockStatisticsCB statistics_cb_object_; private: @@ -313,8 +307,7 @@ class VideoRendererBaseTest : public ::testing::Test { current_frame_ = frame; } - void FrameRequested(const scoped_refptr<DecoderBuffer>& buffer, - const VideoDecoder::ReadCB& read_cb) { + void FrameRequested(const VideoDecoder::ReadCB& read_cb) { DCHECK_EQ(&message_loop_, base::MessageLoop::current()); CHECK(read_cb_.is_null()); read_cb_ = read_cb; diff --git a/media/filters/vpx_video_decoder.cc b/media/filters/vpx_video_decoder.cc index 1754f02..4062352 100644 --- a/media/filters/vpx_video_decoder.cc +++ b/media/filters/vpx_video_decoder.cc @@ -4,9 +4,6 @@ #include "media/filters/vpx_video_decoder.h" -#include <algorithm> -#include <string> - #include "base/bind.h" #include "base/callback_helpers.h" #include "base/command_line.h" @@ -64,6 +61,7 @@ VpxVideoDecoder::VpxVideoDecoder( : message_loop_(message_loop), weak_factory_(this), state_(kUninitialized), + demuxer_stream_(NULL), vpx_codec_(NULL), vpx_codec_alpha_(NULL) { } @@ -73,25 +71,26 @@ VpxVideoDecoder::~VpxVideoDecoder() { CloseDecoder(); } -void VpxVideoDecoder::Initialize(const VideoDecoderConfig& config, - const PipelineStatusCB& status_cb, - const StatisticsCB& statistics_cb) { +void VpxVideoDecoder::Initialize( + DemuxerStream* stream, + const PipelineStatusCB& status_cb, + const StatisticsCB& statistics_cb) { DCHECK(message_loop_->BelongsToCurrentThread()); - DCHECK(config.IsValidConfig()); - DCHECK(!config.is_encrypted()); + DCHECK(stream); DCHECK(read_cb_.is_null()); DCHECK(reset_cb_.is_null()); weak_this_ = weak_factory_.GetWeakPtr(); - if (!ConfigureDecoder(config)) { + demuxer_stream_ = stream; + statistics_cb_ = statistics_cb; + + if (!ConfigureDecoder()) { status_cb.Run(DECODER_ERROR_NOT_SUPPORTED); return; } // Success! - config_ = config; - statistics_cb_ = statistics_cb; state_ = kNormal; status_cb.Run(PIPELINE_OK); } @@ -118,7 +117,10 @@ static vpx_codec_ctx* InitializeVpxContext(vpx_codec_ctx* context, return context; } -bool VpxVideoDecoder::ConfigureDecoder(const VideoDecoderConfig& config) { +bool VpxVideoDecoder::ConfigureDecoder() { + const VideoDecoderConfig& config = demuxer_stream_->video_decoder_config(); + DCHECK(config.IsValidConfig()); + const CommandLine* cmd_line = CommandLine::ForCurrentProcess(); bool can_handle = false; if (config.codec() == kCodecVP9) @@ -158,27 +160,25 @@ void VpxVideoDecoder::CloseDecoder() { } } -void VpxVideoDecoder::Decode(const scoped_refptr<DecoderBuffer>& buffer, - const ReadCB& read_cb) { +void VpxVideoDecoder::Read(const ReadCB& read_cb) { DCHECK(message_loop_->BelongsToCurrentThread()); DCHECK(!read_cb.is_null()); CHECK_NE(state_, kUninitialized); CHECK(read_cb_.is_null()) << "Overlapping decodes are not supported."; - read_cb_ = BindToCurrentLoop(read_cb); if (state_ == kError) { - base::ResetAndReturn(&read_cb_).Run(kDecodeError, NULL); + read_cb.Run(kDecodeError, NULL); return; } // Return empty frames if decoding has finished. if (state_ == kDecodeFinished) { - base::ResetAndReturn(&read_cb_).Run(kOk, VideoFrame::CreateEmptyFrame()); + read_cb.Run(kOk, VideoFrame::CreateEmptyFrame()); return; } - DecodeBuffer(buffer); + ReadFromDemuxerStream(); } void VpxVideoDecoder::Reset(const base::Closure& closure) { @@ -210,18 +210,57 @@ void VpxVideoDecoder::Stop(const base::Closure& closure) { state_ = kUninitialized; } +void VpxVideoDecoder::ReadFromDemuxerStream() { + DCHECK_NE(state_, kUninitialized); + DCHECK_NE(state_, kDecodeFinished); + DCHECK_NE(state_, kError); + DCHECK(!read_cb_.is_null()); + + demuxer_stream_->Read(base::Bind( + &VpxVideoDecoder::DoDecryptOrDecodeBuffer, weak_this_)); +} + bool VpxVideoDecoder::HasAlpha() const { return vpx_codec_alpha_ != NULL; } -void VpxVideoDecoder::DecodeBuffer(const scoped_refptr<DecoderBuffer>& buffer) { +void VpxVideoDecoder::DoDecryptOrDecodeBuffer( + DemuxerStream::Status status, + const scoped_refptr<DecoderBuffer>& buffer) { + DCHECK(message_loop_->BelongsToCurrentThread()); + DCHECK_NE(state_, kDecodeFinished); + DCHECK_EQ(status != DemuxerStream::kOk, !buffer.get()) << status; + + if (state_ == kUninitialized) + return; + + DCHECK(!read_cb_.is_null()); + + if (!reset_cb_.is_null()) { + base::ResetAndReturn(&read_cb_).Run(kOk, NULL); + DoReset(); + return; + } + + if (status == DemuxerStream::kAborted) { + base::ResetAndReturn(&read_cb_).Run(kOk, NULL); + return; + } + + // VideoFrameStream ensures no kConfigChanged is passed to VideoDecoders. + DCHECK_EQ(status, DemuxerStream::kOk) << status; + DecodeBuffer(buffer); +} + +void VpxVideoDecoder::DecodeBuffer( + const scoped_refptr<DecoderBuffer>& buffer) { DCHECK(message_loop_->BelongsToCurrentThread()); DCHECK_NE(state_, kUninitialized); DCHECK_NE(state_, kDecodeFinished); DCHECK_NE(state_, kError); DCHECK(reset_cb_.is_null()); DCHECK(!read_cb_.is_null()); - DCHECK(buffer); + DCHECK(buffer.get()); // Transition to kDecodeFinished on the first end of stream buffer. if (state_ == kNormal && buffer->IsEndOfStream()) { @@ -231,7 +270,7 @@ void VpxVideoDecoder::DecodeBuffer(const scoped_refptr<DecoderBuffer>& buffer) { } scoped_refptr<VideoFrame> video_frame; - if (!VpxDecode(buffer, &video_frame)) { + if (!Decode(buffer, &video_frame)) { state_ = kError; base::ResetAndReturn(&read_cb_).Run(kDecodeError, NULL); return; @@ -244,16 +283,18 @@ void VpxVideoDecoder::DecodeBuffer(const scoped_refptr<DecoderBuffer>& buffer) { statistics_cb_.Run(statistics); } + // If we didn't get a frame we need more data. if (!video_frame.get()) { - base::ResetAndReturn(&read_cb_).Run(kNotEnoughData, NULL); + ReadFromDemuxerStream(); return; } base::ResetAndReturn(&read_cb_).Run(kOk, video_frame); } -bool VpxVideoDecoder::VpxDecode(const scoped_refptr<DecoderBuffer>& buffer, - scoped_refptr<VideoFrame>* video_frame) { +bool VpxVideoDecoder::Decode( + const scoped_refptr<DecoderBuffer>& buffer, + scoped_refptr<VideoFrame>* video_frame) { DCHECK(video_frame); DCHECK(!buffer->IsEndOfStream()); @@ -333,9 +374,10 @@ void VpxVideoDecoder::DoReset() { reset_cb_.Reset(); } -void VpxVideoDecoder::CopyVpxImageTo(const vpx_image* vpx_image, - const struct vpx_image* vpx_image_alpha, - scoped_refptr<VideoFrame>* video_frame) { +void VpxVideoDecoder::CopyVpxImageTo( + const vpx_image* vpx_image, + const struct vpx_image* vpx_image_alpha, + scoped_refptr<VideoFrame>* video_frame) { CHECK(vpx_image); CHECK_EQ(vpx_image->d_w % 2, 0U); CHECK_EQ(vpx_image->d_h % 2, 0U); @@ -343,13 +385,16 @@ void VpxVideoDecoder::CopyVpxImageTo(const vpx_image* vpx_image, vpx_image->fmt == VPX_IMG_FMT_YV12); gfx::Size size(vpx_image->d_w, vpx_image->d_h); - - *video_frame = VideoFrame::CreateFrame( - vpx_codec_alpha_ ? VideoFrame::YV12A : VideoFrame::YV12, - size, - gfx::Rect(size), - config_.natural_size(), - kNoTimestamp()); + gfx::Size natural_size = + demuxer_stream_->video_decoder_config().natural_size(); + + *video_frame = VideoFrame::CreateFrame(vpx_codec_alpha_ ? + VideoFrame::YV12A : + VideoFrame::YV12, + size, + gfx::Rect(size), + natural_size, + kNoTimestamp()); CopyYPlane(vpx_image->planes[VPX_PLANE_Y], vpx_image->stride[VPX_PLANE_Y], diff --git a/media/filters/vpx_video_decoder.h b/media/filters/vpx_video_decoder.h index ebceab6..dae98447 100644 --- a/media/filters/vpx_video_decoder.h +++ b/media/filters/vpx_video_decoder.h @@ -9,7 +9,6 @@ #include "base/memory/weak_ptr.h" #include "media/base/demuxer_stream.h" #include "media/base/video_decoder.h" -#include "media/base/video_decoder_config.h" #include "media/base/video_frame.h" struct vpx_codec_ctx; @@ -32,11 +31,10 @@ class MEDIA_EXPORT VpxVideoDecoder : public VideoDecoder { virtual ~VpxVideoDecoder(); // VideoDecoder implementation. - virtual void Initialize(const VideoDecoderConfig& config, + virtual void Initialize(DemuxerStream* stream, const PipelineStatusCB& status_cb, const StatisticsCB& statistics_cb) OVERRIDE; - virtual void Decode(const scoped_refptr<DecoderBuffer>& buffer, - const ReadCB& read_cb) OVERRIDE; + virtual void Read(const ReadCB& read_cb) OVERRIDE; virtual void Reset(const base::Closure& closure) OVERRIDE; virtual void Stop(const base::Closure& closure) OVERRIDE; virtual bool HasAlpha() const OVERRIDE; @@ -52,13 +50,19 @@ class MEDIA_EXPORT VpxVideoDecoder : public VideoDecoder { // Handles (re-)initializing the decoder with a (new) config. // Returns true when initialization was successful. - bool ConfigureDecoder(const VideoDecoderConfig& config); + bool ConfigureDecoder(); void CloseDecoder(); + void ReadFromDemuxerStream(); + + // Carries out the buffer processing operation scheduled by + // DecryptOrDecodeBuffer(). + void DoDecryptOrDecodeBuffer(DemuxerStream::Status status, + const scoped_refptr<DecoderBuffer>& buffer); void DecodeBuffer(const scoped_refptr<DecoderBuffer>& buffer); - bool VpxDecode(const scoped_refptr<DecoderBuffer>& buffer, - scoped_refptr<VideoFrame>* video_frame); + bool Decode(const scoped_refptr<DecoderBuffer>& buffer, + scoped_refptr<VideoFrame>* video_frame); // Reset decoder and call |reset_cb_|. void DoReset(); @@ -77,7 +81,8 @@ class MEDIA_EXPORT VpxVideoDecoder : public VideoDecoder { ReadCB read_cb_; base::Closure reset_cb_; - VideoDecoderConfig config_; + // Pointer to the demuxer stream that will feed us compressed buffers. + DemuxerStream* demuxer_stream_; vpx_codec_ctx* vpx_codec_; vpx_codec_ctx* vpx_codec_alpha_; diff --git a/media/media.gyp b/media/media.gyp index 1a848f9..88b28dc 100644 --- a/media/media.gyp +++ b/media/media.gyp @@ -233,6 +233,7 @@ 'base/buffers.h', 'base/byte_queue.cc', 'base/byte_queue.h', + 'base/callback_holder.h', 'base/channel_mixer.cc', 'base/channel_mixer.h', 'base/clock.cc', @@ -336,6 +337,10 @@ 'filters/decrypting_demuxer_stream.h', 'filters/decrypting_video_decoder.cc', 'filters/decrypting_video_decoder.h', + 'filters/fake_demuxer_stream.cc', + 'filters/fake_demuxer_stream.h', + 'filters/fake_video_decoder.cc', + 'filters/fake_video_decoder.h', 'filters/ffmpeg_audio_decoder.cc', 'filters/ffmpeg_audio_decoder.h', 'filters/ffmpeg_demuxer.cc', @@ -918,7 +923,6 @@ 'base/audio_timestamp_helper_unittest.cc', 'base/bind_to_loop_unittest.cc', 'base/bit_reader_unittest.cc', - 'base/callback_holder.h', 'base/callback_holder_unittest.cc', 'base/channel_mixer_unittest.cc', 'base/clock_unittest.cc', @@ -953,11 +957,7 @@ 'filters/decrypting_audio_decoder_unittest.cc', 'filters/decrypting_demuxer_stream_unittest.cc', 'filters/decrypting_video_decoder_unittest.cc', - 'filters/fake_demuxer_stream.cc', - 'filters/fake_demuxer_stream.h', 'filters/fake_demuxer_stream_unittest.cc', - 'filters/fake_video_decoder.cc', - 'filters/fake_video_decoder.h', 'filters/fake_video_decoder_unittest.cc', 'filters/ffmpeg_audio_decoder_unittest.cc', 'filters/ffmpeg_demuxer_unittest.cc', |