Parse VPCodecConfiguration and extract VP9 profile

Parse VPCodecConfiguration and extract VP9 profile

Also regenerate the test files with proper VPCodecConfiguration box, which should inherit from FullBox instead of Box.

BUG=604863
Committed: https://crrev.com/9e43284c6c6948793e9e38aa30ad0d59b37a748a
Cr-Commit-Position: refs/heads/master@{#399819}

[kqyang, 2016-04-29 22:25:36]

kqyang@chromium.org changed reviewers:
+ ddorwin@chromium.org, tinskip@chromium.org

[kqyang, 2016-04-29 22:25:37]

Please review!

[kqyang, 2016-05-27 23:50:30]

Description was changed from

==========
Parse VPCodecConfigurationRecord and extract VP9 profile

BUG=604863
==========

to

==========
Parse VPCodecConfiguration and extract VP9 profile

Also regenerate the test files with proper VPCodecConfiguration box, which
should inherit from FullBox instead of Box.

BUG=604863
==========

[tinskip, 2016-06-06 20:13:34]

tinskip@google.com changed reviewers:
+ tinskip@google.com

[tinskip, 2016-06-06 20:13:34]

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

https://codereview.chromium.org/1933793003/diff/20001/media/formats/mp4/box_d...
media/formats/mp4/box_definitions.cc:571:
VPCodecConfigurationRecord::VPCodecConfigurationRecord(
If using the default copy constructor, why bother making this explicit?

https://codereview.chromium.org/1933793003/diff/20001/media/formats/mp4/box_d...
media/formats/mp4/box_definitions.cc:585: RCHECK(profile_indication < 4);
There are, and will only be 2 VP9 profiles: 0 and 2.

[kqyang, 2016-06-06 20:30:19]

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

https://codereview.chromium.org/1933793003/diff/20001/media/formats/mp4/box_d...
media/formats/mp4/box_definitions.cc:571:
VPCodecConfigurationRecord::VPCodecConfigurationRecord(
On 2016/06/06 20:13:34, tinskip wrote:
> If using the default copy constructor, why bother making this explicit?

The box methods are declared using macro:
https://cs.chromium.org/chromium/src/media/formats/mp4/box_definitions.h?rcl=...

The copy constructor needs to be explicitly implemented.

https://codereview.chromium.org/1933793003/diff/20001/media/formats/mp4/box_d...
media/formats/mp4/box_definitions.cc:585: RCHECK(profile_indication < 4);
On 2016/06/06 20:13:34, tinskip wrote:
> There are, and will only be 2 VP9 profiles: 0 and 2.

There are actually four profiles: https://en.wikipedia.org/wiki/VP9#Profiles

Also see
https://storage.googleapis.com/downloads.webmproject.org/docs/vp9/vp9-bitstre...
7.2 Uncompressed header semantics

[kqyang, 2016-06-09 20:18:44]

A friendly ping! Thanks.

[tinskip, 2016-06-09 22:25:34]

lgtm

[tinskip1, 2016-06-09 22:27:50]

LGTM

[kqyang, 2016-06-14 17:03:03]

@ddorwin, do you want to take a look at this cl? I am planning to merge it today
if possible. Thanks.

[kqyang, 2016-06-14 17:03:06]

@ddorwin, do you want to take a look at this cl? I am planning to merge it today
if possible. Thanks.

[ddorwin, 2016-06-14 18:58:22]

LGTM % minor comments

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

https://codereview.chromium.org/1933793003/diff/20001/media/formats/mp4/box_d...
media/formats/mp4/box_definitions.cc:599: }
maybe DVLOG(WARNING) all other cases.

https://codereview.chromium.org/1933793003/diff/20001/media/formats/mp4/box_d...
media/formats/mp4/box_definitions.cc:688: DVLOG(2) << __FUNCTION__ << " parsing
VPCodecConfigurationRecord (vpcC)";
nit: Log at the top of the block for consistency.

[kqyang, 2016-06-14 21:02:32]

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

https://codereview.chromium.org/1933793003/diff/20001/media/formats/mp4/box_d...
media/formats/mp4/box_definitions.cc:599: }
On 2016/06/14 18:58:22, ddorwin wrote:
> maybe DVLOG(WARNING) all other cases.

Removed line 585 and add a default case here.

https://codereview.chromium.org/1933793003/diff/20001/media/formats/mp4/box_d...
media/formats/mp4/box_definitions.cc:688: DVLOG(2) << __FUNCTION__ << " parsing
VPCodecConfigurationRecord (vpcC)";
On 2016/06/14 18:58:22, ddorwin wrote:
> nit: Log at the top of the block for consistency.

Done.

[kqyang, 2016-06-14 21:08:48]

The CQ bit was checked by kqyang@chromium.org

[kqyang, 2016-06-14 21:08:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
tinskip@chromium.org, tinskip@google.com, ddorwin@chromium.org
Link to the patchset: https://codereview.chromium.org/1933793003/#ps60001
(title: " ")

[commit-bot: I haz the power, 2016-06-14 21:09:17]

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

[commit-bot: I haz the power, 2016-06-14 23:10:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-14 23:10:47]

Try jobs failed on following builders:
  ios-device on tryserver.chromium.mac (JOB_TIMED_OUT, no build URL)
  ios-device-gn on tryserver.chromium.mac (JOB_TIMED_OUT, no build URL)

[kqyang, 2016-06-14 23:17:29]

The CQ bit was checked by kqyang@chromium.org

[commit-bot: I haz the power, 2016-06-14 23:18:23]

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

[commit-bot: I haz the power, 2016-06-15 01:20:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-15 01:20:10]

Exceeded global retry quota

[kqyang, 2016-06-15 01:58:44]

The CQ bit was checked by kqyang@chromium.org

[commit-bot: I haz the power, 2016-06-15 01:59:06]

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

[commit-bot: I haz the power, 2016-06-15 02:03:03]

Description was changed from

==========
Parse VPCodecConfiguration and extract VP9 profile

Also regenerate the test files with proper VPCodecConfiguration box, which
should inherit from FullBox instead of Box.

BUG=604863
==========

to

==========
Parse VPCodecConfiguration and extract VP9 profile

Also regenerate the test files with proper VPCodecConfiguration box, which
should inherit from FullBox instead of Box.

BUG=604863
==========

[commit-bot: I haz the power, 2016-06-15 02:03:05]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-06-15 02:03:12]

CQ bit was unchecked

[commit-bot: I haz the power, 2016-06-15 02:04:24]

Description was changed from

==========
Parse VPCodecConfiguration and extract VP9 profile

Also regenerate the test files with proper VPCodecConfiguration box, which
should inherit from FullBox instead of Box.

BUG=604863
==========

to

==========
Parse VPCodecConfiguration and extract VP9 profile

Also regenerate the test files with proper VPCodecConfiguration box, which
should inherit from FullBox instead of Box.

BUG=604863

Committed: https://crrev.com/9e43284c6c6948793e9e38aa30ad0d59b37a748a
Cr-Commit-Position: refs/heads/master@{#399819}
==========

[commit-bot: I haz the power, 2016-06-15 02:04:25]

Patchset 4 (id:??) landed as
https://crrev.com/9e43284c6c6948793e9e38aa30ad0d59b37a748a
Cr-Commit-Position: refs/heads/master@{#399819}