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

Service worker doesn't support seeking in videos #25865

Open
tehXor opened this issue Sep 7, 2018 · 26 comments
Open

Service worker doesn't support seeking in videos #25865

tehXor opened this issue Sep 7, 2018 · 26 comments
Labels
area: service-worker Issues related to the @angular/service-worker package browser: safari freq2: medium help wanted An issue that is suitable for a community contributor (based on its complexity/scope). P4 A relatively minor issue that is not relevant to core functions state: confirmed type: bug/fix workaround2: non-obvious
Milestone

Comments

@tehXor
Copy link

tehXor commented Sep 7, 2018

I'm submitting a...


[x] Bug report  
[x] Feature request (not sure, maybe this is more a feature request?)

Current behavior

If you ng run app:build:production and your pwa comes with a <video> element where you also utilize seeking (setting currentTime depending on your app logic and/or user input) it will not work or only very inconsistently. Most videos will simply run from start and not at the setted currentTime position. This is only the case when videos are delivered/fetched by the service worker. So it works in development mode.

Expected behavior

Videos should start at the setted currentTime.

Minimal reproduction of the problem with instructions

// on template
<video id="myVideo"><source id="myVideoSrc" src="video.mp4" type="video/mp4"></video>

// in page.ts
let videoObject = document.getElementById("myVideo");
videoObject.currentTime = 100;
videoObject.play();

// now build for production
// @angular/pwa has to be installed
> ng run app:build:production

Environment

Angular version: 6.1.2

Browser:
Chrome (desktop) version 69

Others:

This seems to be a known problem since service workers don't know Range requests. However it also looks like very easily fixable if I understand this thread comment correctly: https://bugs.chromium.org/p/chromium/issues/detail?id=575357#c10

A workaround would be a convenient way to completely exclude requests from service worker (not just set strategies for them). But obviously it would be better if we also could cache some videos. For exclusion also this issue may be relevant : #21191

EDIT(gkalpak): Issue summary in #25865 (comment).

@gkalpak
Copy link
Member

gkalpak commented Sep 17, 2018

Fixing #21191 would indeed provide a workaround for this usecase (but this could be properly fixable on its own as well).
Does anyone want to take a stub at incorporating this fix into @angular/service-worker.

@mordka
Copy link

mordka commented Nov 30, 2018

We hit this issue, because requests to video needed to be withCredentials and SW failed over without applying cookies to headers. A workaround was to skip handling request via SW. We used patch-package to maintain this patch. Here is the patch

