Skip to content

Commit

Permalink
Auto merge of #22477 - ferjm:media_time_marches_on_step_6, r=<try>
Browse files Browse the repository at this point in the history
Implement step 6 of media `time marches on` algorithm. Improves stability of media WPTs

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] There are tests for these changes

This should help with the WPTs problems observed in #22348 (comment)

Unfortunately, GStreamer does not seem to be very reliable with Ogg (check https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/issues/520) and most of the media-element WPTs uses a short ogv file, so I had to make our `canPlayType` report that it is not able to play this type to make the tests pick the alternative mp4 version of the same files. Once Ogg support for GStreamer improves, we should be able to revert this change.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22477)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo committed Jan 9, 2019
2 parents 9223d62 + 4d9fb87 commit da5584b
Show file tree
Hide file tree
Showing 189 changed files with 822 additions and 259 deletions.
3 changes: 2 additions & 1 deletion components/script/dom/bindings/trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ use style::values::specified::Length;
use tendril::fmt::UTF8;
use tendril::stream::LossyDecoder;
use tendril::{StrTendril, TendrilSink};
use time::Duration;
use time::{Duration, Timespec};
use uuid::Uuid;
use webrender_api::{DocumentId, ImageKey, RenderApiSender};
use webvr_traits::WebVRGamepadHand;
Expand Down Expand Up @@ -488,6 +488,7 @@ unsafe_no_jsmanaged_fields!(dyn Player<Error = ServoMediaError>);
unsafe_no_jsmanaged_fields!(Mutex<MediaFrameRenderer>);
unsafe_no_jsmanaged_fields!(RenderApiSender);
unsafe_no_jsmanaged_fields!(ResourceFetchTiming);
unsafe_no_jsmanaged_fields!(Timespec);

unsafe impl<'a> JSTraceable for &'a str {
#[inline]
Expand Down
24 changes: 22 additions & 2 deletions components/script/dom/htmlmediaelement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,9 @@ pub struct HTMLMediaElement {
text_tracks_list: MutNullableDom<TextTrackList>,
/// Expected content length of the media asset being fetched or played.
content_length: Cell<Option<u64>>,
/// Time of last timeupdate notification.
#[ignore_malloc_size_of = "Defined in time"]
next_timeupdate_event: Cell<Timespec>,
}

/// <https://html.spec.whatwg.org/multipage/#dom-media-networkstate>
Expand Down Expand Up @@ -261,6 +264,7 @@ impl HTMLMediaElement {
played: Rc::new(DomRefCell::new(TimeRangesContainer::new())),
text_tracks_list: Default::default(),
content_length: Cell::new(None),
next_timeupdate_event: Cell::new(time::get_time() + Duration::milliseconds(250)),
}
}

