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

Basic HTMLMediaElement seeking #22005

Merged
merged 8 commits into from Oct 26, 2018

Conversation

Projects
None yet
6 participants
@ferjm
Member

ferjm commented Oct 23, 2018

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #21998

This allows media seeking only when the server supports byte-range requests.


This change is Reviewable

@highfive

This comment has been minimized.

highfive commented Oct 23, 2018

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/htmlmediaelement.rs, components/script/dom/webidls/HTMLMediaElement.webidl
  • @KiChjang: components/script/dom/htmlmediaelement.rs, components/script/dom/webidls/HTMLMediaElement.webidl
@highfive

This comment has been minimized.

highfive commented Oct 23, 2018

warning Warning warning

  • These commits modify script code, but no tests are modified. Please consider adding a test!
Cargo.lock Outdated
@@ -2820,7 +2820,7 @@ dependencies = [
"lazy_static 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
"libc 0.2.42 (registry+https://github.com/rust-lang/crates.io-index)",
"num_cpus 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
"rand 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)",
"rand 0.3.22 (registry+https://github.com/rust-lang/crates.io-index)",

This comment has been minimized.

@Eijebong

This comment has been minimized.

@ferjm

ferjm Oct 23, 2018

Member

Hmm... no idea why this happened. I run ./mach cargo-update -p servo-media as usual.

@ferjm

This comment has been minimized.

Member

ferjm commented Oct 23, 2018

demo

@ferjm

This comment has been minimized.

Member

ferjm commented Oct 23, 2018

@bors-servo try=wpt

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Oct 23, 2018

⌛️ Trying commit a1a92ad with merge 3a7b7e8...

bors-servo added a commit that referenced this pull request Oct 23, 2018

Auto merge of #22005 - ferjm:media.timeline, r=<try>
Basic HTMLMediaElement seeking

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #21998

This allows media seeking only when the server supports byte-range requests.

<!-- 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/22005)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Oct 23, 2018

☔️ The latest upstream changes (presumably #21999) made this pull request unmergeable. Please resolve the merge conflicts.

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Oct 23, 2018

💔 Test failed - linux-rel-css

@ferjm

This comment has been minimized.

Member

ferjm commented Oct 24, 2018

@bors-servo try=wpt

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Oct 24, 2018

⌛️ Trying commit 8da553f with merge 9ce8a3d...

bors-servo added a commit that referenced this pull request Oct 24, 2018

Auto merge of #22005 - ferjm:media.timeline, r=<try>
Basic HTMLMediaElement seeking

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #21998

This allows media seeking only when the server supports byte-range requests.

<!-- 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/22005)
<!-- Reviewable:end -->
@ferjm

This comment has been minimized.

Member

ferjm commented Oct 24, 2018

r? @ceyusa

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Oct 24, 2018

💔 Test failed - linux-rel-css

@ferjm

This comment has been minimized.

Member

ferjm commented Oct 24, 2018

The remaining WPTs failures are because we don't expose the seekable property yet.

@bors-servo try=wpt

bors-servo added a commit that referenced this pull request Oct 24, 2018

Auto merge of #22005 - ferjm:media.timeline, r=<try>
Basic HTMLMediaElement seeking

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #21998

This allows media seeking only when the server supports byte-range requests.

<!-- 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/22005)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Oct 24, 2018

⌛️ Trying commit 49389a7 with merge 0cf52d8...

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Oct 24, 2018

💔 Test failed - linux-rel-css

}
// https://html.spec.whatwg.org/multipage/#dom-media-seek
fn seek_sync(&self) {

This comment has been minimized.

@ceyusa

ceyusa Oct 25, 2018

Contributor

I'm not sure if seek_sync() is a good name. I'm thinking on seek_end() or seek_next()

let mut headers = Headers::new();
headers.set(HyperRange::Bytes(vec![ByteRangeSpec::AllFrom(
offset.unwrap_or(0),
)]));

This comment has been minimized.

@ceyusa

ceyusa Oct 25, 2018

Contributor

does it worth to add this header if offset is zero?

This comment has been minimized.

@ferjm

ferjm Oct 25, 2018

Member

For simplicity, yes :). A server not supporting byte range requests will simply ignore the header.

@ferjm

This comment has been minimized.

Member

ferjm commented Oct 26, 2018

With #22024 we can seek on media behind file urls. And seeking is obviously way faster. This is with a debug build:

seek_file

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Oct 26, 2018

💔 Test failed - linux-rel-wpt

@ferjm

This comment has been minimized.

Member

ferjm commented Oct 26, 2018

@bors-servo r=ceyusa

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Oct 26, 2018

📌 Commit d7629b7 has been approved by ceyusa

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Oct 26, 2018

⌛️ Testing commit d7629b7 with merge c88f12d...

bors-servo added a commit that referenced this pull request Oct 26, 2018

Auto merge of #22005 - ferjm:media.timeline, r=ceyusa
Basic HTMLMediaElement seeking

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #21998

This allows media seeking only when the server supports byte-range requests.

<!-- 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/22005)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Oct 26, 2018

💔 Test failed - linux-rel-wpt

@ferjm

This comment has been minimized.

Member

ferjm commented Oct 26, 2018

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Oct 26, 2018

⌛️ Testing commit d7629b7 with merge cb915d6...

bors-servo added a commit that referenced this pull request Oct 26, 2018

Auto merge of #22005 - ferjm:media.timeline, r=ceyusa
Basic HTMLMediaElement seeking

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #21998

This allows media seeking only when the server supports byte-range requests.

<!-- 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/22005)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Oct 26, 2018

💔 Test failed - mac-rel-css2

@ferjm

This comment has been minimized.

Member

ferjm commented Oct 26, 2018

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Oct 26, 2018

@bors-servo

This comment has been minimized.

@bors-servo bors-servo merged commit d7629b7 into servo:master Oct 26, 2018

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

@ferjm ferjm deleted the ferjm:media.timeline branch Oct 29, 2018

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