Whatever happened to DataPipe supporting framed read/writes?

Sorry, I don't follow your question... Could you please elaborate a bit more?

Part of the complexity is due to the fact that DataPipe does not support framed IO, which requires using both message pipe and data pipe. I remember seeing a bug in old mojo days to support framed IO. Now I cannot find it. There is also a TODO about this in mojo_demuxer_stream_impl.cc:

// TODO(dalecurtis): Once we can write framed data to the DataPipe, fill via
// the producer handle and then read more to keep the pipe full.  Waiting for
// space can be accomplished using an AsyncWaiter.

That said, sharing the code to read/write DecoderBuffer would be nice. Even when we switch to using DataPipe exclusively, it will be transparent to all client of this DecoderBuffer IO utility.

I don't know exactly what you mean by "framed IO."

Well, it seems an orthogonal issue then. Whatever solution we have, we should not duplicate the solution, which is what this bug is about.

My memory is very vague about the early DataPipe discussions. For that specific comment, my understanding is that if needed, we can proactively fill the DataPipe with more DataBuffers, but then we need a way to tell us when the DataPipe is full (AsyncWaiter). That's still something we can consider in the future, if DemuxerStream read is a bottleneck of the whole pipeline.

Yeah, I have no idea what "we can write framed data to the DataPipe" means... Maybe it's referring to the fact that back then we can only write some fixed length data (packet size), and for writing/reading arbitrary binary data, we have to use packet size of 1 byte.

Oh, so it sounds like you want the pipe to track metadata about the size of each "frame" from the producer, and an API to expose the size of the next available frame to the consumer.

Hmm. I guess I don't see any reason to bake this into the low level data pipe API. We could define a helper class in mojo/public/cpp/system to do this for you using the existing C APIs, or you could just build your own for media code. Doesn't seem too complex to me, though I may be overlooking some important detail.

Agree about not duplicating code.

BTW I found some discussion about framed read/write wrt to decoder buffers:
- https://bugs.chromium.org/p/chromium/issues/detail?id=432960
- https://docs.google.com/document/d/19QzXFcp5EVnYli6yxBO2Y8sDYqmsZojpJu1D-kABaXA/edit?usp=sharing

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1087d4ca6be621c1e687987b40df73c27783a800

commit 1087d4ca6be621c1e687987b40df73c27783a800
Author: xhwang <xhwang@chromium.org>
Date: Tue Jun 28 17:18:34 2016

media: Add MojoDecoderBuffer{Reader|Writer}

This removes duplicate code in clients/services that transfer/receive
media DecoderBuffers over DataPipe.

The implementation of *::ConvertDecoderBuffer() is
copied from MojoAudioDecoder*, which has recently
been updated and reviewed by mojo owners
(https://chromiumcodereview.appspot.com/1982883003/)

BUG= 621784 
TEST=Moved unittest to a new file with improvements.

Review-Url: https://codereview.chromium.org/2096063003
Cr-Commit-Position: refs/heads/master@{#402496}

[modify] https://crrev.com/1087d4ca6be621c1e687987b40df73c27783a800/media/mojo/BUILD.gn
[modify] https://crrev.com/1087d4ca6be621c1e687987b40df73c27783a800/media/mojo/clients/mojo_audio_decoder.cc
[modify] https://crrev.com/1087d4ca6be621c1e687987b40df73c27783a800/media/mojo/clients/mojo_audio_decoder.h
[modify] https://crrev.com/1087d4ca6be621c1e687987b40df73c27783a800/media/mojo/clients/mojo_decryptor.cc
[modify] https://crrev.com/1087d4ca6be621c1e687987b40df73c27783a800/media/mojo/clients/mojo_decryptor.h
[modify] https://crrev.com/1087d4ca6be621c1e687987b40df73c27783a800/media/mojo/clients/mojo_demuxer_stream_impl.cc
[modify] https://crrev.com/1087d4ca6be621c1e687987b40df73c27783a800/media/mojo/clients/mojo_demuxer_stream_impl.h
[modify] https://crrev.com/1087d4ca6be621c1e687987b40df73c27783a800/media/mojo/clients/mojo_video_decoder.cc
[modify] https://crrev.com/1087d4ca6be621c1e687987b40df73c27783a800/media/mojo/clients/mojo_video_decoder.h
[modify] https://crrev.com/1087d4ca6be621c1e687987b40df73c27783a800/media/mojo/common/BUILD.gn
[add] https://crrev.com/1087d4ca6be621c1e687987b40df73c27783a800/media/mojo/common/mojo_decoder_buffer_converter.cc
[add] https://crrev.com/1087d4ca6be621c1e687987b40df73c27783a800/media/mojo/common/mojo_decoder_buffer_converter.h
[add] https://crrev.com/1087d4ca6be621c1e687987b40df73c27783a800/media/mojo/common/mojo_decoder_buffer_converter_unittest.cc
[modify] https://crrev.com/1087d4ca6be621c1e687987b40df73c27783a800/media/mojo/services/mojo_audio_decoder_service.cc
[modify] https://crrev.com/1087d4ca6be621c1e687987b40df73c27783a800/media/mojo/services/mojo_audio_decoder_service.h
[modify] https://crrev.com/1087d4ca6be621c1e687987b40df73c27783a800/media/mojo/services/mojo_decryptor_service.cc
[modify] https://crrev.com/1087d4ca6be621c1e687987b40df73c27783a800/media/mojo/services/mojo_decryptor_service.h
[modify] https://crrev.com/1087d4ca6be621c1e687987b40df73c27783a800/media/mojo/services/mojo_demuxer_stream_adapter.cc
[modify] https://crrev.com/1087d4ca6be621c1e687987b40df73c27783a800/media/mojo/services/mojo_demuxer_stream_adapter.h
[modify] https://crrev.com/1087d4ca6be621c1e687987b40df73c27783a800/media/mojo/services/mojo_video_decoder_service.cc
[modify] https://crrev.com/1087d4ca6be621c1e687987b40df73c27783a800/media/mojo/services/mojo_video_decoder_service.h