Commit a85f2acc authored by Jose Lopes's avatar Jose Lopes Committed by Commit Bot

media: Migrate MojoSharedBufferDoneCB to once callback.

The MojoSharedBufferDoneCB callback is a once callback because it's executed
only once during the class's constructor:
* https://cs.chromium.org/chromium/src/media/mojo/common/mojo_shared_buffer_video_frame.cc?rcl=8184a68c47988fb128372c9c688f77455dfcb5db&l=250

This is also documented in its definition:
* https://cs.chromium.org/chromium/src/media/mojo/common/mojo_shared_buffer_video_frame.cc?rcl=8184a68c47988fb128372c9c688f77455dfcb5db&l=250

The callback is passed in the TransformToVideoFrame method, in:
* https://cs.chromium.org/chromium/src/media/mojo/common/mojo_shared_buffer_video_frame.cc?rcl=8184a68c47988fb128372c9c688f77455dfcb5db&l=250

and this method can only be called once. The evidence of the following statements:
* DCHECK(FrameBuffer()): https://cs.chromium.org/chromium/src/media/mojo/services/mojo_cdm_allocator.cc?rcl=8184a68c47988fb128372c9c688f77455dfcb5db&l=116
* SetFrameBuffer(nullptr): https://cs.chromium.org/chromium/src/media/mojo/services/mojo_cdm_allocator.cc?rcl=8184a68c47988fb128372c9c688f77455dfcb5db&l=116

Together these guarantee that the method can only be called once.

This is part of the base::Callback migration.

Context: https://cs.chromium.org/chromium/src/docs/callback.md?rcl=9fcc3764aea8f97e9f6de4a9ee61d554e67edcda&l=40

