Page MenuHomePhabricator

VideoJS needs to be lazy-loaded on click before we can release it by default
Closed, ResolvedPublic0 Estimated Story Points

Description

mw.inspect size report

Desktop

namesizesizeInBytes
0"ext.tmh.video-js""818.2 KiB"8378601
1"oojs-ui-core""165.9 KiB"1698322
2"jquery""147.0 KiB"150554

Mobile

namesizesizeInBytes
0"ext.tmh.video-js""818.2 KiB"8378601
1"jquery""147.0 KiB"150554

Oh dear.

Event Timeline

so initially we chose this route, to make sure we had full HTML5 fallback.. The technology of lazy loading is somewhat incompatible with html5 fallback content as the controls are always presented.. you can add lots of logic on top to hide the native player and everything, but what if someone already started native html5 playback before your 'lazy load' logic start overlaying it ?

It also causes problems for parsoid serialization, having to know multiple forms of dom representation for the same content and additional overhead.
We should not forget that the old module was once lazy loaded, because RL modules were present on ALL pages, whether they had video or not and had to look at the DOM of a page to figure that out..

I think we should however also look at the size of video.js RL module. We are currently including the full dist version, incl webvtt parser and presentation fallback for platforms that do not support webvtt, Dash and HLS handling, Cea608 handling, mux.js, several components we don't use etc etc..

by handcrafting our own bundle using webpack and packagefiles, we could probably significantly reduce the file initial payload size. I set off doing this at the hackathon, but then got horribly stuck inside trying to lazy load ogvjs into that and got so frustrated that I think i lost the patch/branch i was working on.

Found that experiment back: https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/TimedMediaHandler/+/525145/

Not fun. We have 4 ecosystems that need to communicate (videojs, videojs plugins, mediawiki and ogvjs ), using respectively: cjs+rollup, <whatever the plugin author wanted to use>, package modules+webpack, and umd+webpack

and then we need lazyloading working on top of that...

We can probably reduce the size for now by using the video.js core distribution (without the http-streaming component which we don't use yet) and using more aggressive minification (rather than using our own minifier, which doesn't shorted variable names and such).

As for lazy loading, worst case we can use some kind of click handler I think to display our pretty icon and catch events and then load up the rest of bits, even with a native <video> element showing its poster image. Will experiment with this later if you don't get to it first. :)

Change 529819 had a related patch set uploaded (by Brion VIBBER; owner: Brion VIBBER):
[mediawiki/extensions/TimedMediaHandler@master] WIP Reducing size of video.js payload

https://gerrit.wikimedia.org/r/529819

Patch switches to the core distribution (no http-streaming component) and the more aggressively pre-minified version. Not quite complete, as this means we can't patch in the text track lazy-loading on top as easily.

Should either get that pushed upstream or make a work fork we build from, or both.

Quick fly-by note from talking about this with @Ladsgroup at the hackathon....

... it looks to me like the old Kaltura-based player is not lazy-loaded either. As such, if we can assert that the full videojs payload is smaller than the full Kaltura + jqueryUI payload, then this does not need to block enabling of TMH-videojs in production by default.

I noticed this when inspecting the performance of articles like https://en.wikipedia.org/wiki/Sweden which load all of jqueryUI, apparently due to the anthem audio-clip queuing the TMH-kaltura player unconditionally (not lazy-loaded).

old Kaltura player is not lazy loaded if you use audio-only or videos over $wgMinimumVideoPlayerSize

For a video that triggers player setup but hasn't actually been played, in Chrome/Firefox where no ogv.js shim is needed, they seem to weigh in (as reported from mw.inspect, so uncompressed) at around 293 KiB for MwEmbed+jquery.ui and 288 KiB for video.js with the core.min.js variant.

Note MwEmbed also has a 13 KiB sprite PNG for icons (none of them hi-DPI), while video.js uses a scalable icon font embedded in the CSS for the widgets.

We could possibly trim it down further by lazy-loading the subtitle rendering component, but that's less of a big deal.

We still have the issue that use of very small video embeds currently doesn't load the player until popup while this will pre-load the player components, so total usage may go up (but any use of an audio player already engages it, so eh)

Yeah, I think on the whole, I'd be okay with video.js going ahead as-is (from a perf perspective). With lazy-loading being something we'll get to later (as soon as possible, but not blocking).

Great, I'll tidy up this patch in a bit (once I get the subtitle lazy-loading patched back in) and we'll worry about lazy-loading later.

Does Video.js release imply enablement on MobileFrontend? If so, we may want to require lazy-loading on click after all. Or keep it desktop-only at first.

