Adds WebMClusterParserTest coverage for duration default/estimation/fallback logic

Add WebMClusterParserTest coverage for duration default/estimation/fallback logic

WebMClusterParser recently began supporting using TrackEntry
DefaultDurations, estimating missing durations from inter-buffer
timestamp deltas in a track, or falling back to reusing largest
duration so far or a hardcoded frame duration. This change adds
missing test coverage. The test cluster builder is augmented to allow
generation of clusters containing BlockGroups with blocks optionally
missing the BlockDuration field, to support some of these new tests.

R=acolwell@chromium.org
BUG=361786, 351166
TEST=All media_unittests pass locally on Linux

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

[wolenetz, 2014-04-15 00:05:17]

PTAL at patch set 2.

[acolwell GONE FROM CHROMIUM, 2014-04-15 00:32:24]

lgtm % nits

https://codereview.chromium.org/238273002/diff/20001/media/formats/webm/clust...
File media/formats/webm/cluster_builder.cc (right):

https://codereview.chromium.org/238273002/diff/20001/media/formats/webm/clust...
media/formats/webm/cluster_builder.cc:136:
kBlockGroupWithoutBlockDurationBlockSizeOffset,
nit: This indent looks wrong. newline after ( instead?

https://codereview.chromium.org/238273002/diff/20001/media/formats/webm/webm_...
File media/formats/webm/webm_cluster_parser_unittest.cc (right):

https://codereview.chromium.org/238273002/diff/20001/media/formats/webm/webm_...
media/formats/webm/webm_cluster_parser_unittest.cc:23: typedef
WebMTracksParser::TextTracks TextTracks;
nit: How about "using" instead of a typedef?

https://codereview.chromium.org/238273002/diff/20001/media/formats/webm/webm_...
media/formats/webm/webm_cluster_parser_unittest.cc:727: // cluster, and are
estimated as the highest duration seen so far if the last
Doesn't this become lowest w/ Dale's change?

[wolenetz, 2014-04-15 00:36:04]

On 2014/04/15 00:05:17, wolenetz wrote:
> PTAL at patch set 2.

Also, note that https://codereview.chromium.org/220103002/ (patch set 5) changes
behavior from "max" to "min" duration so far. I expect I'll need to update this
CL to accomodate that behavior change if it lands first.

[wolenetz, 2014-04-15 01:25:40]

Regarding:
>
https://codereview.chromium.org/238273002/diff/20001/media/formats/webm/webm_...
> media/formats/webm/webm_cluster_parser_unittest.cc:23: typedef
> WebMTracksParser::TextTracks TextTracks;
> nit: How about "using" instead of a typedef?

The only way I could get this to work was a using alias, and I'm not sure if all
the toolchains Chromium uses support this:
"using TextTracks = WebMTracksParser::TextTracks".

Is a using alias what you intended? Elsewhere in media code I've typically seen
typedef's used to 'alias' typedefs in other classes.

Note WebMTracksParser::TextTracks itself is a typedef (std::map<int,
TextTrackConfig>). "using WebMTracksParser::TextTracks;" fails to compile:
"error: using declaration cannot refer to class member".

[acolwell GONE FROM CHROMIUM, 2014-04-15 01:28:55]

On 2014/04/15 01:25:40, wolenetz wrote:
> Regarding:
> >
>
https://codereview.chromium.org/238273002/diff/20001/media/formats/webm/webm_...
> > media/formats/webm/webm_cluster_parser_unittest.cc:23: typedef
> > WebMTracksParser::TextTracks TextTracks;
> > nit: How about "using" instead of a typedef?
> 
> The only way I could get this to work was a using alias, and I'm not sure if
all
> the toolchains Chromium uses support this:
> "using TextTracks = WebMTracksParser::TextTracks".
> 
> Is a using alias what you intended? Elsewhere in media code I've typically
seen
> typedef's used to 'alias' typedefs in other classes.
> 
> Note WebMTracksParser::TextTracks itself is a typedef (std::map<int,
> TextTrackConfig>). "using WebMTracksParser::TextTracks;" fails to compile:
> "error: using declaration cannot refer to class member".

:/ ok. Nevermind then.

[wolenetz, 2014-04-15 01:49:12]

Thank you. Patch set 3 is now pending Dale first landing his
https://codereview.chromium.org/220103002/.

https://codereview.chromium.org/238273002/diff/20001/media/formats/webm/clust...
File media/formats/webm/cluster_builder.cc (right):

https://codereview.chromium.org/238273002/diff/20001/media/formats/webm/clust...
media/formats/webm/cluster_builder.cc:136:
kBlockGroupWithoutBlockDurationBlockSizeOffset,
On 2014/04/15 00:32:24, acolwell wrote:
> nit: This indent looks wrong. newline after ( instead?

Done.

https://codereview.chromium.org/238273002/diff/20001/media/formats/webm/webm_...
File media/formats/webm/webm_cluster_parser_unittest.cc (right):

https://codereview.chromium.org/238273002/diff/20001/media/formats/webm/webm_...
media/formats/webm/webm_cluster_parser_unittest.cc:23: typedef
WebMTracksParser::TextTracks TextTracks;
On 2014/04/15 00:32:24, acolwell wrote:
> nit: How about "using" instead of a typedef?

using only worked as an alias. Keeping as typedef. see also this CL's messages
for details.

https://codereview.chromium.org/238273002/diff/20001/media/formats/webm/webm_...
media/formats/webm/webm_cluster_parser_unittest.cc:727: // cluster, and are
estimated as the highest duration seen so far if the last
On 2014/04/15 00:32:24, acolwell wrote:
> Doesn't this become lowest w/ Dale's change?

Correct. In patch set 3, I've rebased onto his CL and reworked these tests'
expectations accordingly.

[wolenetz, 2014-04-18 17:46:23]

The CQ bit was checked by wolenetz@chromium.org

[commit-bot: I haz the power, 2014-04-18 17:46:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wolenetz@chromium.org/238273002/60001

[commit-bot: I haz the power, 2014-04-18 19:10:26]

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

[commit-bot: I haz the power, 2014-04-18 19:10:27]

Retried try job too often on android_dbg for step(s) slave_steps
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...

[wolenetz, 2014-04-18 19:57:10]

PTAL @ Patch set 5. I'm dubious about the line wrap formatting for the updated
COMPILE_ASSERTs, but wrapping at other places looked worse to me. wdyt?

[acolwell GONE FROM CHROMIUM, 2014-04-21 16:42:11]

lgtm 2

[wolenetz, 2014-04-22 17:40:45]

The CQ bit was checked by wolenetz@chromium.org

[commit-bot: I haz the power, 2014-04-22 17:41:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wolenetz@chromium.org/238273002/80001

[commit-bot: I haz the power, 2014-04-22 20:06:43]

Change committed as 265340