Tighten up media::DecoderBuffer data requirements.

Tighten up media::DecoderBuffer data requirements.

Calling DecoderBuffer::CopyFrom(NULL, 0) is a crashable offense as it's strong evidence that there's a bug in a demuxer generating NULL-containing packets.

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

[scherkus (not reviewing), 2013-01-16 23:19:25]

I'm not super happy w/ this CL but it at least makes us more strict while
getting a workaround for FFmpeg returning NULL data + provides a test.

[acolwell GONE FROM CHROMIUM, 2013-01-17 00:00:08]

LGTM % nits

https://codereview.chromium.org/11887011/diff/8001/media/base/decoder_buffer.cc
File media/base/decoder_buffer.cc (right):

https://codereview.chromium.org/11887011/diff/8001/media/base/decoder_buffer....
media/base/decoder_buffer.cc:20: buffer_size_ = 0;
nit: CHECK_EQ(buffer_size, 0);

https://codereview.chromium.org/11887011/diff/8001/media/base/decoder_buffer....
media/base/decoder_buffer.cc:41: CHECK_GE(data_size, 0);
nit: Perhaps move this one into the constructor?

https://codereview.chromium.org/11887011/diff/8001/media/filters/ffmpeg_demux...
File media/filters/ffmpeg_demuxer_unittest.cc (right):

https://codereview.chromium.org/11887011/diff/8001/media/filters/ffmpeg_demux...
media/filters/ffmpeg_demuxer_unittest.cc:622: // mistakently creating end of
stream buffers http://crbug.com/169133
s/mistakently/mistakenly

https://codereview.chromium.org/11887011/diff/8001/media/filters/ffmpeg_demux...
media/filters/ffmpeg_demuxer_unittest.cc:624: #if
!defined(USE_PROPRIETARY_CODECS)
nit: Why don't we just put all these tests in a single #if block or put the
whole test inside an #if block? It doesn't seem like there is much value in
having a empty test run in Chromium builds.

[scherkus (not reviewing), 2013-01-17 00:16:30]

also renamed DecoderBuffer::buffer_size_ to DecoderBuffer::size_ (it was
confusing the heck out of me that we had data_ + buffer_size_)

https://codereview.chromium.org/11887011/diff/8001/media/base/decoder_buffer.cc
File media/base/decoder_buffer.cc (right):

https://codereview.chromium.org/11887011/diff/8001/media/base/decoder_buffer....
media/base/decoder_buffer.cc:20: buffer_size_ = 0;
On 2013/01/17 00:00:08, acolwell wrote:
> nit: CHECK_EQ(buffer_size, 0);

Done.

https://codereview.chromium.org/11887011/diff/8001/media/base/decoder_buffer....
media/base/decoder_buffer.cc:41: CHECK_GE(data_size, 0);
On 2013/01/17 00:00:08, acolwell wrote:
> nit: Perhaps move this one into the constructor?

Looks like a DCHECK equivalent is in Initialize() -- made that one a CHECK()
instead

https://codereview.chromium.org/11887011/diff/8001/media/filters/ffmpeg_demux...
File media/filters/ffmpeg_demuxer_unittest.cc (right):

https://codereview.chromium.org/11887011/diff/8001/media/filters/ffmpeg_demux...
media/filters/ffmpeg_demuxer_unittest.cc:622: // mistakently creating end of
stream buffers http://crbug.com/169133
On 2013/01/17 00:00:08, acolwell wrote:
> s/mistakently/mistakenly

Done.

https://codereview.chromium.org/11887011/diff/8001/media/filters/ffmpeg_demux...
media/filters/ffmpeg_demuxer_unittest.cc:624: #if
!defined(USE_PROPRIETARY_CODECS)
On 2013/01/17 00:00:08, acolwell wrote:
> nit: Why don't we just put all these tests in a single #if block or put the
> whole test inside an #if block? It doesn't seem like there is much value in
> having a empty test run in Chromium builds.

This way they still get compiled -- alternatively we can disable them w/ some
macro magic.