When I last looked into it a couple months ago, I was surprised to see jquery-ui still loaded on e.g. the perf-monitored "Sweden" article. Which was due to TMH loading it unconditionally on page load for the player in the infobox. The payload would be quite a sizeable regression on a lot of popular pages. on mobile. In particular because it also affects audio tracks which lots of articles have e.g. for their pronunciation or national anthem.

At the time, the size of the TMH payload almost entirely represents the difference in JS payload size between desktop and mobile for such articles.

/cc @Jdlrobson

Does Video.js release imply enablement on MobileFrontend? If so, we may want to require lazy-loading on click after all. Or keep it desktop-only at first.

Yes, it would be enabled for both desktop and mobile targets at present; there's no differential behavior between the two targets for video.js mode.

When I last looked into it a couple months ago, I was surprised to see jquery-ui still loaded on e.g. the perf-monitored "Sweden" article. Which was due to TMH loading it unconditionally on page load for the player in the infobox. The payload would be quite a sizeable regression on a lot of popular pages. on mobile. In particular because it also affects audio tracks which lots of articles have e.g. for their pronunciation or national anthem.

Audio is particularly tricky because it shows a player by default and does not provide any opportunity for lazy-loading at present. If we want lazy-loading for audio players, we have to come up with a UI for it -- either a temporary placeholder UI that's replaced by the video.js, or ditch video.js in favor of custom control code that is smaller and can lazy-load things like subtitle management and the ogv.js shim only when actively needed.

That's not impossible, but doing a custom control UI is new work and requires design and implementation, and as a reminder there is no team dedicated to audio/video work. I would need to borrow some designer time or else make up designs myself. ;)

Do we want to reuse wgMinimumVideoPlayerSize with this new lazy load setup ?
https://gerrit.wikimedia.org/r/550926

Change 550926 had a related patch set uploaded (by Jforrester; owner: Brion VIBBER):
[mediawiki/extensions/TimedMediaHandler@master] Lazy-load JS payload & use OOUI dialog in video.js playback mode

https://gerrit.wikimedia.org/r/550926

Change 550926 merged by jenkins-bot:
[mediawiki/extensions/TimedMediaHandler@master] Lazy-load JS payload & use OOUI dialog in video.js playback mode

https://gerrit.wikimedia.org/r/550926

@TheDJ let's try using it without wgMinimumVideoPlayerSize and drop it back in if we decide the always-transformation is confusing.

Ok, we've got the dialog merged which lets us lazy-load. Awesome! Tomorrow I'll fix up the other patch to use the latest video.js release -- the pre-minified sources compress much better than our minified version, and we no longer need to patch the JS source so it's easy to use it. I'll need to make one small change to activate the option for lazy-loading subtitle tracks.

I think the audio player will be a problem. Native players are pretty big and we often have very restrictive spacing. Don’t know a good design either though..

I think the audio player will be a problem. Native players are pretty big and we often have very restrictive spacing. Don’t know a good design either though..

Agreed, in some browsers they're *huge* and it's really inconsistent for styling. I also really want to get captions back for them...

What I'm currently thinking is some kind of nice compact "play" icon that pops up a smaller version of the video dialog. This could be sized to include space for captions if captions are available, and we could run them in the dialog instead of in a separate div.

However I'm not sure if I should have a square-ish stubby play button, or something that is wider similar to the actual transformed player to avoid disrupting style choices made around the old player... Any thoughts/preference?

Change 553442 had a related patch set uploaded (by Brion VIBBER; owner: Brion VIBBER):
[mediawiki/extensions/TimedMediaHandler@master] Transform audio players in video.js mode again

https://gerrit.wikimedia.org/r/553442

Change 529819 merged by jenkins-bot:
[mediawiki/extensions/TimedMediaHandler@master] Reducing size of video.js payload

https://gerrit.wikimedia.org/r/529819

Haven't been able to test audio mode yet. Note for the future however, is that I believe right now the embed/iframe mode is broken due to the lazy load changes.

Acknowledged, ?embedplayer=yes mode is indeed broken with those changes. ;_; I'll fix that shortly.

(Well, not exactly broken, but ... kind of weird. ;) It forces the popup to run inside the iframe, which adds a title bar even at small sizes.)

Latest rev of https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/TimedMediaHandler/+/553442 includes a quick-fix for that: an inline transform property is added which adds the playsinline attribute and a mw-tmh-inline class which has the on-load transforms set up video.js for that player instead of turning it into a popup player trigger. This is then forced on for the embedded-iframe view.

(This is not yet accessible from wikitext, but might be something we want later -- for sparing use only.)

Change 553442 merged by jenkins-bot:
[mediawiki/extensions/TimedMediaHandler@master] Transform audio players in video.js mode again

https://gerrit.wikimedia.org/r/553442