Switch from a DataPipe per DecoderBuffer to a single one.

Switch from a DataPipe per DecoderBuffer to a single one.

Instead of letting the type converter for DecoderBuffer handle
transfer of the DecoderBuffer::data(), clients are now expected
to handle this externally.

MojoDemuxerStreamAdapter now deserializing the data section via
a shared pipe created by MojoDemuxerStreamImpl.  The pipe is
sized dynamically for audio or video content.

We don't have framed DataPipe support yet, but this will at least
prevent us from creating thousands of DataPipes per minute.

BUG=392236
TEST=html_viewer using audio/video urls.

Committed: https://crrev.com/0134ed1292b006e48e293a8863b1dff0bed5e871
Cr-Commit-Position: refs/heads/master@{#307328}

[DaleCurtis, 2014-12-05 20:55:42]

dalecurtis@chromium.org changed reviewers:
+ xhwang@chromium.org

[DaleCurtis, 2014-12-05 20:55:42]

Please just glance through right now; I've been trying to test this for over a
week now since html_viewer has been busted... It looks like it'll be a while
longer, so at least we should discuss this for now.

[DaleCurtis, 2014-12-05 22:48:44]

Okay using the headless shell I can confirm this all works! \o/

[xhwang, 2014-12-06 00:03:45]

It's looking pretty good. Just have some comments.

https://codereview.chromium.org/760523008/diff/20001/media/base/decoder_buffe...
File media/base/decoder_buffer.cc (right):

https://codereview.chromium.org/760523008/diff/20001/media/base/decoder_buffe...
media/base/decoder_buffer.cc:111: base::AlignedAlloc(side_data_size_ +
kPaddingSize, kAlignmentSize)));
Can we have a centralized place to call AlignedAlloc for side data? We are doing
the same on l.46.

https://codereview.chromium.org/760523008/diff/20001/media/mojo/interfaces/de...
File media/mojo/interfaces/demuxer_stream.mojom (right):

https://codereview.chromium.org/760523008/diff/20001/media/mojo/interfaces/de...
media/mojo/interfaces/demuxer_stream.mojom:40: // Informs the client that the
stream is ready for reading.  If |pipe| is
pipe muse be present now

https://codereview.chromium.org/760523008/diff/20001/media/mojo/interfaces/de...
media/mojo/interfaces/demuxer_stream.mojom:45: // payloads from the DataPipe
directly.  If |pipe| is NULL, it means the
pipe cannot be null now.

https://codereview.chromium.org/760523008/diff/20001/media/mojo/interfaces/me...
File media/mojo/interfaces/media_types.mojom (right):

https://codereview.chromium.org/760523008/diff/20001/media/mojo/interfaces/me...
media/mojo/interfaces/media_types.mojom:193: // The number of bytes in |data|.
|data| doesn't exist now. Add a comment that the |data| is in a separate
DataPipe associated with this stream etc.

https://codereview.chromium.org/760523008/diff/20001/media/mojo/services/mojo...
File media/mojo/services/mojo_demuxer_stream_adapter.cc (right):

https://codereview.chromium.org/760523008/diff/20001/media/mojo/services/mojo...
media/mojo/services/mojo_demuxer_stream_adapter.cc:136: } else {
How about kAborted?

https://codereview.chromium.org/760523008/diff/20001/media/mojo/services/mojo...
File media/mojo/services/mojo_demuxer_stream_impl.cc (right):

https://codereview.chromium.org/760523008/diff/20001/media/mojo/services/mojo...
media/mojo/services/mojo_demuxer_stream_impl.cc:88: options.capacity_num_bytes =
1.5 * (1024 * 1024);
nit: we have the config; we might be able to do better in estimating sizes...

https://codereview.chromium.org/760523008/diff/20001/media/mojo/services/mojo...
media/mojo/services/mojo_demuxer_stream_impl.cc:90: 
nit: How about Demuxer::TEXT?

https://codereview.chromium.org/760523008/diff/20001/media/mojo/services/mojo...
File media/mojo/services/mojo_renderer_impl.cc (right):

https://codereview.chromium.org/760523008/diff/20001/media/mojo/services/mojo...
media/mojo/services/mojo_renderer_impl.cc:16: #include
"mojo/public/cpp/system/data_pipe.h"
What is this for?

[DaleCurtis, 2014-12-06 00:42:59]

https://codereview.chromium.org/760523008/diff/20001/media/base/decoder_buffe...
File media/base/decoder_buffer.cc (right):

https://codereview.chromium.org/760523008/diff/20001/media/base/decoder_buffe...
media/base/decoder_buffer.cc:111: base::AlignedAlloc(side_data_size_ +
kPaddingSize, kAlignmentSize)));
On 2014/12/06 00:03:44, xhwang wrote:
> Can we have a centralized place to call AlignedAlloc for side data? We are
doing
> the same on l.46.

