search

henry-rodrick / hlsjs-ipfs-loader

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

Do we need hls.config.ipfsHash at all? #23

Open georgyo opened 3 years ago

georgyo commented 3 years ago

With the changes that got merged in #20, there are no longer any useful references to the ipfs hash specified on the config. It would be much nicer if we could just specify a source that was /ipfs/QmfL9GReWbQbwgrQG4j3aFJaJb6UEyeDfuy8GRQcH5F5NS/manifest.m3u8 instead of specify both the hash and the source separately.

This would be a pretty big breaking change, but it would sure make the library easier to use. IE, if you wanted to allow people to specify their own IPFS path, you would currently need to split the parth first to figure out the parts to feed back into this library.

vaultec81 commented 3 years ago

This suggestions sounds good to me. Any change made must be backwards compatible or rolled out after a couple of versions. I think the original option should stay available, but the ability to use the full ipfs path if specified.

UnKnoWn-Consortium commented 3 years ago

Well the hash specified in config is still referenced to after #20. https://github.com/moshisushi/hlsjs-ipfs-loader/blob/e4ac6e21ca7e8567c06fff3551258ebbbc0ad2ed/src/index.js#L7 https://github.com/moshisushi/hlsjs-ipfs-loader/blob/e4ac6e21ca7e8567c06fff3551258ebbbc0ad2ed/src/index.js#L110

UnKnoWn-Consortium commented 3 years ago

I think it can be implemented without breaking much backward compatibility with cidPath, e.g. QmYjtig7VJQ6XsnUjqqJvj7QaMcCAwtrgNdahSiFofrE7o/path/to/file.

All we need is to make rootHash augment and the backslash in getFile function optional.