Send requested byte range from SW, with correct headers

[mossroy]

Fixes #531

[mossroy]

@Jaifroid : this is still a draft but it seems to solve the issue on my Chromium.
Could you test with the browser/environments where you reproduced the issue?

[Jaifroid]

That's good news! Great sleuthing. I'll do a test of this on Electron as soon as I can.

[mossroy]

It seems to me that it started working when I fixed the response HTTP status code.
So maybe only a part of this PR is really needed.
In particular, I don't know if it's wise to send only the requested bytes: as we in fact always read all the data from the ZIM files, it might make us read it many times, while we could have sent it completely to the browser once for all.
I'll test

[mossroy]

No, it was not only a matter of http status code.
I'm implementing what could be a compromise, where I honor only the beginning of the requested range: if the browsers asks for range x to y, I'll send the bytes x to the end of the stream (with HTTP code 206)
Hopefully it will avoid the browser from asking the following chunks (because it already has them)

[Jaifroid]

OK, interesting. At a very low level, our FileReaders (at least for compressed BLOBs in the archive) decompress data as a stream. When I implemented ZSTD decoding, a lot of the challenge was in understanding how the decoder extracts just enough of the data stream to decompress a few bytes, then loops back and asks for more bytes only if needed, stopping at the end of the requested BLOB. This means that we only decompress just enough of the data stream to get the BLOB we are looking for, even though a compressed cluster may contain several BLOBs.

But I expect video BLOBs are stored without ZSTD compression, like images.

[mossroy]

Apart from the debug logs, that I will have to remove, this PR is ready for test/review (but no rush)

[Jaifroid]

I've tested this in Electron, by incorporating this code into the Service Worker for KJSW (on a branch). I've only tested on Windows so far.

It's a great improvement on the original behaviour. 😀

Skipping forward particularly works very well. Occasionally, skipping backwards, the video will stop, and I have to press play again, but it mostly will resume from where I had skipped back to. Occasionally, it will show a reload icon and ask me to reload the video, after which it works fine again (but from the beginning). Is this your experience?

By clicking backwards and forwards too many times without waiting much between clicks, eventually the video crashes. I assume this is just me overwhelming the back-end.

In any case, I would definitely consider the issue to be fixed, and I wouldn't expect absolute ability to jump around in the stream, given that it is not cached in memory.

@mossroy Would you like me to test other frameworks and other OS? If so, I would do a nightly build based on the branch and test the packages.

[Jaifroid]

Just a thought: if we want truly random access to the video stream, would it be worth adding a simple video cache to the Service Worker (like the memory-based assetsCache we have?). Just a Map. There would have to be logic to clear it (to free up memory) once the SW receives a request for a new video OR a new article. The downside is increased memory use by the app, but it would be no more than we already use in jQuery mode (since making a BLOB URL must be the same kind of memory requirement).

[mossroy]

Thanks for the test!
On my Chromium browsers, it always works without issue, forward and backward. I saw no crash.
I sometimes have the video paused, but it's when my click is a bit above the progress bar, and clicking on the video makes it pause.

Which ZIM file are you using to test?
I've tested with the ones on which I could most easily reproduce the issue before: https://download.kiwix.org/zim/videos/voa_learning_english-word-of-the-day_2021-12.zim and ted_en_global_issues_2018-07.zim

[mossroy]

Just a thought: if we want truly random access to the video stream, would it be worth adding a simple video cache to the Service Worker (like the memory-based assetsCache we have?). Just a Map. There would have to be logic to clear it (to free up memory) once the SW receives a request for a new video OR a new article. The downside is increased memory use by the app, but it would be no more than we already use in jQuery mode (since making a BLOB URL must be the same kind of memory requirement).

It would probably speed up seeking in videos.
But, as you said, videos can be quite big, so keeping them in memory does not seem a good idea to me.
Seeking is probably not the main use-case: I prefer it to be safe, even if a bit slow (BTW I did not find it slow on one of my slowest PC)

[Jaifroid]

Which ZIM file are you using to test?

I tested with: ted_en_global_issues_2021-08.zim and ted_en_playlist-10-days-of-positive-thinking_2021-01.zim.

There could be a different issue with Electron apps, to do with the non-standard use of the file:// protocol. It would be a better test for me to try with NWJS, because this uses a standard extension and standard extension URLs.

I may also be causing the "stop" by the issue you point out (of clicking too far above the line).

I'll test on NWJS.

[Jaifroid]

Good news: it works very well in NWJS, and it's also working well in Electron 👍. The issue with the video stopping was indeed what you identified: because I hadn't changed the video to full screen, it was quite hard to hit the video timeline perfectly. When testing in full screen, I can be sure to click exactly on the line, and then the video doesn't stop, and I can jump around, forwards and backwards, in both frameworks.

There's one possible improvement. If you try "dragging" the video pip in the timeline backwards and forwards, seeking that way is very slow. On phones and tablets this is the main way people move video forward with their finger. Now these ZIMs are not well optimized for this use-case, because it's an old video.js that doesn't seem optimized for touch (video timeline is too narrow to touch properly). But you can sort of simulate it by dragging the pip with the mouse.

It seems clear that when we do this, the SW is (I imagine) processing multiple requests for closely adjacent bits of the file, and it has to cancel those requests and re-request potentially multiple times a second.

Would some kind of rate-limiting be possible? If not, or if it's too complicated, don't worry. Dragging does work, it's just much slower than simply clicking.

To see how it "should" work, try dragging in jQuery mode. Because the video is in memory, it drags forwards and backwards at an acceptable pace, even showing some frames along the way.

Improving dragging could be separate issue. You've already made a very big improvement here!

[Jaifroid]

Very nice PR!

[mossroy]

I've commited the changes you requested, and also removed the debug logs.

I also think that the dragging slowness can be considered a separate improvement.
To me, the right way to do that would be to have a backend that supports reading only a chunk of data. Our custom javascript can currently not, but libzim does: https://libzim.readthedocs.io/en/stable/api/classzim_1_1Item.html#_CPPv4NK3zim4Item7getDataE11offset_type9size_type
So, in theory, this should be possible with libzim wasm, simply by passing the offset/size parameters to getData(). It has to be tested, of course.

I just created #869 for that