Done.

https://codereview.chromium.org/760523008/diff/20001/media/mojo/interfaces/de...
File media/mojo/interfaces/demuxer_stream.mojom (right):

https://codereview.chromium.org/760523008/diff/20001/media/mojo/interfaces/de...
media/mojo/interfaces/demuxer_stream.mojom:40: // Informs the client that the
stream is ready for reading.  If |pipe| is
On 2014/12/06 00:03:44, xhwang wrote:
> pipe muse be present now

Done.

https://codereview.chromium.org/760523008/diff/20001/media/mojo/interfaces/de...
media/mojo/interfaces/demuxer_stream.mojom:45: // payloads from the DataPipe
directly.  If |pipe| is NULL, it means the
On 2014/12/06 00:03:44, xhwang wrote:
> pipe cannot be null now.

Done.

https://codereview.chromium.org/760523008/diff/20001/media/mojo/interfaces/me...
File media/mojo/interfaces/media_types.mojom (right):

https://codereview.chromium.org/760523008/diff/20001/media/mojo/interfaces/me...
media/mojo/interfaces/media_types.mojom:193: // The number of bytes in |data|.
On 2014/12/06 00:03:44, xhwang wrote:
> |data| doesn't exist now. Add a comment that the |data| is in a separate
> DataPipe associated with this stream etc.

Done.

https://codereview.chromium.org/760523008/diff/20001/media/mojo/services/mojo...
File media/mojo/services/mojo_demuxer_stream_adapter.cc (right):

https://codereview.chromium.org/760523008/diff/20001/media/mojo/services/mojo...
media/mojo/services/mojo_demuxer_stream_adapter.cc:136: } else {
On 2014/12/06 00:03:44, xhwang wrote:
> How about kAborted?

Done.

https://codereview.chromium.org/760523008/diff/20001/media/mojo/services/mojo...
File media/mojo/services/mojo_demuxer_stream_impl.cc (right):

https://codereview.chromium.org/760523008/diff/20001/media/mojo/services/mojo...
media/mojo/services/mojo_demuxer_stream_impl.cc:88: options.capacity_num_bytes =
1.5 * (1024 * 1024);
On 2014/12/06 00:03:44, xhwang wrote:
> nit: we have the config; we might be able to do better in estimating sizes...

I can try and figure out some metric based on resolution, but then what about
config changes... this is only created once?

https://codereview.chromium.org/760523008/diff/20001/media/mojo/services/mojo...
media/mojo/services/mojo_demuxer_stream_impl.cc:90: 
On 2014/12/06 00:03:44, xhwang wrote:
> nit: How about Demuxer::TEXT?

Flipped so only video gets the large pipe.

https://codereview.chromium.org/760523008/diff/20001/media/mojo/services/mojo...
File media/mojo/services/mojo_renderer_impl.cc (right):

https://codereview.chromium.org/760523008/diff/20001/media/mojo/services/mojo...
media/mojo/services/mojo_renderer_impl.cc:16: #include
"mojo/public/cpp/system/data_pipe.h"
On 2014/12/06 00:03:44, xhwang wrote:
> What is this for?

Originally to fix a compile error... but it seems no more. Removed.

[xhwang, 2014-12-06 01:16:54]

lgtm

https://codereview.chromium.org/760523008/diff/20001/media/mojo/services/mojo...
File media/mojo/services/mojo_demuxer_stream_impl.cc (right):

https://codereview.chromium.org/760523008/diff/20001/media/mojo/services/mojo...
media/mojo/services/mojo_demuxer_stream_impl.cc:88: options.capacity_num_bytes =
1.5 * (1024 * 1024);
On 2014/12/06 00:42:59, DaleCurtis wrote:
> On 2014/12/06 00:03:44, xhwang wrote:
> > nit: we have the config; we might be able to do better in estimating
sizes...
> 
> I can try and figure out some metric based on resolution, but then what about
> config changes... this is only created once?

Low priority for now. But we may need to have some heuristic way to do this in
the future.

[DaleCurtis, 2014-12-08 19:50:34]

The CQ bit was checked by dalecurtis@chromium.org

[commit-bot: I haz the power, 2014-12-08 19:51:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/760523008/40001

[commit-bot: I haz the power, 2014-12-08 20:44:58]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-12-08 20:46:26]

Patchset 3 (id:??) landed as
https://crrev.com/0134ed1292b006e48e293a8863b1dff0bed5e871
Cr-Commit-Position: refs/heads/master@{#307328}