[web] Add loadable video component
ClosedPublic
Actions

Authored by bartek on May 23 2023, 7:29 AM.

Details

Reviewers

ashoat
atul
ginsu
inka

Commits

rCOMM983f258fb7d9: [web] Add loadable video component

Summary

Extracted the video logic from D7902 as this will also be needed for modals.
This component is intended to display thumbnail while video is loading. And while thumbnail is decrypted/loaded, the thumbhash is displayed. The component also supports encrypted thumbnails.

Test Plan

Tested together with D7902

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

bartek created this revision.May 23 2023, 7:29 AM

bartek held this revision as a draft.

Herald added a subscriber: tomek. · View Herald TranscriptMay 23 2023, 7:29 AM

bartek added a child revision: D7902: [web] Display thumbhash in chat.May 23 2023, 7:37 AM

Harbormaster completed remote builds in B19680: Diff 26888.May 23 2023, 7:49 AM

Memoize component

bartek published this revision for review.May 23 2023, 8:09 AM

bartek added inline comments.

web/media/loadable-video.react.js
19–29 ↗	(On Diff #26901)	Explanation in https://phab.comm.dev/D7902?id=26730#inline-51871

Harbormaster completed remote builds in B19693: Diff 26901.May 23 2023, 8:26 AM

ashoat requested changes to this revision.May 23 2023, 1:08 PM

ashoat added inline comments.

web/media/loadable-video.react.js

29 ↗

(On Diff #26901)

I'd like to show the thumbnailURI as soon as it's loaded, rather than waiting for the whole video to be loaded. More context in my comment on D7902, but it should be a simple change here I think...

React.useEffect(() => {
  (async () => {
    // video thumbnail is used as a poster image when the video is loaded
    // preload it so the browser can immediately load it from cache
    if (!thumbnailURI) {
      return;
    }
    await preloadImage(thumbnailURI);
    setThumbnailLoaded(true);
  })();
}, [thumbnailURI]);

const poster = isThumbnailLoaded ? thumbnailURI : thumbHashDataURL;

Do you think that would work?

This revision now requires changes to proceed.May 23 2023, 1:08 PM

bartek mentioned this in D7902: [web] Display thumbhash in chat.May 23 2023, 1:43 PM

bartek removed a parent revision: D7901: [web] Add hook to generate placeholder image.May 23 2023, 11:57 PM

bartek edited the summary of this revision. (Show Details)

bartek mentioned this in D7900: [web] Add thumbhash to encrypted media, fix spinner.May 24 2023, 12:06 AM

Changed logic to display actual video thumbnail as soon as possible
Added support for encrypted video thumbnails
Added a forwardRef to the video element which will be needed for encrypted media

The whole approach and diff tree changes discussed in D7902

Harbormaster completed remote builds in B19707: Diff 26920.May 24 2023, 12:44 AM

ashoat accepted this revision.May 24 2023, 9:00 AM

ashoat added inline comments.

web/media/loadable-video.react.js
20–22 ↗	(On Diff #26920)	Found myself wondering in what scenarios `uri` and `thumbHashDataURL` will not be set. Answered them by reading later parts of the stack: `uri` is not set for encrypted videos until the decryption concludes and a file URI is available for the actual video (happens in D7900) `thumbHashDataURL` is not set for newly-uploaded videos until the Thumbhash generation (handled by D7901) completes and the value is available (rendered in D7947)