diff options
| -rw-r--r-- | webkit/media/buffered_data_source.cc | 185 | ||||
| -rw-r--r-- | webkit/media/buffered_data_source.h | 36 | ||||
| -rw-r--r-- | webkit/media/buffered_data_source_unittest.cc | 25 | ||||
| -rw-r--r-- | webkit/tools/layout_tests/test_expectations.txt | 2 |
4 files changed, 129 insertions, 119 deletions
diff --git a/webkit/media/buffered_data_source.cc b/webkit/media/buffered_data_source.cc index e5b5320..f3e9b39 100644 --- a/webkit/media/buffered_data_source.cc +++ b/webkit/media/buffered_data_source.cc @@ -27,6 +27,56 @@ const int kNumCacheMissRetries = 3; namespace webkit_media { +class BufferedDataSource::ReadOperation { + public: + ReadOperation(int64 position, int size, uint8* data, + const media::DataSource::ReadCB& callback); + ~ReadOperation(); + + // Runs |callback_| with the given |result|, deleting the operation + // afterwards. + static void Run(scoped_ptr<ReadOperation> read_op, int result); + + // State for the number of times this read operation has been retried. + int retries() { return retries_; } + void IncrementRetries() { ++retries_; } + + int64 position() { return position_; } + int size() { return size_; } + uint8* data() { return data_; } + + private: + int retries_; + + const int64 position_; + const int size_; + uint8* data_; + media::DataSource::ReadCB callback_; + + DISALLOW_IMPLICIT_CONSTRUCTORS(ReadOperation); +}; + +BufferedDataSource::ReadOperation::ReadOperation( + int64 position, int size, uint8* data, + const media::DataSource::ReadCB& callback) + : retries_(0), + position_(position), + size_(size), + data_(data), + callback_(callback) { + DCHECK(!callback_.is_null()); +} + +BufferedDataSource::ReadOperation::~ReadOperation() { + DCHECK(callback_.is_null()); +} + +// static +void BufferedDataSource::ReadOperation::Run( + scoped_ptr<ReadOperation> read_op, int result) { + base::ResetAndReturn(&read_op->callback_).Run(result); +} + BufferedDataSource::BufferedDataSource( MessageLoop* render_loop, WebFrame* frame, @@ -37,17 +87,12 @@ BufferedDataSource::BufferedDataSource( assume_fully_buffered_(false), streaming_(false), frame_(frame), - read_size_(0), - read_buffer_(NULL), - last_read_start_(0), intermediate_read_buffer_(new uint8[kInitialReadBufferSize]), intermediate_read_buffer_size_(kInitialReadBufferSize), render_loop_(render_loop), stop_signal_received_(false), - stopped_on_render_loop_(false), media_has_played_(false), preload_(AUTO), - cache_miss_retries_left_(kNumCacheMissRetries), bitrate_(0), playback_rate_(0.0), media_log_(media_log), @@ -137,23 +182,25 @@ bool BufferedDataSource::DidPassCORSAccessCheck() const { void BufferedDataSource::Abort() { DCHECK(MessageLoop::current() == render_loop_); - - CleanupTask(); + { + base::AutoLock auto_lock(lock_); + StopInternal_Locked(); + } + StopLoader(); frame_ = NULL; } ///////////////////////////////////////////////////////////////////////////// -// media::Filter implementation. +// media::DataSource implementation. void BufferedDataSource::Stop(const base::Closure& closure) { { base::AutoLock auto_lock(lock_); StopInternal_Locked(); } - if (!closure.is_null()) - closure.Run(); + closure.Run(); render_loop_->PostTask(FROM_HERE, - base::Bind(&BufferedDataSource::CleanupTask, this)); + base::Bind(&BufferedDataSource::StopLoader, this)); } void BufferedDataSource::SetPlaybackRate(float playback_rate) { @@ -166,8 +213,6 @@ void BufferedDataSource::SetBitrate(int bitrate) { &BufferedDataSource::SetBitrateTask, this, bitrate)); } -///////////////////////////////////////////////////////////////////////////// -// media::DataSource implementation. void BufferedDataSource::Read( int64 position, int size, uint8* data, const media::DataSource::ReadCB& read_cb) { @@ -176,18 +221,18 @@ void BufferedDataSource::Read( { base::AutoLock auto_lock(lock_); - DCHECK(read_cb_.is_null()); + DCHECK(!read_op_); - if (stop_signal_received_ || stopped_on_render_loop_) { + if (stop_signal_received_) { read_cb.Run(kReadError); return; } - read_cb_ = read_cb; + read_op_.reset(new ReadOperation(position, size, data, read_cb)); } render_loop_->PostTask(FROM_HERE, base::Bind( - &BufferedDataSource::ReadTask, this, position, size, data)); + &BufferedDataSource::ReadTask, this)); } bool BufferedDataSource::GetSize(int64* size_out) { @@ -205,26 +250,8 @@ bool BufferedDataSource::IsStreaming() { ///////////////////////////////////////////////////////////////////////////// // Render thread tasks. -void BufferedDataSource::ReadTask( - int64 position, - int read_size, - uint8* buffer) { +void BufferedDataSource::ReadTask() { DCHECK(MessageLoop::current() == render_loop_); - { - base::AutoLock auto_lock(lock_); - if (stop_signal_received_ || stopped_on_render_loop_) - return; - - DCHECK(!read_cb_.is_null()); - } - - // Saves the read parameters. - last_read_start_ = position; - read_size_ = read_size; - read_buffer_ = buffer; - cache_miss_retries_left_ = kNumCacheMissRetries; - - // Call to read internal to perform the actual read. ReadInternal(); } @@ -239,36 +266,15 @@ void BufferedDataSource::StopInternal_Locked() { // response to Stop(). init_cb_.Reset(); - // TODO(scherkus): Should use DoneRead_Locked() but today |read_size_| and - // |read_buffer_| are only touched on the render thread. We should combine all - // Read()-related variables into a struct to simplify bookkeeping. - if (!read_cb_.is_null()) - base::ResetAndReturn(&read_cb_).Run(media::DataSource::kReadError); + if (read_op_) + ReadOperation::Run(read_op_.Pass(), kReadError); } -void BufferedDataSource::CleanupTask() { +void BufferedDataSource::StopLoader() { DCHECK(MessageLoop::current() == render_loop_); - { - base::AutoLock auto_lock(lock_); - StopInternal_Locked(); - - if (stopped_on_render_loop_) - return; - - // Signal that stop task has finished execution. - // NOTE: it's vital that this be set under lock, as that's how Read() tests - // before registering a new |read_cb_| (which is cleared below). - stopped_on_render_loop_ = true; - } - - // We just need to stop the loader, so it stops activity. if (loader_.get()) loader_->Stop(); - - // Reset the parameters of the current read request. - read_size_ = 0; - read_buffer_ = 0; } void BufferedDataSource::SetPlaybackRateTask(float playback_rate) { @@ -312,31 +318,29 @@ void BufferedDataSource::SetBitrateTask(int bitrate) { // prior to make this method call. void BufferedDataSource::ReadInternal() { DCHECK(MessageLoop::current() == render_loop_); - DCHECK(loader_.get()); + int64 position = 0; + int size = 0; + { + base::AutoLock auto_lock(lock_); + if (stop_signal_received_) + return; + + position = read_op_->position(); + size = read_op_->size(); + } // First we prepare the intermediate read buffer for BufferedResourceLoader // to write to. - if (read_size_ > intermediate_read_buffer_size_) { - intermediate_read_buffer_.reset(new uint8[read_size_]); + if (size > intermediate_read_buffer_size_) { + intermediate_read_buffer_.reset(new uint8[size]); } // Perform the actual read with BufferedResourceLoader. loader_->Read( - last_read_start_, read_size_, intermediate_read_buffer_.get(), + position, size, intermediate_read_buffer_.get(), base::Bind(&BufferedDataSource::ReadCallback, this)); } -void BufferedDataSource::DoneRead_Locked(int bytes_read) { - DVLOG(1) << "DoneRead: " << bytes_read << " bytes"; - DCHECK(MessageLoop::current() == render_loop_); - DCHECK(!read_cb_.is_null()); - DCHECK(bytes_read >= 0 || bytes_read == kReadError); - lock_.AssertAcquired(); - - base::ResetAndReturn(&read_cb_).Run(bytes_read); - read_size_ = 0; - read_buffer_ = 0; -} ///////////////////////////////////////////////////////////////////////////// // BufferedResourceLoader callback methods. @@ -404,7 +408,7 @@ void BufferedDataSource::PartialReadStartCallback( base::AutoLock auto_lock(lock_); if (stop_signal_received_) return; - DoneRead_Locked(kReadError); + ReadOperation::Run(read_op_.Pass(), kReadError); } void BufferedDataSource::ReadCallback( @@ -412,18 +416,24 @@ void BufferedDataSource::ReadCallback( int bytes_read) { DCHECK(MessageLoop::current() == render_loop_); + // TODO(scherkus): we shouldn't have to lock to signal host(), see + // http://crbug.com/113712 for details. + base::AutoLock auto_lock(lock_); + if (stop_signal_received_) + return; + if (status != BufferedResourceLoader::kOk) { // Stop the resource load if it failed. loader_->Stop(); if (status == BufferedResourceLoader::kCacheMiss && - cache_miss_retries_left_ > 0) { - cache_miss_retries_left_--; + read_op_->retries() < kNumCacheMissRetries) { + read_op_->IncrementRetries(); // Recreate a loader starting from where we last left off until the // end of the resource. loader_.reset(CreateResourceLoader( - last_read_start_, kPositionNotSpecified)); + read_op_->position(), kPositionNotSpecified)); loader_->Start( base::Bind(&BufferedDataSource::PartialReadStartCallback, this), base::Bind(&BufferedDataSource::LoadingStateChangedCallback, this), @@ -432,18 +442,12 @@ void BufferedDataSource::ReadCallback( return; } - // Fall through to signal a read error. - bytes_read = kReadError; - } - - // TODO(scherkus): we shouldn't have to lock to signal host(), see - // http://crbug.com/113712 for details. - base::AutoLock auto_lock(lock_); - if (stop_signal_received_) + ReadOperation::Run(read_op_.Pass(), kReadError); return; + } if (bytes_read > 0) { - memcpy(read_buffer_, intermediate_read_buffer_.get(), bytes_read); + memcpy(read_op_->data(), intermediate_read_buffer_.get(), bytes_read); } else if (bytes_read == 0 && total_bytes_ == kPositionNotSpecified) { // We've reached the end of the file and we didn't know the total size // before. Update the total size so Read()s past the end of the file will @@ -456,7 +460,7 @@ void BufferedDataSource::ReadCallback( total_bytes_); } } - DoneRead_Locked(bytes_read); + ReadOperation::Run(read_op_.Pass(), bytes_read); } void BufferedDataSource::LoadingStateChangedCallback( @@ -500,8 +504,7 @@ void BufferedDataSource::ProgressCallback(int64 position) { if (stop_signal_received_) return; - if (position > last_read_start_) - ReportOrQueueBufferedBytes(last_read_start_, position); + ReportOrQueueBufferedBytes(loader_->first_byte_position(), position); } void BufferedDataSource::ReportOrQueueBufferedBytes(int64 start, int64 end) { diff --git a/webkit/media/buffered_data_source.h b/webkit/media/buffered_data_source.h index a620de4..e01fef7bb 100644 --- a/webkit/media/buffered_data_source.h +++ b/webkit/media/buffered_data_source.h @@ -94,16 +94,14 @@ class BufferedDataSource : public media::DataSource { friend class BufferedDataSourceTest; // Task posted to perform actual reading on the render thread. - void ReadTask(int64 position, int read_size, uint8* read_buffer); + void ReadTask(); // Cancels oustanding callbacks and sets |stop_signal_received_|. Safe to call // from any thread. void StopInternal_Locked(); - // Task posted when Stop() is called. Stops |loader_|, resets Read() - // variables, and sets |stopped_on_render_loop_| to signal any remaining - // tasks to stop. - void CleanupTask(); + // Stops |loader_| if present. Used by Abort() and Stop(). + void StopLoader(); // This task uses the current playback rate with the previous playback rate // to determine whether we are going from pause to play and play to pause, @@ -117,11 +115,6 @@ class BufferedDataSource : public media::DataSource { // the render thread. void ReadInternal(); - // Calls |read_cb_| and reset all read parameters. Non-negative |bytes_read| - // values represent successful reads, otherwise |bytes_read| should be - // kReadError. - void DoneRead_Locked(int bytes_read); - // BufferedResourceLoader::Start() callback for initial load. void StartCallback(BufferedResourceLoader::Status status); @@ -167,12 +160,10 @@ class BufferedDataSource : public media::DataSource { // Callback method from the pipeline for initialization. InitializeCB init_cb_; - // Read parameters received from the Read() method call. - media::DataSource::ReadCB read_cb_; - int read_size_; - uint8* read_buffer_; - // Retained between reads to make sense of buffering information. - int64 last_read_start_; + // Read parameters received from the Read() method call. Must be accessed + // under |lock_|. + class ReadOperation; + scoped_ptr<ReadOperation> read_op_; // This buffer is intermediate, we use it for BufferedResourceLoader to write // to. And when read in BufferedResourceLoader is done, we copy data from @@ -190,20 +181,12 @@ class BufferedDataSource : public media::DataSource { // The message loop of the render thread. MessageLoop* render_loop_; - // Protects |stop_signal_received_|, |stopped_on_render_loop_| and - // |init_cb_|. + // Protects |stop_signal_received_| and |read_op_|. base::Lock lock_; // Whether we've been told to stop via Abort() or Stop(). - // - // TODO(scherkus): Combine this bool with |stopped_on_render_loop_| as they're - // redundant. bool stop_signal_received_; - // This variable is set by CleanupTask() that indicates this object is stopped - // on the render thread and work should no longer progress. - bool stopped_on_render_loop_; - // This variable is true when the user has requested the video to play at // least once. bool media_has_played_; @@ -212,9 +195,6 @@ class BufferedDataSource : public media::DataSource { // element. Preload preload_; - // Number of cache miss retries left. - int cache_miss_retries_left_; - // Bitrate of the content, 0 if unknown. int bitrate_; diff --git a/webkit/media/buffered_data_source_unittest.cc b/webkit/media/buffered_data_source_unittest.cc index c8e4deb..170ba98 100644 --- a/webkit/media/buffered_data_source_unittest.cc +++ b/webkit/media/buffered_data_source_unittest.cc @@ -593,6 +593,31 @@ TEST_F(BufferedDataSourceTest, Http_Read) { Stop(); } +TEST_F(BufferedDataSourceTest, Http_Read_Seek) { + InitializeWith206Response(); + + // Read a bit from the beginning. + ReadAt(0); + EXPECT_CALL(*this, ReadCallback(kDataSize)); + EXPECT_CALL(host_, AddBufferedByteRange(0, kDataSize - 1)); + ReceiveData(kDataSize); + + // Simulate a seek by reading a bit beyond kDataSize. + ReadAt(kDataSize * 2); + + // We receive data leading up to but not including our read. + EXPECT_CALL(host_, AddBufferedByteRange(0, kDataSize * 2 - 1)); + ReceiveData(kDataSize); + + // We now receive the rest of the data for our read. + EXPECT_CALL(*this, ReadCallback(kDataSize)); + EXPECT_CALL(host_, AddBufferedByteRange(0, kDataSize * 3 - 1)); + ReceiveData(kDataSize); + + EXPECT_TRUE(data_source_->downloading()); + Stop(); +} + TEST_F(BufferedDataSourceTest, File_Read) { InitializeWithFileResponse(); diff --git a/webkit/tools/layout_tests/test_expectations.txt b/webkit/tools/layout_tests/test_expectations.txt index 2d7c702..bcfb5aeb 100644 --- a/webkit/tools/layout_tests/test_expectations.txt +++ b/webkit/tools/layout_tests/test_expectations.txt @@ -34,3 +34,5 @@ webkit.org/b/103140 inspector/console/command-line-api-inspect.html [ Skip ] webkit.org/b/103140 inspector/runtime/runtime-localStorage-getProperties.html [ Skip ] + +http/tests/media/video-buffered-range-contains-currentTime.html [ ImageOnlyFailure ] |