New issue
Advanced search Search tips

Issue 793550 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

CHECK failure: frame_duration > base::TimeDelta() (0 s vs. 0 s) in video_cadence_estimator.cc

Project Member Reported by ClusterFuzz, Dec 9 2017

Issue description

Detailed report: https://clusterfuzz.com/testcase?key=6196769536081920

Fuzzer: libFuzzer_media_pipeline_integration_fuzzer
Job Type: libfuzzer_chrome_asan_debug
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  frame_duration > base::TimeDelta() (0 s vs. 0 s) in video_cadence_estimator.cc
  media::VideoCadenceEstimator::UpdateCadenceEstimate
  media::VideoRendererAlgorithm::UpdateFrameStatistics
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_asan_debug&range=519950:519981

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6196769536081920

Issue filed automatically.

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md for more information.
 
Cc: kkaluri@chromium.org
Components: Internals>Media>Video
Labels: M-65 Test-Predator-Wrong
Owner: w...@chromium.org
Status: Assigned (was: Untriaged)
Predator and CL could not provide any possible suspects.
Using the code search for the file, “video_cadence_estimator.cc” assigning to concern owner from GIT blame.
Suspecting Commit# https://chromium.googlesource.com/chromium/src/+/2de69291f6f930174f948cf3ac41852a01dd398f


watk@ Could you please look into this issue and kindly reassign if it has nothing to do with the above changes.

Thank You.
Owner: dalecur...@chromium.org
Labels: -M-65 M-64
This can lead to check failures and was landed in m64 with 21ece16c7d9e0d06786579e7d86a7993a706f656, so will merge fix for it too.
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7e9579c8c897cb6512a9e3bb7b1ef975ff39808b

commit 7e9579c8c897cb6512a9e3bb7b1ef975ff39808b
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Fri Dec 15 19:38:39 2017

Don't perform cadence calculations using zero duration.

21ece16c7d9e0d06786579e7d86a7993a706f656 added support for using
the frame duration metadata in the muxed stream, but if the frame
timestamp is already int64_t::max(), computing the end timestamp
based on frame time + duration yields int64_t::max() so we can
end up with a duration of zero.

This updates the conditional to skip frame duration metadata when
we don't have a valid frame duration after the calculation.

BUG= 793550 
TEST=new unittest

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I5bb728f18fbfba7df326b44f7b6159f2395a3fb5
Reviewed-on: https://chromium-review.googlesource.com/828287
Reviewed-by: Thomas Guilbert <tguilbert@chromium.org>
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524434}
[modify] https://crrev.com/7e9579c8c897cb6512a9e3bb7b1ef975ff39808b/media/filters/video_renderer_algorithm.cc
[modify] https://crrev.com/7e9579c8c897cb6512a9e3bb7b1ef975ff39808b/media/filters/video_renderer_algorithm_unittest.cc

Cc: dalecur...@chromium.org
Labels: Merge-Request-64
Owner: liber...@chromium.org
=> liberato to merge this to M64 once it has soaked a while longer since I'll be out on vacation.
Project Member

Comment 6 by ClusterFuzz, Dec 16 2017

ClusterFuzz has detected this issue as fixed in range 524426:524439.

Detailed report: https://clusterfuzz.com/testcase?key=6196769536081920

Fuzzer: libFuzzer_media_pipeline_integration_fuzzer
Job Type: libfuzzer_chrome_asan_debug
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  frame_duration > base::TimeDelta() (0 s vs. 0 s) in video_cadence_estimator.cc
  media::VideoCadenceEstimator::UpdateCadenceEstimate
  media::VideoRendererAlgorithm::UpdateFrameStatistics
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_asan_debug&range=519950:519981
Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_asan_debug&range=524426:524439

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6196769536081920

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 7 by ClusterFuzz, Dec 16 2017

Labels: ClusterFuzz-Verified
Status: Verified (was: Assigned)
ClusterFuzz testcase 6196769536081920 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Project Member

Comment 8 by sheriffbot@chromium.org, Dec 16 2017

Labels: -Merge-Request-64 Hotlist-Merge-Review Merge-Review-64
This bug requires manual review: M64 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Have you verified this in Canary? IS it a safe merge overall for merging to M64?
Labels: -Merge-Review-64 Merge-Approved-64
Confirmed with liberato@. Change has been verified in canary and is a safe merge. Approving merge to M64. Branch:3282
Project Member

Comment 11 by bugdroid1@chromium.org, Jan 4 2018

Labels: -merge-approved-64 merge-merged-3282
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6a6d10a3cd3db90dc0dbf126372d12b2ce5b8673

commit 6a6d10a3cd3db90dc0dbf126372d12b2ce5b8673
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Thu Jan 04 21:09:47 2018

Don't perform cadence calculations using zero duration.

21ece16c7d9e0d06786579e7d86a7993a706f656 added support for using
the frame duration metadata in the muxed stream, but if the frame
timestamp is already int64_t::max(), computing the end timestamp
based on frame time + duration yields int64_t::max() so we can
end up with a duration of zero.

This updates the conditional to skip frame duration metadata when
we don't have a valid frame duration after the calculation.

BUG= 793550 
TEST=new unittest

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I5bb728f18fbfba7df326b44f7b6159f2395a3fb5
Reviewed-on: https://chromium-review.googlesource.com/828287
Reviewed-by: Thomas Guilbert <tguilbert@chromium.org>
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#524434}(cherry picked from commit 7e9579c8c897cb6512a9e3bb7b1ef975ff39808b)
Reviewed-on: https://chromium-review.googlesource.com/851074
Reviewed-by: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/branch-heads/3282@{#409}
Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840}
[modify] https://crrev.com/6a6d10a3cd3db90dc0dbf126372d12b2ce5b8673/media/filters/video_renderer_algorithm.cc
[modify] https://crrev.com/6a6d10a3cd3db90dc0dbf126372d12b2ce5b8673/media/filters/video_renderer_algorithm_unittest.cc

Sign in to add a comment