henry-rodrick / hlsjs-ipfs-loader

A js-ipfs loader for the hls.js JavaScript HLS client
MIT License
60 stars 31 forks source link

Update for latest IPFS, bug fix. #10

Closed vaultec81 closed 4 years ago

alanshaw commented 4 years ago

This switches to using the buffered version of ipfs.cat which means you'll have to wait for the whole video to download before it'll play. The fix in https://github.com/moshisushi/hlsjs-ipfs-loader/pull/9 should be enough.

henry-rodrick commented 4 years ago

Closing this as invalid

vaultec81 commented 4 years ago

@alanshaw It would not make any difference. https://github.com/moshisushi/hlsjs-ipfs-loader/blob/81847d010968995df20f8db97fe41a4894a7b1c9/src/index.js#L98-L100 Callback is only returned when the stream has ended. HLS is chunked into multiple .ts files, only one is needed for about 5-10 seconds of playback depending on the chunk segment length.

alanshaw commented 4 years ago

Ah yes you're correct my apologies.

henry-rodrick commented 4 years ago

@alanshaw @vaultec81 you guys reckon the buffered version is actually better then, as in less code with no loss in performance? I guess this PR overlaps with #9 so can either merge, or if you want to open a new one.

alanshaw commented 4 years ago

Both versions are buffering. The current version using ipfs.catReadableStream is just doing what ipfs.cat does under the hood.

It would be better to stream it if possible.

henry-rodrick commented 4 years ago

@alanshaw But you can't inject a partial ts fragment into the HLS stream, afaik, so what additional benefit would that have? The low latency play and seek functionality is something you get from HLS by design, as streams are always pre-fragmented and only need a handful of fragments to get going (this is what @vaultec81 mentioned above). Sorry if I'm missing the point!

vaultec81 commented 4 years ago

@moshisushi @alanshaw The issue I was experiencing was the stream was never closed. After some repeated testing I could not figure out why the stream wasn't ending. Which is why i put in ipfs.cat instead. This seemed to solve my issue in test case. (This was before @alanshaw's PR was merged)

correction: They are about the same performance/speed wise. ipfs.cat is a little bit faster (side by side comparison). @alanshaw you mentioned it does the same thing under the hood. But does ipfs.cat grab all the file leafs sequentially or all at once?

henry-rodrick commented 4 years ago

@vaultec81 Thanks for this. This PR has been included in release 0.1.5.

https://www.npmjs.com/package/hlsjs-ipfs-loader/v/0.1.5

Cheers!