diff options
Diffstat (limited to 'content')
9 files changed, 489 insertions, 245 deletions
diff --git a/content/browser/media/capture/desktop_capture_device_aura_unittest.cc b/content/browser/media/capture/desktop_capture_device_aura_unittest.cc index 984b661..0bcb066 100644 --- a/content/browser/media/capture/desktop_capture_device_aura_unittest.cc +++ b/content/browser/media/capture/desktop_capture_device_aura_unittest.cc @@ -60,6 +60,7 @@ class MockDeviceClient : public media::VideoCaptureDevice::Client { MOCK_METHOD0(DoReserveOutputBuffer, void(void)); MOCK_METHOD0(DoOnIncomingCapturedBuffer, void(void)); MOCK_METHOD0(DoOnIncomingCapturedVideoFrame, void(void)); + MOCK_METHOD0(DoResurrectLastOutputBuffer, void(void)); MOCK_METHOD2(OnError, void(const tracked_objects::Location& from_here, const std::string& reason)); @@ -85,7 +86,15 @@ class MockDeviceClient : public media::VideoCaptureDevice::Client { const base::TimeTicks& timestamp) override { DoOnIncomingCapturedVideoFrame(); } - + scoped_ptr<Buffer> ResurrectLastOutputBuffer( + const gfx::Size& dimensions, + media::VideoPixelFormat format, + media::VideoPixelStorage storage) override { + EXPECT_EQ(media::PIXEL_FORMAT_I420, format); + EXPECT_EQ(media::PIXEL_STORAGE_CPU, storage); + DoResurrectLastOutputBuffer(); + return scoped_ptr<Buffer>(); + } double GetBufferPoolUtilization() const override { return 0.0; } }; diff --git a/content/browser/media/capture/desktop_capture_device_unittest.cc b/content/browser/media/capture/desktop_capture_device_unittest.cc index ec10e73..3dd6bbc 100644 --- a/content/browser/media/capture/desktop_capture_device_unittest.cc +++ b/content/browser/media/capture/desktop_capture_device_unittest.cc @@ -79,6 +79,7 @@ class MockDeviceClient : public media::VideoCaptureDevice::Client { MOCK_METHOD0(DoReserveOutputBuffer, void(void)); MOCK_METHOD0(DoOnIncomingCapturedBuffer, void(void)); MOCK_METHOD0(DoOnIncomingCapturedVideoFrame, void(void)); + MOCK_METHOD0(DoResurrectLastOutputBuffer, void(void)); MOCK_METHOD2(OnError, void(const tracked_objects::Location& from_here, const std::string& reason)); @@ -104,7 +105,15 @@ class MockDeviceClient : public media::VideoCaptureDevice::Client { const base::TimeTicks& timestamp) override { DoOnIncomingCapturedVideoFrame(); } - + scoped_ptr<Buffer> ResurrectLastOutputBuffer( + const gfx::Size& dimensions, + media::VideoPixelFormat format, + media::VideoPixelStorage storage) override { + EXPECT_TRUE(format == media::PIXEL_FORMAT_I420 && + storage == media::PIXEL_STORAGE_CPU); + DoResurrectLastOutputBuffer(); + return scoped_ptr<Buffer>(); + } double GetBufferPoolUtilization() const override { return 0.0; } }; diff --git a/content/browser/media/capture/web_contents_video_capture_device_unittest.cc b/content/browser/media/capture/web_contents_video_capture_device_unittest.cc index 7b6268f..dfa6548 100644 --- a/content/browser/media/capture/web_contents_video_capture_device_unittest.cc +++ b/content/browser/media/capture/web_contents_video_capture_device_unittest.cc @@ -375,7 +375,7 @@ class StubClient : public media::VideoCaptureDevice::Client { CHECK_EQ(format, media::PIXEL_FORMAT_I420); int buffer_id_to_drop = VideoCaptureBufferPool::kInvalidId; // Ignored. const int buffer_id = buffer_pool_->ReserveForProducer( - format, storage, dimensions, &buffer_id_to_drop); + dimensions, format, storage, &buffer_id_to_drop); if (buffer_id == VideoCaptureBufferPool::kInvalidId) return NULL; @@ -383,6 +383,7 @@ class StubClient : public media::VideoCaptureDevice::Client { new AutoReleaseBuffer( buffer_pool_, buffer_pool_->GetBufferHandle(buffer_id), buffer_id)); } + // Trampoline method to workaround GMOCK problems with scoped_ptr<>. void OnIncomingCapturedBuffer(scoped_ptr<Buffer> buffer, const media::VideoCaptureFormat& frame_format, @@ -419,6 +420,20 @@ class StubClient : public media::VideoCaptureDevice::Client { frame->visible_rect().size()); } + scoped_ptr<media::VideoCaptureDevice::Client::Buffer> + ResurrectLastOutputBuffer(const gfx::Size& dimensions, + media::VideoPixelFormat format, + media::VideoPixelStorage storage) override { + CHECK_EQ(format, media::PIXEL_FORMAT_I420); + const int buffer_id = + buffer_pool_->ResurrectLastForProducer(dimensions, format, storage); + if (buffer_id == VideoCaptureBufferPool::kInvalidId) + return nullptr; + return scoped_ptr<media::VideoCaptureDevice::Client::Buffer>( + new AutoReleaseBuffer( + buffer_pool_, buffer_pool_->GetBufferHandle(buffer_id), buffer_id)); + } + void OnError(const tracked_objects::Location& from_here, const std::string& reason) override { error_callback_.Run(); @@ -439,7 +454,9 @@ class StubClient : public media::VideoCaptureDevice::Client { DCHECK(pool_); } int id() const override { return id_; } - gfx::Size dimensions() const override { return gfx::Size(); } + gfx::Size dimensions() const override { + return buffer_handle_->dimensions(); + } size_t mapped_size() const override { return buffer_handle_->mapped_size(); } diff --git a/content/browser/renderer_host/media/video_capture_buffer_pool.cc b/content/browser/renderer_host/media/video_capture_buffer_pool.cc index cd3e0be..831c060 100644 --- a/content/browser/renderer_host/media/video_capture_buffer_pool.cc +++ b/content/browser/renderer_host/media/video_capture_buffer_pool.cc @@ -16,99 +16,31 @@ namespace content { const int VideoCaptureBufferPool::kInvalidId = -1; -// A simple holder of a memory-backed buffer and accessors to it. -class SimpleBufferHandle final : public VideoCaptureBufferPool::BufferHandle { - public: - SimpleBufferHandle(void* data, - size_t mapped_size, - base::SharedMemoryHandle handle) - : data_(data), - mapped_size_(mapped_size) -#if defined(OS_POSIX) && !defined(OS_MACOSX) - , - handle_(handle) -#endif - { - } - ~SimpleBufferHandle() override {} - - gfx::Size dimensions() const override { - NOTREACHED(); - return gfx::Size(); - } - size_t mapped_size() const override { return mapped_size_; } - void* data(int plane) override { - DCHECK_EQ(plane, 0); - return data_; - } - ClientBuffer AsClientBuffer(int plane) override { - NOTREACHED(); - return nullptr; - } -#if defined(OS_POSIX) && !defined(OS_MACOSX) - base::FileDescriptor AsPlatformFile() override { - return handle_; - } -#endif - - private: - void* const data_; - const size_t mapped_size_; -#if defined(OS_POSIX) && !defined(OS_MACOSX) - const base::SharedMemoryHandle handle_; -#endif -}; - -// A holder of a GpuMemoryBuffer-backed buffer. Holds weak references to -// GpuMemoryBuffer-backed buffers and provides accessors to their data. -class GpuMemoryBufferBufferHandle final - : public VideoCaptureBufferPool::BufferHandle { - public: - GpuMemoryBufferBufferHandle(const gfx::Size& dimensions, - std::vector< - scoped_ptr<gfx::GpuMemoryBuffer>>* gmbs) - : dimensions_(dimensions), gmbs_(gmbs) { - DCHECK(gmbs); - } - ~GpuMemoryBufferBufferHandle() override {} - - gfx::Size dimensions() const override { return dimensions_; } - size_t mapped_size() const override { return dimensions_.GetArea(); } - void* data(int plane) override { - DCHECK_GE(plane, 0); - DCHECK_LT(plane, static_cast<int>(gmbs_->size())); - DCHECK((*gmbs_)[plane]); - return (*gmbs_)[plane]->memory(0); - } - ClientBuffer AsClientBuffer(int plane) override { - DCHECK_GE(plane, 0); - DCHECK_LT(plane, static_cast<int>(gmbs_->size())); - return (*gmbs_)[plane]->AsClientBuffer(); - } -#if defined(OS_POSIX) && !defined(OS_MACOSX) - base::FileDescriptor AsPlatformFile() override { - NOTREACHED(); - return base::FileDescriptor(); - } -#endif - - private: - const gfx::Size dimensions_; - std::vector<scoped_ptr<gfx::GpuMemoryBuffer>>* const gmbs_; -}; - // Tracker specifics for SharedMemory. class VideoCaptureBufferPool::SharedMemTracker final : public Tracker { public: - SharedMemTracker(); - bool Init(media::VideoPixelFormat format, + SharedMemTracker() : Tracker() {} + + bool Init(const gfx::Size& dimensions, + media::VideoPixelFormat format, media::VideoPixelStorage storage_type, - const gfx::Size& dimensions, - base::Lock* lock) override; + base::Lock* lock) override { + DVLOG(2) << "allocating ShMem of " << dimensions.ToString(); + set_dimensions(dimensions); + // |dimensions| can be 0x0 for trackers that do not require memory backing. + set_max_pixel_count(dimensions.GetArea()); + set_pixel_format(format); + set_storage_type(storage_type); + mapped_size_ = + media::VideoCaptureFormat(dimensions, 0.0f, format, storage_type) + .ImageAllocationSize(); + if (!mapped_size_) + return true; + return shared_memory_.CreateAndMapAnonymous(mapped_size_); + } scoped_ptr<BufferHandle> GetBufferHandle() override { - return make_scoped_ptr(new SimpleBufferHandle( - shared_memory_.memory(), mapped_size_, shared_memory_.handle())); + return make_scoped_ptr(new SharedMemBufferHandle(this)); } bool ShareToProcess(base::ProcessHandle process_handle, base::SharedMemoryHandle* new_handle) override { @@ -122,6 +54,37 @@ class VideoCaptureBufferPool::SharedMemTracker final : public Tracker { } private: + // A simple proxy that implements the BufferHandle interface, providing + // accessors to SharedMemTracker's memory and properties. + class SharedMemBufferHandle : public VideoCaptureBufferPool::BufferHandle { + public: + // |tracker| must outlive SimpleBufferHandle. This is ensured since a + // tracker is pinned until ownership of this SimpleBufferHandle is returned + // to VideoCaptureBufferPool. + explicit SharedMemBufferHandle(SharedMemTracker* tracker) + : tracker_(tracker) {} + ~SharedMemBufferHandle() override {} + + gfx::Size dimensions() const override { return tracker_->dimensions(); } + size_t mapped_size() const override { return tracker_->mapped_size_; } + void* data(int plane) override { + DCHECK_EQ(plane, 0); + return tracker_->shared_memory_.memory(); + } + ClientBuffer AsClientBuffer(int plane) override { + NOTREACHED(); + return nullptr; + } +#if defined(OS_POSIX) && !defined(OS_MACOSX) + base::FileDescriptor AsPlatformFile() override { + return tracker_->shared_memory_.handle(); + } +#endif + + private: + SharedMemTracker* const tracker_; + }; + // The memory created to be shared with renderer processes. base::SharedMemory shared_memory_; size_t mapped_size_; @@ -131,131 +94,134 @@ class VideoCaptureBufferPool::SharedMemTracker final : public Tracker { // associated pixel dimensions. class VideoCaptureBufferPool::GpuMemoryBufferTracker final : public Tracker { public: - GpuMemoryBufferTracker(); - bool Init(media::VideoPixelFormat format, + GpuMemoryBufferTracker() : Tracker() {} + + ~GpuMemoryBufferTracker() override { + for (const auto& gmb : gpu_memory_buffers_) + gmb->Unmap(); + } + + bool Init(const gfx::Size& dimensions, + media::VideoPixelFormat format, media::VideoPixelStorage storage_type, - const gfx::Size& dimensions, - base::Lock* lock) override; - ~GpuMemoryBufferTracker() override; + base::Lock* lock) override { + DVLOG(2) << "allocating GMB for " << dimensions.ToString(); + // BrowserGpuMemoryBufferManager::current() may not be accessed on IO + // Thread. + DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::IO)); + DCHECK(BrowserGpuMemoryBufferManager::current()); + // This class is only expected to be called with I420 buffer requests at + // this point. + DCHECK_EQ(format, media::PIXEL_FORMAT_I420); + set_dimensions(dimensions); + set_max_pixel_count(dimensions.GetArea()); + set_pixel_format(format); + set_storage_type(storage_type); + // |dimensions| can be 0x0 for trackers that do not require memory backing. + if (dimensions.GetArea() == 0u) + return true; + + base::AutoUnlock auto_unlock(*lock); + const size_t num_planes = media::VideoFrame::NumPlanes(pixel_format()); + for (size_t i = 0; i < num_planes; ++i) { + const gfx::Size& size = + media::VideoFrame::PlaneSize(pixel_format(), i, dimensions); + gpu_memory_buffers_.push_back( + BrowserGpuMemoryBufferManager::current()->AllocateGpuMemoryBuffer( + size, gfx::BufferFormat::R_8, + gfx::BufferUsage::GPU_READ_CPU_READ_WRITE)); + + DLOG_IF(ERROR, !gpu_memory_buffers_[i]) << "Allocating GpuMemoryBuffer"; + if (!gpu_memory_buffers_[i] || !gpu_memory_buffers_[i]->Map()) + return false; + } + return true; + } scoped_ptr<BufferHandle> GetBufferHandle() override { DCHECK_EQ(gpu_memory_buffers_.size(), media::VideoFrame::NumPlanes(pixel_format())); - return make_scoped_ptr( - new GpuMemoryBufferBufferHandle(dimensions_, &gpu_memory_buffers_)); + return make_scoped_ptr(new GpuMemoryBufferBufferHandle(this)); } + bool ShareToProcess(base::ProcessHandle process_handle, base::SharedMemoryHandle* new_handle) override { NOTREACHED(); return false; } + bool ShareToProcess2(int plane, base::ProcessHandle process_handle, - gfx::GpuMemoryBufferHandle* new_handle) override; - - private: - gfx::Size dimensions_; - // Owned references to GpuMemoryBuffers. - std::vector<scoped_ptr<gfx::GpuMemoryBuffer>> gpu_memory_buffers_; -}; - -VideoCaptureBufferPool::SharedMemTracker::SharedMemTracker() : Tracker() {} - -bool VideoCaptureBufferPool::SharedMemTracker::Init( - media::VideoPixelFormat format, - media::VideoPixelStorage storage_type, - const gfx::Size& dimensions, - base::Lock* lock) { - DVLOG(2) << "allocating ShMem of " << dimensions.ToString(); - set_pixel_format(format); - set_storage_type(storage_type); - // |dimensions| can be 0x0 for trackers that do not require memory backing. - set_pixel_count(dimensions.GetArea()); - mapped_size_ = - media::VideoCaptureFormat(dimensions, 0.0f, format, storage_type) - .ImageAllocationSize(); - if (!mapped_size_) - return true; - return shared_memory_.CreateAndMapAnonymous(mapped_size_); -} - -VideoCaptureBufferPool::GpuMemoryBufferTracker::GpuMemoryBufferTracker() - : Tracker() { -} - -VideoCaptureBufferPool::GpuMemoryBufferTracker::~GpuMemoryBufferTracker() { - for (const auto& gmb : gpu_memory_buffers_) - gmb->Unmap(); -} - -bool VideoCaptureBufferPool::GpuMemoryBufferTracker::Init( - media::VideoPixelFormat format, - media::VideoPixelStorage storage_type, - const gfx::Size& dimensions, - base::Lock* lock) { - DVLOG(2) << "allocating GMB for " << dimensions.ToString(); - // BrowserGpuMemoryBufferManager::current() may not be accessed on IO Thread. - DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::IO)); - DCHECK(BrowserGpuMemoryBufferManager::current()); - // This class is only expected to be called with I420 buffer requests at this - // point. - DCHECK_EQ(format, media::PIXEL_FORMAT_I420); - set_pixel_format(format); - set_storage_type(storage_type); - set_pixel_count(dimensions.GetArea()); - // |dimensions| can be 0x0 for trackers that do not require memory backing. - if (dimensions.GetArea() == 0u) + gfx::GpuMemoryBufferHandle* new_handle) override { + DCHECK_LE(plane, static_cast<int>(gpu_memory_buffers_.size())); + + const auto& current_gmb_handle = gpu_memory_buffers_[plane]->GetHandle(); + switch (current_gmb_handle.type) { + case gfx::EMPTY_BUFFER: + NOTREACHED(); + return false; + case gfx::SHARED_MEMORY_BUFFER: { + DCHECK(base::SharedMemory::IsHandleValid(current_gmb_handle.handle)); + base::SharedMemory shared_memory( + base::SharedMemory::DuplicateHandle(current_gmb_handle.handle), + false); + shared_memory.ShareToProcess(process_handle, &new_handle->handle); + DCHECK(base::SharedMemory::IsHandleValid(new_handle->handle)); + new_handle->type = gfx::SHARED_MEMORY_BUFFER; + return true; + } + case gfx::IO_SURFACE_BUFFER: + case gfx::SURFACE_TEXTURE_BUFFER: + case gfx::OZONE_NATIVE_PIXMAP: + *new_handle = current_gmb_handle; + return true; + } + NOTREACHED(); return true; - dimensions_ = dimensions; - - lock->Release(); - const size_t num_planes = media::VideoFrame::NumPlanes(pixel_format()); - for (size_t i = 0; i < num_planes; ++i) { - const gfx::Size& size = - media::VideoFrame::PlaneSize(pixel_format(), i, dimensions); - gpu_memory_buffers_.push_back( - BrowserGpuMemoryBufferManager::current()->AllocateGpuMemoryBuffer( - size, gfx::BufferFormat::R_8, - gfx::BufferUsage::GPU_READ_CPU_READ_WRITE)); - - DLOG_IF(ERROR, !gpu_memory_buffers_[i]) << "Allocating GpuMemoryBuffer"; - if (!gpu_memory_buffers_[i] || !gpu_memory_buffers_[i]->Map()) - return false; } - lock->Acquire(); - return true; -} - -bool VideoCaptureBufferPool::GpuMemoryBufferTracker::ShareToProcess2( - int plane, - base::ProcessHandle process_handle, - gfx::GpuMemoryBufferHandle* new_handle) { - DCHECK_LE(plane, static_cast<int>(gpu_memory_buffers_.size())); - const auto& current_gmb_handle = gpu_memory_buffers_[plane]->GetHandle(); - switch (current_gmb_handle.type) { - case gfx::EMPTY_BUFFER: + private: + // A simple proxy that implements the BufferHandle interface, providing + // accessors to GpuMemoryBufferTracker's memory and properties. + class GpuMemoryBufferBufferHandle + : public VideoCaptureBufferPool::BufferHandle { + public: + // |tracker| must outlive GpuMemoryBufferBufferHandle. This is ensured since + // a tracker is pinned until ownership of this GpuMemoryBufferBufferHandle + // is returned to VideoCaptureBufferPool. + explicit GpuMemoryBufferBufferHandle(GpuMemoryBufferTracker* tracker) + : tracker_(tracker) {} + ~GpuMemoryBufferBufferHandle() override {} + + gfx::Size dimensions() const override { return tracker_->dimensions(); } + size_t mapped_size() const override { + return tracker_->dimensions().GetArea(); + } + void* data(int plane) override { + DCHECK_GE(plane, 0); + DCHECK_LT(plane, static_cast<int>(tracker_->gpu_memory_buffers_.size())); + DCHECK(tracker_->gpu_memory_buffers_[plane]); + return tracker_->gpu_memory_buffers_[plane]->memory(0); + } + ClientBuffer AsClientBuffer(int plane) override { + DCHECK_GE(plane, 0); + DCHECK_LT(plane, static_cast<int>(tracker_->gpu_memory_buffers_.size())); + return tracker_->gpu_memory_buffers_[plane]->AsClientBuffer(); + } +#if defined(OS_POSIX) && !defined(OS_MACOSX) + base::FileDescriptor AsPlatformFile() override { NOTREACHED(); - return false; - case gfx::SHARED_MEMORY_BUFFER: { - DCHECK(base::SharedMemory::IsHandleValid(current_gmb_handle.handle)); - base::SharedMemory shared_memory( - base::SharedMemory::DuplicateHandle(current_gmb_handle.handle), - false); - shared_memory.ShareToProcess(process_handle, &new_handle->handle); - DCHECK(base::SharedMemory::IsHandleValid(new_handle->handle)); - new_handle->type = gfx::SHARED_MEMORY_BUFFER; - return true; + return base::FileDescriptor(); } - case gfx::IO_SURFACE_BUFFER: - case gfx::SURFACE_TEXTURE_BUFFER: - case gfx::OZONE_NATIVE_PIXMAP: - *new_handle = current_gmb_handle; - return true; - } - NOTREACHED(); - return true; -} +#endif + + private: + GpuMemoryBufferTracker* const tracker_; + }; + + // Owned references to GpuMemoryBuffers. + std::vector<scoped_ptr<gfx::GpuMemoryBuffer>> gpu_memory_buffers_; +}; // static scoped_ptr<VideoCaptureBufferPool::Tracker> @@ -275,7 +241,8 @@ VideoCaptureBufferPool::Tracker::~Tracker() {} VideoCaptureBufferPool::VideoCaptureBufferPool(int count) : count_(count), - next_buffer_id_(0) { + next_buffer_id_(0), + last_relinquished_buffer_id_(kInvalidId) { DCHECK_GT(count, 0); } @@ -332,13 +299,12 @@ VideoCaptureBufferPool::GetBufferHandle(int buffer_id) { return tracker->GetBufferHandle(); } -int VideoCaptureBufferPool::ReserveForProducer( - media::VideoPixelFormat format, - media::VideoPixelStorage storage, - const gfx::Size& dimensions, - int* buffer_id_to_drop) { +int VideoCaptureBufferPool::ReserveForProducer(const gfx::Size& dimensions, + media::VideoPixelFormat format, + media::VideoPixelStorage storage, + int* buffer_id_to_drop) { base::AutoLock lock(lock_); - return ReserveForProducerInternal(format, storage, dimensions, + return ReserveForProducerInternal(dimensions, format, storage, buffer_id_to_drop); } @@ -351,6 +317,7 @@ void VideoCaptureBufferPool::RelinquishProducerReservation(int buffer_id) { } DCHECK(tracker->held_by_producer()); tracker->set_held_by_producer(false); + last_relinquished_buffer_id_ = buffer_id; } void VideoCaptureBufferPool::HoldForConsumers( @@ -385,6 +352,34 @@ void VideoCaptureBufferPool::RelinquishConsumerHold(int buffer_id, num_clients); } +int VideoCaptureBufferPool::ResurrectLastForProducer( + const gfx::Size& dimensions, + media::VideoPixelFormat format, + media::VideoPixelStorage storage) { + base::AutoLock lock(lock_); + + // Return early if the last relinquished buffer has been re-used already. + if (last_relinquished_buffer_id_ == kInvalidId) + return kInvalidId; + + // If there are no consumers reading from this buffer, then it's safe to + // provide this buffer back to the producer (because the producer may + // potentially modify the content). Check that the expected dimensions, + // format, and storage match. + TrackerMap::iterator it = trackers_.find(last_relinquished_buffer_id_); + DCHECK(it != trackers_.end()); + DCHECK(!it->second->held_by_producer()); + if (it->second->consumer_hold_count() == 0 && + it->second->dimensions() == dimensions && + it->second->pixel_format() == format && + it->second->storage_type() == storage) { + it->second->set_held_by_producer(true); + return last_relinquished_buffer_id_; + } + + return kInvalidId; +} + double VideoCaptureBufferPool::GetBufferPoolUtilization() const { base::AutoLock lock(lock_); int num_buffers_held = 0; @@ -397,9 +392,9 @@ double VideoCaptureBufferPool::GetBufferPoolUtilization() const { } int VideoCaptureBufferPool::ReserveForProducerInternal( + const gfx::Size& dimensions, media::VideoPixelFormat pixel_format, media::VideoPixelStorage storage_type, - const gfx::Size& dimensions, int* buffer_id_to_drop) { lock_.AssertAcquired(); @@ -408,32 +403,50 @@ int VideoCaptureBufferPool::ReserveForProducerInternal( // largest one that's not big enough, in case we have to reallocate a tracker. *buffer_id_to_drop = kInvalidId; size_t largest_size_in_pixels = 0; + TrackerMap::iterator tracker_of_last_resort = trackers_.end(); TrackerMap::iterator tracker_to_drop = trackers_.end(); for (TrackerMap::iterator it = trackers_.begin(); it != trackers_.end(); ++it) { Tracker* const tracker = it->second; if (!tracker->consumer_hold_count() && !tracker->held_by_producer()) { - if (tracker->pixel_count() >= size_in_pixels && + if (tracker->max_pixel_count() >= size_in_pixels && (tracker->pixel_format() == pixel_format) && (tracker->storage_type() == storage_type)) { + if (it->first == last_relinquished_buffer_id_) { + // This buffer would do just fine, but avoid returning it because the + // client may want to resurrect it. It will be returned perforce if + // the pool has reached it's maximum limit (see code below). + tracker_of_last_resort = it; + continue; + } // Existing tracker is big enough and has correct format. Reuse it. + tracker->set_dimensions(dimensions); tracker->set_held_by_producer(true); return it->first; } - if (tracker->pixel_count() > largest_size_in_pixels) { - largest_size_in_pixels = tracker->pixel_count(); + if (tracker->max_pixel_count() > largest_size_in_pixels) { + largest_size_in_pixels = tracker->max_pixel_count(); tracker_to_drop = it; } } } // Preferably grow the pool by creating a new tracker. If we're at maximum - // size, then reallocate by deleting an existing one instead. + // size, then try using |tracker_of_last_resort| or reallocate by deleting an + // existing one instead. if (trackers_.size() == static_cast<size_t>(count_)) { + if (tracker_of_last_resort != trackers_.end()) { + last_relinquished_buffer_id_ = kInvalidId; + tracker_of_last_resort->second->set_dimensions(dimensions); + tracker_of_last_resort->second->set_held_by_producer(true); + return tracker_of_last_resort->first; + } if (tracker_to_drop == trackers_.end()) { // We're out of space, and can't find an unused tracker to reallocate. return kInvalidId; } + if (tracker_to_drop->first == last_relinquished_buffer_id_) + last_relinquished_buffer_id_ = kInvalidId; *buffer_id_to_drop = tracker_to_drop->first; delete tracker_to_drop->second; trackers_.erase(tracker_to_drop); @@ -445,7 +458,7 @@ int VideoCaptureBufferPool::ReserveForProducerInternal( scoped_ptr<Tracker> tracker = Tracker::CreateTracker(storage_type); // TODO(emircan): We pass the lock here to solve GMB allocation issue, see // crbug.com/545238. - if (!tracker->Init(pixel_format, storage_type, dimensions, &lock_)) { + if (!tracker->Init(dimensions, pixel_format, storage_type, &lock_)) { DLOG(ERROR) << "Error initializing Tracker"; return kInvalidId; } diff --git a/content/browser/renderer_host/media/video_capture_buffer_pool.h b/content/browser/renderer_host/media/video_capture_buffer_pool.h index e5e4e2f..bc86447 100644 --- a/content/browser/renderer_host/media/video_capture_buffer_pool.h +++ b/content/browser/renderer_host/media/video_capture_buffer_pool.h @@ -91,9 +91,9 @@ class CONTENT_EXPORT VideoCaptureBufferPool // On occasion, this call will decide to free an old buffer to make room for a // new allocation at a larger size. If so, the ID of the destroyed buffer is // returned via |buffer_id_to_drop|. - int ReserveForProducer(media::VideoPixelFormat format, + int ReserveForProducer(const gfx::Size& dimensions, + media::VideoPixelFormat format, media::VideoPixelStorage storage, - const gfx::Size& dimensions, int* buffer_id_to_drop); // Indicate that a buffer held for the producer should be returned back to the @@ -111,6 +111,18 @@ class CONTENT_EXPORT VideoCaptureBufferPool // done, a buffer is returned to the pool for reuse. void RelinquishConsumerHold(int buffer_id, int num_clients); + // Attempt to reserve the same buffer that was relinquished in the last call + // to RelinquishProducerReservation(). If the buffer is not still being + // consumed, and has not yet been re-used since being consumed, and the + // specified |dimensions|, |format|, and |storage| agree with its last + // reservation, this will succeed. Otherwise, |kInvalidId| will be returned. + // + // A producer may assume the content of the buffer has been preserved and may + // also make modifications. + int ResurrectLastForProducer(const gfx::Size& dimensions, + media::VideoPixelFormat format, + media::VideoPixelStorage storage); + // Returns a snapshot of the current number of buffers in-use divided by the // maximum |count_|. double GetBufferPoolUtilization() const; @@ -125,15 +137,19 @@ class CONTENT_EXPORT VideoCaptureBufferPool static scoped_ptr<Tracker> CreateTracker(media::VideoPixelStorage storage); Tracker() - : pixel_count_(0), held_by_producer_(false), consumer_hold_count_(0) {} - virtual bool Init(media::VideoPixelFormat format, + : max_pixel_count_(0), + held_by_producer_(false), + consumer_hold_count_(0) {} + virtual bool Init(const gfx::Size& dimensions, + media::VideoPixelFormat format, media::VideoPixelStorage storage_type, - const gfx::Size& dimensions, base::Lock* lock) = 0; virtual ~Tracker(); - size_t pixel_count() const { return pixel_count_; } - void set_pixel_count(size_t count) { pixel_count_ = count; } + const gfx::Size& dimensions() const { return dimensions_; } + void set_dimensions(const gfx::Size& dim) { dimensions_ = dim; } + size_t max_pixel_count() const { return max_pixel_count_; } + void set_max_pixel_count(size_t count) { max_pixel_count_ = count; } media::VideoPixelFormat pixel_format() const { return pixel_format_; } @@ -160,11 +176,17 @@ class CONTENT_EXPORT VideoCaptureBufferPool gfx::GpuMemoryBufferHandle* new_handle) = 0; private: - size_t pixel_count_; + // |dimensions_| may change as a Tracker is re-used, but |max_pixel_count_|, + // |pixel_format_|, and |storage_type_| are set once for the lifetime of a + // Tracker. + gfx::Size dimensions_; + size_t max_pixel_count_; media::VideoPixelFormat pixel_format_; media::VideoPixelStorage storage_type_; + // Indicates whether this Tracker is currently referenced by the producer. bool held_by_producer_; + // Number of consumer processes which hold this Tracker. int consumer_hold_count_; }; @@ -172,9 +194,9 @@ class CONTENT_EXPORT VideoCaptureBufferPool friend class base::RefCountedThreadSafe<VideoCaptureBufferPool>; virtual ~VideoCaptureBufferPool(); - int ReserveForProducerInternal(media::VideoPixelFormat format, + int ReserveForProducerInternal(const gfx::Size& dimensions, + media::VideoPixelFormat format, media::VideoPixelStorage storage, - const gfx::Size& dimensions, int* tracker_id_to_drop); Tracker* GetTracker(int buffer_id); @@ -188,6 +210,10 @@ class CONTENT_EXPORT VideoCaptureBufferPool // The ID of the next buffer. int next_buffer_id_; + // The ID of the buffer last relinquished by the producer (a candidate for + // resurrection). + int last_relinquished_buffer_id_; + // The buffers, indexed by the first parameter, a buffer id. using TrackerMap = std::map<int, Tracker*>; TrackerMap trackers_; diff --git a/content/browser/renderer_host/media/video_capture_buffer_pool_unittest.cc b/content/browser/renderer_host/media/video_capture_buffer_pool_unittest.cc index 21c6132..f06837e 100644 --- a/content/browser/renderer_host/media/video_capture_buffer_pool_unittest.cc +++ b/content/browser/renderer_host/media/video_capture_buffer_pool_unittest.cc @@ -9,7 +9,9 @@ #include <stddef.h> #include <stdint.h> #include <string.h> + #include <utility> +#include <vector> #include "base/bind.h" #include "base/macros.h" @@ -175,8 +177,8 @@ class VideoCaptureBufferPoolTest << media::VideoPixelFormatToString(format_and_storage.pixel_format) << " " << dimensions.ToString(); const int buffer_id = pool_->ReserveForProducer( - format_and_storage.pixel_format, format_and_storage.pixel_storage, - dimensions, &buffer_id_to_drop); + dimensions, format_and_storage.pixel_format, + format_and_storage.pixel_storage, &buffer_id_to_drop); if (buffer_id == VideoCaptureBufferPool::kInvalidId) return scoped_ptr<Buffer>(); EXPECT_EQ(expected_dropped_id_, buffer_id_to_drop); @@ -187,6 +189,18 @@ class VideoCaptureBufferPoolTest new Buffer(pool_, std::move(buffer_handle), buffer_id)); } + scoped_ptr<Buffer> ResurrectLastBuffer( + const gfx::Size& dimensions, + PixelFormatAndStorage format_and_storage) { + const int buffer_id = pool_->ResurrectLastForProducer( + dimensions, format_and_storage.pixel_format, + format_and_storage.pixel_storage); + if (buffer_id == VideoCaptureBufferPool::kInvalidId) + return scoped_ptr<Buffer>(); + return scoped_ptr<Buffer>( + new Buffer(pool_, pool_->GetBufferHandle(buffer_id), buffer_id)); + } + base::MessageLoop loop_; int expected_dropped_id_; scoped_refptr<VideoCaptureBufferPool> pool_; @@ -371,6 +385,135 @@ TEST_P(VideoCaptureBufferPoolTest, BufferPool) { buffer4.reset(); } +// Tests that a previously-released buffer can be immediately resurrected under +// normal conditions. +TEST_P(VideoCaptureBufferPoolTest, ResurrectsLastBuffer) { + ExpectDroppedId(VideoCaptureBufferPool::kInvalidId); + + // At the start, there should be nothing to resurrect. + scoped_ptr<Buffer> resurrected = + ResurrectLastBuffer(gfx::Size(10, 10), GetParam()); + ASSERT_EQ(nullptr, resurrected.get()); + + // Reserve a 10x10 buffer and fill it with 0xab values. + scoped_ptr<Buffer> original = ReserveBuffer(gfx::Size(10, 10), GetParam()); + ASSERT_NE(nullptr, original.get()); + const size_t original_mapped_size = original->mapped_size(); + memset(original->data(), 0xab, original_mapped_size); + + // Try to resurrect a buffer BEFORE releasing |original|. This should fail. + resurrected = ResurrectLastBuffer(gfx::Size(10, 10), GetParam()); + ASSERT_EQ(nullptr, resurrected.get()); + + // Release |original| and then try to resurrect it. Confirm the content of + // the resurrected buffer is a fill of 0xab values. + original.reset(); + resurrected = ResurrectLastBuffer(gfx::Size(10, 10), GetParam()); + ASSERT_NE(nullptr, resurrected.get()); + ASSERT_EQ(original_mapped_size, resurrected->mapped_size()); + uint8_t* resurrected_memory = reinterpret_cast<uint8_t*>(resurrected->data()); + for (size_t i = 0; i < original_mapped_size; ++i) + EXPECT_EQ(0xab, resurrected_memory[i]) << "Mismatch at byte offset: " << i; + + // Now, fill the resurrected buffer with 0xbc values and release it. + memset(resurrected_memory, 0xbc, original_mapped_size); + resurrected.reset(); + + // Finally, resurrect the buffer again, and confirm it contains a fill of 0xbc + // values. + resurrected = ResurrectLastBuffer(gfx::Size(10, 10), GetParam()); + ASSERT_NE(nullptr, resurrected.get()); + ASSERT_EQ(original_mapped_size, resurrected->mapped_size()); + resurrected_memory = reinterpret_cast<uint8_t*>(resurrected->data()); + for (size_t i = 0; i < original_mapped_size; ++i) + EXPECT_EQ(0xbc, resurrected_memory[i]) << "Mismatch at byte offset: " << i; +} + +// Tests that a buffer cannot be resurrected if its properties do not match. +TEST_P(VideoCaptureBufferPoolTest, DoesNotResurrectIfPropertiesNotMatched) { + ExpectDroppedId(VideoCaptureBufferPool::kInvalidId); + + // Reserve a 10x10 buffer, fill it with 0xcd values, and release it. + scoped_ptr<Buffer> original = ReserveBuffer(gfx::Size(10, 10), GetParam()); + ASSERT_NE(nullptr, original.get()); + const size_t original_mapped_size = original->mapped_size(); + memset(original->data(), 0xcd, original_mapped_size); + original.reset(); + + // Expect that the buffer cannot be resurrected if the dimensions do not + // match. + scoped_ptr<Buffer> resurrected = + ResurrectLastBuffer(gfx::Size(8, 8), GetParam()); + ASSERT_EQ(nullptr, resurrected.get()); + + // Expect that the buffer cannot be resurrected if the pixel format does not + // match. + PixelFormatAndStorage altered_format_or_storage = GetParam(); + altered_format_or_storage.pixel_format = + (altered_format_or_storage.pixel_format == media::PIXEL_FORMAT_I420 + ? media::PIXEL_FORMAT_ARGB + : media::PIXEL_FORMAT_I420); + resurrected = + ResurrectLastBuffer(gfx::Size(10, 10), altered_format_or_storage); + ASSERT_EQ(nullptr, resurrected.get()); + + // Expect that the buffer cannot be resurrected if the pixel storage does not + // match. + altered_format_or_storage = GetParam(); + altered_format_or_storage.pixel_storage = + (altered_format_or_storage.pixel_storage == media::PIXEL_STORAGE_CPU) + ? media::PIXEL_STORAGE_GPUMEMORYBUFFER + : media::PIXEL_STORAGE_CPU; + resurrected = + ResurrectLastBuffer(gfx::Size(10, 10), altered_format_or_storage); + ASSERT_EQ(nullptr, resurrected.get()); + + // Finally, check that the buffer CAN be resurrected if all properties match. + resurrected = ResurrectLastBuffer(gfx::Size(10, 10), GetParam()); + ASSERT_NE(nullptr, resurrected.get()); + ASSERT_EQ(original_mapped_size, resurrected->mapped_size()); + uint8_t* resurrected_memory = reinterpret_cast<uint8_t*>(resurrected->data()); + for (size_t i = 0; i < original_mapped_size; ++i) + EXPECT_EQ(0xcd, resurrected_memory[i]) << "Mismatch at byte offset: " << i; +} + +// Tests that the buffers are managed by the pool such that the last-released +// buffer is kept around as long as possible (for successful resurrection). +TEST_P(VideoCaptureBufferPoolTest, AvoidsClobberingForResurrectingLastBuffer) { + ExpectDroppedId(VideoCaptureBufferPool::kInvalidId); + + // Reserve a 10x10 buffer, fill it with 0xde values, and release it. + scoped_ptr<Buffer> original = ReserveBuffer(gfx::Size(10, 10), GetParam()); + ASSERT_NE(nullptr, original.get()); + const size_t original_mapped_size = original->mapped_size(); + memset(original->data(), 0xde, original_mapped_size); + original.reset(); + + // Reserve all but one of the pool's buffers. + std::vector<scoped_ptr<Buffer>> held_buffers; + for (int i = 0; i < kTestBufferPoolSize - 1; ++i) { + held_buffers.push_back(ReserveBuffer(gfx::Size(10, 10), GetParam())); + ASSERT_NE(nullptr, held_buffers.back().get()); + } + + // Now, attempt to resurrect the original buffer. This should succeed. + scoped_ptr<Buffer> resurrected = + ResurrectLastBuffer(gfx::Size(10, 10), GetParam()); + ASSERT_NE(nullptr, resurrected.get()); + ASSERT_EQ(original_mapped_size, resurrected->mapped_size()); + uint8_t* resurrected_memory = reinterpret_cast<uint8_t*>(resurrected->data()); + for (size_t i = 0; i < original_mapped_size; ++i) + EXPECT_EQ(0xde, resurrected_memory[i]) << "Mismatch at byte offset: " << i; + resurrected.reset(); + + // Reserve the final buffer in the pool, and then confirm resurrection does + // not succeed. + held_buffers.push_back(ReserveBuffer(gfx::Size(10, 10), GetParam())); + ASSERT_NE(nullptr, held_buffers.back().get()); + resurrected = ResurrectLastBuffer(gfx::Size(10, 10), GetParam()); + ASSERT_EQ(nullptr, resurrected.get()); +} + INSTANTIATE_TEST_CASE_P(, VideoCaptureBufferPoolTest, testing::ValuesIn(kCapturePixelFormatAndStorages)); diff --git a/content/browser/renderer_host/media/video_capture_controller_unittest.cc b/content/browser/renderer_host/media/video_capture_controller_unittest.cc index 99e563f..cdd8c16 100644 --- a/content/browser/renderer_host/media/video_capture_controller_unittest.cc +++ b/content/browser/renderer_host/media/video_capture_controller_unittest.cc @@ -331,6 +331,7 @@ TEST_F(VideoCaptureControllerTest, NormalCaptureMultipleClients) { base::RunLoop().RunUntilIdle(); Mock::VerifyAndClearExpectations(client_a_.get()); Mock::VerifyAndClearExpectations(client_b_.get()); + // Expect VideoCaptureController set the metadata in |video_frame| to hold a // resource utilization of 0.5 (the largest of all reported values). double resource_utilization_in_metadata = -1.0; @@ -359,15 +360,27 @@ TEST_F(VideoCaptureControllerTest, NormalCaptureMultipleClients) { base::TimeTicks()); // The buffer should be delivered to the clients in any order. - EXPECT_CALL(*client_a_, - DoI420BufferReady(client_a_route_1, capture_resolution)) - .Times(1); - EXPECT_CALL(*client_b_, - DoI420BufferReady(client_b_route_1, capture_resolution)) - .Times(1); - EXPECT_CALL(*client_a_, - DoI420BufferReady(client_a_route_2, capture_resolution)) - .Times(1); + { + InSequence s; + EXPECT_CALL(*client_a_, DoBufferCreated(client_a_route_1)).Times(1); + EXPECT_CALL(*client_a_, + DoI420BufferReady(client_a_route_1, capture_resolution)) + .Times(1); + } + { + InSequence s; + EXPECT_CALL(*client_b_, DoBufferCreated(client_b_route_1)).Times(1); + EXPECT_CALL(*client_b_, + DoI420BufferReady(client_b_route_1, capture_resolution)) + .Times(1); + } + { + InSequence s; + EXPECT_CALL(*client_a_, DoBufferCreated(client_a_route_2)).Times(1); + EXPECT_CALL(*client_a_, + DoI420BufferReady(client_a_route_2, capture_resolution)) + .Times(1); + } base::RunLoop().RunUntilIdle(); Mock::VerifyAndClearExpectations(client_a_.get()); Mock::VerifyAndClearExpectations(client_b_.get()); @@ -407,23 +420,24 @@ TEST_F(VideoCaptureControllerTest, NormalCaptureMultipleClients) { media::PIXEL_FORMAT_I420, media::PIXEL_STORAGE_CPU).get()); - // The new client needs to be told of 3 buffers; the old clients only 2. + // The new client needs to be notified of the creation of |kPoolSize| buffers; + // the old clients only |kPoolSize - 2|. EXPECT_CALL(*client_b_, DoBufferCreated(client_b_route_2)).Times(kPoolSize); EXPECT_CALL(*client_b_, DoI420BufferReady(client_b_route_2, capture_resolution)) .Times(kPoolSize); EXPECT_CALL(*client_a_, DoBufferCreated(client_a_route_1)) - .Times(kPoolSize - 1); + .Times(kPoolSize - 2); EXPECT_CALL(*client_a_, DoI420BufferReady(client_a_route_1, capture_resolution)) .Times(kPoolSize); EXPECT_CALL(*client_a_, DoBufferCreated(client_a_route_2)) - .Times(kPoolSize - 1); + .Times(kPoolSize - 2); EXPECT_CALL(*client_a_, DoI420BufferReady(client_a_route_2, capture_resolution)) .Times(kPoolSize); EXPECT_CALL(*client_b_, DoBufferCreated(client_b_route_1)) - .Times(kPoolSize - 1); + .Times(kPoolSize - 2); EXPECT_CALL(*client_b_, DoI420BufferReady(client_b_route_1, capture_resolution)) .Times(kPoolSize); diff --git a/content/browser/renderer_host/media/video_capture_device_client.cc b/content/browser/renderer_host/media/video_capture_device_client.cc index d5cb149..4d508c4 100644 --- a/content/browser/renderer_host/media/video_capture_device_client.cc +++ b/content/browser/renderer_host/media/video_capture_device_client.cc @@ -270,21 +270,17 @@ VideoCaptureDeviceClient::ReserveOutputBuffer( // it's a ShMem GMB or a DmaBuf GMB. int buffer_id_to_drop = VideoCaptureBufferPool::kInvalidId; const int buffer_id = buffer_pool_->ReserveForProducer( - pixel_format, pixel_storage, frame_size, &buffer_id_to_drop); - if (buffer_id == VideoCaptureBufferPool::kInvalidId) - return NULL; - - scoped_ptr<media::VideoCaptureDevice::Client::Buffer> output_buffer( - new AutoReleaseBuffer(buffer_pool_, buffer_id)); - + frame_size, pixel_format, pixel_storage, &buffer_id_to_drop); if (buffer_id_to_drop != VideoCaptureBufferPool::kInvalidId) { BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, base::Bind(&VideoCaptureController::DoBufferDestroyedOnIOThread, controller_, buffer_id_to_drop)); } - - return output_buffer; + if (buffer_id == VideoCaptureBufferPool::kInvalidId) + return nullptr; + return make_scoped_ptr<Buffer>( + new AutoReleaseBuffer(buffer_pool_, buffer_id)); } void VideoCaptureDeviceClient::OnIncomingCapturedBuffer( @@ -340,6 +336,19 @@ void VideoCaptureDeviceClient::OnIncomingCapturedVideoFrame( timestamp)); } +scoped_ptr<media::VideoCaptureDevice::Client::Buffer> +VideoCaptureDeviceClient::ResurrectLastOutputBuffer( + const gfx::Size& dimensions, + media::VideoPixelFormat format, + media::VideoPixelStorage storage) { + const int buffer_id = + buffer_pool_->ResurrectLastForProducer(dimensions, format, storage); + if (buffer_id == VideoCaptureBufferPool::kInvalidId) + return nullptr; + return make_scoped_ptr<Buffer>( + new AutoReleaseBuffer(buffer_pool_, buffer_id)); +} + void VideoCaptureDeviceClient::OnError( const tracked_objects::Location& from_here, const std::string& reason) { diff --git a/content/browser/renderer_host/media/video_capture_device_client.h b/content/browser/renderer_host/media/video_capture_device_client.h index 1b982a2..0273c32 100644 --- a/content/browser/renderer_host/media/video_capture_device_client.h +++ b/content/browser/renderer_host/media/video_capture_device_client.h @@ -60,6 +60,10 @@ class CONTENT_EXPORT VideoCaptureDeviceClient scoped_ptr<Buffer> buffer, const scoped_refptr<media::VideoFrame>& frame, const base::TimeTicks& timestamp) override; + scoped_ptr<Buffer> ResurrectLastOutputBuffer( + const gfx::Size& dimensions, + media::VideoPixelFormat format, + media::VideoPixelStorage storage) override; void OnError(const tracked_objects::Location& from_here, const std::string& reason) override; void OnLog(const std::string& message) override; |