Bug: 714018
Change-Id: Ib4dff4d9c491682b8abd134723ef88bcf5252bc0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2069540
Commit-Queue: Jose Lopes <jabolopes@google.com>
Reviewed-by: default avatarXiaohan Wang <xhwang@chromium.org>
Reviewed-by: default avatardanakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#744610}
parent 33f26eae
...@@ -246,9 +246,10 @@ bool MojoSharedBufferVideoFrame::Init(size_t y_offset, ...@@ -246,9 +246,10 @@ bool MojoSharedBufferVideoFrame::Init(size_t y_offset,
MojoSharedBufferVideoFrame::~MojoSharedBufferVideoFrame() { MojoSharedBufferVideoFrame::~MojoSharedBufferVideoFrame() {
// Call |mojo_shared_buffer_done_cb_| to take ownership of // Call |mojo_shared_buffer_done_cb_| to take ownership of
// |shared_buffer_handle_|. // |shared_buffer_handle_|.
if (mojo_shared_buffer_done_cb_) if (mojo_shared_buffer_done_cb_) {
mojo_shared_buffer_done_cb_.Run(std::move(shared_buffer_handle_), std::move(mojo_shared_buffer_done_cb_)
shared_buffer_size_); .Run(std::move(shared_buffer_handle_), shared_buffer_size_);
}
} }
size_t MojoSharedBufferVideoFrame::PlaneOffset(size_t plane) const { size_t MojoSharedBufferVideoFrame::PlaneOffset(size_t plane) const {
...@@ -257,8 +258,8 @@ size_t MojoSharedBufferVideoFrame::PlaneOffset(size_t plane) const { ...@@ -257,8 +258,8 @@ size_t MojoSharedBufferVideoFrame::PlaneOffset(size_t plane) const {
} }
void MojoSharedBufferVideoFrame::SetMojoSharedBufferDoneCB( void MojoSharedBufferVideoFrame::SetMojoSharedBufferDoneCB(
const MojoSharedBufferDoneCB& mojo_shared_buffer_done_cb) { MojoSharedBufferDoneCB mojo_shared_buffer_done_cb) {
mojo_shared_buffer_done_cb_ = mojo_shared_buffer_done_cb; mojo_shared_buffer_done_cb_ = std::move(mojo_shared_buffer_done_cb);
} }
const mojo::SharedBufferHandle& MojoSharedBufferVideoFrame::Handle() const { const mojo::SharedBufferHandle& MojoSharedBufferVideoFrame::Handle() const {
......
...@@ -27,8 +27,8 @@ class MojoSharedBufferVideoFrame : public VideoFrame { ...@@ -27,8 +27,8 @@ class MojoSharedBufferVideoFrame : public VideoFrame {
// Callback called when this object is destructed. Ownership of the shared // Callback called when this object is destructed. Ownership of the shared
// memory is transferred to the callee. // memory is transferred to the callee.
using MojoSharedBufferDoneCB = using MojoSharedBufferDoneCB =
base::Callback<void(mojo::ScopedSharedBufferHandle buffer, base::OnceCallback<void(mojo::ScopedSharedBufferHandle buffer,
size_t capacity)>; size_t capacity)>;
// Creates a new I420 frame in shared memory with provided parameters // Creates a new I420 frame in shared memory with provided parameters
// (coded_size() == natural_size() == visible_rect()), or returns nullptr. // (coded_size() == natural_size() == visible_rect()), or returns nullptr.
...@@ -78,7 +78,7 @@ class MojoSharedBufferVideoFrame : public VideoFrame { ...@@ -78,7 +78,7 @@ class MojoSharedBufferVideoFrame : public VideoFrame {
// Sets the callback to be called to free the shared buffer. If not null, // Sets the callback to be called to free the shared buffer. If not null,
// it is called on destruction, and is passed ownership of |handle|. // it is called on destruction, and is passed ownership of |handle|.
void SetMojoSharedBufferDoneCB( void SetMojoSharedBufferDoneCB(
const MojoSharedBufferDoneCB& mojo_shared_buffer_done_cb); MojoSharedBufferDoneCB mojo_shared_buffer_done_cb);
private: private:
friend class MojoDecryptorService; friend class MojoDecryptorService;
......
...@@ -165,9 +165,9 @@ TEST(MojoSharedBufferVideoFrameTest, TestDestructionCallback) { ...@@ -165,9 +165,9 @@ TEST(MojoSharedBufferVideoFrameTest, TestDestructionCallback) {
// Set the destruction callback. // Set the destruction callback.
bool callback_called = false; bool callback_called = false;
frame->SetMojoSharedBufferDoneCB(base::Bind(&CompareDestructionCallbackValues, frame->SetMojoSharedBufferDoneCB(
original_handle, requested_size, base::BindOnce(&CompareDestructionCallbackValues, original_handle,
&callback_called)); requested_size, &callback_called));
EXPECT_FALSE(callback_called); EXPECT_FALSE(callback_called);
// Force destruction of |frame|. // Force destruction of |frame|.
......
...@@ -24,10 +24,6 @@ namespace media { ...@@ -24,10 +24,6 @@ namespace media {
namespace { namespace {
typedef base::Callback<void(mojo::ScopedSharedBufferHandle buffer,
size_t capacity)>
MojoSharedBufferDoneCB;
// cdm::Buffer implementation that provides access to mojo shared memory. // cdm::Buffer implementation that provides access to mojo shared memory.
// It owns the memory until Destroy() is called. // It owns the memory until Destroy() is called.
class MojoCdmBuffer : public cdm::Buffer { class MojoCdmBuffer : public cdm::Buffer {
...@@ -35,7 +31,8 @@ class MojoCdmBuffer : public cdm::Buffer { ...@@ -35,7 +31,8 @@ class MojoCdmBuffer : public cdm::Buffer {
static MojoCdmBuffer* Create( static MojoCdmBuffer* Create(
mojo::ScopedSharedBufferHandle buffer, mojo::ScopedSharedBufferHandle buffer,
size_t capacity, size_t capacity,
const MojoSharedBufferDoneCB& mojo_shared_buffer_done_cb) { MojoSharedBufferVideoFrame::MojoSharedBufferDoneCB
mojo_shared_buffer_done_cb) {
DCHECK(buffer.is_valid()); DCHECK(buffer.is_valid());
DCHECK(mojo_shared_buffer_done_cb); DCHECK(mojo_shared_buffer_done_cb);
...@@ -43,7 +40,7 @@ class MojoCdmBuffer : public cdm::Buffer { ...@@ -43,7 +40,7 @@ class MojoCdmBuffer : public cdm::Buffer {
DCHECK_LE(capacity, std::numeric_limits<uint32_t>::max()); DCHECK_LE(capacity, std::numeric_limits<uint32_t>::max());
return new MojoCdmBuffer(std::move(buffer), return new MojoCdmBuffer(std::move(buffer),
base::checked_cast<uint32_t>(capacity), base::checked_cast<uint32_t>(capacity),
mojo_shared_buffer_done_cb); std::move(mojo_shared_buffer_done_cb));
} }
// cdm::Buffer implementation. // cdm::Buffer implementation.
...@@ -52,8 +49,9 @@ class MojoCdmBuffer : public cdm::Buffer { ...@@ -52,8 +49,9 @@ class MojoCdmBuffer : public cdm::Buffer {
mapping_.reset(); mapping_.reset();
// If nobody has claimed the handle, then return it. // If nobody has claimed the handle, then return it.
if (buffer_.is_valid()) if (buffer_.is_valid()) {
mojo_shared_buffer_done_cb_.Run(std::move(buffer_), capacity_); std::move(mojo_shared_buffer_done_cb_).Run(std::move(buffer_), capacity_);
}
// No need to exist anymore. // No need to exist anymore.
delete this; delete this;
...@@ -77,9 +75,10 @@ class MojoCdmBuffer : public cdm::Buffer { ...@@ -77,9 +75,10 @@ class MojoCdmBuffer : public cdm::Buffer {
private: private:
MojoCdmBuffer(mojo::ScopedSharedBufferHandle buffer, MojoCdmBuffer(mojo::ScopedSharedBufferHandle buffer,
uint32_t capacity, uint32_t capacity,
const MojoSharedBufferDoneCB& mojo_shared_buffer_done_cb) MojoSharedBufferVideoFrame::MojoSharedBufferDoneCB
mojo_shared_buffer_done_cb)
: buffer_(std::move(buffer)), : buffer_(std::move(buffer)),
mojo_shared_buffer_done_cb_(mojo_shared_buffer_done_cb), mojo_shared_buffer_done_cb_(std::move(mojo_shared_buffer_done_cb)),
capacity_(capacity), capacity_(capacity),
size_(0) { size_(0) {
mapping_ = buffer_->Map(capacity_); mapping_ = buffer_->Map(capacity_);
...@@ -92,7 +91,8 @@ class MojoCdmBuffer : public cdm::Buffer { ...@@ -92,7 +91,8 @@ class MojoCdmBuffer : public cdm::Buffer {
} }
mojo::ScopedSharedBufferHandle buffer_; mojo::ScopedSharedBufferHandle buffer_;
MojoSharedBufferDoneCB mojo_shared_buffer_done_cb_; MojoSharedBufferVideoFrame::MojoSharedBufferDoneCB
mojo_shared_buffer_done_cb_;
mojo::ScopedSharedBufferMapping mapping_; mojo::ScopedSharedBufferMapping mapping_;
uint32_t capacity_; uint32_t capacity_;
...@@ -105,9 +105,9 @@ class MojoCdmBuffer : public cdm::Buffer { ...@@ -105,9 +105,9 @@ class MojoCdmBuffer : public cdm::Buffer {
// out of the data. // out of the data.
class MojoCdmVideoFrame : public VideoFrameImpl { class MojoCdmVideoFrame : public VideoFrameImpl {
public: public:
explicit MojoCdmVideoFrame( explicit MojoCdmVideoFrame(MojoSharedBufferVideoFrame::MojoSharedBufferDoneCB
const MojoSharedBufferDoneCB& mojo_shared_buffer_done_cb) mojo_shared_buffer_done_cb)
: mojo_shared_buffer_done_cb_(mojo_shared_buffer_done_cb) {} : mojo_shared_buffer_done_cb_(std::move(mojo_shared_buffer_done_cb)) {}
~MojoCdmVideoFrame() final = default; ~MojoCdmVideoFrame() final = default;
// VideoFrameImpl implementation. // VideoFrameImpl implementation.
...@@ -143,13 +143,16 @@ class MojoCdmVideoFrame : public VideoFrameImpl { ...@@ -143,13 +143,16 @@ class MojoCdmVideoFrame : public VideoFrameImpl {
// |frame| could fail to be created if the memory can't be mapped into // |frame| could fail to be created if the memory can't be mapped into
// this address space. // this address space.
if (frame) if (frame) {
frame->SetMojoSharedBufferDoneCB(mojo_shared_buffer_done_cb_); frame->SetMojoSharedBufferDoneCB(std::move(mojo_shared_buffer_done_cb_));
}
return frame; return frame;
} }
private: private:
MojoSharedBufferDoneCB mojo_shared_buffer_done_cb_; MojoSharedBufferVideoFrame::MojoSharedBufferDoneCB
mojo_shared_buffer_done_cb_;
DISALLOW_COPY_AND_ASSIGN(MojoCdmVideoFrame); DISALLOW_COPY_AND_ASSIGN(MojoCdmVideoFrame);
}; };
...@@ -175,8 +178,9 @@ cdm::Buffer* MojoCdmAllocator::CreateCdmBuffer(size_t capacity) { ...@@ -175,8 +178,9 @@ cdm::Buffer* MojoCdmAllocator::CreateCdmBuffer(size_t capacity) {
auto found = available_buffers_.lower_bound(capacity); auto found = available_buffers_.lower_bound(capacity);
if (found == available_buffers_.end()) { if (found == available_buffers_.end()) {
buffer = AllocateNewBuffer(&capacity); buffer = AllocateNewBuffer(&capacity);
if (!buffer.is_valid()) if (!buffer.is_valid()) {
return nullptr; return nullptr;
}
} else { } else {
capacity = found->first; capacity = found->first;
buffer = std::move(found->second); buffer = std::move(found->second);
...@@ -196,8 +200,8 @@ cdm::Buffer* MojoCdmAllocator::CreateCdmBuffer(size_t capacity) { ...@@ -196,8 +200,8 @@ cdm::Buffer* MojoCdmAllocator::CreateCdmBuffer(size_t capacity) {
std::unique_ptr<VideoFrameImpl> MojoCdmAllocator::CreateCdmVideoFrame() { std::unique_ptr<VideoFrameImpl> MojoCdmAllocator::CreateCdmVideoFrame() {
DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(thread_checker_.CalledOnValidThread());
return std::make_unique<MojoCdmVideoFrame>( return std::make_unique<MojoCdmVideoFrame>(
base::Bind(&MojoCdmAllocator::AddBufferToAvailableMap, base::BindOnce(&MojoCdmAllocator::AddBufferToAvailableMap,
weak_ptr_factory_.GetWeakPtr())); weak_ptr_factory_.GetWeakPtr()));
} }
mojo::ScopedSharedBufferHandle MojoCdmAllocator::AllocateNewBuffer( mojo::ScopedSharedBufferHandle MojoCdmAllocator::AllocateNewBuffer(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment