Add the ability for presentation timestamps to be specified in test strings.

Add the ability for presentation timestamps to be specified in test strings.

All SourceBufferStream string based unit tests assume that decode and
presentation timestamps are the same. This change extends the string
syntax so that buffers can be created where the presentation and
decode timestamps are different. This will make it easier to write
tests that verify proper behavior when B-frames are used.

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

[acolwell GONE FROM CHROMIUM, 2014-07-16 20:42:52]

This patch depends on https://codereview.chromium.org/393403002/

[wolenetz, 2014-07-16 22:19:24]

https://codereview.chromium.org/395303002/diff/20001/media/filters/source_buf...
File media/filters/source_buffer_stream_unittest.cc (right):

https://codereview.chromium.org/395303002/diff/20001/media/filters/source_buf...
media/filters/source_buffer_stream_unittest.cc:443: // is determined by the
difference between the decode timestamp in ## and
This seems a little confusing: should buffer duration in real streams be
essentially the PTS delta (once reordered into monotonically increasing PTS),
instead of DTS delta? If so, perhaps we should disallow auto-duration
calculation from previous DTS delta here and always require explicit duration zz
be specified (or disallow any pts!=dts in the entire string if using delta to
derive duration)?

https://codereview.chromium.org/395303002/diff/20001/media/filters/source_buf...
media/filters/source_buffer_stream_unittest.cc:555: CHECK_GT(
IIUC, splicing is not enabled on streams with reordering. (MP3, AAC, Vorbis are
currently the only supported splice stream codecs).
nit: Note this in comment here and add CHECK_GT(..) on pts here, too.

[acolwell GONE FROM CHROMIUM, 2014-07-16 22:37:24]

https://codereview.chromium.org/395303002/diff/20001/media/filters/source_buf...
File media/filters/source_buffer_stream_unittest.cc (right):

https://codereview.chromium.org/395303002/diff/20001/media/filters/source_buf...
media/filters/source_buffer_stream_unittest.cc:443: // is determined by the
difference between the decode timestamp in ## and
On 2014/07/16 22:19:24, wolenetz wrote:
> This seems a little confusing: should buffer duration in real streams be
> essentially the PTS delta (once reordered into monotonically increasing PTS),
> instead of DTS delta? If so, perhaps we should disallow auto-duration
> calculation from previous DTS delta here and always require explicit duration
zz
> be specified (or disallow any pts!=dts in the entire string if using delta to
> derive duration)?

This is a compromise to avoid the strings getting too messy. Typically the delta
in dts is the same as the ultimate frame duration after pts reordering because a
single frame is decoded per frame interval. There are exceptions to this of
course, but in general this is true for most content. We can still represent the
weird cases if we need to by using explicit ##Dzz syntax, but I don't think it
is worth forcing this syntax for all buffers.

https://codereview.chromium.org/395303002/diff/20001/media/filters/source_buf...
media/filters/source_buffer_stream_unittest.cc:555: CHECK_GT(
On 2014/07/16 22:19:24, wolenetz wrote:
> IIUC, splicing is not enabled on streams with reordering. (MP3, AAC, Vorbis
are
> currently the only supported splice stream codecs).
> nit: Note this in comment here and add CHECK_GT(..) on pts here, too.

Done.

[wolenetz, 2014-07-16 23:02:51]

lgtm % nit. Thanks!

https://codereview.chromium.org/395303002/diff/20001/media/filters/source_buf...
File media/filters/source_buffer_stream_unittest.cc (right):

https://codereview.chromium.org/395303002/diff/20001/media/filters/source_buf...
media/filters/source_buffer_stream_unittest.cc:443: // is determined by the
difference between the decode timestamp in ## and
On 2014/07/16 22:37:24, acolwell wrote:
> On 2014/07/16 22:19:24, wolenetz wrote:
> > This seems a little confusing: should buffer duration in real streams be
> > essentially the PTS delta (once reordered into monotonically increasing
PTS),
> > instead of DTS delta? If so, perhaps we should disallow auto-duration
> > calculation from previous DTS delta here and always require explicit
duration
> zz
> > be specified (or disallow any pts!=dts in the entire string if using delta
to
> > derive duration)?
> 
> This is a compromise to avoid the strings getting too messy. Typically the
delta
> in dts is the same as the ultimate frame duration after pts reordering because
a
> single frame is decoded per frame interval. There are exceptions to this of
> course, but in general this is true for most content. We can still represent
the
> weird cases if we need to by using explicit ##Dzz syntax, but I don't think it
> is worth forcing this syntax for all buffers.

Acknowledged.

https://codereview.chromium.org/395303002/diff/40001/media/filters/source_buf...
File media/filters/source_buffer_stream_unittest.cc (right):

https://codereview.chromium.org/395303002/diff/40001/media/filters/source_buf...
media/filters/source_buffer_stream_unittest.cc:557:
CHECK_EQ(buffer->GetDecodeTimestamp().InMicroseconds(),
nit: move this first CHECK_EQ(), and its portion of the comment, up one level in
the if(){} hierarchy, so it enforces this for all, even the first, pre splice
buffers.

[acolwell GONE FROM CHROMIUM, 2014-07-16 23:15:47]

https://codereview.chromium.org/395303002/diff/40001/media/filters/source_buf...
File media/filters/source_buffer_stream_unittest.cc (right):

https://codereview.chromium.org/395303002/diff/40001/media/filters/source_buf...
media/filters/source_buffer_stream_unittest.cc:557:
CHECK_EQ(buffer->GetDecodeTimestamp().InMicroseconds(),
On 2014/07/16 23:02:51, wolenetz wrote:
> nit: move this first CHECK_EQ(), and its portion of the comment, up one level
in
> the if(){} hierarchy, so it enforces this for all, even the first, pre splice
> buffers.

Done.

[acolwell GONE FROM CHROMIUM, 2014-07-17 19:23:14]

The CQ bit was checked by acolwell@chromium.org

[commit-bot: I haz the power, 2014-07-17 19:24:26]

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

[commit-bot: I haz the power, 2014-07-17 23:07:12]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[commit-bot: I haz the power, 2014-07-18 00:17:32]

Change committed as 283944