Skip to content
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

Full asset is downloaded in progressive playback with "chunked" transfer encoding #1257

Closed
filipe-norte-red opened this issue Dec 13, 2023 · 6 comments
Assignees
Labels
upstream Related to an upstream bug (or should be at some point) wpe-2.38

Comments

@filipe-norte-red
Copy link

When a server transfers a video resource using "chunked" transfer encoding, wpe will download the full file, irrespective of size (e.g. 150MB). However, if "chunked" transfer encoding is not used, wpe will download small parts, progressively requesting further parts of the file as needed.
The attached test.zip contains:

  • test.html - Sample page that plays a video file
  • myserver.js - Node based server that serves the test page and video asset. It uses https://github.com/pillarjs/send

Repo steps:

If you'd like to disable chunked transfer mode for test, edit myserver.js and comment line 20:
res.setHeader('Transfer-Encoding', 'chunked');

test_chunked1

@magomez
Copy link

magomez commented Dec 14, 2023

This happens on wpe-2.38 so we're using libsoup3 and HTTP2 requests. According to google, Transfer-Encoding:chunked is deprecated when using HTTP2 as all of the requests are chunked by design. The server shouldn't be replying with a Transfer-Content header when using an HTTP2 connection.
Then, we may have a bug on our side handling that incorrect header that's causing that the full file is being downloaded. I'll ask internally about it.

@magomez magomez assigned eocanha and unassigned magomez Dec 15, 2023
@magomez
Copy link

magomez commented Dec 15, 2023

Mi previous comment is not valid because the request is HTTP 1.1 and not 2.0.
The problem seems to be that when Transfer-Encoding chunked is used, the Content-Lenght of the header is not available. Due to that, WebKitWebSourceGStreamer assumes that the content is not seekable and downloads the whole file. A way to handle chunked data should be added.

@eocanha
Copy link
Member

eocanha commented Dec 20, 2023

I coded a patch to honor Content-Length when the Transfer-Encoding is chunked (so the WebKitWebSrc element has an idea of the actual length, considers that the content isn't "live" and can therefore unblock the seek and partial download features) and submitted it upstream for review as https://bugs.webkit.org/show_bug.cgi?id=266691 / WebKit/WebKit#22093.

@eocanha eocanha added the upstream Related to an upstream bug (or should be at some point) label Dec 20, 2023
@filipe-norte-red
Copy link
Author

Thanks for the feedback, @magomez . Will that be done via an upstream issue to be created? Or is it something you are working on?

@modeveci
Copy link

@filipe-norte-red , The PR is in review, in the meantime you can also give a try: WebKit/WebKit@68c0a93
Regards
Ozgur

webkit-commit-queue pushed a commit to eocanha/WebKit that referenced this issue Dec 20, 2023
…hunked content encoding

https://bugs.webkit.org/show_bug.cgi?id=266691

Reviewed by Xabier Rodriguez-Calvar.

Currently, chunked content-encoding media content transfers are considered not to have a
defined length and are treated as live videos (no seek available, the whole content is
downloaded in a monolithic way). However, sometimes the web server specifies a
content-length that, even if not mandatory, gives a hint about the total length. If that
info was used, we could treat the download in a much more flexible way, downloading only the
required pieces on demand, as the player needs them, and also enabling seek.

See: WebPlatformForEmbedded/WPEWebKit#1257

This patch leverages the information supplied by the content-length header when
content-encoding is set as chunked.

* Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:
(CachedResourceStreamingClient::responseReceived): Set length with the right information when the circumstances are appropriate.

Canonical link: https://commits.webkit.org/272355@main
eocanha added a commit that referenced this issue Dec 20, 2023
…hunked content encoding

https://bugs.webkit.org/show_bug.cgi?id=266691

Reviewed by Xabier Rodriguez-Calvar.

Currently, chunked content-encoding media content transfers are considered not to have a
defined length and are treated as live videos (no seek available, the whole content is
downloaded in a monolithic way). However, sometimes the web server specifies a
content-length that, even if not mandatory, gives a hint about the total length. If that
info was used, we could treat the download in a much more flexible way, downloading only the
required pieces on demand, as the player needs them, and also enabling seek.

See: #1257

This patch leverages the information supplied by the content-length header when
content-encoding is set as chunked.

* Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:
(CachedResourceStreamingClient::responseReceived): Set length with the right information when the circumstances are appropriate.

Canonical link: https://commits.webkit.org/272355@main
eocanha added a commit that referenced this issue Dec 20, 2023
…hunked content encoding

https://bugs.webkit.org/show_bug.cgi?id=266691

Reviewed by Xabier Rodriguez-Calvar.

Currently, chunked content-encoding media content transfers are considered not to have a
defined length and are treated as live videos (no seek available, the whole content is
downloaded in a monolithic way). However, sometimes the web server specifies a
content-length that, even if not mandatory, gives a hint about the total length. If that
info was used, we could treat the download in a much more flexible way, downloading only the
required pieces on demand, as the player needs them, and also enabling seek.

See: #1257

This patch leverages the information supplied by the content-length header when
content-encoding is set as chunked.

* Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:
(CachedResourceStreamingClient::responseReceived): Set length with the right information when the circumstances are appropriate.

Canonical link: https://commits.webkit.org/272355@main
@eocanha
Copy link
Member

eocanha commented Dec 20, 2023

The fix landed upstream as WebKit/WebKit@dc46983 and was backported to wpe-2.38 as b56a9c7 and to wpe-2.42 as 4e6f703.
Closing issue.

@eocanha eocanha closed this as completed Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upstream Related to an upstream bug (or should be at some point) wpe-2.38
Development

No branches or pull requests

4 participants