Fix int-overflow due to absent stream timestamp

Fix int-overflow due to absent stream timestamp

crbug.com/665305 introduced some fixes for buffers with no timestamps.
However, if we did not receive a valid stream timestamp from the
demuxer, the first buffer that we output can still end up having no
timestamp (which eventually leads to an integer overflow in
VideoRendererAlgorithm).

This CL fixes the issue by reporting a demuxer error, rather than
emitting a buffer with no timestamp and relying on the decoder to error
out.

BUG=665305, 656763
TEST= fixes media_pipeline_integration_fuzzer. Ran ffmpeg_integration_tests as ASAN/MSAN/UBSAN. Ran Media UTs

Review-Url: https://codereview.chromium.org/2635573002
Cr-Commit-Position: refs/heads/master@{#443769}
Committed: https://chromium.googlesource.com/chromium/src/+/067e03ca0fc3a4f64fc5f324d665e5c10268cd14

[tguilbert, 2017-01-13 22:02:11]

tguilbert@chromium.org changed reviewers:
+ dalecurtis@chromium.org

[tguilbert, 2017-01-13 22:02:12]

PTAL.

Is this what you had in mind? Or is this too aggressive of a fix? It seems like
there are cases when |fixup_negative_timestamps_| is true that are currently
handled, but that would now error out instead. 

cc: Liberato

[tguilbert, 2017-01-13 22:03:48]

The CQ bit was checked by tguilbert@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-13 22:04:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[DaleCurtis, 2017-01-13 22:05:00]

https://codereview.chromium.org/2635573002/diff/1/media/filters/ffmpeg_demuxe...
File media/filters/ffmpeg_demuxer.cc (right):

https://codereview.chromium.org/2635573002/diff/1/media/filters/ffmpeg_demuxe...
media/filters/ffmpeg_demuxer.cc:545: // timestamp.  The decoder will rewrite the
timestamps to be sample accurate
Comment seems busted?

https://codereview.chromium.org/2635573002/diff/1/media/filters/ffmpeg_demuxe...
media/filters/ffmpeg_demuxer.cc:547: if (fixup_negative_timestamps_ &&
Do any of the tests hit this block of code? It's the only place I could see that
we'll error out eearlier on now.

[tguilbert, 2017-01-13 23:51:34]

Description was changed from

==========
Fix int-overflow due to absent stream timestamp

crbug.com/665305 introduced some fixes for buffers with no timestamps.
However, if we did not receive a valid stream timestamp from the
demuxer, the first buffer that we output can still end up having no
timestamp (which eventually leads to an integer overflow in
VideoRendererAlgorithm).

This CL fixes the issue by reporting a demuxer error, rather than
emitting a buffer with no timestamp and relying on the decoder to error
out.

BUG=665305, 656763
TEST= fixes media_pipeline_integration_fuzzer.
==========

to

==========
Fix int-overflow due to absent stream timestamp

crbug.com/665305 introduced some fixes for buffers with no timestamps.
However, if we did not receive a valid stream timestamp from the
demuxer, the first buffer that we output can still end up having no
timestamp (which eventually leads to an integer overflow in
VideoRendererAlgorithm).

This CL fixes the issue by reporting a demuxer error, rather than
emitting a buffer with no timestamp and relying on the decoder to error
out.

BUG=665305, 656763
TEST= fixes media_pipeline_integration_fuzzer. Ran ffmpeg_integration_tests as
ASAN/MSAN/UBSAN. Ran Media UTs
==========

[tguilbert, 2017-01-13 23:52:02]

https://codereview.chromium.org/2635573002/diff/1/media/filters/ffmpeg_demuxe...
File media/filters/ffmpeg_demuxer.cc (right):

https://codereview.chromium.org/2635573002/diff/1/media/filters/ffmpeg_demuxe...
media/filters/ffmpeg_demuxer.cc:545: // timestamp.  The decoder will rewrite the
timestamps to be sample accurate
On 2017/01/13 22:05:00, DaleCurtis wrote:
> Comment seems busted?

Done. (Assuming that was the portion of the comment that you were pointing out
as busted)

https://codereview.chromium.org/2635573002/diff/1/media/filters/ffmpeg_demuxe...
media/filters/ffmpeg_demuxer.cc:547: if (fixup_negative_timestamps_ &&
On 2017/01/13 22:05:00, DaleCurtis wrote:
> Do any of the tests hit this block of code? It's the only place I could see
that
> we'll error out eearlier on now.

Confirming that BasicPlaybackChainedOgg and BasicPlaybackChainedOggVideo hit
this code block (Thanks for adding them 3 years ago :) )

[DaleCurtis, 2017-01-14 00:02:26]

lgtm!

[tguilbert, 2017-01-14 00:13:30]

The CQ bit was checked by tguilbert@chromium.org

[commit-bot: I haz the power, 2017-01-14 00:13:57]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-14 02:14:29]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484352810956070,
"parent_rev": "fdf8404d839bbdaa8e836e28826e7cc268a4c2bb", "commit_rev":
"067e03ca0fc3a4f64fc5f324d665e5c10268cd14"}

[commit-bot: I haz the power, 2017-01-14 02:15:03]

Description was changed from

==========
Fix int-overflow due to absent stream timestamp

crbug.com/665305 introduced some fixes for buffers with no timestamps.
However, if we did not receive a valid stream timestamp from the
demuxer, the first buffer that we output can still end up having no
timestamp (which eventually leads to an integer overflow in
VideoRendererAlgorithm).

This CL fixes the issue by reporting a demuxer error, rather than
emitting a buffer with no timestamp and relying on the decoder to error
out.

BUG=665305, 656763
TEST= fixes media_pipeline_integration_fuzzer. Ran ffmpeg_integration_tests as
ASAN/MSAN/UBSAN. Ran Media UTs
==========

to

==========
Fix int-overflow due to absent stream timestamp

crbug.com/665305 introduced some fixes for buffers with no timestamps.
However, if we did not receive a valid stream timestamp from the
demuxer, the first buffer that we output can still end up having no
timestamp (which eventually leads to an integer overflow in
VideoRendererAlgorithm).

This CL fixes the issue by reporting a demuxer error, rather than
emitting a buffer with no timestamp and relying on the decoder to error
out.

BUG=665305, 656763
TEST= fixes media_pipeline_integration_fuzzer. Ran ffmpeg_integration_tests as
ASAN/MSAN/UBSAN. Ran Media UTs

Review-Url: https://codereview.chromium.org/2635573002
Cr-Commit-Position: refs/heads/master@{#443769}
Committed:
https://chromium.googlesource.com/chromium/src/+/067e03ca0fc3a4f64fc5f324d665...
==========

[commit-bot: I haz the power, 2017-01-14 02:15:04]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/067e03ca0fc3a4f64fc5f324d665...