youtube icon indicating copy to clipboard operation
youtube copied to clipboard

--all-subtitles is slow

Open rgaudin opened this issue 5 years ago • 6 comments

When building ZIM with --all-subtitles, all videos gets more than a hundred subtitles. Subtitles are loaded asynchronously on page load but the video and controls only starts/become active when all of them have been called which can takes seconds.

rgaudin avatar Sep 16 '19 17:09 rgaudin

@rgaudin This sounds like an enhancement for video.js?! Or can we do something about that?

kelson42 avatar Sep 17 '19 04:09 kelson42

Maybe we could be smarter and offer the ability to select some subtitles to include instead of this enormous list which is not very usable anyway.

rgaudin avatar Sep 17 '19 08:09 rgaudin

This issue has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

stale[bot] avatar Nov 27 '19 04:11 stale[bot]

@rgaudin I think that we should actually have some way to select subtitles. Maybe we introduce another argument which would take in a list of language codes. I've also opened a similar issue on ted and I think we should harmonize this behaviour on scrapers dealing with videos.

satyamtg avatar May 08 '20 10:05 satyamtg

Agrees.

rgaudin avatar May 08 '20 11:05 rgaudin

This issue has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

stale[bot] avatar Jul 08 '20 02:07 stale[bot]

I can say that doing performance check on video loading on Android has shown that this pb can significantly slow down video playback (more than one second). To me it sounds like a pretty serious fix to implement.

kelson42 avatar Jul 23 '24 14:07 kelson42

Do you have a sample ZIM/recipe to consider for the tests?

benoit74 avatar Jul 23 '24 14:07 benoit74

Do you have a sample ZIM/recipe to consider for the tests?

No, but Mohit knows as he was the tester. It might be @rgaudin videi test zim.

kelson42 avatar Jul 23 '24 14:07 kelson42

There seems to be a clear architecture problem around this at video.js level. Solution does not scale properly/without serious perf impact. This is really unrelated to ZIM storage i'm really in favour of attempting to implement a fix at video.js level. Most obvious idea would be to load subtitle file only when needed/chose. First thing first: opening issue upstream (at video js repo).

kelson42 avatar Jul 23 '24 14:07 kelson42

If the issue is really that we pass too many subtitles, then I don't agree about opening a video.js issue ; video.js is simply presenting the subtitles we configure. Even if there was no perf issue, presenting 10s of subtitles languages in a dropdown is never going to be a good idea from a UX perspective. So I'm glad there is a perf issue in video.JS because it will force us to revisit the UI/UX.

I think this problem should be handled in the scraper with proper UI to select which subtitles the user is interested in, and JS code to configure video.JS with only these subtitles. It was probably difficult to anticipate and even more difficult to code when only plain JS was used. Now that we have a powerful framework like Vue.JS almost in place and a real problem, it seems pretty straigthforward to implement. At least a proposition in this direction has been made in #278

benoit74 avatar Jul 23 '24 14:07 benoit74

@benoit74 OK, but considering your proposal runs at runtime, it will end up like an issue for video.js probably.

kelson42 avatar Jul 23 '24 14:07 kelson42

I don't get why it would end up like an issue for video.js, sorry, could you elaborate? if the problem is that we pass too many subtitles like it has been suggested so far in this issue, and if we adapt the UI to never pass too many subtitles, then there is no more video.js issue to handle.

benoit74 avatar Jul 23 '24 15:07 benoit74

Problem to the issue that subtitles are too slow to load when there are many seems pretty straightforward: tell video.js to not preload tracks with a data-setup='{"html5": {"preloadTextTracks": false}}' attribute on <video> tag. Note: this is supposed to work only with the html5 tech ; to be tested a bit further with ogv tech (but then the tracks are handled by whom? the native browser player or ogv.js?)

Whole code used to test this:

<!DOCTYPE html>

<head>
    <link href="https://vjs.zencdn.net/8.16.1/video-js.css" rel="stylesheet" />
</head>

<body>
    <video class="video-js" controls preload="metadata" width="640" height="264" data-setup='{"html5": {"preloadTextTracks": false}}'>
        <source src="//vjs.zencdn.net/v/oceans.mp4" type="video/mp4">
        <source src="//vjs.zencdn.net/v/oceans.webm" type="video/webm">
        <track kind="captions" src="./captions_en.vtt" srclang="en" label="English">
        <track kind="captions" src="./captions_fr.vtt" srclang="fr" label="French">
        <track kind="captions" src="./captions_de.vtt" srclang="de" label="German">
        <track kind="captions" src="./captions_es.vtt" srclang="es" label="Spanish">
    </video>


    <script src="https://vjs.zencdn.net/8.16.1/video.min.js"></script>
</body>

I suggest that we never preload subtitles in Youtube scraper, I do not see any benefit of doing this.

benoit74 avatar Jul 29 '24 20:07 benoit74

@benoit74 Great, should be implemented and released quickly. Same for TED. Proper testing on all readers is mandatory here.

kelson42 avatar Jul 30 '24 07:07 kelson42

Adding the following line from tech.js in the video.js library, to the videojs-ogvjs.js plugin in zimui let's us disable preloading subtitles in ogv.js also.

this.preloadTextTracks = options.preloadTextTracks !== false;

dan-niles avatar Aug 02 '24 04:08 dan-niles

This is even better then, thank you!

benoit74 avatar Aug 02 '24 05:08 benoit74