diff options
Diffstat (limited to 'media')
| -rw-r--r-- | media/base/filters.cc | 4 | ||||
| -rw-r--r-- | media/base/filters.h | 25 | ||||
| -rw-r--r-- | media/base/mock_filters.h | 5 | ||||
| -rw-r--r-- | media/filters/audio_renderer_algorithm_base.cc | 4 | ||||
| -rw-r--r-- | media/filters/audio_renderer_algorithm_base.h | 7 | ||||
| -rw-r--r-- | media/filters/audio_renderer_base.cc | 55 | ||||
| -rw-r--r-- | media/filters/audio_renderer_base.h | 19 | ||||
| -rw-r--r-- | media/filters/audio_renderer_base_unittest.cc | 470 | ||||
| -rw-r--r-- | media/filters/ffmpeg_audio_decoder.cc | 50 | ||||
| -rw-r--r-- | media/filters/ffmpeg_audio_decoder.h | 13 | ||||
| -rw-r--r-- | media/filters/ffmpeg_audio_decoder_unittest.cc | 24 | ||||
| -rw-r--r-- | media/filters/ffmpeg_video_decoder.cc | 10 |
12 files changed, 288 insertions, 398 deletions
diff --git a/media/base/filters.cc b/media/base/filters.cc index c137216..7132661 100644 --- a/media/base/filters.cc +++ b/media/base/filters.cc @@ -79,8 +79,4 @@ AudioDecoder::AudioDecoder() {} AudioDecoder::~AudioDecoder() {} -void AudioDecoder::ConsumeAudioSamples(scoped_refptr<Buffer> buffer) { - consume_audio_samples_callback_.Run(buffer); -} - } // namespace media diff --git a/media/base/filters.h b/media/base/filters.h index 654dae1..ed3af89 100644 --- a/media/base/filters.h +++ b/media/base/filters.h @@ -164,18 +164,15 @@ class MEDIA_EXPORT AudioDecoder : public Filter { virtual void Initialize(DemuxerStream* stream, const base::Closure& callback, const StatisticsCallback& stats_callback) = 0; - // Renderer provides an output buffer for Decoder to write to. These buffers - // will be recycled to renderer via the permanent callback. + // Request samples to be decoded and returned via the provided callback. + // Only one read may be in flight at any given time. // - // We could also pass empty pointer here to let decoder provide buffers pool. - virtual void ProduceAudioSamples(scoped_refptr<Buffer> buffer) = 0; - - // Installs a permanent callback for passing decoded audio output. - typedef base::Callback<void(scoped_refptr<Buffer>)> ConsumeAudioSamplesCB; - void set_consume_audio_samples_callback( - const ConsumeAudioSamplesCB& callback) { - consume_audio_samples_callback_ = callback; - } + // Implementations guarantee that the callback will not be called from within + // this method. + // + // Sample buffers will be non-NULL yet may be end of stream buffers. + typedef base::Callback<void(scoped_refptr<Buffer>)> ReadCB; + virtual void Read(const ReadCB& callback) = 0; // Returns various information about the decoded audio format. virtual int bits_per_channel() = 0; @@ -185,12 +182,6 @@ class MEDIA_EXPORT AudioDecoder : public Filter { protected: AudioDecoder(); virtual ~AudioDecoder(); - - // Executes the permanent callback to pass off decoded audio. - void ConsumeAudioSamples(scoped_refptr<Buffer> buffer); - - private: - ConsumeAudioSamplesCB consume_audio_samples_callback_; }; diff --git a/media/base/mock_filters.h b/media/base/mock_filters.h index b3758f2..7961e44 100644 --- a/media/base/mock_filters.h +++ b/media/base/mock_filters.h @@ -213,15 +213,12 @@ class MockAudioDecoder : public AudioDecoder { MOCK_METHOD3(Initialize, void(DemuxerStream* stream, const base::Closure& callback, const StatisticsCallback& stats_callback)); + MOCK_METHOD1(Read, void(const ReadCB& callback)); MOCK_METHOD1(ProduceAudioSamples, void(scoped_refptr<Buffer>)); MOCK_METHOD0(bits_per_channel, int(void)); MOCK_METHOD0(channel_layout, ChannelLayout(void)); MOCK_METHOD0(samples_per_second, int(void)); - void ConsumeAudioSamplesForTest(scoped_refptr<Buffer> buffer) { - AudioDecoder::ConsumeAudioSamples(buffer); - } - protected: virtual ~MockAudioDecoder(); diff --git a/media/filters/audio_renderer_algorithm_base.cc b/media/filters/audio_renderer_algorithm_base.cc index 2a05a24..25b6a29 100644 --- a/media/filters/audio_renderer_algorithm_base.cc +++ b/media/filters/audio_renderer_algorithm_base.cc @@ -283,6 +283,10 @@ uint32 AudioRendererAlgorithmBase::QueueSize() { return queue_.forward_bytes(); } +uint32 AudioRendererAlgorithmBase::QueueCapacity() { + return queue_.forward_capacity(); +} + void AudioRendererAlgorithmBase::IncreaseQueueCapacity() { queue_.set_forward_capacity( std::min(2 * queue_.forward_capacity(), max_queue_capacity_)); diff --git a/media/filters/audio_renderer_algorithm_base.h b/media/filters/audio_renderer_algorithm_base.h index c6888c2..6ea8db5 100644 --- a/media/filters/audio_renderer_algorithm_base.h +++ b/media/filters/audio_renderer_algorithm_base.h @@ -71,9 +71,14 @@ class MEDIA_EXPORT AudioRendererAlgorithmBase { // Returns true if we have enough data bool IsQueueFull(); - // Returns the number of bytes left in |queue_|. + // Returns the number of bytes left in |queue_|, which may be larger than + // QueueCapacity() in the event that a read callback delivered more data than + // |queue_| was intending to hold. uint32 QueueSize(); + // Returns the capacity of |queue_|. + uint32 QueueCapacity(); + // Increase the capacity of |queue_| if possible. void IncreaseQueueCapacity(); diff --git a/media/filters/audio_renderer_base.cc b/media/filters/audio_renderer_base.cc index 03c9ed0..e3f2164 100644 --- a/media/filters/audio_renderer_base.cc +++ b/media/filters/audio_renderer_base.cc @@ -14,15 +14,13 @@ namespace media { -// Upper bound on the number of pending AudioDecoder reads. -// TODO(acolwell): Experiment with reducing this to 1. -const size_t kMaxPendingReads = 4; - AudioRendererBase::AudioRendererBase() : state_(kUninitialized), + pending_read_(false), recieved_end_of_stream_(false), rendered_end_of_stream_(false), - pending_reads_(0) { + read_cb_(base::Bind(&AudioRendererBase::DecodedAudioReady, + base::Unretained(this))) { } AudioRendererBase::~AudioRendererBase() { @@ -44,8 +42,8 @@ void AudioRendererBase::Pause(const base::Closure& callback) { pause_callback_ = callback; state_ = kPaused; - // We'll only pause when we've finished all pending reads. - if (pending_reads_ == 0) { + // Pause only when we've completed our pending read. + if (!pending_read_) { pause_callback_.Run(); pause_callback_.Reset(); } else { @@ -68,7 +66,7 @@ void AudioRendererBase::Stop(const base::Closure& callback) { void AudioRendererBase::Seek(base::TimeDelta time, const FilterStatusCB& cb) { base::AutoLock auto_lock(lock_); DCHECK_EQ(kPaused, state_); - DCHECK_EQ(0u, pending_reads_) << "Pending reads should have completed"; + DCHECK(!pending_read_) << "Pending read must complete before seeking"; DCHECK(seek_cb_.is_null()); state_ = kSeeking; seek_cb_ = cb; @@ -93,11 +91,6 @@ void AudioRendererBase::Initialize(AudioDecoder* decoder, decoder_ = decoder; underflow_callback_ = underflow_callback; - // Use base::Unretained() as the decoder doesn't need to ref us. - decoder_->set_consume_audio_samples_callback( - base::Bind(&AudioRendererBase::ConsumeAudioSamples, - base::Unretained(this))); - // Create a callback so our algorithm can request more reads. base::Closure cb = base::Bind(&AudioRendererBase::ScheduleRead_Locked, this); @@ -143,15 +136,13 @@ void AudioRendererBase::ResumeAfterUnderflow(bool buffer_more_audio) { } } -void AudioRendererBase::ConsumeAudioSamples(scoped_refptr<Buffer> buffer_in) { +void AudioRendererBase::DecodedAudioReady(scoped_refptr<Buffer> buffer) { base::AutoLock auto_lock(lock_); DCHECK(state_ == kPaused || state_ == kSeeking || state_ == kPlaying || - state_ == kUnderflow || state_ == kRebuffering || - state_ == kStopped); - if (!pending_reads_) - return; + state_ == kUnderflow || state_ == kRebuffering || state_ == kStopped); - --pending_reads_; + CHECK(pending_read_); + pending_read_ = false; // TODO(scherkus): this happens due to a race, primarily because Stop() is a // synchronous call when it should be asynchronous and accept a callback. @@ -162,20 +153,20 @@ void AudioRendererBase::ConsumeAudioSamples(scoped_refptr<Buffer> buffer_in) { // Don't enqueue an end-of-stream buffer because it has no data, otherwise // discard decoded audio data until we reach our desired seek timestamp. - if (buffer_in->IsEndOfStream()) { + if (buffer->IsEndOfStream()) { recieved_end_of_stream_ = true; // Transition to kPlaying if we are currently handling an underflow since no // more data will be arriving. if (state_ == kUnderflow || state_ == kRebuffering) state_ = kPlaying; - } else if (state_ == kSeeking && !buffer_in->IsEndOfStream() && - (buffer_in->GetTimestamp() + buffer_in->GetDuration()) < + } else if (state_ == kSeeking && !buffer->IsEndOfStream() && + (buffer->GetTimestamp() + buffer->GetDuration()) < seek_timestamp_) { ScheduleRead_Locked(); } else { // Note: Calling this may schedule more reads. - algorithm_->EnqueueBuffer(buffer_in); + algorithm_->EnqueueBuffer(buffer); } // Check for our preroll complete condition. @@ -187,8 +178,8 @@ void AudioRendererBase::ConsumeAudioSamples(scoped_refptr<Buffer> buffer_in) { state_ = kPaused; ResetAndRunCB(&seek_cb_, PIPELINE_OK); } - } else if (state_ == kPaused && pending_reads_ == 0) { - // No more pending reads! We're now officially "paused". + } else if (state_ == kPaused && !pending_read_) { + // No more pending read! We're now officially "paused". if (!pause_callback_.is_null()) { pause_callback_.Run(); pause_callback_.Reset(); @@ -216,6 +207,8 @@ uint32 AudioRendererBase::FillBuffer(uint8* dest, // zeros. This gets around the tricky situation of pausing and resuming // the audio IPC layer in Chrome. Ideally, we should return zero and then // the subclass can restart the conversation. + // + // This should get handled by the subclass http://crbug.com/106600 const uint32 kZeroLength = 8192; dest_written = std::min(kZeroLength, dest_len); memset(dest, 0, dest_written); @@ -274,14 +267,10 @@ uint32 AudioRendererBase::FillBuffer(uint8* dest, void AudioRendererBase::ScheduleRead_Locked() { lock_.AssertAcquired(); - if (pending_reads_ < kMaxPendingReads) { - ++pending_reads_; - // TODO(jiesun): We use dummy buffer to feed decoder to let decoder to - // provide buffer pools. In the future, we may want to implement real - // buffer pool to recycle buffers. - scoped_refptr<Buffer> buffer; - decoder_->ProduceAudioSamples(buffer); - } + if (pending_read_) + return; + pending_read_ = true; + decoder_->Read(read_cb_); } void AudioRendererBase::SetPlaybackRate(float playback_rate) { diff --git a/media/filters/audio_renderer_base.h b/media/filters/audio_renderer_base.h index 26aa76b..a578204 100644 --- a/media/filters/audio_renderer_base.h +++ b/media/filters/audio_renderer_base.h @@ -57,9 +57,8 @@ class MEDIA_EXPORT AudioRendererBase : public AudioRenderer { // this time, such as stopping any running threads. virtual void OnStop() = 0; - // Called when a AudioDecoder completes decoding and decrements - // |pending_reads_|. - virtual void ConsumeAudioSamples(scoped_refptr<Buffer> buffer_in); + // Callback from the audio decoder delivering decoded audio samples. + void DecodedAudioReady(scoped_refptr<Buffer> buffer); // Fills the given buffer with audio data by delegating to its |algorithm_|. // FillBuffer() also takes care of updating the clock. Returns the number of @@ -92,6 +91,8 @@ class MEDIA_EXPORT AudioRendererBase : public AudioRenderer { virtual float GetPlaybackRate(); private: + friend class AudioRendererBaseTest; + // Helper method that schedules an asynchronous read from the decoder and // increments |pending_reads_|. // @@ -119,17 +120,13 @@ class MEDIA_EXPORT AudioRendererBase : public AudioRenderer { }; State state_; + // Keep track of our outstanding read to |decoder_|. + bool pending_read_; + // Keeps track of whether we received and rendered the end of stream buffer. bool recieved_end_of_stream_; bool rendered_end_of_stream_; - // Keeps track of our pending reads. We *must* have no pending reads before - // executing the pause callback, otherwise we breach the contract that all - // filters are idling. - // - // We use size_t since we compare against std::deque::size(). - size_t pending_reads_; - // Audio time at end of last call to FillBuffer(). // TODO(ralphl): Update this value after seeking. base::TimeDelta last_fill_buffer_time_; @@ -142,6 +139,8 @@ class MEDIA_EXPORT AudioRendererBase : public AudioRenderer { base::TimeDelta seek_timestamp_; + AudioDecoder::ReadCB read_cb_; + DISALLOW_COPY_AND_ASSIGN(AudioRendererBase); }; diff --git a/media/filters/audio_renderer_base_unittest.cc b/media/filters/audio_renderer_base_unittest.cc index 6f53c0c..964686e 100644 --- a/media/filters/audio_renderer_base_unittest.cc +++ b/media/filters/audio_renderer_base_unittest.cc @@ -14,14 +14,17 @@ using ::testing::_; using ::testing::AnyNumber; -using ::testing::InSequence; using ::testing::Invoke; -using ::testing::NotNull; using ::testing::Return; using ::testing::StrictMock; namespace media { +// Constants for distinguishing between muted audio and playing audio when using +// ConsumeBufferedData(). +static uint8 kMutedAudio = 0x00; +static uint8 kPlayingAudio = 0x99; + // Mocked subclass of AudioRendererBase for testing purposes. class MockAudioRendererBase : public AudioRendererBase { public: @@ -36,15 +39,7 @@ class MockAudioRendererBase : public AudioRendererBase { MOCK_METHOD3(OnInitialize, bool(int, ChannelLayout, int)); MOCK_METHOD0(OnStop, void()); - // Used for verifying check points during tests. - MOCK_METHOD1(CheckPoint, void(int id)); - private: - FRIEND_TEST_ALL_PREFIXES(AudioRendererBaseTest, OneCompleteReadCycle); - FRIEND_TEST_ALL_PREFIXES(AudioRendererBaseTest, Underflow); - FRIEND_TEST_ALL_PREFIXES(AudioRendererBaseTest, Underflow_EndOfStream); - friend class AudioRendererBaseTest; - DISALLOW_COPY_AND_ASSIGN(MockAudioRendererBase); }; @@ -53,13 +48,12 @@ class AudioRendererBaseTest : public ::testing::Test { // Give the decoder some non-garbage media properties. AudioRendererBaseTest() : renderer_(new MockAudioRendererBase()), - decoder_(new MockAudioDecoder()), - pending_reads_(0) { + decoder_(new MockAudioDecoder()) { renderer_->set_host(&host_); - // Queue all reads from the decoder. - EXPECT_CALL(*decoder_, ProduceAudioSamples(_)) - .WillRepeatedly(Invoke(this, &AudioRendererBaseTest::EnqueueCallback)); + // Queue all reads from the decoder by default. + ON_CALL(*decoder_, Read(_)) + .WillByDefault(Invoke(this, &AudioRendererBaseTest::SaveReadCallback)); // Set up audio properties. ON_CALL(*decoder_, bits_per_channel()) @@ -78,337 +72,253 @@ class AudioRendererBaseTest : public ::testing::Test { } virtual ~AudioRendererBaseTest() { - // Expect a call into the subclass. EXPECT_CALL(*renderer_, OnStop()); renderer_->Stop(NewExpectedClosure()); } - MOCK_METHOD0(OnUnderflow, void()); + MOCK_METHOD1(OnSeekComplete, void(PipelineStatus)); + FilterStatusCB NewSeekCB() { + return base::Bind(&AudioRendererBaseTest::OnSeekComplete, + base::Unretained(this)); + } + MOCK_METHOD0(OnUnderflow, void()); base::Closure NewUnderflowClosure() { return base::Bind(&AudioRendererBaseTest::OnUnderflow, base::Unretained(this)); } - scoped_refptr<DataBuffer> CreateBuffer(int data_size, uint8 value) { - scoped_refptr<DataBuffer> buffer(new DataBuffer(data_size)); - buffer->SetDataSize(data_size); - memset(buffer->GetWritableData(), value, buffer->GetDataSize()); - return buffer; + void Initialize() { + EXPECT_CALL(*renderer_, OnInitialize(_, _, _)) + .WillOnce(Return(true)); + renderer_->Initialize( + decoder_, NewExpectedClosure(), NewUnderflowClosure()); } - void WriteUntilNoPendingReads(int data_size, uint8 value, - uint32* bytes_buffered) { - while (pending_reads_ > 0) { - --pending_reads_; - *bytes_buffered += data_size; - decoder_->ConsumeAudioSamplesForTest(CreateBuffer(data_size, value)); - } + void Preroll() { + // Seek to trigger prerolling. + EXPECT_CALL(*decoder_, Read(_)); + renderer_->Seek(base::TimeDelta(), NewSeekCB()); + + // Fill entire buffer to complete prerolling. + EXPECT_CALL(*this, OnSeekComplete(PIPELINE_OK)); + DeliverRemainingAudio(); } - void ConsumeBufferedData(uint32 data_size, uint8 expected_value, - uint32* bytes_buffered) { - DCHECK(bytes_buffered); - DCHECK_GT(*bytes_buffered, 0u); - - base::TimeDelta playback_delay(base::TimeDelta::FromSeconds(1)); - scoped_array<uint8> buffer(new uint8[data_size]); - while (*bytes_buffered > 0) { - EXPECT_EQ(data_size, renderer_->FillBuffer(buffer.get(), data_size, - playback_delay, false)); - EXPECT_EQ(expected_value, buffer[0]); - *bytes_buffered -= data_size; - } + void Play() { + renderer_->Play(NewExpectedClosure()); + renderer_->SetPlaybackRate(1.0f); + } + + // Delivers |size| bytes with value kPlayingAudio to |renderer_|. + // + // There must be a pending read callback. + void FulfillPendingRead(size_t size) { + CHECK(!read_cb_.is_null()); + scoped_refptr<DataBuffer> buffer(new DataBuffer(size)); + buffer->SetDataSize(size); + memset(buffer->GetWritableData(), kPlayingAudio, buffer->GetDataSize()); + + AudioDecoder::ReadCB read_cb; + std::swap(read_cb, read_cb_); + read_cb.Run(buffer); + } + + // Delivers an end of stream buffer to |renderer_|. + // + // There must be a pending read callback. + void DeliverEndOfStream() { + FulfillPendingRead(0); + } + + // Delivers bytes until |renderer_|'s internal buffer is full and no longer + // has pending reads. + void DeliverRemainingAudio() { + CHECK(!read_cb_.is_null()); + FulfillPendingRead(bytes_remaining_in_buffer()); + CHECK(read_cb_.is_null()); } - void ExpectUnderflow(uint32 data_size, int checkpoint_value) { - scoped_array<uint8> buffer(new uint8[data_size]); - base::TimeDelta playback_delay(base::TimeDelta::FromSeconds(1)); + // Attempts to consume |size| bytes from |renderer_|'s internal buffer, + // returning true if all |size| bytes were consumed, false if less than + // |size| bytes were consumed. + // + // |muted| is optional and if passed will get set if the byte value of + // the consumed data is muted audio. + bool ConsumeBufferedData(uint32 size, bool* muted) { + scoped_array<uint8> buffer(new uint8[size]); + uint32 bytes_read = renderer_->FillBuffer(buffer.get(), size, + base::TimeDelta(), true); + if (bytes_read > 0 && muted) { + *muted = (buffer[0] == kMutedAudio); + } + return (bytes_read == size); + } - EXPECT_CALL(*this, OnUnderflow()); - EXPECT_CALL(*renderer_, CheckPoint(checkpoint_value)); - EXPECT_EQ(0u, renderer_->FillBuffer(buffer.get(), data_size, playback_delay, - false)); - renderer_->CheckPoint(checkpoint_value); + uint32 bytes_buffered() { + return renderer_->algorithm_->QueueSize(); } + uint32 buffer_capacity() { + return renderer_->algorithm_->QueueCapacity(); + } - protected: - static const size_t kMaxQueueSize; + uint32 bytes_remaining_in_buffer() { + // This can happen if too much data was delivered, in which case the buffer + // will accept the data but not increase capacity. + if (bytes_buffered() > buffer_capacity()) { + return 0; + } + return buffer_capacity() - bytes_buffered(); + } // Fixture members. scoped_refptr<MockAudioRendererBase> renderer_; scoped_refptr<MockAudioDecoder> decoder_; StrictMock<MockFilterHost> host_; - - // Number of asynchronous read requests sent to |decoder_|. - size_t pending_reads_; + AudioDecoder::ReadCB read_cb_; private: - void EnqueueCallback(scoped_refptr<Buffer> buffer) { - ++pending_reads_; + void SaveReadCallback(const AudioDecoder::ReadCB& callback) { + CHECK(read_cb_.is_null()) << "Overlapping reads are not permitted"; + read_cb_ = callback; } DISALLOW_COPY_AND_ASSIGN(AudioRendererBaseTest); }; -const size_t AudioRendererBaseTest::kMaxQueueSize = 1u; - TEST_F(AudioRendererBaseTest, Initialize_Failed) { - InSequence s; - - // Our subclass will fail when asked to initialize. EXPECT_CALL(*renderer_, OnInitialize(_, _, _)) .WillOnce(Return(false)); - - // We expect to receive an error. EXPECT_CALL(host_, SetError(PIPELINE_ERROR_INITIALIZATION_FAILED)); - - // Initialize, we expect to have no reads. renderer_->Initialize(decoder_, NewExpectedClosure(), NewUnderflowClosure()); - EXPECT_EQ(0u, pending_reads_); + + // We should have no reads. + EXPECT_TRUE(read_cb_.is_null()); } TEST_F(AudioRendererBaseTest, Initialize_Successful) { - InSequence s; - - // Then our subclass will be asked to initialize. EXPECT_CALL(*renderer_, OnInitialize(_, _, _)) .WillOnce(Return(true)); - - // Initialize, we shouldn't have any reads. renderer_->Initialize(decoder_, NewExpectedClosure(), NewUnderflowClosure()); - EXPECT_EQ(0u, pending_reads_); - - // Now seek to trigger prerolling, verifying the callback hasn't been - // executed yet. - EXPECT_CALL(*renderer_, CheckPoint(0)); - renderer_->Seek(base::TimeDelta(), NewExpectedStatusCB(PIPELINE_OK)); - EXPECT_EQ(kMaxQueueSize, pending_reads_); - renderer_->CheckPoint(0); - - // Now satisfy the read requests. Our callback should be executed after - // exiting this loop. - while (pending_reads_) { - scoped_refptr<DataBuffer> buffer(new DataBuffer(1024)); - buffer->SetDataSize(1024); - --pending_reads_; - decoder_->ConsumeAudioSamplesForTest(buffer); - } + // We should have no reads. + EXPECT_TRUE(read_cb_.is_null()); } -TEST_F(AudioRendererBaseTest, OneCompleteReadCycle) { - InSequence s; - - // Then our subclass will be asked to initialize. - EXPECT_CALL(*renderer_, OnInitialize(_, _, _)) - .WillOnce(Return(true)); - - // Initialize, we shouldn't have any reads. - renderer_->Initialize(decoder_, NewExpectedClosure(), NewUnderflowClosure()); - EXPECT_EQ(0u, pending_reads_); - - // Now seek to trigger prerolling, verifying the callback hasn't been - // executed yet. - EXPECT_CALL(*renderer_, CheckPoint(0)); - renderer_->Seek(base::TimeDelta(), NewExpectedStatusCB(PIPELINE_OK)); - EXPECT_EQ(kMaxQueueSize, pending_reads_); - renderer_->CheckPoint(0); - - // Now satisfy the read requests. Our callback should be executed after - // exiting this loop. - const uint32 kDataSize = 1024; - uint32 bytes_buffered = 0; - - WriteUntilNoPendingReads(kDataSize, 1, &bytes_buffered); - - // Then set the renderer to play state. - renderer_->Play(NewExpectedClosure()); - renderer_->SetPlaybackRate(1.0f); - EXPECT_EQ(1.0f, renderer_->GetPlaybackRate()); - - // Then flush the data in the renderer by reading from it. - uint8 buffer[kDataSize]; - for (size_t i = 0; i < kMaxQueueSize; ++i) { - EXPECT_EQ(kDataSize, - renderer_->FillBuffer(buffer, kDataSize, - base::TimeDelta(), true)); - bytes_buffered -= kDataSize; - } +TEST_F(AudioRendererBaseTest, Preroll) { + Initialize(); + Preroll(); +} - // Make sure the read request queue is full. - EXPECT_EQ(kMaxQueueSize, pending_reads_); +TEST_F(AudioRendererBaseTest, Play) { + Initialize(); + Preroll(); + Play(); - // Fulfill the read with an end-of-stream packet. - scoped_refptr<DataBuffer> last_buffer(new DataBuffer(0)); - decoder_->ConsumeAudioSamplesForTest(last_buffer); - --pending_reads_; + // Drain internal buffer, we should have a pending read. + EXPECT_CALL(*decoder_, Read(_)); + EXPECT_TRUE(ConsumeBufferedData(bytes_buffered(), NULL)); +} - // We shouldn't report ended until all data has been flushed out. - EXPECT_FALSE(renderer_->HasEnded()); +TEST_F(AudioRendererBaseTest, EndOfStream) { + Initialize(); + Preroll(); + Play(); - // We should have one less read request in the queue. - EXPECT_EQ(kMaxQueueSize - 1, pending_reads_); - - // Flush the entire internal buffer and verify NotifyEnded() isn't called - // right away. - EXPECT_CALL(*renderer_, CheckPoint(1)); - EXPECT_EQ(0u, bytes_buffered % kDataSize); - while (bytes_buffered > 0) { - EXPECT_EQ(kDataSize, - renderer_->FillBuffer(buffer, kDataSize, - base::TimeDelta(), true)); - bytes_buffered -= kDataSize; - } + // Drain internal buffer, we should have a pending read. + EXPECT_CALL(*decoder_, Read(_)); + EXPECT_TRUE(ConsumeBufferedData(bytes_buffered(), NULL)); - // Although we've emptied the buffer, we don't consider ourselves ended until - // we request another buffer. This way we know the last of the audio has - // played. + // Fulfill the read with an end-of-stream packet, we shouldn't report ended + // nor have a read until we drain the internal buffer. + DeliverEndOfStream(); EXPECT_FALSE(renderer_->HasEnded()); - renderer_->CheckPoint(1); - // Do an additional read to trigger NotifyEnded(). + // Drain internal buffer, now we should report ended. EXPECT_CALL(host_, NotifyEnded()); - EXPECT_EQ(0u, renderer_->FillBuffer(buffer, kDataSize, - base::TimeDelta(), true)); - - // We should now report ended. + EXPECT_TRUE(ConsumeBufferedData(bytes_buffered(), NULL)); EXPECT_TRUE(renderer_->HasEnded()); - - // Further reads should return muted audio and not notify any more. - EXPECT_EQ(0u, renderer_->FillBuffer(buffer, kDataSize, - base::TimeDelta(), true)); } TEST_F(AudioRendererBaseTest, Underflow) { - InSequence s; - - base::TimeDelta playback_delay(base::TimeDelta::FromSeconds(1)); - - // Then our subclass will be asked to initialize. - EXPECT_CALL(*renderer_, OnInitialize(_, _, _)) - .WillOnce(Return(true)); - - // Initialize, we shouldn't have any reads. - renderer_->Initialize(decoder_, NewExpectedClosure(), NewUnderflowClosure()); - EXPECT_EQ(0u, pending_reads_); - - // Now seek to trigger prerolling, verifying the callback hasn't been - // executed yet. - EXPECT_CALL(*renderer_, CheckPoint(0)); - renderer_->Seek(base::TimeDelta(), NewExpectedStatusCB(PIPELINE_OK)); - EXPECT_EQ(kMaxQueueSize, pending_reads_); - renderer_->CheckPoint(0); - - // Now satisfy the read requests. Our callback should be executed after - // exiting this loop. - const uint32 kDataSize = 1024; - uint32 bytes_buffered = 0; - - WriteUntilNoPendingReads(kDataSize, 1, &bytes_buffered); + Initialize(); + Preroll(); + Play(); - uint32 bytes_for_preroll = bytes_buffered; + // Drain internal buffer, we should have a pending read. + EXPECT_CALL(*decoder_, Read(_)); + EXPECT_TRUE(ConsumeBufferedData(bytes_buffered(), NULL)); - // Then set the renderer to play state. - renderer_->Play(NewExpectedClosure()); - renderer_->SetPlaybackRate(1.0f); - EXPECT_EQ(1.0f, renderer_->GetPlaybackRate()); - - // Consume all of the data passed into the renderer. - ConsumeBufferedData(kDataSize, 1, &bytes_buffered); - - // Make sure there are read requests pending. - EXPECT_GT(pending_reads_, 0u); - - // Verify the next FillBuffer() call triggers calls the underflow callback - // since the queue is empty. - ExpectUnderflow(kDataSize, 1); - - // Verify that zeroed out buffers are being returned during the underflow. - uint32 zeros_to_read = 5 * kDataSize; - ConsumeBufferedData(kDataSize, 0, &zeros_to_read); + // Verify the next FillBuffer() call triggers the underflow callback + // since the decoder hasn't delivered any data after it was drained. + const size_t kDataSize = 1024; + EXPECT_CALL(*this, OnUnderflow()); + EXPECT_FALSE(ConsumeBufferedData(kDataSize, NULL)); renderer_->ResumeAfterUnderflow(false); - // Verify we are still getting zeroed out buffers since no new data has been - // pushed to the renderer. - zeros_to_read = 5 * kDataSize; - ConsumeBufferedData(kDataSize, 0, &zeros_to_read); - - // Satisfy all pending read requests. - WriteUntilNoPendingReads(kDataSize, 2, &bytes_buffered); - - EXPECT_GE(bytes_buffered, bytes_for_preroll); - - // Verify that we are now getting the new data. - ConsumeBufferedData(kDataSize, 2, &bytes_buffered); + // Verify after resuming that we're still not getting data. + // + // NOTE: FillBuffer() satisfies the read but returns muted audio, which + // is crazy http://crbug.com/106600 + bool muted = false; + EXPECT_EQ(0u, bytes_buffered()); + EXPECT_TRUE(ConsumeBufferedData(kDataSize, &muted)); + EXPECT_TRUE(muted); + + // Deliver data, we should get non-muted audio. + DeliverRemainingAudio(); + EXPECT_CALL(*decoder_, Read(_)); + EXPECT_TRUE(ConsumeBufferedData(kDataSize, &muted)); + EXPECT_FALSE(muted); } - TEST_F(AudioRendererBaseTest, Underflow_EndOfStream) { - InSequence s; - - base::TimeDelta playback_delay(base::TimeDelta::FromSeconds(1)); - - // Then our subclass will be asked to initialize. - EXPECT_CALL(*renderer_, OnInitialize(_, _, _)) - .WillOnce(Return(true)); - - // Initialize, we shouldn't have any reads. - renderer_->Initialize(decoder_, NewExpectedClosure(), NewUnderflowClosure()); - EXPECT_EQ(0u, pending_reads_); - - // Now seek to trigger prerolling, verifying the callback hasn't been - // executed yet. - EXPECT_CALL(*renderer_, CheckPoint(0)); - renderer_->Seek(base::TimeDelta(), NewExpectedStatusCB(PIPELINE_OK)); - EXPECT_EQ(kMaxQueueSize, pending_reads_); - renderer_->CheckPoint(0); - - // Now satisfy the read requests. Our callback should be executed after - // exiting this loop. - const uint32 kDataSize = 1024; - uint32 bytes_buffered = 0; - - WriteUntilNoPendingReads(kDataSize, 1, &bytes_buffered); - - // Then set the renderer to play state. - renderer_->Play(NewExpectedClosure()); - renderer_->SetPlaybackRate(1.0f); - EXPECT_EQ(1.0f, renderer_->GetPlaybackRate()); - - // Consume all of the data passed into the renderer. - ConsumeBufferedData(kDataSize, 1, &bytes_buffered); - - // Make sure there are read requests pending. - EXPECT_GT(pending_reads_, 0u); - - // Verify the next FillBuffer() call triggers calls the underflow callback - // since the queue is empty. - ExpectUnderflow(kDataSize, 1); - - DCHECK_GE(pending_reads_, 2u); - - // Write a buffer. - bytes_buffered += kDataSize; - --pending_reads_; - decoder_->ConsumeAudioSamplesForTest(CreateBuffer(kDataSize, 3)); - - // Verify we are getting zeroed out buffers since we haven't pushed enough - // data to satisfy preroll. - uint32 zeros_to_read = 5 * kDataSize; - ConsumeBufferedData(kDataSize, 0, &zeros_to_read); - - // Send end of stream buffers for all pending reads. - while (pending_reads_ > 0) { - scoped_refptr<DataBuffer> buffer(new DataBuffer(0)); - --pending_reads_; - decoder_->ConsumeAudioSamplesForTest(buffer); - } - - // Verify that we are now getting the new data. - ConsumeBufferedData(kDataSize, 3, &bytes_buffered); + Initialize(); + Preroll(); + Play(); + + // Drain internal buffer, we should have a pending read. + EXPECT_CALL(*decoder_, Read(_)); + EXPECT_TRUE(ConsumeBufferedData(bytes_buffered(), NULL)); + + // Verify the next FillBuffer() call triggers the underflow callback + // since the decoder hasn't delivered any data after it was drained. + const size_t kDataSize = 1024; + EXPECT_CALL(*this, OnUnderflow()); + EXPECT_FALSE(ConsumeBufferedData(kDataSize, NULL)); + + // Deliver a little bit of data. + EXPECT_CALL(*decoder_, Read(_)); + FulfillPendingRead(kDataSize); + + // Verify we're getting muted audio during underflow. + // + // NOTE: FillBuffer() satisfies the read but returns muted audio, which + // is crazy http://crbug.com/106600 + bool muted = false; + EXPECT_EQ(kDataSize, bytes_buffered()); + EXPECT_TRUE(ConsumeBufferedData(kDataSize, &muted)); + EXPECT_TRUE(muted); + + // Now deliver end of stream, we should get our little bit of data back. + DeliverEndOfStream(); + EXPECT_CALL(*decoder_, Read(_)); + EXPECT_EQ(kDataSize, bytes_buffered()); + EXPECT_TRUE(ConsumeBufferedData(kDataSize, &muted)); + EXPECT_FALSE(muted); + + // Deliver another end of stream buffer and attempt to read to make sure + // we're truly at the end of stream. + // + // TODO(scherkus): fix AudioRendererBase and AudioRendererAlgorithmBase to + // stop reading after receiving an end of stream buffer. It should have also + // called NotifyEnded() http://crbug.com/106641 + DeliverEndOfStream(); + EXPECT_CALL(host_, NotifyEnded()); + EXPECT_FALSE(ConsumeBufferedData(kDataSize, &muted)); + EXPECT_FALSE(muted); } } // namespace media diff --git a/media/filters/ffmpeg_audio_decoder.cc b/media/filters/ffmpeg_audio_decoder.cc index 3a0ab34..61aa02b 100644 --- a/media/filters/ffmpeg_audio_decoder.cc +++ b/media/filters/ffmpeg_audio_decoder.cc @@ -52,8 +52,7 @@ FFmpegAudioDecoder::FFmpegAudioDecoder(MessageLoop* message_loop) channel_layout_(CHANNEL_LAYOUT_NONE), samples_per_second_(0), decoded_audio_size_(AVCODEC_MAX_AUDIO_FRAME_SIZE), - decoded_audio_(static_cast<uint8*>(av_malloc(decoded_audio_size_))), - pending_reads_(0) { + decoded_audio_(static_cast<uint8*>(av_malloc(decoded_audio_size_))) { } FFmpegAudioDecoder::~FFmpegAudioDecoder() { @@ -87,11 +86,11 @@ void FFmpegAudioDecoder::Initialize( ref_stream, callback, stats_callback)); } -void FFmpegAudioDecoder::ProduceAudioSamples(scoped_refptr<Buffer> buffer) { - message_loop_->PostTask( - FROM_HERE, - base::Bind(&FFmpegAudioDecoder::DoProduceAudioSamples, this, - buffer)); +void FFmpegAudioDecoder::Read(const ReadCB& callback) { + // Complete operation asynchronously on different stack of execution as per + // the API contract of AudioDecoder::Read() + message_loop_->PostTask(FROM_HERE, base::Bind( + &FFmpegAudioDecoder::DoRead, this, callback)); } int FFmpegAudioDecoder::bits_per_channel() { @@ -156,16 +155,18 @@ void FFmpegAudioDecoder::DoFlush(const base::Closure& callback) { callback.Run(); } -void FFmpegAudioDecoder::DoProduceAudioSamples( - const scoped_refptr<Buffer>& output) { - output_buffers_.push_back(output); +void FFmpegAudioDecoder::DoRead(const ReadCB& callback) { + DCHECK_EQ(MessageLoop::current(), message_loop_); + DCHECK(!callback.is_null()); + CHECK(read_cb_.is_null()) << "Overlapping decodes are not supported."; + + read_cb_ = callback; ReadFromDemuxerStream(); } void FFmpegAudioDecoder::DoDecodeBuffer(const scoped_refptr<Buffer>& input) { - DCHECK(!output_buffers_.empty()); - DCHECK_GT(pending_reads_, 0); - pending_reads_--; + DCHECK_EQ(MessageLoop::current(), message_loop_); + DCHECK(!read_cb_.is_null()); // FFmpeg tends to seek Ogg audio streams in the middle of nowhere, giving us // a whole bunch of AV_NOPTS_VALUE packets. Discard them until we find @@ -233,27 +234,23 @@ void FFmpegAudioDecoder::DoDecodeBuffer(const scoped_refptr<Buffer>& input) { // Decoding finished successfully, update stats and execute callback. stats_callback_.Run(statistics); if (output) { - DCHECK_GT(output_buffers_.size(), 0u); - output_buffers_.pop_front(); - - ConsumeAudioSamples(output); + DeliverSamples(output); } else { ReadFromDemuxerStream(); } } void FFmpegAudioDecoder::ReadFromDemuxerStream() { - DCHECK(!output_buffers_.empty()) - << "Reads should only occur if there are output buffers."; + DCHECK(!read_cb_.is_null()); - pending_reads_++; demuxer_stream_->Read(base::Bind(&FFmpegAudioDecoder::DecodeBuffer, this)); } void FFmpegAudioDecoder::DecodeBuffer(const scoped_refptr<Buffer>& buffer) { - message_loop_->PostTask( - FROM_HERE, - base::Bind(&FFmpegAudioDecoder::DoDecodeBuffer, this, buffer)); + // TODO(scherkus): fix FFmpegDemuxerStream::Read() to not execute our read + // callback on the same execution stack so we can get rid of forced task post. + message_loop_->PostTask(FROM_HERE, base::Bind( + &FFmpegAudioDecoder::DoDecodeBuffer, this, buffer)); } void FFmpegAudioDecoder::UpdateDurationAndTimestamp( @@ -286,4 +283,11 @@ base::TimeDelta FFmpegAudioDecoder::CalculateDuration(int size) { return base::TimeDelta::FromMicroseconds(static_cast<int64>(microseconds)); } +void FFmpegAudioDecoder::DeliverSamples(const scoped_refptr<Buffer>& samples) { + // Reset the callback before running to protect against reentrancy. + ReadCB read_cb = read_cb_; + read_cb_.Reset(); + read_cb.Run(samples); +} + } // namespace media diff --git a/media/filters/ffmpeg_audio_decoder.h b/media/filters/ffmpeg_audio_decoder.h index 3f33abf..2ee96ff 100644 --- a/media/filters/ffmpeg_audio_decoder.h +++ b/media/filters/ffmpeg_audio_decoder.h @@ -27,7 +27,7 @@ class MEDIA_EXPORT FFmpegAudioDecoder : public AudioDecoder { // AudioDecoder implementation. virtual void Initialize(DemuxerStream* stream, const base::Closure& callback, const StatisticsCallback& stats_callback) OVERRIDE; - virtual void ProduceAudioSamples(scoped_refptr<Buffer> output) OVERRIDE; + virtual void Read(const ReadCB& callback) OVERRIDE; virtual int bits_per_channel() OVERRIDE; virtual ChannelLayout channel_layout() OVERRIDE; virtual int samples_per_second() OVERRIDE; @@ -38,7 +38,7 @@ class MEDIA_EXPORT FFmpegAudioDecoder : public AudioDecoder { const base::Closure& callback, const StatisticsCallback& stats_callback); void DoFlush(const base::Closure& callback); - void DoProduceAudioSamples(const scoped_refptr<Buffer>& output); + void DoRead(const ReadCB& callback); void DoDecodeBuffer(const scoped_refptr<Buffer>& input); // Reads from the demuxer stream with corresponding callback method. @@ -53,6 +53,9 @@ class MEDIA_EXPORT FFmpegAudioDecoder : public AudioDecoder { // Calculates duration based on size of decoded audio bytes. base::TimeDelta CalculateDuration(int size); + // Delivers decoded samples to |read_cb_| and resets the callback. + void DeliverSamples(const scoped_refptr<Buffer>& samples); + MessageLoop* message_loop_; scoped_refptr<DemuxerStream> demuxer_stream_; @@ -72,11 +75,7 @@ class MEDIA_EXPORT FFmpegAudioDecoder : public AudioDecoder { const int decoded_audio_size_; uint8* decoded_audio_; // Allocated via av_malloc(). - // Holds downstream-provided buffers. - std::list<scoped_refptr<Buffer> > output_buffers_; - - // Tracks reads issued for compressed data. - int pending_reads_; + ReadCB read_cb_; DISALLOW_IMPLICIT_CONSTRUCTORS(FFmpegAudioDecoder); }; diff --git a/media/filters/ffmpeg_audio_decoder_unittest.cc b/media/filters/ffmpeg_audio_decoder_unittest.cc index 0e67c63..f8ba102 100644 --- a/media/filters/ffmpeg_audio_decoder_unittest.cc +++ b/media/filters/ffmpeg_audio_decoder_unittest.cc @@ -54,10 +54,6 @@ class FFmpegAudioDecoderTest : public testing::Test { // Push in an EOS buffer. encoded_audio_.push_back(new DataBuffer(0)); - decoder_->set_consume_audio_samples_callback( - base::Bind(&FFmpegAudioDecoderTest::DecodeFinished, - base::Unretained(this))); - config_.Initialize(kCodecVorbis, 16, CHANNEL_LAYOUT_STEREO, @@ -93,6 +89,12 @@ class FFmpegAudioDecoderTest : public testing::Test { read_callback.Run(buffer); } + void Read() { + decoder_->Read(base::Bind( + &FFmpegAudioDecoderTest::DecodeFinished, base::Unretained(this))); + message_loop_.RunAllPending(); + } + void DecodeFinished(scoped_refptr<Buffer> buffer) { decoded_audio_.push_back(buffer); } @@ -157,12 +159,9 @@ TEST_F(FFmpegAudioDecoderTest, ProduceAudioSamples) { EXPECT_CALL(statistics_callback_, OnStatistics(_)) .Times(5); - // We have to use a buffer to trigger a read. Silly. - scoped_refptr<DataBuffer> buffer(0); - decoder_->ProduceAudioSamples(buffer); - decoder_->ProduceAudioSamples(buffer); - decoder_->ProduceAudioSamples(buffer); - message_loop_.RunAllPending(); + Read(); + Read(); + Read(); // We should have three decoded audio buffers. // @@ -173,10 +172,7 @@ TEST_F(FFmpegAudioDecoderTest, ProduceAudioSamples) { ExpectDecodedAudio(2, 2902, 23219); // Call one more time to trigger EOS. - // - // TODO(scherkus): EOS should flush data, not overwrite timestamps with zero. - decoder_->ProduceAudioSamples(buffer); - message_loop_.RunAllPending(); + Read(); ASSERT_EQ(4u, decoded_audio_.size()); ExpectEndOfStream(3); diff --git a/media/filters/ffmpeg_video_decoder.cc b/media/filters/ffmpeg_video_decoder.cc index 66b25b6..c1fa54d 100644 --- a/media/filters/ffmpeg_video_decoder.cc +++ b/media/filters/ffmpeg_video_decoder.cc @@ -171,8 +171,8 @@ void FFmpegVideoDecoder::Flush(const base::Closure& callback) { } void FFmpegVideoDecoder::Read(const ReadCB& callback) { - // TODO(scherkus): forced task post since VideoRendererBase::FrameReady() will - // call Read() on FFmpegVideoDecoder's thread as we executed |read_cb_|. + // Complete operation asynchronously on different stack of execution as per + // the API contract of VideoDecoder::Read() message_loop_->PostTask(FROM_HERE, base::Bind( &FFmpegVideoDecoder::DoRead, this, callback)); } @@ -183,7 +183,7 @@ const gfx::Size& FFmpegVideoDecoder::natural_size() { void FFmpegVideoDecoder::DoRead(const ReadCB& callback) { DCHECK_EQ(MessageLoop::current(), message_loop_); - CHECK(!callback.is_null()); + DCHECK(!callback.is_null()); CHECK(read_cb_.is_null()) << "Overlapping decodes are not supported."; // This can happen during shutdown after Stop() has been called. @@ -211,8 +211,8 @@ void FFmpegVideoDecoder::ReadFromDemuxerStream() { } void FFmpegVideoDecoder::DecodeBuffer(const scoped_refptr<Buffer>& buffer) { - // TODO(scherkus): forced task post since FFmpegDemuxerStream::Read() can - // immediately execute our callback on FFmpegVideoDecoder's thread. + // TODO(scherkus): fix FFmpegDemuxerStream::Read() to not execute our read + // callback on the same execution stack so we can get rid of forced task post. message_loop_->PostTask(FROM_HERE, base::Bind( &FFmpegVideoDecoder::DoDecodeBuffer, this, buffer)); } |