Checks MediaSource.activeSourceBuffers and changes to track state #3182
Conversation
…state The tests only cover the configurations required by the specification, namely: - one SourceBuffer with one audio and/or one video track - one SourceBuffer with an audio track and one SourceBuffer with a video track The tests do not try to create more SourceBuffer objects in particular and do not create text tracks. The tests should cover all steps in section 2.4.5 "Changes to selected/enabled track state": https://w3c.github.io/media-source/#active-source-buffer-changes
|
Critic review: https://critic.hoppipolla.co.uk/r/6628 This is an external review system which you may optionally use for the code review of your pull request. In order to help critic track your changes, please do not make in-place history rewrites (e.g. via |
|
Reviewers for this pull request are: @acolwell, @bit, @shishimaru, @sideshowbarker, and @wolenetz. |
|
I'm reviewing this now. Apologies for delay while I had been working on the MSE spec issue drive-down. |
| assert_equals(mediaSource.activeSourceBuffers.length, 0, | ||
| "activeSourceBuffers is empty to start with"); | ||
|
|
||
| test.expectEvent(mediaElement, "loadedmetadata"); |
There was a problem hiding this comment.
It shouldn't matter much, but the actual sequence of these events being enqueued is: addsourcebuffer first, then loadedmetadata later once the first init segments across all sourcebuffers has appended and parsed. I'll swap the ordering as part of merging this.
|
This PR LGTM modulo the comments I added. I'll merge it with commit(s) that address my comments shortly.
|
|
It looks like tip-of-tree Chrome with --enable-experimental-platform-features is failing the latter two tests because the sourceBuffer.{audio,video}Tracks "change" expectation isn't met. I'll investigate further and fix one or both of these tests and Chrome. |
|
Investigating per #3182 (comment), the test looks right, Chrome needs fixing (only HTMLMediaElement.{audio,video}Tracks lists get the 'change' events, not SourceBuffer.{audio,video}Tracks, when selected/enabled track state changes). I've filed https://crbug.com/639151 to get Chrome compliant with 'change' events on SourceBuffer.{audio,video}Tracks. edit: That same Chrome bug tracks the other implementation issue exposed by this test: MediaSource.activeSourceBuffers needs to get appropriate '{add,remove}sourcebuffer' events when track enable/select state changes require those events per the MSE spec. I'll merge this PR with additional commit(s) that address my comments shortly. |
The tests only cover the configurations required by the specification, namely:
The tests do not try to create more SourceBuffer objects in particular and do not create text tracks.
The tests should cover all steps in section 2.4.5 "Changes to selected/enabled track state":
https://w3c.github.io/media-source/#active-source-buffer-changes
Note that Edge apparently does not yet support resetting the delaying-the-load-event-flag of the media element, and does not want to reach HAVE_CURRENT_DATA for video-only tracks, unless "endOfStream()" is called. This makes the test harness time out in some cases (because the "load" event is not fired on "window"), even though the underlying test cases actually pass. If needed, I may add calls to "mediaSource.endOfStream()".