New issue
Advanced search Search tips

Issue 803018 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug

Blocking:
issue 803293



Sign in to add a comment

CHECK failure: IsRangeListSorted(ranges_) in source_buffer_stream.cc

Project Member Reported by ClusterFuzz, Jan 17 2018

Issue description

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

Fuzzer: libFuzzer_mediasource_MP4_AACSBR_pipeline_integration_fuzzer
Job Type: libfuzzer_chrome_asan_debug
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  IsRangeListSorted(ranges_) in source_buffer_stream.cc
  media::SourceBufferStream<media::SourceBufferRangeByPts>::Append
  media::ChunkDemuxerStream::Append
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_asan_debug&range=529129:529141

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

Issue filed automatically.

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

Comment 1 by ClusterFuzz, Jan 17 2018

Components: Internals>Media
Labels: Test-Predator-Auto-Components
Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.
Project Member

Comment 2 by ClusterFuzz, Jan 17 2018

Labels: Test-Predator-Auto-Owner
Owner: wolenetz@chromium.org
Status: Assigned (was: Untriaged)
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/chromium/src/+/03c6fb32c54b64bb9424b7e2dbdf8753e409e7ce (MSE: Improve sorted range checking in debug builds).

If this is incorrect, please remove the owner and apply the Test-Predator-Wrong-CLs label.
Cc: chcunningham@chromium.org sande...@chromium.org
Components: -Internals>Media Internals>Media>Source
Status: Started (was: Assigned)
I have a local repro on ToT. This looks like a ByPts case that fails the stricter IsRangeListSorted verification added to debug builds as part of the fixes for  bug 791095 . I'll investigate further...
The case fails the sort check eventually on this set of ranges:
   Append AUDIO: done. ranges_=[0us;749043us(1298650us)] [766088us;774514us(1324121us)] [800470us;1188489us(1738096us)]
Labels: -Pri-1 M-65 Pri-2
=> P2, this is ByPts (only available by opt-in flag or possibly by experiment soon).
It looks like the fuzzer found a case (probably could find similar in ByDts) where a bunch of 0-duration audio buffers are buffered into a bunch of disjoint ranges (some with multiple, 1 microsecond-apart buffers), and then effectively increases the fudge room by appending buffers with larger duration. That fudge room increase causes the previous buffered ranges to overlap each other because we have logic that uses that dynamically growing fudge room for part of range presentation interval's calculation when the duration of the buffer from the parser was 0. Still P2 - this case would result in stalls in Release mode at buffered range gaps (which aren't coalesced, yet overlap), and probably predates the ByPts/Dts logic (it was detected when I strengthened the IsRangeListSorted check last week to operate on range's BufferedEndTimestamp, rather than just EndTimestamp.)
Blocking: 803293
Project Member

Comment 8 by bugdroid1@chromium.org, Jan 22 2018

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

commit ef5f357c1f1375f3167bc1d9ecc9b5188ad421f6
Author: Matt Wolenetz <wolenetz@chromium.org>
Date: Mon Jan 22 21:13:14 2018

MSE: Fix a couple fudge room usage faults

When fudge room increases, MergeAllAdjacentRanges should merge all
ranges that are adjacent based on the new fudge room. Previous code
would not try to merge sets of more than 2 adjacent ranges, resulting in
disjoint ranges which should have been coalesced. This change updates
MergeAllAdjacentRanges to try to merge the current range with the next
one repeatedly if the previous merge was successful.

This change also stops using the approximate buffer duration (based on
the maximum duration or continuous interbuffer DTS distance seen so far)
as the reported range end time for ranges ending with a zero duration
buffer. In combination with the previously broken MergeAllAdjacentRanges
logic, approximated range end times that grow dynamically as a side
effect of fudge room growth led to previously disjoint ranges
overlapping each other (and failing the strengthened IsRangeListSorted()
debug build verifications added as part of fixing  bug 791095 .)

This change uses a constant 1 microsecond for the duration of a buffer
that is reported as zero duration by the stream parsing and frame
processor when calculating the buffered end time of a range.

Since public base::TimeDelta::FromInternalValue usage is deprecated,
this change also switches usage to FromMicroseconds in SBS and SBR.

BUG= 803018 , 803293 , 791095 
TEST=SBS.*ZeroDurationBuffersThenIncreasingFudgeRoom, and removal of a
FrameProcessorTest that verified the obsolete fudge-room-based buffered
range reporting. The clusterfuzz cases in bugs  803018  and  803293  also
no longer repro locally with this change.

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: I8b4adfeb5064c15e57211d2f07132e7723f7f66a
Reviewed-on: https://chromium-review.googlesource.com/871517
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530992}
[modify] https://crrev.com/ef5f357c1f1375f3167bc1d9ecc9b5188ad421f6/media/filters/frame_processor_unittest.cc
[modify] https://crrev.com/ef5f357c1f1375f3167bc1d9ecc9b5188ad421f6/media/filters/source_buffer_range_by_dts.cc
[modify] https://crrev.com/ef5f357c1f1375f3167bc1d9ecc9b5188ad421f6/media/filters/source_buffer_range_by_pts.cc
[modify] https://crrev.com/ef5f357c1f1375f3167bc1d9ecc9b5188ad421f6/media/filters/source_buffer_stream.cc
[modify] https://crrev.com/ef5f357c1f1375f3167bc1d9ecc9b5188ad421f6/media/filters/source_buffer_stream.h
[modify] https://crrev.com/ef5f357c1f1375f3167bc1d9ecc9b5188ad421f6/media/filters/source_buffer_stream_unittest.cc

Pending CF verification, I believe this is fixed by #8.
Project Member

Comment 10 by ClusterFuzz, Jan 23 2018

ClusterFuzz has detected this issue as fixed in range 530991:530997.

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

Fuzzer: libFuzzer_mediasource_MP4_AACSBR_pipeline_integration_fuzzer
Job Type: libfuzzer_chrome_asan_debug
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  IsRangeListSorted(ranges_) in source_buffer_stream.cc
  media::SourceBufferStream<media::SourceBufferRangeByPts>::Append
  media::ChunkDemuxerStream::Append
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_asan_debug&range=529129:529141
Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_asan_debug&range=530991:530997

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

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 11 by ClusterFuzz, Jan 23 2018

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

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

Sign in to add a comment