Expand Down Expand Up @@ -303,7 +307,16 @@ impl HTMLMediaElement {

/// https://html.spec.whatwg.org/multipage/#time-marches-on
fn time_marches_on(&self) {
// TODO: implement this.
// Step 6.
if time::get_time() > self.next_timeupdate_event.get() {
let window = window_from_node(self);
window
.task_manager()
.media_element_task_source()
.queue_simple_event(self.upcast(), atom!("timeupdate"), &window);
self.next_timeupdate_event
.set(time::get_time() + Duration::milliseconds(350));
}
}

/// <https://html.spec.whatwg.org/multipage/#internal-pause-steps>
Expand Down Expand Up @@ -1173,6 +1186,7 @@ impl HTMLMediaElement {
.borrow_mut()
.add(self.playback_position.get(), position);
self.playback_position.set(position);
self.time_marches_on();
},
PlayerEvent::StateChanged(ref state) => match *state {
PlaybackState::Paused => {
Expand Down Expand Up @@ -1270,8 +1284,14 @@ impl HTMLMediaElementMethods for HTMLMediaElement {
// https://html.spec.whatwg.org/multipage/#dom-navigator-canplaytype
fn CanPlayType(&self, type_: DOMString) -> CanPlayTypeResult {
match type_.parse::<Mime>() {
// XXX GStreamer is currently not very reliable playing OGG and most of
// the media related WPTs uses OGG if we report that we are able to
// play this type. So we report that we are unable to play it to force
// the usage of other types.
// https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/issues/520
Ok(ref mime)
if (mime.type_() == mime::APPLICATION && mime.subtype() == mime::OCTET_STREAM) =>
if (mime.type_() == mime::APPLICATION && mime.subtype() == mime::OCTET_STREAM) ||
(mime.subtype() == mime::OGG) =>
{
CanPlayTypeResult::_empty
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
[audio_loop_base.html]
type: testharness
disabled: extreme timeout
expected: TIMEOUT
[Check if audio.loop is set to true that expecting the seeking event is fired more than once]
expected: FAIL
expected: NOTRUN

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,27 +1,20 @@
[event_volumechange.html]
type: testharness
expected: TIMEOUT
[setting audio.volume fires volumechange]
expected: PASS

[setting audio.muted fires volumechange]
expected: FAIL

[setting audio.volume/muted to the same value does not fire volumechange]
[setting video.volume/muted repeatedly fires volumechange repeatedly]
expected: TIMEOUT

[setting audio.volume/muted repeatedly fires volumechange repeatedly]
[setting audio.volume/muted to the same value does not fire volumechange]
expected: TIMEOUT

[setting video.volume fires volumechange]
expected: PASS

[setting video.muted fires volumechange]
expected: FAIL

[setting video.volume/muted to the same value does not fire volumechange]
[setting audio.volume/muted repeatedly fires volumechange repeatedly]
expected: TIMEOUT

[setting video.volume/muted repeatedly fires volumechange repeatedly]
[setting video.volume/muted to the same value does not fire volumechange]
expected: TIMEOUT

Original file line number Diff line number Diff line change
@@ -1,31 +1,31 @@
[crossOrigin.html]
[HTMLMediaElement.crossOrigin]
[HTMLMediaElement.crossOrigin, setting to invalid value]
expected: FAIL

[HTMLMediaElement.crossOrigin, content attribute missing]
[HTMLMediaElement.crossOrigin, content attribute use-credentials]
expected: FAIL

[HTMLMediaElement.crossOrigin, content attribute invalid value]
expected: FAIL

[HTMLMediaElement.crossOrigin, content attribute empty string]
[HTMLMediaElement.crossOrigin, setting to uppercase ANONYMOUS]
expected: FAIL

[HTMLMediaElement.crossOrigin, content attribute uppercase ANONYMOUS]
expected: FAIL

[HTMLMediaElement.crossOrigin, content attribute use-credentials]
expected: FAIL

[HTMLMediaElement.crossOrigin, setting to empty string]
expected: FAIL

[HTMLMediaElement.crossOrigin, setting to invalid value]
[HTMLMediaElement.crossOrigin, content attribute empty string]
expected: FAIL

[HTMLMediaElement.crossOrigin, setting to uppercase ANONYMOUS]
[HTMLMediaElement.crossOrigin, content attribute missing]
expected: FAIL

[HTMLMediaElement.crossOrigin, setting to use-credentials]
expected: FAIL

[HTMLMediaElement.crossOrigin]
expected: FAIL

Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
[addCue.html]
type: testharness
expected: TIMEOUT
[TextTrack.addCue(), adding a cue to two different tracks]
expected: FAIL

[TextTrack.addCue(), adding a cue to a track twice]
expected: FAIL
[TextTrack.addCue(), adding a cue associated with a track element to other track]
expected: TIMEOUT

[TextTrack.addCue(), adding a removed cue to a different track]
expected: FAIL

[TextTrack.addCue(), adding an associated but removed cue to the same track]
expected: FAIL

[TextTrack.addCue(), adding a cue associated with a track element to other track]
expected: TIMEOUT
[TextTrack.addCue(), adding a cue to a track twice]
expected: FAIL

Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
[cues.html]
type: testharness
[TextTrack.cues, after addCue()]
expected: FAIL

[TextTrack.cues, different modes]
expected: FAIL

[TextTrack.cues, changing order]
[TextTrack.cues, default attribute]
expected: FAIL

[TextTrack.cues, default attribute]
[TextTrack.cues, changing order]
expected: FAIL

Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
[kind.html]
type: testharness
[TextTrack.kind, track element]
[TextTrack.kind, \\u0000]
expected: FAIL

[TextTrack.kind, \\u0000]
[TextTrack.kind, track element]
expected: FAIL

Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
[label.html]
type: testharness
[TextTrack.label]
[TextTrack.label, \\u0000]
expected: FAIL

[TextTrack.label, \\u0000]
[TextTrack.label]
expected: FAIL

Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
[language.html]
type: testharness
[TextTrack.language]
[TextTrack.language, \\u0000]
expected: FAIL

[TextTrack.language, \\u0000]
[TextTrack.language]
expected: FAIL

Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
[oncuechange.html]
type: testharness
[TextTrack.oncuechange]
expected: FAIL

Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
[removeCue.html]
type: testharness
expected: TIMEOUT
[TextTrack.removeCue(), two elementless tracks]
expected: FAIL

[TextTrack.removeCue(), cue from track element]
expected: TIMEOUT

[TextTrack.removeCue(), two elementless tracks]
expected: FAIL

Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
[endTime.html]
type: testharness
expected: TIMEOUT
[TextTrackCue.endTime, script-created cue]
expected: FAIL
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
[id.html]
type: testharness
expected: TIMEOUT
[TextTrackCue.id, script-created cue]
expected: FAIL
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
[onenter.html]
type: testharness
expected: ERROR
[TextTrackCue.onenter]
[TextTrackCue.addEventListener/removeEventListener]
expected: FAIL

[TextTrackCue.addEventListener/removeEventListener]
[TextTrackCue.onenter]
expected: FAIL

Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
[onexit.html]
type: testharness
expected: ERROR
[TextTrackCue.onexit]
[TextTrackCue.addEventListener/removeEventListener]
expected: FAIL

[TextTrackCue.addEventListener/removeEventListener]
[TextTrackCue.onexit]
expected: FAIL

Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
[pauseOnExit.html]
type: testharness
expected: TIMEOUT
[TextTrackCue.pauseOnExit, script-created cue]
expected: FAIL

[TextTrackCue.pauseOnExit, parsed cue]
expected: TIMEOUT

[TextTrackCue.pauseOnExit, script-created cue]
expected: FAIL

Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
[startTime.html]
type: testharness
expected: TIMEOUT
[TextTrackCue.startTime, script-created cue]
expected: FAIL

[TextTrackCue.startTime, parsed cue]
expected: TIMEOUT

[TextTrackCue.startTime, script-created cue]
expected: FAIL

Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
[track.html]
type: testharness
expected: TIMEOUT
[TextTrackCue.track, script-created cue]
expected: FAIL
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
[getCueById.html]
type: testharness
[TextTrackCueList.getCueById, no id]
[TextTrackCueList.getCueById, id a\\u0000b]
expected: FAIL

[TextTrackCueList.getCueById, id foo]
expected: FAIL

[TextTrackCueList.getCueById, no 1]
[TextTrackCueList.getCueById, no id]
expected: FAIL

[TextTrackCueList.getCueById, id a\\u0000b]
[TextTrackCueList.getCueById, no 1]
expected: FAIL

Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
[getter.html]
type: testharness
[TextTrackCueList getter]
[TextTrackCueList getter, no indexed set/create (strict)]
expected: FAIL

[TextTrackCueList getter, no indexed set/create]
[TextTrackCueList getter]
expected: FAIL

[TextTrackCueList getter, no indexed set/create (strict)]
[TextTrackCueList getter, no indexed set/create]
expected: FAIL

Loading

0 comments on commit da5584b

Please sign in to comment.