1088472 - test_SeekableBeforeEndOfStream.html hangs in seeking within test.webm

Status: NEW

(Core :: Audio/Video: Playback, defect, P5)

Description

[JW Wang [:jwwang] [:jw_wang]]

Attachment: test_SeekableBeforeEndOfStream_html_mod.patch

I tried to have test_SeekableBeforeEndOfStream.html test different files to increase the test coverage:

1. copy testing/web-platform/tests/media-source/webm/test.webm to content/media/mediasource/test
2. apply test_SeekableBeforeEndOfStream_html_mod.patch
3. run |./mach mochitest-plain content/media/mediasource/test/test_SeekableBeforeEndOfStream.html|

I found it hang during seeking within test.webm. However it didn't with the original seek.webm.

I wonder there is something wrong to test.webm which is used by our web-platform media-source tests. And it could be the reason why the web-platform media-source tests tend to time out.

Comment 1

[Matthew Gregan [:kinetik]]

Nice find!  The main difference is that test.webm has the Cues at the end of the file whereas seek.webm has them at the beginning.

The seek blocks in WebMReader::Seek when the script sets currentTime to 2s with the following stack:

    mozilla::ReentrantMonitorAutoEnter::Wait
    mozilla::SourceBufferResource::Read
    mozilla::webm_read
    ne_io_read
    ne_bare_read_vint
    ne_read_id
    ne_peek_element
    ne_parse
    ne_init_cue_points
    nestegg_track_seek
    mozilla::WebMReader::Seek

In SourceBufferResource::Read, blockingRead is true, mEnded is false, and mOffset is 219448.  The length of test.webm is also 219448.

ne_parse attempts to read until it sees a non-Cue element or EOF.  Since we're not at EOF (because EndOfStream has not been called and we don't know if more data will be appended) and we're never going to receive more data (to see a non-Cue element), ne_parse never completes because it's waiting on data that will never arrive.

Comment 2

[JW Wang [:jwwang] [:jw_wang]]

(In reply to Matthew Gregan [:kinetik] from comment #1)
> Nice find!  The main difference is that test.webm has the Cues at the end of
> the file whereas seek.webm has them at the beginning.
Does that mean test.webm has an invalid format or our webm reader should handle this case?

Comment 3

[Jean-Yves Avenard [:jya]]

I wouldn't be surprised if the reason was related to the issues I'm seeing in bug 1065827.
I believe I have identified the issue in the MediaDecoderStateMachine, there's a racing condition that can happen when we are in DECODER_SEEKING state and we enter DECODER_DECODING state before MediaDecoderStateMachine::DecoderSeek gets the chance to run

Comment 4

[Matthew Gregan [:kinetik]]

(In reply to JW Wang [:jwwang] from comment #2)
> Does that mean test.webm has an invalid format or our webm reader should
> handle this case?

It's a perfectly valid WebM file.  The problem is a bug/bad interaction between our WebM parser and the way MSE resources are presented to it.

(In reply to Jean-Yves Avenard [:jya] from comment #3)
> I wouldn't be surprised if the reason was related to the issues I'm seeing
> in bug 1065827.
> I believe I have identified the issue in the MediaDecoderStateMachine,
> there's a racing condition that can happen when we are in DECODER_SEEKING
> state and we enter DECODER_DECODING state before
> MediaDecoderStateMachine::DecoderSeek gets the chance to run

It's unrelated.

Comment 5

[JW Wang [:jwwang] [:jw_wang]]

(In reply to Jean-Yves Avenard [:jya] from comment #3)
How is that a problem? MediaDecoderStateMachine::DecodeSeek() returns immediately when seeing |mState != DECODER_STATE_SEEKING| to handle the shutdown case by aborting seeking.

Comment 6

[Jean-Yves Avenard [:jya]]

(In reply to JW Wang [:jwwang] from comment #5)
> (In reply to Jean-Yves Avenard [:jya] from comment #3)
> How is that a problem? MediaDecoderStateMachine::DecodeSeek() returns
> immediately when seeing |mState != DECODER_STATE_SEEKING| to handle the
> shutdown case by aborting seeking.

Because it happens when seeking shouldn't be aborted, it just happens that we're receiving decoded data queued from before seeking. That race causes most of the seek mochi test to intermittently fail

But let's move this conversation to bug 1065827, or maybe I should open another bug