New issue
Advanced search Search tips

Issue 806181 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

CHECK failure: IsRangeListSorted(ranges_) in source_buffer_stream.cc

Project Member Reported by ClusterFuzz, Jan 26 2018

Issue description

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

Fuzzer: libFuzzer_mediasource_WEBM_VP9_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=5068583517028352

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 26 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 26 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 let us know why and apply the Test-Predator-Wrong-CLs label. If you aren't the correct owner for this issue, please unassign yourself as soon as possible so it can be re-triaged.
Cc: chcunningham@chromium.org sande...@chromium.org
Components: -Internals>Media Internals>Media>Source
Labels: M-66
Status: Started (was: Assigned)
I have a local repro of this on tip-of-tree. Investigating...
This occurs in both BufferingByPts and BufferingByDts (it'd be weird if it didn't, considering the repro case comes from a WebM stream where PTS==DTS).

Subset of the set of ranges that triggers the !IsRangeListSorted determination:
[679908us;723077us(852583us)] [852583us;852583us(915583us)]

Investigating further... I would have thought the extra MergeWithNextRangeIfNecessary operations for both the current range_for_next_append_ and (if there's one) it's predecessor, at the end of SBS::AppendData would have prevented this, but obviously there's a case where it isn't prevented.
This looks like it might be related to recent changes that allow for more adjacency (range start times (? and end times)) are adjusted as the result of remove operations, but the IsNextInPts/Dts helpers that determine adjacency don't look at buffered end timestamps of the current range, just the highest frame DTS pr PTS (respectively) in the current range as the lower bound of the adjacency window). I'll look further to understand this better tomorrow.
In the repro case, all of the appended buffers have estimated durations. They're estimated to be 63ms, but adjacency checks are done on the minimum possible intervals of 1us in at least some cases. Looking further.
Partially simplified a repro unit test case:
  NewCodedFrameGroupAppend("723077uD63EK");
  AppendBuffers("777038uD63E");
  NewCodedFrameGroupAppend("11725725uD63EK");
  NewCodedFrameGroupAppend("852583uD63EK");
  NewCodedFrameGroupAppend("723077uD63EK");
  NewCodedFrameGroupAppend("852583uD63EK"); --> Boom.

A simpler repro:

TEST_P(SourceBufferStreamTest, MultipleOverlaps_PreserveAdjacency_EstimatedDurationOnOneOfTheAppendedBuffers) {
  NewCodedFrameGroupAppend("0D10K");
  AppendBuffers("8D10");
  NewCodedFrameGroupAppend("100D10K");
  NewCodedFrameGroupAppend("21D10K");
  NewCodedFrameGroupAppend("0D10EK");
  NewCodedFrameGroupAppend("21D10K");
}

Repro is understood. 3 solution approaches identified. Will proceed further upon discussing with chcunningham@ ideally first. (See https://chromium-review.googlesource.com/c/chromium/src/+/898604/2/media/filters/source_buffer_stream_unittest.cc)
Project Member

Comment 10 by bugdroid1@chromium.org, Feb 7 2018

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

commit 7adfb7752591c843ef6c90d15b9449d698d8cdea
Author: Matt Wolenetz <wolenetz@chromium.org>
Date: Wed Feb 07 01:46:57 2018

MSE: Allow range merge in specific estimated buffer duration scenarios

If a range B starts with the same timestamp as the end of the previous
range A, these would normally merge due to being adjacent within the fudge
room based on the end timestamp of range A (not the buffered end
timestamp of A). This is sufficient for coalescing adjacent ranges in
all cases except where fudge room has not been increased to account for
an increased estimated buffer's duration. In complex overlap/abutting
append scenarios, A and B previously would have remained outside of
fudge room of each other and not have been coalesced, if A ended with an
estimated duration buffer longer than the fudge room.

This change patches CanAppendBuffersToEnd to recognize this specific
scenario and allow A and B to coalesce.

BUG= 806181 
TEST=CF case no longer repros locally, new SBSTest.MergeAllowedIfRangeEndTimeWithEstimatedDurationMatchesNextRangeStart

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

Project Member

Comment 11 by ClusterFuzz, Feb 7 2018

ClusterFuzz has detected this issue as fixed in range 534868:534887.

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

Fuzzer: libFuzzer_mediasource_WEBM_VP9_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=534868:534887

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

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 12 by ClusterFuzz, Feb 7 2018

Labels: ClusterFuzz-Verified
Status: Verified (was: Started)
ClusterFuzz testcase 5068583517028352 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