'sequence' appendMode and remove() calls can spuriously fail subsequent appends |
|||
Issue descriptionRepro (we have a private report with more details): 1. Set 'sequence' appendMode on a single mp3 audio-track SourceBuffer. 2. Append some audio, say [0,60) 3. sourceBuffer.abort() 4. Remove [40,sourceBuffer.buffered.end(sourceBuffer.buffered.length - 1)) 5. sourceBuffer.abort() 6. Set timeStampOffset = 40 7. Remove [0,sourceBuffer.buffered.end(sourceBuffer.buffered.length - 1)) 8. Set timeStampOffset = 0 9. Attempt to re-append the same audio as in step 2. Expected: No decode error Actual: Decode error from SourceBufferStream due to "Buffers did not monotonically increase). Note: If just step #1 is skipped, no repro. This uses default 'segments' appendMode (with current Chromium workaround that emulates 'sequence' appendMode in the engine for mp3). Note: If just step #4 is skipped, no repro.
,
Jun 1 2016
And when in 'segments' mode, discontinuity is detected and signalled by the frame processor, side-stepping the issue encountered in 'sequence' mode later in the frames' append processing. Regardless, if last_appended_buffer is removed, then last_appended_buffer_timestamp_ should be reset IIUC.
,
Jun 1 2016
Hmm. Digging a little further, I believe I found a related 'sequence' mode issue: NotifyStartOfCodedFrameGroup() isn't called in sequence mode, even if there is an associated discontinuity. Possible example: 1. Set 'sequence' AppendMode. 2. Set 'timestampOffset' to 100. 3. Append buffers [100,200) (originally with DTS [0,100)) 4. Set 'timestampOffset' to 0. 5. Append buffers [0,10) (originally with DTS [0,10)). Somewhere betweeen steps 3 and 5, SourceBufferStream needs to know that a new coded frame group is starting. Otherwise, a similar enforcement failure as that in the original repro in this bug will occur (also within the IsMonotonicallyIncreasing within this coded frame group helper method). I haven't tried this second flavor of repro, but I suspect it will also need to be fixed.
,
Jun 2 2016
I found a workaround for this issue, enabling slight deprioritization: To forcibly indicate to Chromium that it needs to treat a 'sequence' mode append as beginning a new coded frame group: a) following a prior remove()'s "updateend" such as between steps 4 and 6, and between steps 7 and 8 in comment #1), or b) if the scenario in comment #3 is also causing trouble, between its steps 3 and 4: sourceBuffer.mode='segments'; sourceBuffer.abort(); // Chromium's abort() processing in 'segments' mode forces the coded frame processor to begin a new coded frame group. sourceBuffer.mode='sequence'; For those hitting this issue, please make comment here, especially if the workaround is incomplete or unfeasible for your scenario.
,
Jun 2 2016
Indeed the scenario in comment #3 leads at least to a DCHECK failure in debug builds (with, for mp3 mimetype, also including an abort() after step 3: FATAL:source_buffer_stream.cc(234)] Check failed: coded_frame_group_start_time_ <= buffers.front()->GetDecodeTimestamp(). The workaround suggested in comment #4 fixes the scenario in comment #3, too. Specifically, the additional steps needed to make comment #3 work in Chromium currently are: 3.1 Upon 'updateend', sourceBuffer.abort(); 3.2 sourceBuffer.mode = 'segments'; 3.3 sourceBuffer.abort(); // Yes, again. 3.4 sourceBuffer.mode = 'sequence';
,
Jun 3 2016
I have the beginnings of a potential fix for this out for review at https://codereview.chromium.org/2035003002/
,
Jun 16 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b7982341ce30c1dfb165551b8c641d781eeb0159 commit b7982341ce30c1dfb165551b8c641d781eeb0159 Author: wolenetz <wolenetz@chromium.org> Date: Thu Jun 16 20:23:40 2016 MSE: Signal new coded frame group more aggressively For 'sequence' mode coded frame processing, we may need to signal a new coded frame group because application may override timestampOffset and cause the processed frame sequence to go backwards in time. Jumps forward are allowed and kept within the same coded frame group. Jumps backward are now detected and trigger new coded frame group signalling so the overlap-append logic in SourceBufferStream handles this situation more correctly. TEST=*FrameProcessorTest.AudioVideo_Discontinuity_TimestampOffset*, http/tests/media/media-source/mediasource-crbug-616565.html BUG= 616565 , 620523 Review-Url: https://codereview.chromium.org/2035003002 Cr-Commit-Position: refs/heads/master@{#400249} [modify] https://crrev.com/b7982341ce30c1dfb165551b8c641d781eeb0159/media/filters/frame_processor.cc [modify] https://crrev.com/b7982341ce30c1dfb165551b8c641d781eeb0159/media/filters/frame_processor.h [modify] https://crrev.com/b7982341ce30c1dfb165551b8c641d781eeb0159/media/filters/frame_processor_unittest.cc [add] https://crrev.com/b7982341ce30c1dfb165551b8c641d781eeb0159/third_party/WebKit/LayoutTests/http/tests/media/media-source/mediasource-sequencemode-crbug-616565.html
,
Jun 16 2016
|
|||
►
Sign in to add a comment |
|||
Comment 1 by wolenetz@chromium.org
, Jun 1 2016