Abrt in media::SourceBufferRangeByPts::AppendBuffersToEnd |
|||||||
Issue descriptionDetailed report: https://clusterfuzz.com/testcase?key=4710837939077120 Fuzzer: libFuzzer_mediasource_MP2T_AACLC_AVC_pipeline_integration_fuzzer Job Type: libfuzzer_chrome_msan Platform Id: linux Crash Type: Abrt Crash Address: 0x03e900004902 Crash State: media::SourceBufferRangeByPts::AppendBuffersToEnd media::SourceBufferStream<media::SourceBufferRangeByPts>::Append media::ChunkDemuxerStream::Append Sanitizer: memory (MSAN) Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=518239:518261 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=4710837939077120 Issue filed automatically. See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md for more information.
,
Jan 4 2018
With reference to the following Issue 793247 , assigning it to wolenetz@ wolenetz@ Could you please look into it
,
Jan 16 2018
I have a local repro in the debug ASAN version of this fuzzer on linux.
,
Jan 17 2018
Interesting. SAP-Type-2 and 0 duration buffer is in the repro case. At least the SAP-Type-2 is triggering the reset of |range_for_next_append_|, and then AppendBuffersToEnd is done to the wrong range. I'll investigate further to see why...
,
Jan 22 2018
I'm still working on reproducing the fault in a minimized unit test. I have found, however, that there is an increase in the fudge room at the start of the faulting append operation (and after the OnStartOfCodedFrameGroup's prior signalling), and the first buffer in the new append is at (first range's buffered end time + fudge room + 1) microseconds. So I think the issue is inconsistent |range_for_next_append_| determination (the first range) versus the lack of merging the two ranges (MergeAllAdjacentRanges), probably due to an off-by-one microsecond between the two code paths.
,
Jan 23 2018
,
Jan 23 2018
The root cause is https://cs.chromium.org/chromium/src/media/filters/source_buffer_stream.cc?sq=package:chromium&dr&l=700 (and perhaps also https://cs.chromium.org/chromium/src/media/filters/source_buffer_stream.cc?sq=package:chromium&dr&l=652): PotentialNextAppendTimestamp(), in the presence of the last appended buffer in the current CFG being a SAP-Type-2 nonkeyframe prior to the start of |range_for_next_append_|, but within fudge room of some other range, can cause |range_for_next_append_| to be reset incorrectly to that other range. Subsequent attempted append of the new GOP onto that other range fails the adjacency check. Neither 0-duration, nor increasing fudge room, are required to reproduce this issue. Investigating fix(es).
,
Jan 23 2018
Two different rough approaches to fixing this that I'm considering: 1) Modify PotentialNextAppendTimestamp() to have some state from OnStartOfCodedFrameGroup()?? (ugly, fragile, hacky) 2) Take the high road and have SourceBufferRangeByPts move its start time earlier if it contains a SAP-Type-2 nonkeyframe with timestamp prior to its "start time" otherwise. We're already do the associated overlap removals in such situations; this would be a cleaner fix and tie into existing adjacency / fudge room better IIUC. I'll take a stab at #2 first, and see how well it works.
,
Jan 23 2018
@#8 if a later GOP besides the first GOP in the range "owns" the nonkeyframe prior to the range start time, then approach #2 might regress the time-range remove operation (which is based on keyframe timestamps). I need to think further about this...
,
Jan 24 2018
@#8-9: Approach #2's modification of the range preparation and buffer removal code would be extensive in cases like the repro. Note that the repro, if in one bytestream media segment, might not even be considered SAP-2. However, the repro could just as easily be accomplished by splicing a SAP-2 stream over previously buffered media. In light of the relatively small portion of Chrome SAP-2 playbacks encountered and lack of consensus on the MSE v1 spec on how to interoperably process such frames in the coded frame processing algorithm, I'll try smaller fixes along the lines of approach #1 first. That approach should allow for SAP-2 overlaps of previously buffered media, with the caveat that range start times for a range beginning with a SAP-2 GOP (or containing some extreme SAP 2 GOP with a nonkeyframe PTS well before the range's first GOP's keyframe PTS) would not be adjusted backwards (like current code) to that extremely early PTS. The handling of extremely "early" nonkeyframe PTS for such SAP 2 sequences would still use the range's first GOP's keyframe PTS as the range start time (excepting existing cases where the |range_start_time_| is earlier as a result of muxed CFG signalling or range splitting.)
,
Jan 25 2018
This impacts only BufferingByPts mode. => M-66 Fix in CR: https://chromium-review.googlesource.com/c/chromium/src/+/879408
,
Jan 25 2018
,
Jan 28 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fdeaf6e25bb2d2596de09eca9a902ea98e70d84c commit fdeaf6e25bb2d2596de09eca9a902ea98e70d84c Author: Matt Wolenetz <wolenetz@chromium.org> Date: Sun Jan 28 16:44:16 2018 MSE: Use |highest_timestamp_in_append_sequence_| in PotentialNextAppendTimestamp() SAP-Type-2 sequences may contain a nonkeyframe so far in advance of the range's first GOP's keyframe timestamp that it may appear to belong to the presentation interval of some other range. If we continued to use |last_appended_buffer_timestamp_| when re-determining |range_for_next_append_| upon consulting PotentialNextAppendTimestamp() during buffer removal operations that touched the current |range_for_next_append_|, we could incorrectly append to the earlier range. This change uses the |highest_timestamp_in_append_sequence_| in PotentialNextAppendTimestamp(), enabling correct determination of |range_for_next_append_| when such SAP-Type-2 scenarios are encountered when BufferingByPts. Note: This change continues the direction of the BufferingByPts logic so far: it doesn't move a range's start time earlier if that range contains a nonkeyframe from a SAP-Type-2 GOP before the range's starting keyframe. This greatly simplifies adapting our existing logic to gracefully handle SAP-Type-2, with the known caveat that range start times don't indicate any potentially buffered earlier SAP-Type-2 nonkeyframes. See also https://crbug.com/798935#c10 BUG= 798935 , 804802 , 795655 TEST=Added SBST.Sap2GopWithNonKeyframeInEarlierRangesInterval, and the CF cases for bugs 798935 and 804802 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: Ieff9b086ec9ac73f02a6bfbfa406765b434d9728 Reviewed-on: https://chromium-review.googlesource.com/879408 Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org> Reviewed-by: Chrome Cunningham <chcunningham@chromium.org> Cr-Commit-Position: refs/heads/master@{#532280} [modify] https://crrev.com/fdeaf6e25bb2d2596de09eca9a902ea98e70d84c/media/filters/source_buffer_stream.cc [modify] https://crrev.com/fdeaf6e25bb2d2596de09eca9a902ea98e70d84c/media/filters/source_buffer_stream_unittest.cc
,
Jan 29 2018
ClusterFuzz has detected this issue as fixed in range 532267:532326. Detailed report: https://clusterfuzz.com/testcase?key=4710837939077120 Fuzzer: libFuzzer_mediasource_MP2T_AACLC_AVC_pipeline_integration_fuzzer Job Type: libfuzzer_chrome_msan Platform Id: linux Crash Type: Abrt Crash Address: 0x03e900004902 Crash State: media::SourceBufferRangeByPts::AppendBuffersToEnd media::SourceBufferStream<media::SourceBufferRangeByPts>::Append media::ChunkDemuxerStream::Append Sanitizer: memory (MSAN) Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=518239:518261 Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=532267:532326 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=4710837939077120 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.
,
Jan 29 2018
ClusterFuzz testcase 4710837939077120 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 |
|||||||
Comment 1 by ClusterFuzz
, Jan 4 2018Labels: Test-Predator-Auto-Components