New issue
Advanced search Search tips

Issue 737710 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Append is allowed to continue after another sourcebuffer has hit appendError on same MediaSource

Project Member Reported by wolenetz@chromium.org, Jun 28 2017

Issue description

Repro (one way, internal only due to found while investigating a b/):

(not a 100% repro since this is a race between operations on 2 sourcebuffers):

1) Make sure https://chromium-review.googlesource.com/c/552824/ is applied, otherwise this later DCHECK failure won't be reached 
2) With a debug build of Chrome, repro b/62450277 using its info in its comment 33, and with verbose MSE vmodule logging.

Successful repro indicates something like:

:0628/130342.048676:ERROR:render_media_log.cc(30)] MediaEvent: MEDIA_ERROR_LOG_ENTRY {"error":"Append: stream parsing failed. Data size=43411 append_window_start=0 append_window_end=inf"}
[1:1:0628/130342.072895:FATAL:chunk_demuxer.cc(1054)] Check failed: state_ == ENDED (4 vs. 3)

Stack shows this DCHECK failure is due to a new SourceBuffer.appendBuffer() call
--> AppendBufferInternal
--> PrepareAppend
--> OpenIfInEndedState
--> WMSI::UnMmarkEndOfStream
--> CD::UnmarkEndOfStream
DCHECK failure.

2 questions:
1) Why was this next append call allowed to proceed after appendError algorithm had occurred (on some SourceBuffer, maybe a different one on same MediaSource/element)?
2) Why wasn't the state of the ChunkDemuxer 'ended' (previous appendError on *some* WebSourceBufferImpl associated with (I think) that same ChunkDemuxer should have changed it to ended)?

 
Labels: -Pri-2 Pri-3
This isn't a recent regression, nor is it visible on release builds (though web apps will be able to briefly do more work against MSE API after appendError, but before HTMLMediaElement error is determined and closes the MediaSource).
for 1) I suspect it's because the SourceBuffer's end-of-stream algorithm implemented in Chrome *doesn't* block the main renderer thread while the ChunkDemuxer propogates the error from renderer->media->renderer threads, and
for 2) I'm not sure.

This is something I'll keep in mind as perhaps being more fixable once async append operations are done on the media thread, not the main renderer thread.

Perhaps just closing the MediaSource as part of appendError is necessary; not sure if that would violate spec though.
For 2) ChunkDemuxer was beyond 'ended' state --> it was in parse error state (correctly).

This is understood and definitely a valid bug now (See #1 explanation of 1) ).

Sign in to add a comment