Skip to content

htmlmediaelement: Support seek requests for non seekable fetch context#37264

Merged
jdm merged 1 commit intoservo:mainfrom
tharkum:htmlmediaelement-data-url
Jun 6, 2025
Merged

htmlmediaelement: Support seek requests for non seekable fetch context#37264
jdm merged 1 commit intoservo:mainfrom
tharkum:htmlmediaelement-data-url

Conversation

@tharkum
Copy link
Copy Markdown
Contributor

@tharkum tharkum commented Jun 5, 2025

According to specification the data:// URL protocol doesn't support range request
so be able make any seek request to required content position let's allow
for non seekable fetch context discard fetched content bytes until seek offset.
https://fetch.spec.whatwg.org/#scheme-fetch

Some scheme URLs (like data:// URL) doesn't expose "Content-Length" header in response
so the total expected size of the stream is unknown and it causes some additional
seek request (SeekData) from the media player. Try to post configure stream size
after we reached fetch EOS response.

Related source code which breaks WPT tests:
tests/wpt/tests/html/canvas/element/manual/imagebitmap/common.sub.js#L56-L78

Testing: Improvements in the following tests:

  • html/canvas/element/manual/imagebitmap/createImageBitmap*

Fixes: #32645
Fixes: #32745
Fixes: #34119
Fixes: #34120
Fixes: #34151

@tharkum tharkum requested a review from gterzian as a code owner June 5, 2025 14:05
@tharkum
Copy link
Copy Markdown
Contributor Author

tharkum commented Jun 5, 2025

NOTE: The "Content-Length" header for data URL response change is optional and could be remove from PR.
Everything will works but with extra fetch request (to make seek to required position)

@mrobinson mrobinson added the T-linux-wpt Do a try run of the WPT label Jun 5, 2025
Copy link
Copy Markdown
Member

@jdm jdm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice solution! Let's merge this and revisit the Content-Length once we reach a conclusion in the linked issue.

@jdm
Copy link
Copy Markdown
Member

jdm commented Jun 5, 2025

There are some unit tests that are now failing that need to be investigated.

@tharkum tharkum force-pushed the htmlmediaelement-data-url branch from 13699c0 to 043cf1e Compare June 6, 2025 10:33
@tharkum
Copy link
Copy Markdown
Contributor Author

tharkum commented Jun 6, 2025

Change to data:// URL processing ("Content-Length" header) didn't pass unit tests
(expected header maps size should be equal to 1 - "Content-Type" header).

I removed this change from PR and proposed another change (post configuration of the total stream size) for non seekable fetch context.

According to specification the data:// URL protocol doesn't support range request
so be able make any seek request to required content position let's allow
for non seekable fetch context discard fetched content bytes until seek offset.
https://fetch.spec.whatwg.org/#scheme-fetch

Some scheme URLs (like data:// URL) doesn't expose "Content-Length" header in response
so the total expected size of the stream is unknown and it causes some additional
seek request (SeekData) from the media player. Try to post configure stream size
after we reached fetch EOS response.

Related source code which breaks WPT tests:
tests/wpt/tests/html/canvas/element/manual/imagebitmap/common.sub.js#L56-L78

Testing: Improvements in the following tests:
 - html/canvas/element/manual/imagebitmap/createImageBitmap*
 - mixed-content/gen/top.***/opt-in/video-tag.https.html
 - webgl/tests/conformance/textures/misc/texture-srgb-upload.html

Fixes: servo#32645
Fixes: servo#32745
Fixes: servo#34119
Fixes: servo#34120
Fixes: servo#34151

Signed-off-by: Andrei Volykhin <[email protected]>
@tharkum tharkum force-pushed the htmlmediaelement-data-url branch from 043cf1e to 0706594 Compare June 6, 2025 12:26
@tharkum
Copy link
Copy Markdown
Contributor Author

tharkum commented Jun 6, 2025

@tharkum
Copy link
Copy Markdown
Contributor Author

tharkum commented Jun 6, 2025

Copy link
Copy Markdown
Member

@jdm jdm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice.

@jdm jdm added this pull request to the merge queue Jun 6, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 6, 2025
@jdm jdm added this pull request to the merge queue Jun 6, 2025
Merged via the queue into servo:main with commit b81f1b1 Jun 6, 2025
21 checks passed
@tharkum tharkum deleted the htmlmediaelement-data-url branch June 8, 2025 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment