Change who allocated the MediaPipe's shared buffer.
Have the MediaPipe's implementor receive a client allocated shared buffer
instead of having the client fetch a shared buffer from the implementor.
BUG=
R=dalesat@chromium.org
Review URL: https://codereview.chromium.org/1694963002 .
diff --git a/examples/audio_play_test/play_tone.cc b/examples/audio_play_test/play_tone.cc
index fdc18f2..321261b 100644
--- a/examples/audio_play_test/play_tone.cc
+++ b/examples/audio_play_test/play_tone.cc
@@ -121,7 +121,6 @@
// Configure our sink for 16-bit 48KHz mono.
AudioTrackConfigurationPtr cfg = AudioTrackConfiguration::New();
- cfg->max_frames = USecToBytes(BUF_DEPTH_USEC) / FRAME_BYTES;
LpcmMediaTypeDetailsPtr pcm_cfg = LpcmMediaTypeDetails::New();
pcm_cfg->sample_format = LpcmSampleFormat::SIGNED_16;
@@ -140,6 +139,7 @@
// proxy, pass its interface to our circular buffer helper, set up our
// high/low water marks, register our callback, and start to buffer our audio.
audio_pipe_.reset(new CircularBufferMediaPipeAdapter(pipe.Pass()));
+ audio_pipe_->Init(USecToBytes(BUF_DEPTH_USEC));
audio_pipe_->SetSignalCallback(
[this](MediaResult res) -> void {
GenerateToneCbk(res);
diff --git a/examples/audio_play_test/play_wav.cc b/examples/audio_play_test/play_wav.cc
index 03cb392..6b89231 100644
--- a/examples/audio_play_test/play_wav.cc
+++ b/examples/audio_play_test/play_wav.cc
@@ -267,7 +267,6 @@
AudioTrackConfigurationPtr cfg;
cfg = AudioTrackConfiguration::New();
- cfg->max_frames = USecToFrames(BUF_DEPTH_USEC);
cfg->audio_frame_ratio = tmp.numerator;
cfg->media_time_ratio = tmp.denominator;
@@ -296,6 +295,7 @@
// Set up our media pipe helper, configure its callback and water marks to
// kick off the playback process.
audio_pipe_.reset(new CircularBufferMediaPipeAdapter(media_pipe.Pass()));
+ audio_pipe_->Init(USecToBytes(BUF_DEPTH_USEC));
audio_pipe_->SetWatermarks(USecToBytes(BUF_HI_WATER_USEC),
USecToBytes(BUF_LO_WATER_USEC));
audio_pipe_->SetSignalCallback(
diff --git a/mojo/dart/packages/mojo_services/lib/mojo/media/audio_track.mojom.dart b/mojo/dart/packages/mojo_services/lib/mojo/media/audio_track.mojom.dart
index 127421d..4121604 100644
--- a/mojo/dart/packages/mojo_services/lib/mojo/media/audio_track.mojom.dart
+++ b/mojo/dart/packages/mojo_services/lib/mojo/media/audio_track.mojom.dart
@@ -109,10 +109,9 @@
class AudioTrackConfiguration extends bindings.Struct {
static const List<bindings.StructDataHeader> kVersions = const [
- const bindings.StructDataHeader(32, 0)
+ const bindings.StructDataHeader(24, 0)
];
media_types_mojom.MediaType mediaType = null;
- int maxFrames = 0;
int audioFrameRatio = 1;
int mediaTimeRatio = 1;
@@ -158,15 +157,11 @@
}
if (mainDataHeader.version >= 0) {
- result.maxFrames = decoder0.decodeUint64(16);
+ result.audioFrameRatio = decoder0.decodeUint32(16);
}
if (mainDataHeader.version >= 0) {
- result.audioFrameRatio = decoder0.decodeUint32(24);
- }
- if (mainDataHeader.version >= 0) {
-
- result.mediaTimeRatio = decoder0.decodeUint32(28);
+ result.mediaTimeRatio = decoder0.decodeUint32(20);
}
return result;
}
@@ -181,21 +176,14 @@
rethrow;
}
try {
- encoder0.encodeUint64(maxFrames, 16);
- } on bindings.MojoCodecError catch(e) {
- e.message = "Error encountered while encoding field "
- "maxFrames of struct AudioTrackConfiguration: $e";
- rethrow;
- }
- try {
- encoder0.encodeUint32(audioFrameRatio, 24);
+ encoder0.encodeUint32(audioFrameRatio, 16);
} on bindings.MojoCodecError catch(e) {
e.message = "Error encountered while encoding field "
"audioFrameRatio of struct AudioTrackConfiguration: $e";
rethrow;
}
try {
- encoder0.encodeUint32(mediaTimeRatio, 28);
+ encoder0.encodeUint32(mediaTimeRatio, 20);
} on bindings.MojoCodecError catch(e) {
e.message = "Error encountered while encoding field "
"mediaTimeRatio of struct AudioTrackConfiguration: $e";
@@ -206,7 +194,6 @@
String toString() {
return "AudioTrackConfiguration("
"mediaType: $mediaType" ", "
- "maxFrames: $maxFrames" ", "
"audioFrameRatio: $audioFrameRatio" ", "
"mediaTimeRatio: $mediaTimeRatio" ")";
}
@@ -214,7 +201,6 @@
Map toJson() {
Map map = new Map();
map["mediaType"] = mediaType;
- map["maxFrames"] = maxFrames;
map["audioFrameRatio"] = audioFrameRatio;
map["mediaTimeRatio"] = mediaTimeRatio;
return map;
diff --git a/mojo/dart/packages/mojo_services/lib/mojo/media/media_pipe.mojom.dart b/mojo/dart/packages/mojo_services/lib/mojo/media/media_pipe.mojom.dart
index a367aa6..ecf7139 100644
--- a/mojo/dart/packages/mojo_services/lib/mojo/media/media_pipe.mojom.dart
+++ b/mojo/dart/packages/mojo_services/lib/mojo/media/media_pipe.mojom.dart
@@ -252,17 +252,16 @@
-class MediaPipeState extends bindings.Struct {
+class _MediaPipeSetBufferParams extends bindings.Struct {
static const List<bindings.StructDataHeader> kVersions = const [
const bindings.StructDataHeader(24, 0)
];
- static const int kMaxPayloadLen = 4611686018427387903;
- core.MojoSharedBuffer payloadBuffer = null;
- int payloadBufferLen = 0;
+ core.MojoSharedBuffer buffer = null;
+ int size = 0;
- MediaPipeState() : super(kVersions.last.size);
+ _MediaPipeSetBufferParams() : super(kVersions.last.size);
- static MediaPipeState deserialize(bindings.Message message) {
+ static _MediaPipeSetBufferParams deserialize(bindings.Message message) {
var decoder = new bindings.Decoder(message);
var result = decode(decoder);
if (decoder.excessHandles != null) {
@@ -271,11 +270,11 @@
return result;
}
- static MediaPipeState decode(bindings.Decoder decoder0) {
+ static _MediaPipeSetBufferParams decode(bindings.Decoder decoder0) {
if (decoder0 == null) {
return null;
}
- MediaPipeState result = new MediaPipeState();
+ _MediaPipeSetBufferParams result = new _MediaPipeSetBufferParams();
var mainDataHeader = decoder0.decodeStructDataHeader();
if (mainDataHeader.version <= kVersions.last.version) {
@@ -297,11 +296,11 @@
}
if (mainDataHeader.version >= 0) {
- result.payloadBuffer = decoder0.decodeSharedBufferHandle(8, false);
+ result.buffer = decoder0.decodeSharedBufferHandle(8, false);
}
if (mainDataHeader.version >= 0) {
- result.payloadBufferLen = decoder0.decodeUint64(16);
+ result.size = decoder0.decodeUint64(16);
}
return result;
}
@@ -309,159 +308,25 @@
void encode(bindings.Encoder encoder) {
var encoder0 = encoder.getStructEncoderAtOffset(kVersions.last);
try {
- encoder0.encodeSharedBufferHandle(payloadBuffer, 8, false);
+ encoder0.encodeSharedBufferHandle(buffer, 8, false);
} on bindings.MojoCodecError catch(e) {
e.message = "Error encountered while encoding field "
- "payloadBuffer of struct MediaPipeState: $e";
+ "buffer of struct _MediaPipeSetBufferParams: $e";
rethrow;
}
try {
- encoder0.encodeUint64(payloadBufferLen, 16);
+ encoder0.encodeUint64(size, 16);
} on bindings.MojoCodecError catch(e) {
e.message = "Error encountered while encoding field "
- "payloadBufferLen of struct MediaPipeState: $e";
+ "size of struct _MediaPipeSetBufferParams: $e";
rethrow;
}
}
String toString() {
- return "MediaPipeState("
- "payloadBuffer: $payloadBuffer" ", "
- "payloadBufferLen: $payloadBufferLen" ")";
- }
-
- Map toJson() {
- throw new bindings.MojoCodecError(
- 'Object containing handles cannot be encoded to JSON.');
- }
-}
-
-
-
-
-class _MediaPipeGetStateParams extends bindings.Struct {
- static const List<bindings.StructDataHeader> kVersions = const [
- const bindings.StructDataHeader(8, 0)
- ];
-
- _MediaPipeGetStateParams() : super(kVersions.last.size);
-
- static _MediaPipeGetStateParams deserialize(bindings.Message message) {
- var decoder = new bindings.Decoder(message);
- var result = decode(decoder);
- if (decoder.excessHandles != null) {
- decoder.excessHandles.forEach((h) => h.close());
- }
- return result;
- }
-
- static _MediaPipeGetStateParams decode(bindings.Decoder decoder0) {
- if (decoder0 == null) {
- return null;
- }
- _MediaPipeGetStateParams result = new _MediaPipeGetStateParams();
-
- var mainDataHeader = decoder0.decodeStructDataHeader();
- if (mainDataHeader.version <= kVersions.last.version) {
- // Scan in reverse order to optimize for more recent versions.
- for (int i = kVersions.length - 1; i >= 0; --i) {
- if (mainDataHeader.version >= kVersions[i].version) {
- if (mainDataHeader.size == kVersions[i].size) {
- // Found a match.
- break;
- }
- throw new bindings.MojoCodecError(
- 'Header size doesn\'t correspond to known version size.');
- }
- }
- } else if (mainDataHeader.size < kVersions.last.size) {
- throw new bindings.MojoCodecError(
- 'Message newer than the last known version cannot be shorter than '
- 'required by the last known version.');
- }
- return result;
- }
-
- void encode(bindings.Encoder encoder) {
- encoder.getStructEncoderAtOffset(kVersions.last);
- }
-
- String toString() {
- return "_MediaPipeGetStateParams("")";
- }
-
- Map toJson() {
- Map map = new Map();
- return map;
- }
-}
-
-
-
-
-class MediaPipeGetStateResponseParams extends bindings.Struct {
- static const List<bindings.StructDataHeader> kVersions = const [
- const bindings.StructDataHeader(16, 0)
- ];
- MediaPipeState state = null;
-
- MediaPipeGetStateResponseParams() : super(kVersions.last.size);
-
- static MediaPipeGetStateResponseParams deserialize(bindings.Message message) {
- var decoder = new bindings.Decoder(message);
- var result = decode(decoder);
- if (decoder.excessHandles != null) {
- decoder.excessHandles.forEach((h) => h.close());
- }
- return result;
- }
-
- static MediaPipeGetStateResponseParams decode(bindings.Decoder decoder0) {
- if (decoder0 == null) {
- return null;
- }
- MediaPipeGetStateResponseParams result = new MediaPipeGetStateResponseParams();
-
- var mainDataHeader = decoder0.decodeStructDataHeader();
- if (mainDataHeader.version <= kVersions.last.version) {
- // Scan in reverse order to optimize for more recent versions.
- for (int i = kVersions.length - 1; i >= 0; --i) {
- if (mainDataHeader.version >= kVersions[i].version) {
- if (mainDataHeader.size == kVersions[i].size) {
- // Found a match.
- break;
- }
- throw new bindings.MojoCodecError(
- 'Header size doesn\'t correspond to known version size.');
- }
- }
- } else if (mainDataHeader.size < kVersions.last.size) {
- throw new bindings.MojoCodecError(
- 'Message newer than the last known version cannot be shorter than '
- 'required by the last known version.');
- }
- if (mainDataHeader.version >= 0) {
-
- var decoder1 = decoder0.decodePointer(8, false);
- result.state = MediaPipeState.decode(decoder1);
- }
- return result;
- }
-
- void encode(bindings.Encoder encoder) {
- var encoder0 = encoder.getStructEncoderAtOffset(kVersions.last);
- try {
- encoder0.encodeStruct(state, 8, false);
- } on bindings.MojoCodecError catch(e) {
- e.message = "Error encountered while encoding field "
- "state of struct MediaPipeGetStateResponseParams: $e";
- rethrow;
- }
- }
-
- String toString() {
- return "MediaPipeGetStateResponseParams("
- "state: $state" ")";
+ return "_MediaPipeSetBufferParams("
+ "buffer: $buffer" ", "
+ "size: $size" ")";
}
Map toJson() {
@@ -746,7 +611,7 @@
-const int _MediaPipe_getStateName = 0;
+const int _MediaPipe_setBufferName = 0;
const int _MediaPipe_sendPacketName = 1;
const int _MediaPipe_flushName = 2;
@@ -819,9 +684,10 @@
abstract class MediaPipe {
static const String serviceName = null;
- dynamic getState([Function responseFactory = null]);
+ void setBuffer(core.MojoSharedBuffer buffer, int size);
dynamic sendPacket(MediaPacket packet,[Function responseFactory = null]);
dynamic flush([Function responseFactory = null]);
+ static const int kMaxBufferLen = 4611686018427387903;
}
@@ -845,26 +711,6 @@
void handleResponse(bindings.ServiceMessage message) {
switch (message.header.type) {
- case _MediaPipe_getStateName:
- var r = MediaPipeGetStateResponseParams.deserialize(
- message.payload);
- if (!message.header.hasRequestId) {
- proxyError("Expected a message with a valid request Id.");
- return;
- }
- Completer c = completerMap[message.header.requestId];
- if (c == null) {
- proxyError(
- "Message had unknown request Id: ${message.header.requestId}");
- return;
- }
- completerMap.remove(message.header.requestId);
- if (c.isCompleted) {
- proxyError("Response completer already completed");
- return;
- }
- c.complete(r);
- break;
case _MediaPipe_sendPacketName:
var r = MediaPipeSendPacketResponseParams.deserialize(
message.payload);
@@ -923,13 +769,15 @@
_MediaPipeProxyImpl _proxyImpl;
_MediaPipeProxyCalls(this._proxyImpl);
- dynamic getState([Function responseFactory = null]) {
- var params = new _MediaPipeGetStateParams();
- return _proxyImpl.sendMessageWithRequestId(
- params,
- _MediaPipe_getStateName,
- -1,
- bindings.MessageHeader.kMessageExpectsResponse);
+ void setBuffer(core.MojoSharedBuffer buffer, int size) {
+ if (!_proxyImpl.isBound) {
+ _proxyImpl.proxyError("The Proxy is closed.");
+ return;
+ }
+ var params = new _MediaPipeSetBufferParams();
+ params.buffer = buffer;
+ params.size = size;
+ _proxyImpl.sendMessage(params, _MediaPipe_setBufferName);
}
dynamic sendPacket(MediaPacket packet,[Function responseFactory = null]) {
var params = new _MediaPipeSendPacketParams();
@@ -1029,11 +877,6 @@
}
- MediaPipeGetStateResponseParams _MediaPipeGetStateResponseParamsFactory(MediaPipeState state) {
- var mojo_factory_result = new MediaPipeGetStateResponseParams();
- mojo_factory_result.state = state;
- return mojo_factory_result;
- }
MediaPipeSendPacketResponseParams _MediaPipeSendPacketResponseParamsFactory(MediaPipeSendResult result) {
var mojo_factory_result = new MediaPipeSendPacketResponseParams();
mojo_factory_result.result = result;
@@ -1052,27 +895,10 @@
}
assert(_impl != null);
switch (message.header.type) {
- case _MediaPipe_getStateName:
- var params = _MediaPipeGetStateParams.deserialize(
+ case _MediaPipe_setBufferName:
+ var params = _MediaPipeSetBufferParams.deserialize(
message.payload);
- var response = _impl.getState(_MediaPipeGetStateResponseParamsFactory);
- if (response is Future) {
- return response.then((response) {
- if (response != null) {
- return buildResponseWithId(
- response,
- _MediaPipe_getStateName,
- message.header.requestId,
- bindings.MessageHeader.kMessageIsResponse);
- }
- });
- } else if (response != null) {
- return buildResponseWithId(
- response,
- _MediaPipe_getStateName,
- message.header.requestId,
- bindings.MessageHeader.kMessageIsResponse);
- }
+ _impl.setBuffer(params.buffer, params.size);
break;
case _MediaPipe_sendPacketName:
var params = _MediaPipeSendPacketParams.deserialize(
diff --git a/mojo/services/media/audio/interfaces/audio_track.mojom b/mojo/services/media/audio/interfaces/audio_track.mojom
index 3456485..a8b8e07 100644
--- a/mojo/services/media/audio/interfaces/audio_track.mojom
+++ b/mojo/services/media/audio/interfaces/audio_track.mojom
@@ -19,9 +19,6 @@
// The media type to use.
MediaType media_type;
- // Desired maximum buffer size, in frames of audio.
- uint64 max_frames;
-
// Ratio of audio frames to media time ticks.
//
// Presentation time stamps on audio packets are expressed in units of media
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 4ce301f..406c50a 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
@@ -39,11 +39,6 @@
MOJO_DCHECK(pipe_);
MOJO_DCHECK(RunLoop::current());
- pipe_get_state_cbk_ = MediaPipe::GetStateCallback(
- [this] (MediaPipeStatePtr state) {
- HandleGetState(state.Pass());
- });
-
pipe_flush_cbk_ = MediaPipe::FlushCallback(
[this] () {
HandleFlush();
@@ -61,12 +56,6 @@
(*thiz)->HandleSignalCallback();
}
});
-
-
- // Begin by getting a hold of the shared buffer from our pipe over which we
- // will push data.
- MOJO_DCHECK(get_state_in_progress_);
- pipe_->GetState(pipe_get_state_cbk_);
}
CircularBufferMediaPipeAdapter::~CircularBufferMediaPipeAdapter() {
@@ -74,6 +63,63 @@
Cleanup();
}
+void CircularBufferMediaPipeAdapter::Init(uint64_t size) {
+ // Double Init? Fault unless the user is asking us to set up a buffer of the
+ // same size.
+ if (buffer_handle_.is_valid()) {
+ if (buffer_size_ != size) {
+ Fault(MediaResult::BAD_STATE);
+ }
+ return;
+ }
+
+ // Make the buffer
+ MojoResult res = CreateSharedBuffer(nullptr, size, &buffer_handle_);
+ if (res != MOJO_RESULT_OK) {
+ MOJO_LOG(ERROR) << "Failed to allocate buffer of size " << size
+ << " in " << __PRETTY_FUNCTION__
+ << " (error " << res << ")";
+ Fault(MediaResult::INSUFFICIENT_RESOURCES);
+ return;
+ }
+
+ buffer_size_ = size;
+
+ // Map the buffer
+ //
+ // TODO(johngro) : We really only need write access to this buffer.
+ // Ideally, we could request that using flags, but there does not seem to be
+ // a way to do this right now.
+ res = MapBuffer(buffer_handle_.get(),
+ 0,
+ buffer_size_,
+ &buffer_,
+ MOJO_MAP_BUFFER_FLAG_NONE);
+ if (res != MOJO_RESULT_OK) {
+ MOJO_LOG(ERROR) << "Failed to map buffer in " << __PRETTY_FUNCTION__
+ << " (error " << res << ")";
+ Fault(MediaResult::UNKNOWN_ERROR);
+ return;
+ }
+
+ // Duplicate the buffer and send it to the other side of the pipe.
+ //
+ // TODO(johngro) : It would be nice if we could restrict this handle to be
+ // read-only.
+ ScopedSharedBufferHandle duplicated_handle;
+ res = DuplicateBuffer(buffer_handle_.get(), nullptr, &duplicated_handle);
+ if (res != MOJO_RESULT_OK) {
+ MOJO_LOG(ERROR) << "Failed to duplicate handle in " << __PRETTY_FUNCTION__
+ << " (error " << res << ")";
+ Fault(MediaResult::UNKNOWN_ERROR);
+ 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_);
+}
+
void CircularBufferMediaPipeAdapter::SetSignalCallback(SignalCbk cbk) {
bool schedule;
signal_cbk_ = cbk;
@@ -298,78 +344,12 @@
return MediaResult::OK;
}
-void CircularBufferMediaPipeAdapter::HandleGetState(MediaPipeStatePtr state) {
- MOJO_DCHECK(state); // We must have a state structure.
- MOJO_DCHECK(!buffer_); // We must not have already mapped a buffer.
- MOJO_DCHECK(get_state_in_progress_); // We should be waiting for our cbk.
-
- // Success or failure, we are no longer waiting for our get state callback.
- get_state_in_progress_ = false;
- rd_ = wr_ = 0;
-
- // Double init? How did that happen?
- if (buffer_handle_.is_valid() || (nullptr != buffer_)) {
- MOJO_LOG(ERROR) << "Double init during " << __PRETTY_FUNCTION__;
- Fault(MediaResult::UNKNOWN_ERROR);
- return;
- }
-
- // No shared buffer? That's a fatal error.
- if (!state->payload_buffer.is_valid()) {
- MOJO_LOG(ERROR) << "Null payload buffer in " << __PRETTY_FUNCTION__;
- Fault(MediaResult::UNKNOWN_ERROR);
- return;
- }
-
- // stash our state
- buffer_handle_ = state->payload_buffer.Pass();
- buffer_size_ = state->payload_buffer_len;
-
- // Sanity checks the buffer size.
- if (!buffer_size_ || (buffer_size_ > MediaPipeState::kMaxPayloadLen)) {
- MOJO_LOG(ERROR) << "Bad buffer size in " << __PRETTY_FUNCTION__
- << " (" << buffer_size_ << ")";
- Fault(MediaResult::BAD_STATE);
- return;
- }
-
- // Map our buffer
- //
- // TODO(johngro) : We really only need write access to this buffer.
- // Ideally, we could request that using flags, but there does not seem to be
- // a way to do this right now.
- MojoResult res;
- res = MapBuffer(buffer_handle_.get(),
- 0,
- buffer_size_,
- &buffer_,
- MOJO_MAP_BUFFER_FLAG_NONE);
- if (res != MOJO_RESULT_OK) {
- MOJO_LOG(ERROR) << "Failed to map buffer in " << __PRETTY_FUNCTION__
- << " (error " << res << ")";
- Fault(MediaResult::UNKNOWN_ERROR);
- return;
- }
-
- // Init is complete, we may be signalled now.
- UpdateSignalled();
-}
-
void CircularBufferMediaPipeAdapter::HandleSendPacket(
uint32_t seq_num,
MediaPipe::SendResult result) {
MediaPipe::SendPacketCallback cbk;
do {
- if (get_state_in_progress_) {
- // If we are in the process of getting the initial state of the system,
- // then something is seriously wrong. The other end of this interface is
- // sending us Send callbacks while we are in the process of initializing
- // (something which should be impossible)
- Fault(MediaResult::PROTOCOL_ERROR);
- break;
- }
-
// There should be at least one element in the in-flight queue, and the
// front of the queue's sequence number should match the sequence number of
// the payload being returned to us.
diff --git a/mojo/services/media/common/cpp/circular_buffer_media_pipe_adapter.h b/mojo/services/media/common/cpp/circular_buffer_media_pipe_adapter.h
index 73fe68d..679b624 100644
--- a/mojo/services/media/common/cpp/circular_buffer_media_pipe_adapter.h
+++ b/mojo/services/media/common/cpp/circular_buffer_media_pipe_adapter.h
@@ -70,10 +70,34 @@
*/
using SignalCbk = std::function<void(MediaResult state)>;
+ /**
+ * Constructor
+ *
+ * Create an adapter which will take ownership of the provided MediaPipe
+ * interface and assist in the process of generating MediaPackets and
+ * marshalling them to the other side of the MediaPipe.
+ *
+ * @param pipe A pointer to the MediaPipe interface which will be used as the
+ * target for MediaPackets.
+ */
explicit CircularBufferMediaPipeAdapter(MediaPipePtr pipe);
+
+ /**
+ * Destructor
+ */
~CircularBufferMediaPipeAdapter();
/**
+ * Init
+ *
+ * Allocate a shared memory buffer of the specified size and begin the process
+ * of marshalling it to the other side of the MediaPipe.
+ *
+ * @param size The size in bytes of the shared memory buffer to allocate.
+ */
+ void Init(uint64_t size);
+
+ /**
* Set the signal callback for this media pipe adapter. This callback will be
* called when the adapter transitions from un-signalled to signalled and has
* a valid callback, or when a valid callback is assigned (via a call to
@@ -205,7 +229,6 @@
};
using PacketStateQueue = std::deque<PacketState>;
- void HandleGetState(MediaPipeStatePtr state);
void HandleSendPacket(uint32_t seq_num, MediaPipe::SendResult result);
void HandleFlush();
void HandleSignalCallback();
@@ -219,12 +242,11 @@
}
bool Busy() const {
- return (get_state_in_progress_ || flush_in_progress_);
+ return flush_in_progress_;
}
// Pipe interface callbacks
MediaPipePtr pipe_;
- MediaPipe::GetStateCallback pipe_get_state_cbk_;
MediaPipe::FlushCallback pipe_flush_cbk_;
Closure signalled_callback_;
@@ -234,7 +256,6 @@
// State for managing signalled/un-signalled status and signal callbacks.
SignalCbk signal_cbk_;
- bool get_state_in_progress_ = true;
bool flush_in_progress_ = false;
bool fault_cbk_made_ = false;
bool cbk_scheduled_ = false;
@@ -246,7 +267,7 @@
ScopedSharedBufferHandle buffer_handle_;
void* buffer_ = nullptr;
- uint64_t buffer_size_;
+ uint64_t buffer_size_ = 0;
uint64_t rd_, wr_;
// Packet queue state
diff --git a/mojo/services/media/common/interfaces/media_pipe.mojom b/mojo/services/media/common/interfaces/media_pipe.mojom
index ca5e400..7efbd2f 100644
--- a/mojo/services/media/common/interfaces/media_pipe.mojom
+++ b/mojo/services/media/common/interfaces/media_pipe.mojom
@@ -61,25 +61,6 @@
// packet?
};
-// MediaPipeState
-//
-// A small structure used by the user of a media pipe to fetch the shared buffer
-// used by the media pipe to move bulk data.
-//
-// TODO(johngro): Right now, the only real purpose for this struture is to
-// bundle the size of the buffer with the buffer handle itself. When buffer
-// handles can have their properties queried directly, it can go away. See
-// domokit/mojo issue #501
-struct MediaPipeState {
- const uint64 kMaxPayloadLen = 0x3FFFFFFFFFFFFFFF;
-
- handle<shared_buffer> payload_buffer;
-
- // TODO(johngro) : Why do I have to send this? Why can't I just query the
- // shared_buffer handle for its length?
- uint64 payload_buffer_len;
-};
-
// MediaPipe
//
// An interface exposed by consumers of media which provides the means for
@@ -90,6 +71,8 @@
// the media has been consumed via the SendPacket callback they provide.
// Finally, the pipeline may be flushed using the Flush method.
interface MediaPipe {
+ const uint64 kMaxBufferLen = 0x3FFFFFFFFFFFFFFF;
+
// An enumeration used to indicate the ultimate fate of packets sent across
// the pipe using the SendPacket method.
enum SendResult {
@@ -97,9 +80,8 @@
FLUSHED, // Some or all of the media was flushed before being consumed.
};
- // Request that a reference to the pipe's state be sent to the caller via
- // callback.
- GetState() => (MediaPipeState state);
+ // Sets the shared buffer in which sent Packet payloads will be located.
+ SetBuffer(handle<shared_buffer> buffer, uint64 size);
// Place a media packet into the pipeline to be consumed. When the consumer
// is finished with the packet, it will invoke the supplied callback to
diff --git a/services/media/audio/audio_track_impl.cc b/services/media/audio/audio_track_impl.cc
index 84ecf25..a12c3af 100644
--- a/services/media/audio/audio_track_impl.cc
+++ b/services/media/audio/audio_track_impl.cc
@@ -225,23 +225,9 @@
}
bytes_per_frame_ *= cfg->channels;
- // Overflow trying to convert from frames to bytes?
- uint64_t requested_frames = configuration->max_frames;
- if (requested_frames >
- (std::numeric_limits<size_t>::max() / bytes_per_frame_)) {
- LOG(ERROR) << "Insufficient resources to create "
- << requested_frames << " frame audio buffer.";
- Shutdown();
- return;
- }
-
- size_t requested_bytes = (requested_frames * bytes_per_frame_);
-
- // Attempt to initialize our shared buffer and bind it to our interface
- // request.
- if (pipe_.Init(req.Pass(), requested_bytes) != MOJO_RESULT_OK) {
- LOG(ERROR) << "Insufficient resources to create "
- << requested_frames << " frame audio buffer.";
+ // Bind our pipe to the interface request.
+ if (pipe_.Init(req.Pass()) != MOJO_RESULT_OK) {
+ LOG(ERROR) << "Failed to media pipe to interface request.";
Shutdown();
return;
}
diff --git a/services/media/common/media_pipe_base.cc b/services/media/common/media_pipe_base.cc
index 642c686..4a7df2d 100644
--- a/services/media/common/media_pipe_base.cc
+++ b/services/media/common/media_pipe_base.cc
@@ -15,27 +15,12 @@
MediaPipeBase::~MediaPipeBase() {
}
-MojoResult MediaPipeBase::Init(InterfaceRequest<MediaPipe> request,
- uint64_t shared_buffer_size) {
+MojoResult MediaPipeBase::Init(InterfaceRequest<MediaPipe> request) {
// Double init?
if (IsInitialized()) {
return MOJO_RESULT_ALREADY_EXISTS;
}
- // Valid size?
- DCHECK_GT(shared_buffer_size, 0u);
- DCHECK_LE(shared_buffer_size, MediaPipeState::kMaxPayloadLen);
- if (!shared_buffer_size ||
- (shared_buffer_size > MediaPipeState::kMaxPayloadLen)) {
- return MOJO_RESULT_INVALID_ARGUMENT;
- }
-
- DCHECK(!buffer_);
- buffer_ = MappedSharedBuffer::Create(shared_buffer_size);
- if (buffer_ == nullptr) {
- return MOJO_RESULT_UNKNOWN;
- }
-
binding_.Bind(request.Pass());
binding_.set_connection_error_handler([this]() -> void {
Reset();
@@ -44,9 +29,7 @@
}
bool MediaPipeBase::IsInitialized() const {
- DCHECK((!binding_.is_bound() && (buffer_ == nullptr)) ||
- (binding_.is_bound() && (buffer_ != nullptr)));
- return !!buffer_;
+ return binding_.is_bound();
}
void MediaPipeBase::Reset() {
@@ -56,27 +39,33 @@
buffer_ = nullptr;
}
-void MediaPipeBase::GetState(const GetStateCallback& cbk) {
- static const MojoDuplicateBufferHandleOptions options = {
- .struct_size = sizeof(*this),
- .flags = MOJO_DUPLICATE_BUFFER_HANDLE_OPTIONS_FLAG_NONE,
- };
-
- MediaPipeStatePtr state_ptr(MediaPipeState::New());
-
- // If we have not been successfully initialized, send back an invalid handle
- // and a zero length.
- if (!buffer_) {
- state_ptr->payload_buffer_len = 0;
- } else {
- MojoResult res = DuplicateBuffer(buffer_->handle().get(),
- &options,
- &state_ptr->payload_buffer);
- state_ptr->payload_buffer_len = buffer_->size();
- DCHECK(MOJO_RESULT_OK == res);
+void MediaPipeBase::SetBuffer(ScopedSharedBufferHandle handle, uint64_t size) {
+ // Double init? Close the connection.
+ if (buffer_) {
+ LOG(ERROR) << "Attempting to set a new buffer (size = "
+ << size
+ << ") on a MediaPipe which already has a buffer (size = "
+ << buffer_->size()
+ << ")";
+ Reset();
+ return;
}
- cbk.Run(state_ptr.Pass());
+ // Invalid size? Close the connection.
+ if (!size || (size > MediaPipe::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_) {
+ LOG(ERROR) << "Failed to map shared memory buffer (size = " << size << ")";
+ Reset();
+ return;
+ }
}
void MediaPipeBase::SendPacket(MediaPacketPtr packet,
@@ -167,8 +156,9 @@
}
MediaPipeBase::MappedSharedBufferPtr MediaPipeBase::MappedSharedBuffer::Create(
- size_t size) {
- MappedSharedBufferPtr ret(new MappedSharedBuffer(size));
+ ScopedSharedBufferHandle handle,
+ uint64_t size) {
+ MappedSharedBufferPtr ret(new MappedSharedBuffer(handle.Pass(), size));
return ret->base() ? ret : nullptr;
}
@@ -179,35 +169,22 @@
}
}
-MediaPipeBase::MappedSharedBuffer::MappedSharedBuffer(size_t size)
- : size_(size) {
- static const MojoCreateSharedBufferOptions opt {
- .struct_size = sizeof(MojoDuplicateBufferHandleOptions),
- .flags = MOJO_CREATE_SHARED_BUFFER_OPTIONS_FLAG_NONE,
- };
- MojoResult res;
+MediaPipeBase::MappedSharedBuffer::MappedSharedBuffer(
+ ScopedSharedBufferHandle handle,
+ size_t size)
+ : handle_(handle.Pass()),
+ size_(size) {
+ MojoResult res = MapBuffer(handle_.get(),
+ 0u,
+ size_,
+ &base_,
+ MOJO_MAP_BUFFER_FLAG_NONE);
- res = CreateSharedBuffer(&opt, size_, &handle_);
- if (MOJO_RESULT_OK == res) {
- // TODO(johngro) : We really only need read access to this buffer. Ideally,
- // we could request that using flags, but there does not seem to be a way to
- // do this right now.
- res = MapBuffer(handle_.get(),
- 0u,
- size_,
- &base_,
- MOJO_MAP_BUFFER_FLAG_NONE);
-
- if (MOJO_RESULT_OK != res) {
- LOG(ERROR) << "Failed to map shared buffer of size " << size_
- << " (res " << res << ")";
- DCHECK(base_ == nullptr);
- handle_.reset();
- }
- } else {
- LOG(ERROR) << "Failed to create shared buffer of size " << size_
+ if (MOJO_RESULT_OK != res) {
+ LOG(ERROR) << "Failed to map shared buffer of size " << size_
<< " (res " << res << ")";
- DCHECK(!handle_.is_valid());
+ DCHECK(base_ == nullptr);
+ handle_.reset();
}
}
diff --git a/services/media/common/media_pipe_base.h b/services/media/common/media_pipe_base.h
index 07bea65..9e7499f 100644
--- a/services/media/common/media_pipe_base.h
+++ b/services/media/common/media_pipe_base.h
@@ -50,8 +50,7 @@
~MediaPipeBase() override;
// Initialize the internal state of the pipe (allocate resources, etc..)
- MojoResult Init(InterfaceRequest<MediaPipe> request,
- uint64_t shared_buffer_size);
+ MojoResult Init(InterfaceRequest<MediaPipe> request);
bool IsInitialized() const;
void Reset();
@@ -59,15 +58,16 @@
protected:
class MappedSharedBuffer {
public:
- static MappedSharedBufferPtr Create(size_t size);
+ static MappedSharedBufferPtr Create(ScopedSharedBufferHandle handle,
+ uint64_t size);
~MappedSharedBuffer();
const ScopedSharedBufferHandle& handle() const { return handle_; }
- size_t size() const { return size_; }
- void* base() const { return base_; }
+ uint64_t size() const { return size_; }
+ void* base() const { return base_; }
private:
- explicit MappedSharedBuffer(size_t size);
+ MappedSharedBuffer(ScopedSharedBufferHandle handle, size_t size);
ScopedSharedBufferHandle handle_;
size_t size_;
@@ -84,7 +84,7 @@
Binding<MediaPipe> binding_;
// MediaPipe.mojom implementation.
- void GetState(const GetStateCallback& cbk) final;
+ void SetBuffer(ScopedSharedBufferHandle handle, uint64_t size) final;
void SendPacket(MediaPacketPtr packet,
const SendPacketCallback& cbk) final;
void Flush(const FlushCallback& cbk) final;
