Change H264AnnexBBitstreamConverter to use AVCDecoderConfigurationRecord.

Change H264AnnexBBitstreamConverter to use AVCDecoderConfigurationRecord.

H264AnnexBBitstreamConverter has a bunch of parsing code that essentially
duplicates what is in AVCDecoderConfigurationRecord. This change removes
the duplicate code so that the same logic is used by
H264AnnexBBitstreamConverter and the MP4StreamParser.

BUG=364925
TESTS=All existing tests still pass.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=268305

[scherkus (not reviewing), 2014-05-05 18:22:17]

nits

https://codereview.chromium.org/264743016/diff/20001/media/filters/ffmpeg_dem...
File media/filters/ffmpeg_demuxer.cc (right):

https://codereview.chromium.org/264743016/diff/20001/media/filters/ffmpeg_dem...
media/filters/ffmpeg_demuxer.cc:305: NOTREACHED() << "Proprietary codecs not
enabled.";
will this end up being logspam?

https://codereview.chromium.org/264743016/diff/20001/media/filters/h264_to_an...
File media/filters/h264_to_annex_b_bitstream_converter.cc (right):

https://codereview.chromium.org/264743016/diff/20001/media/filters/h264_to_an...
media/filters/h264_to_annex_b_bitstream_converter.cc:72: DCHECK(input);
ditto

https://codereview.chromium.org/264743016/diff/20001/media/filters/h264_to_an...
media/filters/h264_to_annex_b_bitstream_converter.cc:133: DCHECK(output_size);
non helpful dchecks

https://codereview.chromium.org/264743016/diff/20001/media/filters/h264_to_an...
media/filters/h264_to_annex_b_bitstream_converter.cc:156: DCHECK(input);
non-helpful DCHECKS as they get derefed

https://codereview.chromium.org/264743016/diff/20001/media/formats/mp4/box_de...
File media/formats/mp4/box_definitions.cc (right):

https://codereview.chromium.org/264743016/diff/20001/media/formats/mp4/box_de...
media/formats/mp4/box_definitions.cc:356: DCHECK(data);
you DCHECK here but BufferReader CHECK()s

remove?

https://codereview.chromium.org/264743016/diff/20001/media/formats/mp4/box_de...
File media/formats/mp4/box_definitions.h (right):

https://codereview.chromium.org/264743016/diff/20001/media/formats/mp4/box_de...
media/formats/mp4/box_definitions.h:153: bool Parse(const uint8* data, int
data_size);
docs / how is this different from the one from DECLARE_BOX_METHODS():

virtual bool Parse(BoxReader* reader) OVERRIDE;


can we have the caller construct a BoxReader instead?

https://codereview.chromium.org/264743016/diff/20001/media/formats/mp4/box_de...
media/formats/mp4/box_definitions.h:167: private:
indent one more

https://codereview.chromium.org/264743016/diff/20001/media/formats/mp4/box_re...
File media/formats/mp4/box_reader.h (right):

https://codereview.chromium.org/264743016/diff/20001/media/formats/mp4/box_re...
media/formats/mp4/box_reader.h:33: CHECK(buf);
did you mean to add these?, or are these leftover from development?

[acolwell GONE FROM CHROMIUM, 2014-05-05 18:49:02]

https://codereview.chromium.org/264743016/diff/20001/media/filters/ffmpeg_dem...
File media/filters/ffmpeg_demuxer.cc (right):

https://codereview.chromium.org/264743016/diff/20001/media/filters/ffmpeg_dem...
media/filters/ffmpeg_demuxer.cc:305: NOTREACHED() << "Proprietary codecs not
enabled.";
On 2014/05/05 18:22:17, scherkus wrote:
> will this end up being logspam?
It shouldn't. Nothing should be calling this method if proprietary codecs isn't
enabled. If it is, then we want to know about it and I think it is worth the
spam in that case.

https://codereview.chromium.org/264743016/diff/20001/media/filters/h264_to_an...
File media/filters/h264_to_annex_b_bitstream_converter.cc (right):

https://codereview.chromium.org/264743016/diff/20001/media/filters/h264_to_an...
media/filters/h264_to_annex_b_bitstream_converter.cc:72: DCHECK(input);
On 2014/05/05 18:22:17, scherkus wrote:
> ditto

Done.

https://codereview.chromium.org/264743016/diff/20001/media/filters/h264_to_an...
media/filters/h264_to_annex_b_bitstream_converter.cc:133: DCHECK(output_size);
On 2014/05/05 18:22:17, scherkus wrote:
> non helpful dchecks

Done.

https://codereview.chromium.org/264743016/diff/20001/media/filters/h264_to_an...
media/filters/h264_to_annex_b_bitstream_converter.cc:156: DCHECK(input);
On 2014/05/05 18:22:17, scherkus wrote:
> non-helpful DCHECKS as they get derefed

Done.

https://codereview.chromium.org/264743016/diff/20001/media/formats/mp4/box_de...
File media/formats/mp4/box_definitions.cc (right):

https://codereview.chromium.org/264743016/diff/20001/media/formats/mp4/box_de...
media/formats/mp4/box_definitions.cc:356: DCHECK(data);
On 2014/05/05 18:22:17, scherkus wrote:
> you DCHECK here but BufferReader CHECK()s
> 
> remove?

Done.

https://codereview.chromium.org/264743016/diff/20001/media/formats/mp4/box_de...
File media/formats/mp4/box_definitions.h (right):

https://codereview.chromium.org/264743016/diff/20001/media/formats/mp4/box_de...
media/formats/mp4/box_definitions.h:153: bool Parse(const uint8* data, int
data_size);
On 2014/05/05 18:22:17, scherkus wrote:
> docs / how is this different from the one from DECLARE_BOX_METHODS():

Done.

> 
> virtual bool Parse(BoxReader* reader) OVERRIDE;
> 
> 
> can we have the caller construct a BoxReader instead?
No we can't use the BoxReader because the data passed to this function doesn't
actually have the box header. This function allows you to parse the contents of
the box, w/o the need for the box header.

https://codereview.chromium.org/264743016/diff/20001/media/formats/mp4/box_de...
media/formats/mp4/box_definitions.h:167: private:
On 2014/05/05 18:22:17, scherkus wrote:
> indent one more

Done.

https://codereview.chromium.org/264743016/diff/20001/media/formats/mp4/box_re...
File media/formats/mp4/box_reader.h (right):

https://codereview.chromium.org/264743016/diff/20001/media/formats/mp4/box_re...
media/formats/mp4/box_reader.h:33: CHECK(buf);
On 2014/05/05 18:22:17, scherkus wrote:
> did you mean to add these?, or are these leftover from development?
Yes. This is intended to protect release code from invalid data that may have
snuck by our normal defenses. The CHECK(buf) isn't probably a big deal, but a
negative size could be problematic.

[scherkus (not reviewing), 2014-05-05 18:50:38]

lgtm

[acolwell GONE FROM CHROMIUM, 2014-05-05 19:06:55]

The CQ bit was checked by acolwell@chromium.org

[commit-bot: I haz the power, 2014-05-05 19:07:23]

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

[commit-bot: I haz the power, 2014-05-05 23:13:33]

Change committed as 268305