Set error on unrecognized top-level BMFF boxes.

Set error on unrecognized top-level BMFF boxes.

A common error when using BMFF with Media Source is to begin appending at an
incorrect offset, such that the parser sees an extremely large value for the
box size and consequently waits forever for input without failing. This change
adds a whitelist of permitted top-level boxes, which allows these situations
to be detected as soon as the header is read.

BUG=
TEST=BoxReaderTest.WrongFourCCTest


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

[acolwell GONE FROM CHROMIUM, 2012-08-02 20:38:18]

https://chromiumcodereview.appspot.com/10823139/diff/2002/media/mp4/box_reade...
File media/mp4/box_reader.cc (right):

https://chromiumcodereview.appspot.com/10823139/diff/2002/media/mp4/box_reade...
media/mp4/box_reader.cc:109: << std::hex << reader->type();
Any reason not to use FourCCToString() here like you do in other places?

https://chromiumcodereview.appspot.com/10823139/diff/2002/media/mp4/box_reade...
media/mp4/box_reader.cc:139: return (type == FOURCC_FTYP ||
nit: WDYT about using a switch() here w/ a return true and putting the logging
statement you have above in default: ? That way you'll log for ReadTopLevelBox()
& StartTopLevelBox().

https://chromiumcodereview.appspot.com/10823139/diff/2002/media/mp4/box_reade...
File media/mp4/box_reader_unittest.cc (right):

https://chromiumcodereview.appspot.com/10823139/diff/2002/media/mp4/box_reade...
media/mp4/box_reader_unittest.cc:19: // chosen arbitrarily.
Why did these need to change if they are chosen arbitrarily?

https://chromiumcodereview.appspot.com/10823139/diff/2002/media/mp4/box_reade...
media/mp4/box_reader_unittest.cc:34: struct EmptyBox : Box {
Rename this and the ones below to match the new 4ccs?

[strobe_, 2012-08-02 21:27:49]

https://chromiumcodereview.appspot.com/10823139/diff/2002/media/mp4/box_reade...
File media/mp4/box_reader.cc (right):

https://chromiumcodereview.appspot.com/10823139/diff/2002/media/mp4/box_reade...
media/mp4/box_reader.cc:109: << std::hex << reader->type();
On 2012/08/02 20:38:18, acolwell wrote:
> Any reason not to use FourCCToString() here like you do in other places?

In the expected case of misregistration, the box type will often contain
unprintable characters. Comment added.

https://chromiumcodereview.appspot.com/10823139/diff/2002/media/mp4/box_reade...
media/mp4/box_reader.cc:139: return (type == FOURCC_FTYP ||
On 2012/08/02 20:38:18, acolwell wrote:
> nit: WDYT about using a switch() here w/ a return true and putting the logging
> statement you have above in default: ? That way you'll log for
ReadTopLevelBox()
> & StartTopLevelBox().

Done.

https://chromiumcodereview.appspot.com/10823139/diff/2002/media/mp4/box_reade...
File media/mp4/box_reader_unittest.cc (right):

https://chromiumcodereview.appspot.com/10823139/diff/2002/media/mp4/box_reade...
media/mp4/box_reader_unittest.cc:19: // chosen arbitrarily.
On 2012/08/02 20:38:18, acolwell wrote:
> Why did these need to change if they are chosen arbitrarily?

They didn't, I was just removing the test variables since they had to leak into
IsValidTopLevelBox() as well as fourccs.h. I can revert that change if you like?

https://chromiumcodereview.appspot.com/10823139/diff/2002/media/mp4/box_reade...
media/mp4/box_reader_unittest.cc:34: struct EmptyBox : Box {
On 2012/08/02 20:38:18, acolwell wrote:
> Rename this and the ones below to match the new 4ccs?

Done.

[acolwell GONE FROM CHROMIUM, 2012-08-02 21:40:25]

LGTM

https://chromiumcodereview.appspot.com/10823139/diff/2002/media/mp4/box_reade...
File media/mp4/box_reader.cc (right):

https://chromiumcodereview.appspot.com/10823139/diff/2002/media/mp4/box_reade...
media/mp4/box_reader.cc:109: << std::hex << reader->type();
On 2012/08/02 21:27:50, strobe wrote:
> On 2012/08/02 20:38:18, acolwell wrote:
> > Any reason not to use FourCCToString() here like you do in other places?
> 
> In the expected case of misregistration, the box type will often contain
> unprintable characters. Comment added.
Ok. Makes sense.

https://chromiumcodereview.appspot.com/10823139/diff/2002/media/mp4/box_reade...
File media/mp4/box_reader_unittest.cc (right):

https://chromiumcodereview.appspot.com/10823139/diff/2002/media/mp4/box_reade...
media/mp4/box_reader_unittest.cc:19: // chosen arbitrarily.
On 2012/08/02 21:27:50, strobe wrote:
> On 2012/08/02 20:38:18, acolwell wrote:
> > Why did these need to change if they are chosen arbitrarily?
> 
> They didn't, I was just removing the test variables since they had to leak
into
> IsValidTopLevelBox() as well as fourccs.h. I can revert that change if you
like?
Yeah. I think it is best to revert this text since it isn't really arbitrary.
Perhaps s/Test box/Test top level box/ would help as well if your test relies on
this.

[commit-bot: I haz the power, 2012-08-02 21:47:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/strobe@google.com/10823139/3006

[commit-bot: I haz the power, 2012-08-02 23:15:19]

Change committed as 149735