New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement step 6 of media `time marches on` algorithm. Improves stability of media WPTs #22477

Merged
merged 3 commits into from Jan 10, 2019

Conversation

6 participants
@ferjm
Copy link
Member

ferjm commented Dec 16, 2018

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • 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.


This change is Reviewable

@highfive

This comment has been minimized.

Copy link

highfive commented Dec 16, 2018

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/htmlmediaelement.rs, components/script/dom/bindings/trace.rs
  • @KiChjang: components/script/dom/htmlmediaelement.rs, components/script/dom/bindings/trace.rs
@ferjm

This comment has been minimized.

Copy link
Member Author

ferjm commented Dec 16, 2018

r? @jdm

@highfive highfive assigned jdm and unassigned nox Dec 16, 2018

@jdm

This comment has been minimized.

Copy link
Member

jdm commented Dec 17, 2018

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Dec 17, 2018

📌 Commit d19834e has been approved by jdm

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Dec 17, 2018

⌛️ Testing commit d19834e with merge d2f66cb...

bors-servo added a commit that referenced this pull request Dec 17, 2018

Auto merge of #22477 - ferjm:media_time_marches_on_step_6, r=jdm
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 -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Dec 17, 2018

💔 Test failed - status-taskcluster

@ferjm

This comment has been minimized.

Copy link
Member Author

ferjm commented Dec 17, 2018

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Dec 17, 2018

⌛️ Testing commit d19834e with merge 04afbe4...

bors-servo added a commit that referenced this pull request Dec 17, 2018

Auto merge of #22477 - ferjm:media_time_marches_on_step_6, r=jdm
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 -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Dec 17, 2018

💔 Test failed - status-taskcluster

@ferjm

This comment has been minimized.

Copy link
Member Author

ferjm commented Dec 18, 2018

@bors-servo retry

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Dec 18, 2018

⌛️ Testing commit d19834e with merge 340aa9e...

bors-servo added a commit that referenced this pull request Dec 18, 2018

Auto merge of #22477 - ferjm:media_time_marches_on_step_6, r=jdm
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 -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Dec 18, 2018

💔 Test failed - status-taskcluster

@ferjm

This comment has been minimized.

Copy link
Member Author

ferjm commented Jan 9, 2019

@bors-servo try=wpt

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Jan 9, 2019

⌛️ Trying commit 4d9fb87 with merge ca7f362...

bors-servo added a commit that referenced this pull request Jan 9, 2019

Auto merge of #22477 - ferjm:media_time_marches_on_step_6, r=<try>
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 -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Jan 9, 2019

💔 Test failed - linux-rel-css

@ferjm

This comment has been minimized.

Copy link
Member Author

ferjm commented Jan 9, 2019

Oh gosh...

  │ 0:00:00.043080348 61918 0x7f9ba44095e0 ERROR             gst-player gstplayer.c:1139:emit_warning:<player0> Warning: Warning from element /GstPlayBin:playbin/GstURIDecodeBin:uridecodebin0: There is no codec present that can handle the stream's type.
  │ No decoder available for type 'audio/mpeg, mpegversion=(int)1, mpegaudioversion=(int)2, layer=(int)3, rate=(int)22050, channels=(int)1, parsed=(boolean)true'.
  │ ../subprojects/gst-plugins-base/gst/playback/gsturidecodebin.c(921): unknown_type_cb (): /GstPlayBin:playbin/GstURIDecodeBin:uridecodebin0 (gst-player-error-quark, 0)
  │ 0:00:00.043580538 61918 0x7f9b5002d230 WARN            uridecodebin gsturidecodebin.c:988:no_more_pads_full:<uridecodebin0> error: no suitable plugins found:
  │ ../subprojects/gst-plugins-base/gst/playback/gstdecodebin2.c(4640): gst_decode_bin_expose (): /GstPlayBin:playbin/GstURIDecodeBin:uridecodebin0/GstDecodeBin:decodebin0:
  │ no suitable plugins found:
  │ Missing decoder: MPEG-1 Layer 3 (MP3) (audio/mpeg, mpegversion=(int)1, mpegaudioversion=(int)2, layer=(int)3, rate=(int)22050, channels=(int)1, parsed=(boolean)true)

More missing decoders.

@ferjm

This comment has been minimized.

Copy link
Member Author

ferjm commented Jan 9, 2019

@bors-servo try=wpt

@jdm

This comment has been minimized.

Copy link
Member

jdm commented Jan 9, 2019

@bors-servo retry

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Jan 9, 2019

⌛️ Trying commit 4d9fb87 with merge da5584b...

bors-servo added a commit that referenced this pull request Jan 9, 2019

Auto merge of #22477 - ferjm:media_time_marches_on_step_6, r=<try>
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 -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Jan 9, 2019

💔 Test failed - linux-rel-css

@ferjm

This comment has been minimized.

Copy link
Member Author

ferjm commented Jan 10, 2019

With servo/saltfs@1ec4e8f I am able to play an mp4 file on servo-linux1.servo.org

gst-play-1.0 movie_5.mp4
Press 'k' to see a list of keyboard shortcuts.
Now playing /tmp/movie_5.mp4
Redistribute latency...
Redistribute latency...
Redistribute latency...
0:00:00.8 / 0:00:05.1

@bors-servo try=wpt

@ferjm

This comment has been minimized.

Copy link
Member Author

ferjm commented Jan 10, 2019

@bors-servo retry

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Jan 10, 2019

⌛️ Trying commit 4d9fb87 with merge f387508...

bors-servo added a commit that referenced this pull request Jan 10, 2019

Auto merge of #22477 - ferjm:media_time_marches_on_step_6, r=<try>
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 -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Jan 10, 2019

☀️ Test successful - linux-rel-css, linux-rel-wpt, status-taskcluster
State: approved= try=True

@ferjm

This comment has been minimized.

Copy link
Member Author

ferjm commented Jan 10, 2019

@bors-servo r=jdm

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Jan 10, 2019

📌 Commit 4d9fb87 has been approved by jdm

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Jan 10, 2019

⌛️ Testing commit 4d9fb87 with merge 4ac5b8a...

bors-servo added a commit that referenced this pull request Jan 10, 2019

Auto merge of #22477 - ferjm:media_time_marches_on_step_6, r=jdm
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 -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Jan 10, 2019

@bors-servo bors-servo merged commit 4d9fb87 into servo:master Jan 10, 2019

4 checks passed

Taskcluster (pull_request) TaskGroup: success
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

Media playback automation moved this from In progress to Done Jan 10, 2019

@ferjm ferjm deleted the ferjm:media_time_marches_on_step_6 branch Jan 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment