Downloading videos using Background Fetch API #169

dero · 2021-11-19T12:33:18Z

Short Screencast

kino-background-fetch-api.mp4

Summary

This PR adds support for downloading single videos and streamed videos using Background Fetch API.

We're adding an offline version of MPEG DASH manifests to all streamed videos. This offline manifest only contains one selected video and audio representation (HD H.264 MP4), which allows us to only download this one representation for offline playback.

We've been doing this on-the-fly in the past, but with the introduction of Background Fetch API, it arguably make more sense to move towards using the more explicit separate offline manifests, because it would bring a larger amount of complexity to the codebase if we tried to manipulate DASH manifest data on the fly in the service worker.

The DOMParser object is not available in the worker context, meaning that parsing the manifest file would have to be done by an external library like xmldom. (Extra bundle weight, one more thing to maintain and rely on.)
We can't instantiate a video element or use its canPlayType method statically from the worker context. That means we can't test which of the representations in the manifest are actually playable on the current device and in the current browser. We would need to build a cache layer in IDB in order to figure that out ahead of time and cache it for later when SW is invoked with the downloaded files.

We can avoid both issues by uploading the offline version of each manifest upfront. Plus this approach allows us to simplify the code quite a bit.

The main downside, I think, is that users will not be able to select their preferred download resolution with static offline manifests in place. Note: This is not a regression, we never had a setting for download quality. But this change does not allow for such a setting should we decide to implement it in the future.

Edit: Thinking about this a bit more, maybe it would be possible to leverage the Media Capabilities API to detect support and gain playback performance estimates for individual resources listed in DASH manifests – the API should be available from worker threads.

We might decide to pick this back up later.

Questions

Should the "Download videos in the background" setting be enabled by default?
Do we create a new video post for "Background Fetch API"?

Fixes #25

beaufortfrancois

Here's my first pass.
Thanks @dero for working on it!

beaufortfrancois · 2021-11-22T14:08:38Z

src/js/classes/BackgroundFetch.js

+          this.id,
+          urls,
+          {
+            title: videoData.title,


videoData.title may end up being confusing. See video at http://localhost:5000/multiple-sources/ for instance.
Shall we have something like Downloading "${videoData.title}" video? (Source)

Updated in 7979e32.

beaufortfrancois · 2021-11-22T14:15:10Z

src/js/classes/BackgroundFetch.js

+
+      // eslint-disable-next-line compat/compat
+      navigator.serviceWorker.ready.then(async (swReg) => {
+        this.maybeAbort(swReg);


I guess you'd want to await there before or is this intentional?

Suggested change

this.maybeAbort(swReg);

await this.maybeAbort(swReg);

Added in 7979e32.

beaufortfrancois · 2021-11-22T14:28:31Z

src/js/sw/sw.js

@@ -217,3 +220,53 @@ const fetchHandler = async (event) => {
 self.addEventListener('install', precacheAssets);
 self.addEventListener('activate', clearOldCaches);
 self.addEventListener('fetch', fetchHandler);
+
+/** @type {MessagePort} */
+let messageChannelPort;


Some quick comment to explain where it comes from may be helpful to readers.

Added in 7979e32.

beaufortfrancois · 2021-11-22T14:38:40Z

src/js/sw/sw.js

+const bgFetchHandler = async (e) => {
+  /** @type {BackgroundFetchRegistration} */
+  const bgFetchRegistration = e.registration;
+  const bgFetch = new BackgroundFetch();


Nit: Shall we move this until we actually need videoId?

const bgFetchHandler = async (e) => { /** @type {BackgroundFetchRegistration} */ const bgFetchRegistration = e.registration; const records = await bgFetchRegistration.matchAll(); const urls = records.map((record) => record.request.url); if (urls.length === 0) return; const responsePromises = records.map((record) => record.responseReady); const responses = await Promise.all(responsePromises); const bgFetch = new BackgroundFetch(); bgFetch.fromRegistration(bgFetchRegistration); ...

@beaufortfrancois Good suggestion, thanks! Addressed in 7979e32.

beaufortfrancois · 2021-11-22T14:41:03Z

src/js/sw/sw.js

@@ -217,3 +220,53 @@ const fetchHandler = async (event) => {
 self.addEventListener('install', precacheAssets);
 self.addEventListener('activate', clearOldCaches);
 self.addEventListener('fetch', fetchHandler);
+


Shall we gate all the code added only if (!('BackgroundFetchManager' in self))?

@beaufortfrancois Thank you, addressed in 7979e32.

beaufortfrancois · 2021-12-09T14:01:53Z

LGTM. Thanks @dero!

Swansoffie02 · 2021-12-30T12:36:33Z

Short Screencast

kino-background-fetch-api.mp4

Summary

This PR adds support for downloading single videos and streamed videos using Background Fetch API.

We're adding an offline version of MPEG DASH manifests to all streamed videos. This offline manifest only contains one selected video and audio representation (HD H.264 MP4), which allows us to only download this one representation for offline playback.

We've been doing this on-the-fly in the past, but with the introduction of Background Fetch API, it arguably make more sense to move towards using the more explicit separate offline manifests, because it would bring a larger amount of complexity to the codebase if we tried to manipulate DASH manifest data on the fly in the service worker.

The DOMParser object is not available in the worker context, meaning that parsing the manifest file would have to be done by an external library like xmldom. (Extra bundle weight, one more thing to maintain and rely on.)

We can't instantiate a video element or use its canPlayType method statically from the worker context. That means we can't test which of the representations in the manifest are actually playable on the current device and in the current browser. We would need to build a cache layer in IDB in order to figure that out ahead of time and cache it for later when SW is invoked with the downloaded files.

We can avoid both issues by uploading the offline version of each manifest upfront. Plus this approach allows us to simplify the code quite a bit.

The main downside, I think, is that users will not be able to select their preferred download resolution with static offline manifests in place. Note: This is not a regression, we never had a setting for download quality. But this change does not allow for such a setting should we decide to implement it in the future.

Edit: Thinking about this a bit more, maybe it would be possible to leverage the Media Capabilities API to detect support and gain playback performance estimates for individual resources listed in DASH manifests – the API should be available from worker threads.

We might decide to pick this back up later.

Questions

Should the "Download videos in the background" setting be enabled by default?

Do we create a new video post for "Background Fetch API"?

Fixes #25

lkraav · 2021-12-30T13:26:37Z

Why the revert?

dero added 6 commits November 16, 2021 15:07

Background Fetch API initial implementation.

45c71d1

Add offline URLs rewrites for MPD manifests.

d563412

Remove the file transformers concept.

9d6d36d

Merge remote-tracking branch 'origin/develop' into add/background-fetch

cd13c85

Abstract getProgress away from the VideoDownloader class.

d8b2757

Background Fetch has to be controlled using browser UI.

5be1b50

dero requested a review from derekherman November 19, 2021 12:33

dero self-assigned this Nov 19, 2021

beaufortfrancois reviewed Nov 22, 2021

View reviewed changes

dero added 2 commits December 9, 2021 14:55

Address PR feedback related to Background Fetch API.

7979e32

Only display the BG Fetch API settings in supporting browsers.

fb53402

Merge branch 'develop' into add/background-fetch

098aea6

derekherman approved these changes Dec 14, 2021

View reviewed changes

derekherman merged commit ba4f9ce into develop Dec 14, 2021

derekherman deleted the add/background-fetch branch December 14, 2021 00:55

Swansoffie02 mentioned this pull request Dec 30, 2021

Revert "Downloading videos using Background Fetch API" #174

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Downloading videos using Background Fetch API #169

Downloading videos using Background Fetch API #169

dero commented Nov 19, 2021 •

edited

Loading

beaufortfrancois left a comment

beaufortfrancois Nov 22, 2021

dero Dec 9, 2021

beaufortfrancois Nov 22, 2021

dero Dec 9, 2021

beaufortfrancois Nov 22, 2021

dero Dec 9, 2021

beaufortfrancois Nov 22, 2021

dero Dec 9, 2021

beaufortfrancois Nov 22, 2021

dero Dec 9, 2021

beaufortfrancois commented Dec 9, 2021

Swansoffie02 commented Dec 30, 2021

Short Screencast

Summary

Questions

lkraav commented Dec 30, 2021

Downloading videos using Background Fetch API #169

Downloading videos using Background Fetch API #169

Conversation

dero commented Nov 19, 2021 • edited Loading

Short Screencast

Summary

Questions

beaufortfrancois left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

beaufortfrancois commented Dec 9, 2021

Swansoffie02 commented Dec 30, 2021

Short Screencast

Summary

Questions

lkraav commented Dec 30, 2021

dero commented Nov 19, 2021 •

edited

Loading