New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 761567 link

Starred by 2 users

Issue metadata

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

Blocking:
issue 718641



Sign in to add a comment

CHECK failure: last_appended_buffer.get() == highest_frame_.get() in source_buffer_range.cc

Project Member Reported by ClusterFuzz, Sep 1 2017

Issue description

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

Fuzzer: libFuzzer_mediasource_WEBM_VP9_pipeline_integration_fuzzer
Job Type: libfuzzer_chrome_ubsan
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  last_appended_buffer.get() == highest_frame_.get() in source_buffer_range.cc
  media::SourceBufferRange::AdjustEstimatedDurationForNewAppend
  media::SourceBufferRange::AppendBuffersToEnd
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=497087:497155

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

Issue filed automatically.

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.
 
Cc: msrchandra@chromium.org
Labels: M-63 Test-Predator-Wrong
Owner: wolenetz@chromium.org
Status: Assigned (was: Untriaged)
Predator and CL could not provide any possible suspects.
Using Code Search for the file, "source_buffer_range.cc" assigning to the concern owner.

Suspecting Commit#
https://chromium.googlesource.com/chromium/src/+/c485d6c706fe27268948c43e71ef9f68e743bcf7

@wolenetz -- Could you please look into the issue, kindly re-assign if this is not related to your changes.
Thank You.
Cc: wolenetz@chromium.org
Components: Internals>Media>Source
Owner: chcunningham@chromium.org
Repro'd in a debug=false ubsan build:
source_buffer_range.cc(102)] Check failed: last_appended_buffer.get() == highest_frame_.get() (0x60d000000d40 vs. 0x60d000000ba0)

In a debug asan build, I get: "source_buffer_range.cc(91)] Check failed: timestamp_delta > base::TimeDelta()"

=> chcunningham@ to fix these issues with SourceBufferRange::AdjustEstimatedDurationForNewAppend(...)

This might be a duplicate of  bug 758802 .
Cc: chcunningham@chromium.org
Owner: wolenetz@chromium.org
I'm taking  bug 758802  since it's likely going to get fixed as I fix  bug 759336 .
I'll also take a stab at fixing this one, too.

For this bug in particular, I suspect a simple repro:
In same WEBM coded frame group (simplified so PTS==DTS):
Append KeyFrame PTS=0, *estimated* DUR=20ms
Append KeyFrame PTS=0, *estimated* DUR=10ms 
Append KeyFrame PTS=0, estimated or for-realz DUR=something...

Since there has not been a discontinuity, and since we've long since relaxed the same-timestamp logic, all these frames should become buffered in the same range, in order by append. Also, the first and second keyframes' durations should be adjusted to 0. On adjusting the second one to zero, the CHECK gets hit because the (I think; I need to verify) prior adjustments didn't adjust the highest_frame_ ptr.
Status: Started (was: Assigned)
A quick attempt at a repro unit test with scenario in #3 didn't repro the issue. I'll dig a bit further (maybe one of the earlier durations *wasn't* estimated?)
I have a simplified repro unit test. Working on a fix.

Interim CL: https://chromium-review.googlesource.com/c/chromium/src/+/653243
The issue is understood. Since our parsers can emit an "allowed, but strange" sequence of frames with same timestamp but varying durations, even our WebM parser, then there is no guarantee that the most recently appended frame (even in WebM, even if it was with an estimated duration) has the highest timestamp+duration value.

Looking through the existing SourceBufferRange code managing |highest_frame_|, that code still appears correct, and the fix is to remove the offending CHECK which was attempting to verify the invalid assumption (the lack of guarantee mentioned in the first paragraph in this comment.)

(This analysis included a chat with chcunningham@ to make sure we're on the same page about this.)
Project Member

Comment 8 by bugdroid1@chromium.org, Sep 6 2017

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

commit 1c66bda4ea8562c3874f0f8d3f56e2067ca8d129
Author: Matt Wolenetz <wolenetz@chromium.org>
Date: Wed Sep 06 21:32:55 2017

MSE: Remove unnecessary and incorrect CHECK_EQ

Even in WebM (with PTS==DTS), there is no guarantee that the most
recently appended frame in a coded frame group is the frame with the
highest timestamp *and* the highest timestamp+duration. There could be
same-timestamp sequences (strange, but allowed) for video where an
earlier frame with same highest-timestamp could have had the higher
duration than the most recent append.

This CL adds a test demonstrating the behavior and removes the failing
CHECK_EQ (which was incorrect because it was assuming the guarantee,
above) from a helper method that adjusted (WebM) buffered frame
estimated durations on new appends to the end of a SourceBufferRange.

To get beyond the DCHECK fixed in  bug 758802 , this CL depends on first
landing https://chromium-review.googlesource.com/c/chromium/src/+/651352

BUG= 761567 
TEST=SBS.SameTimestampEstimatedDurations_Video and fuzzer case no longer
repros locally.

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: Iec44ebd379950c59294aab10088362256c6b221e
Reviewed-on: https://chromium-review.googlesource.com/653243
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500095}
[modify] https://crrev.com/1c66bda4ea8562c3874f0f8d3f56e2067ca8d129/media/filters/source_buffer_range.cc
[modify] https://crrev.com/1c66bda4ea8562c3874f0f8d3f56e2067ca8d129/media/filters/source_buffer_range.h
[modify] https://crrev.com/1c66bda4ea8562c3874f0f8d3f56e2067ca8d129/media/filters/source_buffer_stream_unittest.cc

Blocking: 718641
Status: Fixed (was: Started)
#8 should fix this. Pending CF verification...
Project Member

Comment 10 by ClusterFuzz, Sep 7 2017

ClusterFuzz has detected this issue as fixed in range 500054:500109.

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

Fuzzer: libFuzzer_mediasource_WEBM_VP9_pipeline_integration_fuzzer
Job Type: libfuzzer_chrome_ubsan
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  last_appended_buffer.get() == highest_frame_.get() in source_buffer_range.cc
  media::SourceBufferRange::AdjustEstimatedDurationForNewAppend
  media::SourceBufferRange::AppendBuffersToEnd
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=497087:497155
Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=500054:500109

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

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.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 11 by ClusterFuzz, Sep 7 2017

Labels: ClusterFuzz-Verified
Status: Verified (was: Fixed)
ClusterFuzz testcase 6208500861763584 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 12 by ClusterFuzz, Sep 7 2017

ClusterFuzz has detected this issue as fixed in range 500054:500109.

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

Fuzzer: libFuzzer_mediasource_WEBM_VP9_pipeline_integration_fuzzer
Job Type: libfuzzer_chrome_ubsan
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  last_appended_buffer.get() == highest_frame_.get() in source_buffer_range.cc
  media::SourceBufferRange::AdjustEstimatedDurationForNewAppend
  media::SourceBufferRange::AppendBuffersToEnd
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=497087:497155
Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=500054:500109

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

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

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

Sign in to add a comment