Take advantage of MojoGetBufferInformation
Now that we can ask a shared buffer what size it is before we map it, stop
explicitly passing the size across the media subsystem's MediaConsumer
interface.
BUG=
R=dalesat@chromium.org, viettrungluu@chromium.org
Review URL: https://codereview.chromium.org/1823833003 .
diff --git a/mojo/dart/packages/mojo_services/lib/mojo/media/media_transport.mojom.dart b/mojo/dart/packages/mojo_services/lib/mojo/media/media_transport.mojom.dart
index c8ffc41..14b2d3a 100644
--- a/mojo/dart/packages/mojo_services/lib/mojo/media/media_transport.mojom.dart
+++ b/mojo/dart/packages/mojo_services/lib/mojo/media/media_transport.mojom.dart
@@ -769,10 +769,9 @@
class _MediaConsumerSetBufferParams extends bindings.Struct {
static const List<bindings.StructDataHeader> kVersions = const [
- const bindings.StructDataHeader(24, 0)
+ const bindings.StructDataHeader(16, 0)
];
core.MojoSharedBuffer buffer = null;
- int size = 0;
_MediaConsumerSetBufferParams() : super(kVersions.last.size);
@@ -813,10 +812,6 @@
result.buffer = decoder0.decodeSharedBufferHandle(8, false);
}
- if (mainDataHeader.version >= 0) {
-
- result.size = decoder0.decodeUint64(16);
- }
return result;
}
@@ -829,19 +824,11 @@
"buffer of struct _MediaConsumerSetBufferParams: $e";
rethrow;
}
- try {
- encoder0.encodeUint64(size, 16);
- } on bindings.MojoCodecError catch(e) {
- e.message = "Error encountered while encoding field "
- "size of struct _MediaConsumerSetBufferParams: $e";
- rethrow;
- }
}
String toString() {
return "_MediaConsumerSetBufferParams("
- "buffer: $buffer" ", "
- "size: $size" ")";
+ "buffer: $buffer" ")";
}
Map toJson() {
@@ -1906,7 +1893,7 @@
abstract class MediaConsumer {
static const String serviceName = null;
- dynamic setBuffer(core.MojoSharedBuffer buffer,int size,[Function responseFactory = null]);
+ dynamic setBuffer(core.MojoSharedBuffer buffer,[Function responseFactory = null]);
dynamic sendPacket(MediaPacket packet,[Function responseFactory = null]);
dynamic prime([Function responseFactory = null]);
dynamic flush([Function responseFactory = null]);
@@ -2032,10 +2019,9 @@
_MediaConsumerProxyImpl _proxyImpl;
_MediaConsumerProxyCalls(this._proxyImpl);
- dynamic setBuffer(core.MojoSharedBuffer buffer,int size,[Function responseFactory = null]) {
+ dynamic setBuffer(core.MojoSharedBuffer buffer,[Function responseFactory = null]) {
var params = new _MediaConsumerSetBufferParams();
params.buffer = buffer;
- params.size = size;
return _proxyImpl.sendMessageWithRequestId(
params,
_mediaConsumerMethodSetBufferName,
@@ -2177,7 +2163,7 @@
case _mediaConsumerMethodSetBufferName:
var params = _MediaConsumerSetBufferParams.deserialize(
message.payload);
- var response = _impl.setBuffer(params.buffer,params.size,_mediaConsumerSetBufferResponseParamsFactory);
+ var response = _impl.setBuffer(params.buffer,_mediaConsumerSetBufferResponseParamsFactory);
if (response is Future) {
return response.then((response) {
if (response != null) {
diff --git a/mojo/services/media/common/cpp/circular_buffer_media_pipe_adapter.cc b/mojo/services/media/common/cpp/circular_buffer_media_pipe_adapter.cc
index 180577c..abf23f4 100644
--- a/mojo/services/media/common/cpp/circular_buffer_media_pipe_adapter.cc
+++ b/mojo/services/media/common/cpp/circular_buffer_media_pipe_adapter.cc
@@ -115,9 +115,7 @@
return;
}
- // TODO(johngro) : We should not have to send the buffer size, it should be an
- // intrinsic property of the buffer itself and be query-able via the handle.
- pipe_->SetBuffer(duplicated_handle.Pass(), buffer_size_, []() {});
+ pipe_->SetBuffer(duplicated_handle.Pass(), []() {});
}
void CircularBufferMediaPipeAdapter::SetSignalCallback(SignalCbk cbk) {
diff --git a/mojo/services/media/common/cpp/mapped_shared_buffer.cc b/mojo/services/media/common/cpp/mapped_shared_buffer.cc
index 5d0cb4b..976858a 100644
--- a/mojo/services/media/common/cpp/mapped_shared_buffer.cc
+++ b/mojo/services/media/common/cpp/mapped_shared_buffer.cc
@@ -18,25 +18,30 @@
buffer_.reset(new SharedBuffer(size));
handle_.reset();
- InitInternal(buffer_->handle, size);
+ InitInternal(buffer_->handle);
}
-void MappedSharedBuffer::InitFromHandle(
- ScopedSharedBufferHandle handle,
- uint64_t size) {
+void MappedSharedBuffer::InitFromHandle(ScopedSharedBufferHandle handle) {
MOJO_DCHECK(handle.is_valid());
- MOJO_DCHECK(size > 0);
buffer_.reset();
handle_ = handle.Pass();
- InitInternal(handle_, size);
+ InitInternal(handle_);
}
-void MappedSharedBuffer::InitInternal(
- ScopedSharedBufferHandle& handle,
- uint64_t size) {
+void MappedSharedBuffer::InitInternal(const ScopedSharedBufferHandle& handle) {
MOJO_DCHECK(handle.is_valid());
+
+ // Query the buffer for its size.
+ // TODO(johngro) : It would be nice if we could do something other than
+ // DCHECK if things don't go exactly our way.
+ MojoBufferInformation info;
+ MojoResult res = MojoGetBufferInformation(handle.get().value(),
+ &info,
+ sizeof(info));
+ uint64_t size = info.num_bytes;
+ MOJO_DCHECK(res == MOJO_RESULT_OK);
MOJO_DCHECK(size > 0);
size_ = size;
@@ -45,7 +50,7 @@
void* ptr;
auto result = MapBuffer(
handle.get(),
- 0, // offset
+ 0, // offset
size,
&ptr,
MOJO_MAP_BUFFER_FLAG_NONE);
@@ -100,5 +105,5 @@
void MappedSharedBuffer::OnInit() {}
-} // namespace media
-} // namespace mojo
+} // namespace media
+} // namespace mojo
diff --git a/mojo/services/media/common/cpp/mapped_shared_buffer.h b/mojo/services/media/common/cpp/mapped_shared_buffer.h
index 5fd7e8a..9002393 100644
--- a/mojo/services/media/common/cpp/mapped_shared_buffer.h
+++ b/mojo/services/media/common/cpp/mapped_shared_buffer.h
@@ -31,7 +31,7 @@
void InitNew(uint64_t size);
// Initializes from a handle to an existing shared buffer.
- void InitFromHandle(ScopedSharedBufferHandle handle, uint64_t size);
+ void InitFromHandle(ScopedSharedBufferHandle handle);
// Indicates whether the buffer is initialized.
bool initialized() const;
@@ -49,7 +49,7 @@
uint64_t OffsetFromPtr(void *payload_ptr) const;
protected:
- void InitInternal(ScopedSharedBufferHandle& handle, uint64_t size);
+ void InitInternal(const ScopedSharedBufferHandle& handle);
// Does nothing. Called when initialization is complete. Subclasses may
// override.
@@ -78,4 +78,4 @@
} // namespace media
} // namespace mojo
-#endif // MOJO_SERVICES_MEDIA_COMMON_CPP_MAPPED_SHARED_BUFFER_H_
+#endif // MOJO_SERVICES_MEDIA_COMMON_CPP_MAPPED_SHARED_BUFFER_H_
diff --git a/mojo/services/media/common/interfaces/media_transport.mojom b/mojo/services/media/common/interfaces/media_transport.mojom
index da31dfd..736a8ce 100644
--- a/mojo/services/media/common/interfaces/media_transport.mojom
+++ b/mojo/services/media/common/interfaces/media_transport.mojom
@@ -119,7 +119,7 @@
};
// Sets the shared buffer in which packet payload will be located.
- SetBuffer(handle<shared_buffer> buffer, uint64 size) => ();
+ SetBuffer(handle<shared_buffer> buffer) => ();
// Sends a packet to the consumer. The callback signals that the consumer
// is done with the packet buffer region.
diff --git a/services/media/common/media_pipe_base.cc b/services/media/common/media_pipe_base.cc
index 069283c..6c8e2c5 100644
--- a/services/media/common/media_pipe_base.cc
+++ b/services/media/common/media_pipe_base.cc
@@ -40,27 +40,38 @@
}
void MediaPipeBase::SetBuffer(ScopedSharedBufferHandle handle,
- uint64_t size,
const SetBufferCallback& cbk) {
+ DCHECK(handle.is_valid());
+
// Double init? Close the connection.
if (buffer_) {
- LOG(ERROR) << "Attempting to set a new buffer (size = "
- << size
- << ") on a MediaConsumer which already has a buffer (size = "
+ LOG(ERROR) << "Attempting to set a new buffer on a MediaConsumer which "
+ "already has a buffer assigned. (size = "
<< buffer_->size()
<< ")";
Reset();
return;
}
+ // Query the buffer for its size. If we fail to query the info, close the
+ // connection.
+ MojoResult res;
+ MojoBufferInformation info;
+ res = MojoGetBufferInformation(handle.get().value(), &info, sizeof(info));
+ if (res != MOJO_RESULT_OK) {
+ LOG(ERROR) << "Failed to query shared buffer info (res = " << res << ")";
+ Reset();
+ return;
+ }
+
// Invalid size? Close the connection.
+ uint64_t size = info.num_bytes;
if (!size || (size > MediaConsumer::kMaxBufferLen)) {
LOG(ERROR) << "Invalid shared buffer size (size = " << size << ")";
Reset();
return;
}
-
// Failed to map the buffer? Close the connection.
buffer_ = MappedSharedBuffer::Create(handle.Pass(), size);
if (!buffer_) {
@@ -183,7 +194,7 @@
MediaPipeBase::MappedSharedBuffer::MappedSharedBuffer(
ScopedSharedBufferHandle handle,
- size_t size)
+ uint64_t size)
: handle_(handle.Pass()),
size_(size) {
MojoResult res = MapBuffer(handle_.get(),
diff --git a/services/media/common/media_pipe_base.h b/services/media/common/media_pipe_base.h
index e5d3284..96c8afc 100644
--- a/services/media/common/media_pipe_base.h
+++ b/services/media/common/media_pipe_base.h
@@ -67,10 +67,10 @@
void* base() const { return base_; }
private:
- MappedSharedBuffer(ScopedSharedBufferHandle handle, size_t size);
+ MappedSharedBuffer(ScopedSharedBufferHandle handle, uint64_t size);
ScopedSharedBufferHandle handle_;
- size_t size_;
+ uint64_t size_;
void* base_ = nullptr;
};
@@ -85,7 +85,6 @@
// MediaConsumer.mojom implementation.
void SetBuffer(ScopedSharedBufferHandle handle,
- uint64_t size,
const SetBufferCallback& cbk) final;
void SendPacket(MediaPacketPtr packet,
const SendPacketCallback& cbk) final;
diff --git a/services/media/framework_mojo/mojo_consumer.cc b/services/media/framework_mojo/mojo_consumer.cc
index 2efb5e3..37f3032 100644
--- a/services/media/framework_mojo/mojo_consumer.cc
+++ b/services/media/framework_mojo/mojo_consumer.cc
@@ -37,9 +37,8 @@
void MojoConsumer::SetBuffer(
ScopedSharedBufferHandle buffer,
- uint64_t size,
const SetBufferCallback& callback) {
- buffer_.InitFromHandle(buffer.Pass(), size);
+ buffer_.InitFromHandle(buffer.Pass());
callback.Run();
}
@@ -116,5 +115,5 @@
delete this;
}
-} // namespace media
-} // namespace mojo
+} // namespace media
+} // namespace mojo
diff --git a/services/media/framework_mojo/mojo_consumer.h b/services/media/framework_mojo/mojo_consumer.h
index 076623e..c464373 100644
--- a/services/media/framework_mojo/mojo_consumer.h
+++ b/services/media/framework_mojo/mojo_consumer.h
@@ -52,7 +52,6 @@
// MediaConsumer implementation.
void SetBuffer(
ScopedSharedBufferHandle buffer,
- uint64_t size,
const SetBufferCallback& callback) override;
void SendPacket(MediaPacketPtr packet, const SendPacketCallback& callback)
diff --git a/services/media/framework_mojo/mojo_producer.cc b/services/media/framework_mojo/mojo_producer.cc
index 744feed..a876395 100644
--- a/services/media/framework_mojo/mojo_producer.cc
+++ b/services/media/framework_mojo/mojo_producer.cc
@@ -29,7 +29,7 @@
{
base::AutoLock lock(lock_);
- max_pushes_outstanding_ = 10; // TODO(dalesat): Made up!
+ max_pushes_outstanding_ = 10; // TODO(dalesat): Made up!
demand = current_pushes_outstanding_ < max_pushes_outstanding_ ?
Demand::kPositive :
Demand::kNegative;
@@ -57,7 +57,7 @@
DCHECK(consumer_.is_bound());
consumer_->Flush(callback);
first_pts_since_flush_ = Packet::kUnknownPts;
- end_of_stream_= false;
+ end_of_stream_ = false;
}
void MojoProducer::SetStatusCallback(
@@ -129,12 +129,11 @@
consumer_ = MediaConsumerPtr::Create(std::move(consumer));
if (!mojo_allocator_.initialized()) {
- mojo_allocator_.InitNew(256 * 1024); // TODO(dalesat): Made up!
+ mojo_allocator_.InitNew(256 * 1024); // TODO(dalesat): Made up!
}
consumer_->SetBuffer(
mojo_allocator_.GetDuplicateHandle(),
- mojo_allocator_.size(),
[callback]() {
callback.Run();
});
@@ -198,5 +197,5 @@
return media_packet.Pass();
}
-} // namespace media
-} // namespace mojo
+} // namespace media
+} // namespace mojo
