Refactoring StreamParser & StreamParserHost interfaces.

Refactoring StreamParser & StreamParserHost interfaces.
- Moving ByteQueue from ChunkDemuxer into WebMStreamParser to simplify handling
  multiple StreamParsers.
- Replacing OnAudioConfig & OnVideoConfig with a single OnNewConfigs method to
  make it easier to verify that initialization segments all contain the same
  number and type of streams.


BUG=122909
TEST=Existing ChunkDemuxer tests.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=133725

[scherkus (not reviewing), 2012-04-24 01:40:04]

nice ALL_CAPS-to-kCamelCasing!

LGTM w/ nits

http://codereview.chromium.org/10205004/diff/1/media/base/stream_parser.h
File media/base/stream_parser.h (right):

http://codereview.chromium.org/10205004/diff/1/media/base/stream_parser.h#new...
media/base/stream_parser.h:33: virtual bool OnNewConfigs(const
AudioDecoderConfig& audio_config,
what does true/false mean?

http://codereview.chromium.org/10205004/diff/1/media/filters/chunk_demuxer.cc
File media/filters/chunk_demuxer.cc (right):

http://codereview.chromium.org/10205004/diff/1/media/filters/chunk_demuxer.cc...
media/filters/chunk_demuxer.cc:660: if (!audio_config.IsValidConfig() &&
!video_config.IsValidConfig())
Considering we handle the DCHECK (and that the previous code did not check for
valid configs) -- does this mean we require a unit test for this code path?

If so, then it's tricky because we'll DCHECK so either:
  1) Remove the DCHECK and passing in two invalid configs is valid + write test
  2) Promote the DCHECK to a CHECK and remove this handler

http://codereview.chromium.org/10205004/diff/1/media/filters/chunk_demuxer.cc...
media/filters/chunk_demuxer.cc:663: // Only allow a single audio config for now.
nit: you can either dupe this comment below for video or just include video in
this comment OR if you add LOG(ERROR) (or similar) for the false case the code
should be self documenting

[acolwell GONE FROM CHROMIUM, 2012-04-24 16:25:55]

http://codereview.chromium.org/10205004/diff/1/media/base/stream_parser.h
File media/base/stream_parser.h (right):

http://codereview.chromium.org/10205004/diff/1/media/base/stream_parser.h#new...
media/base/stream_parser.h:33: virtual bool OnNewConfigs(const
AudioDecoderConfig& audio_config,
On 2012/04/24 01:40:04, scherkus wrote:
> what does true/false mean?

Done.

http://codereview.chromium.org/10205004/diff/1/media/filters/chunk_demuxer.cc
File media/filters/chunk_demuxer.cc (right):

http://codereview.chromium.org/10205004/diff/1/media/filters/chunk_demuxer.cc...
media/filters/chunk_demuxer.cc:660: if (!audio_config.IsValidConfig() &&
!video_config.IsValidConfig())
On 2012/04/24 01:40:04, scherkus wrote:
> Considering we handle the DCHECK (and that the previous code did not check for
> valid configs) -- does this mean we require a unit test for this code path?
> 
> If so, then it's tricky because we'll DCHECK so either:
>   1) Remove the DCHECK and passing in two invalid configs is valid + write
test
>   2) Promote the DCHECK to a CHECK and remove this handler

Done. Picked option 2

http://codereview.chromium.org/10205004/diff/1/media/filters/chunk_demuxer.cc...
media/filters/chunk_demuxer.cc:663: // Only allow a single audio config for now.
On 2012/04/24 01:40:04, scherkus wrote:
> nit: you can either dupe this comment below for video or just include video in
> this comment OR if you add LOG(ERROR) (or similar) for the false case the code
> should be self documenting

Done.

[commit-bot: I haz the power, 2012-04-24 16:29:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/acolwell@chromium.org/10205004/8001

[commit-bot: I haz the power, 2012-04-24 18:00:49]

Change committed as 133725