diff --git a/node_modules/@angular/service-worker/ngsw-worker.js b/node_modules/@angular/service-worker/ngsw-worker.js
index 51c431b..1216506 100755
--- a/node_modules/@angular/service-worker/ngsw-worker.js
+++ b/node_modules/@angular/service-worker/ngsw-worker.js
@@ -1896,6 +1896,10 @@ ${msgIdle}`, { headers: this.adapter.newHeaders({ 'Content-Type': 'text/plain' }
          */
         onFetch(event) {
             const req = event.request;
+            //PATCH to prevent caching certain kinds of requests
+            // - PUT requests which breaks files upload
+            // - requests with 'Range' header which breaks credentialed video irequests
+            if (req.method==="PUT" || !!req.headers.get('range')) return;
             // The only thing that is served unconditionally is the debug page.
             if (this.adapter.parseUrl(req.url, this.scope.registration.scope).path === '/ngsw/state') {
                 // Allow the debugger to handle the request, but don't affect SW state in any

@marod424
Copy link

@mordka Thank you for your suggested fix.

I was having a slightly different issue where range requests were breaking audio in Safari due to the SW failing to apply the Content-Range header. I applied the range header patch using patch-package and it fixed the issue.

fergalmoran added a commit to podnoms/podnoms-backend that referenced this issue Jan 13, 2019
fergalmoran added a commit to podnoms/podnoms-web that referenced this issue Jan 13, 2019
@alxhub alxhub added freq2: medium severity2: inconvenient feature Issue that requests a new feature labels Apr 23, 2019
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Apr 23, 2019
@fergalmoran
Copy link

Turns out this is actually fixed now by appending ngsw-ignore to your querystring or as a header.
https://github.com/angular/angular/blob/master/aio/content/guide/service-worker-devops.md

@tehXor
Copy link
Author

tehXor commented Jun 10, 2019

Turns out this is actually fixed now by appending ngsw-ignore to your querystring or as a header.
https://github.com/angular/angular/blob/master/aio/content/guide/service-worker-devops.md

That is just another workaround to ignore videos (though it is one of the best workarounds in that regard so far).
But you still cannot include videos in your service worker's caching strategy when you need seeking. So no, I wouldn't say this is fixed.

@gkalpak
Copy link
Member

gkalpak commented Jun 10, 2019

As mentioned in #25865 (comment), if someone wants to take a stub at incorporating range handling into the SW, I'd be happy to help/review. How do other implementations (e.g. workbox) handle this?

@n-belokopytov
Copy link

We are having the same issue now. Was there any progress on that?

@saaniaki
Copy link

saaniaki commented Feb 1, 2020

Hopefully, this issue gets solved soon. We are using a nasty hack, after building, in the build script we're adding a line to ignore the video:

sed -i $'/^            const req = event.request;.*/a if (req.headers.get(\'range\')) return;' ./dist/ngsw-worker.js

This way we're avoiding a new fork from the main repo.

@gkalpak
Copy link
Member

gkalpak commented Feb 1, 2020

#25865 (comment)

@jay-babu
Copy link

jay-babu commented Mar 5, 2020

I have also been facing an issue with this. I have been using audio and doing seeking with currentTime. I obtained the audio in my component using ViewChild and not by it's ID. I was able to get my application working when I cleared the cache every time I opened it and trying to seek audio work successfully only in that edge case. In the end, I just removed the PWA aka the serviceWorker to fix the problem

EDIT:
I added in ?ngsw-bypass=true into the params and that fixed it I believe

@SystemR
Copy link

SystemR commented Apr 18, 2020

One way to do this is to create a patched ngsw-worker.js with Phil Nash's @philnash code to handle range request here: https://github.com/philnash/philna.sh/blob/ba798a2d5d8364fc7c1dae1819cbd8ef103c8b67/sw.js#L50-L94 (from https://philna.sh/blog/2018/10/23/service-workers-beware-safaris-range-request/)

First copy a ngsw-worker.js file from dist and create ngsw-worker-patched.js. Then prepend the following codes to the onFetch function in ngsw-worker.js

onFetch(event) {
  if (event.request.headers.get("range")) {

    // You need to grab the cache name. On my version of ngsw-worker.js: 
    const cacheName = `${this.adapter.cacheNamePrefix}:${this.latestHash}:assets:app:cache`;
    // note: adjust assets:app with where you put the videos (could be assets:assets group).

    // do respondWith with the promise returned from returnRangeRequest()
    event.respondWith(returnRangeRequest(event.request, cacheName));

    // @philnash code:
    function returnRangeRequest(request, cacheName) {
      return caches
        .open(cacheName)
        .then(function (cache) {
          return cache.match(request.url);
        })
        .then(function (res) {
          if (!res) {
            return fetch(request)
              .then((res) => {
                const clonedRes = res.clone();
                return caches
                  .open(cacheName)
                  .then((cache) => cache.put(request, clonedRes))
                  .then(() => res);
              })
              .then((res) => {
                return res.arrayBuffer();
              });
          }
          return res.arrayBuffer();
        })
        .then(function (arrayBuffer) {
          const bytes = /^bytes\=(\d+)\-(\d+)?$/g.exec(
            request.headers.get("range")
          );
          if (bytes) {
            const start = Number(bytes[1]);
            const end = Number(bytes[2]) || arrayBuffer.byteLength - 1;
            return new Response(arrayBuffer.slice(start, end + 1), {
              status: 206,
              statusText: "Partial Content",
              headers: [
                [
                  "Content-Range",
                  `bytes ${start}-${end}/${arrayBuffer.byteLength}`,
                ],
              ],
            });
          } else {
            return new Response(null, {
              status: 416,
              statusText: "Range Not Satisfiable",
              headers: [["Content-Range", `*/${arrayBuffer.byteLength}`]],
            });
          }
        });
    }
    return;
  }
  ... // rest of codes onFetch codes

Then on your npm postbuild script you can just cp the file back to your dist folder:

"scripts": {
  "build": "ng build --prod",
  "postbuild": "cp ngsw-worker-patched.js ./dist/ngsw-worker.js",

If your videos are small and you want to avoid doing manual patch of ngsw-worker.js, you can convert the videos into base 64 and use data uri on the video src. Or append ?ngsw-bypass to your video links: src="video.mp4?ngsw-bypass" to bypass the service worker.

@philnash
Copy link

Hello, I actually had to update that SW code recently (turned out the caches API didn't like storing 206 responses), so I'd check the latest from here: https://github.com/philnash/philna.sh/blob/master/sw.js#L49-L100.

@gkalpak gkalpak added browser: safari and removed feature Issue that requests a new feature labels May 26, 2020
@gkalpak
Copy link
Member

gkalpak commented May 26, 2020

To sum up (for future reference):

  • The problem is caused by the ServiceWorker's not handling range requests (i.e. requests with a Range header) correctly. See this blog post for more details.
  • An easy work around is to make the SW ignore affected requests by adding an ngsw-bypass header or query param. This has the downside that it breaks caching of such resources by the SW and so they won't be available offline.
  • A more involved work around is to manually augment the SW to handle range requests correctly, e.g. using something similar to this code by @philnash.

BTW, Workbox has a module to handle range requests: Workbox Range Requests (source code)

If anyone is interested in trying to incorporate the fix into the SW and submit a pull request, I would be happy to help/review.

@gkalpak gkalpak added the help wanted An issue that is suitable for a community contributor (based on its complexity/scope). label May 26, 2020
@ajitsinghkaler
Copy link
Contributor

@gkalpak what king of a fix are we looking into should we add another property in SW-config file. Which if made true add code which handles it

@gkalpak
Copy link
Member

gkalpak commented May 28, 2020

No, we don't need a config option. We just need to teach the SW how to handle range requests (by incorporating code similar to this and potentially making other necessary adjustments).

@jelbourn jelbourn added P4 A relatively minor issue that is not relevant to core functions and removed severity2: inconvenient labels Oct 1, 2020
@mbagiella
Copy link

It seems that we have a lot of workarounds in order to fix the video issues in Safari iOS, but I wonder why the fix is not yet implemented ?

@gkalpak
Copy link
Member

gkalpak commented Jan 18, 2021

As mentioned in #25865 (comment) (and other comments), implementing the fix is up for grabs if anyone is interested. I would be happy to help out.

Otherwise, I will hopefully get to it at some point, but this is not high in priority, so no timeline and no promises 😁

@mbagiella
Copy link

No timeline, no promise, not high in priority. However this issue is open since June 2018

Of course there is a lot of people interested to see a fix in order to have video working with Safari without having to extand manually the ngsw. I'm one of them :)

@lukfel
Copy link

lukfel commented Feb 8, 2021

My issue was that the videos of my PWA (Angular + NGSW) simply wouldn´t play on iOS using Safari. I finally found the solution in this answer: https://stackoverflow.com/a/58966397/8581106.

The solution for me was to bypass the Angular Service Worker with ?ngsw-bypass=true after the video url. This might not be related to everyones issue in here, however, this issue was linked as a reference to people sharing the same problem as me.

Example:

<video muted preload="metadata" controls playsinline>
     <source src="assets/vid/tutorial_invoice.mp4?ngsw-bypass=true" type="video/mp4">
</video>

@katgolek
Copy link

Is it possible to add it to :src that renders dynamically?

@muuvmuuv
Copy link

Hm, what is needed here to get this to work properly?

In our case, we load many PDFs, each must be cached for offline usage, which works, but range requests do not. Weirdly, it only does not work for giant PDFs, above 6 MB.

@StrixOSG
Copy link

Same issue here with range requests and the service worker. We're using @philnash's patch, but we shouldn't have to. Any updates on this?

@nabby27
Copy link

nabby27 commented May 1, 2024

I would like this issue to be resolved, which is why I have posted a $60 reward. I would like to contribute but I don't have time to solve it and that's why I thought that a monetary contribution could help attract people who want to help (especially seeing that it is an issue that has been open for quite some time)

@turijanAldo
Copy link

If the video file is still being served by the Service Worker, you can force the video to load directly from the network by appending a unique parameter to the video URL, such as a timestamp or identifier:

const videoElement = document.getElementById('myVideo');
const videoSource = document.getElementById('myVideoSrc');
videoSource.src = video.mp4?nocache=${Date.now()}; // Prevent caching
videoElement.load();
videoElement.currentTime = 100;
videoElement.play();

If you need to serve large videos, consider configuring a web server with range request support, such as Nginx or Apache. This ensures that the browser can handle seeking (currentTime) properly.

@TheMeanBeanMachine
Copy link

TheMeanBeanMachine commented Feb 19, 2025

#60007

Hey i took a stab at it ... is this looking like what you all wanted to do?

I just made a quick draft to confirm if this is the solution desired since there was a lot of discussion over this. Open to any and all feedback or requested changes.

@TheMeanBeanMachine
Copy link

can anyone take a look and tell me if im missing anything ? i think this is all we need to do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: service-worker Issues related to the @angular/service-worker package browser: safari freq2: medium help wanted An issue that is suitable for a community contributor (based on its complexity/scope). P4 A relatively minor issue that is not relevant to core functions state: confirmed type: bug/fix workaround2: non-obvious
Projects
None yet
Development

No branches or pull requests