Fix WebM parser not to wait for a next frame when DefaultDuration is specified.

Fix WebM parser not to wait for a next frame when DefaultDuration is specified.

Per Matroska spec difference between frames timestamps must be used as
block duration when neither Duration nor DefaultDuration is specified.
Problem was that WebMClusterParser was waiting for the next frame
even when DefaultDuration is specified.

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

[wolenetz, 2014-04-09 21:53:47]

lgtm % nits.
Thank you for catching (and fixing) my mistake :) !
My apologies for introducing unnecessary frame delay when TrackEntry
DefaultDuration was present when using SimpleBlocks.

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

https://codereview.chromium.org/230173002/diff/20001/media/formats/webm/webm_...
media/formats/webm/webm_cluster_parser.cc:411:
buffer->set_duration(track->default_duration());
nit: DCHECK_NE(DemuxerStream::TEXT, buffer_type) to reinforce text buffers must
have duration specified; no default or estimate.

https://codereview.chromium.org/230173002/diff/20001/media/formats/webm/webm_...
File media/formats/webm/webm_cluster_parser.h (right):

https://codereview.chromium.org/230173002/diff/20001/media/formats/webm/webm_...
media/formats/webm/webm_cluster_parser.h:85: // If kNoTimestamp(), then
|estimated_next_frame_duration_| will be used.
nit: webm_cluster_parser_unittest could use some Default vs Estimated vs
Arbitrary vs Specified-in-block duration coverage. Please add a TODO(wolenetz)
referencing http://crbug.com/361786

[Sergey Ulanov, 2014-04-10 02:00:49]

+scherkus for OWNERS approval

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

https://codereview.chromium.org/230173002/diff/20001/media/formats/webm/webm_...
media/formats/webm/webm_cluster_parser.cc:411:
buffer->set_duration(track->default_duration());
On 2014/04/09 21:53:47, wolenetz wrote:
> nit: DCHECK_NE(DemuxerStream::TEXT, buffer_type) to reinforce text buffers
must
> have duration specified; no default or estimate.

Done.

https://codereview.chromium.org/230173002/diff/20001/media/formats/webm/webm_...
File media/formats/webm/webm_cluster_parser.h (right):

https://codereview.chromium.org/230173002/diff/20001/media/formats/webm/webm_...
media/formats/webm/webm_cluster_parser.h:85: // If kNoTimestamp(), then
|estimated_next_frame_duration_| will be used.
On 2014/04/09 21:53:47, wolenetz wrote:
> nit: webm_cluster_parser_unittest could use some Default vs Estimated vs
> Arbitrary vs Specified-in-block duration coverage. Please add a TODO(wolenetz)
> referencing http://crbug.com/361786

Done.

[scherkus (not reviewing), 2014-04-10 18:19:02]

we have media/formats/webm/webm_cluster_parser_unittest.cc -- should we consider
adding a test?

[Sergey Ulanov, 2014-04-10 19:33:24]

On 2014/04/10 18:19:02, scherkus wrote:
> we have media/formats/webm/webm_cluster_parser_unittest.cc -- should we
consider
> adding a test?

Matt opened this bug to add tests: http://crbug.com/361786

[scherkus (not reviewing), 2014-04-10 19:38:20]

lgtm

[Sergey Ulanov, 2014-04-10 19:42:01]

The CQ bit was checked by sergeyu@chromium.org

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

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

[commit-bot: I haz the power, 2014-04-10 21:53:05]

Change committed as 263097