ios Caching Support

[n1ru4l]

I want some discussion about this (related to #99). I am currently working on an iOS app so I have no needs for android right now. Edit: Android integration will follow soon!

Would love some feedback from guys that are more into objective c. This is almost the first objective c I have ever written 😅.

I added the libraries that I am using SPTPersistentCache and DVAssetLoaderDelegate as git submodules as I have seen it react-native-fast-image cocoapod dependencies.
I am using a the setup described here: http://facebook.github.io/react-native/docs/integration-with-existing-apps.html.

if you would setup react-native with this library your Podfile should look similar to this:

# Uncomment the next line to define a global platform for your project
platform :ios, '10.0'

target 'VideoCaching' do
  rn_path = '../node_modules/react-native'

  pod 'yoga', path: "#{rn_path}/ReactCommon/yoga/yoga.podspec"
  pod 'DoubleConversion', :podspec => "#{rn_path}/third-party-podspecs/DoubleConversion.podspec"
  pod 'Folly', :podspec => "#{rn_path}/third-party-podspecs/Folly.podspec"
  pod 'glog', :podspec => "#{rn_path}/third-party-podspecs/GLog.podspec"
  pod 'React', path: rn_path, subspecs: [
    'Core',
    'CxxBridge',
    'RCTAnimation',
    'RCTActionSheet',
    'RCTImage',
    'RCTLinkingIOS',
    'RCTNetwork',
    'RCTSettings',
    'RCTText',
    'RCTVibration',
    'RCTWebSocket',
    'RCTPushNotification',
    'RCTCameraRoll',
    'RCTSettings',
    'RCTBlob',
    'RCTGeolocation',
    'DevSupport'
  ]

  pod 'react-native-video/VideoCaching', :path => '../node_modules/react-native-video/react-native-video.podspec'
end

Functionality

Aggressive Caching based on the uri (similar to react-native-fast-image).
The storage is backed by SPTPersistentCache. (LRU Cache). Might be possible to make this someway more customisable.

Since it is not possible to create an AVAsset from NSData which SPTPersistentCache returns. I also added another layer which saves the file to the devices temporary folder (Google says it is cleared after 3 days). So after three days it will have to be refetched from the persistent storage (SPTPersistentCache). This results in additional storage size.

Testing

Method 1:

Check out the example app located in examples/video-caching

UPDATE: There is now a README.md file which describes how you can test the implementation

Commands for initializing the project:

yarn
cd ios
pod install
cd ..

After you initialized the project, you can open the ios/VideoCaching.xcworkspace project in xcode (and run it your device/simulator).

Method 2:

Use my private fork @n1ru4l/react-native-video@3.1.0-alpha.4 in your project. (The project will also require a proper cocoapods setup). You can copy it from the example app (examples/video-caching) and replace the pod with the following line:

pod 'react-native-video/VideoCaching', :path => '../node_modules/@n1ru4l/react-native-video/react-native-video.podspec'

TODO

• Clean up debug logging
• Make feature optional (react-native-camera did this with additional frameworks https://github.com/react-native-community/react-native-camera/pull/1341/files)

[roycclu]

Thanks man.
This is a very welcome feature.
Especially given the large costs of loading video from network everytime.
Has any core contributors looked at this?

[n1ru4l]

roycclu wrote:

@n1ru4l I see you added SPTPersistentCache in your podspec as dependency. Any idea why the header isn't found?

Have you followed all steps for configuring react-native with CocoaPods and opened the .xcworkspace project?

I updated the Podfile above to match the latest react-native version 0.56.0.

FYI: I am using my fork that is published to npm @n1ru4l/react-native-video), without any issues.

roycclu wrote

Has any core contributors looked at this?

Nobody did comment this Pull Request yet, so I doubt that anybody checked it out.

I will rebase on master soon (and the android implementation should also follow soon).

[n1ru4l]

@BunHouth wrote (#99 (comment)):

I got error as image bellow.

There was indeed an issue. I merged in master and fixed these issues, I also updated the example app (examples/video-caching).

My new private fork version is also published as @n1ru4l/react-native-video@3.1.0-alpha.2

I would love if you could give me some feedback whether it now works for you or not.

[cobarx]

@n1ru4l Do you think it would be valuable to make the cache size a configurable prop?

I have not gotten a chance to test this yet btw. I'm hoping the community can help out with that, as doing extensive testing will eat into the time I have to do new feature dev myself.

With the Cocoapods, I don't want to get into a situation where everyone who installs react-native-video needs to setup Cocoapods, installing needs to be as simple as possible. I'm happy to help out with making it an optional dependency in whatever way I can, let me know if you need assistance.

A number of people have requested the ability to save videos. That's beyond the scope of this PR, but if you happened to want to implement it down the road, I'm sure you'd have a lot of happy fans.

[n1ru4l]

@cobarx done. might be best if someone that uses the audio and subtitle features checks if something got destroyed in the process.

[cobarx]

I set it up via Podfile like you put in the description, and I'm getting an error:
Video has no PropType for native prop RCTVideo.cache of native type BOOL

Also, if I create a new project and use react-native link, the paths are coming in with a Vendor directory (see screenshot). So when it goes to compile it can't find the iOS code. Can you take a look into that.

[n1ru4l]

@cobarx You have to remove the cache property from the <Video/> component.

I also removed the Vendor code and adjusted the file paths in b83f3a5.

[cobarx]

@n1ru4l I'm pretty sure the cache error is coming from being defined in RCTVideoManager.m.

I'm trying to test this on the Simulator and not having much luck, do you know if it will work there? If not, we should document that. I keep getting -11800 or -11828 errors if I turn WiFi off on my laptop, even with extremely short videos.

[n1ru4l]

@cobarx I forgot to remove the cache property, however for me it was not causing any errors 😕

Both examples basic and video-caching are fully functional for me.

Also the caching works fine on the simulator as well as on real devices. Can you please provide a repository that shows these errors? I would like to investigate more on that!

[cobarx]

Ok, I went back and tested with the video you're using in the example and it works great online and offline. It appears that certain videos can cause problems.

URLs with query strings at the end, e.g. ?size=large
The video will load fine while online, but if I switch to offline it throws a -11828 error. Once I go online, it still throws the error. If I get rid of the query string, everything works.

Example:
http://techslides.com/demos/sample-videos/small.mp4?variable=value

HLS Playlists
Won't load at all, throws a -11800 error. I made sure to turn off all the audio & text track selection code.

Examples:
https://bitdash-a.akamaihd.net/content/sintel/hls/playlist.m3u8
http://devimages.apple.com/iphone/samples/bipbop/bipbopall.m3u8

[cobarx]

I can't test the HLS text track selection since that's not working yet, but I did test with a sideloaded track. The video loads fine without textTracks included but doesn't start loading if I include textTracks. If you need help with this, lmk.

Here's an example:

<Video source={{ uri: 'http://hamptonmaxwell.com:8080/sintel.mp4' }}
  textTracks={[{
    title: 'English VTT',
    uri: 'https://bitdash-a.akamaihd.net/content/sintel/subtitles/subtitles_en.vtt',
    language: 'en',
    type: TextTrackType.VTT
  }]}
  style={{ width: 300, height: 200 }} />

Edit: forgot to convert subtitle variable to a URL

[n1ru4l]

@cobarx wrote:

URLs with query strings at the end, e.g. ?size=large
The video will load fine while online, but if I switch to offline it throws a -11828 error. Once I go online, it still throws the error. If I get rid of the query string, everything works.

We should also handle URLs without file extension. Best way would be to parse the Content-Type header and save the file with the corresponding file extension.
Any further concerns before I start working on this?

Edit: This requires work on a dependency (I already created a pull request vdugnist/DVAssetLoaderDelegate#5)

@cobarx wrote:

HLS Playlists
Won't load at all, throws a -11800 error. I made sure to turn off all the audio & text track selection code.

Are we fine with just skipping those files from being cached?

[cobarx]

@n1ru4l wrote:

We should also handle URLs without file extension. Best way would be to parse the Content-Type header and save the file with the corresponding file extension.
Any further concerns before I start working on this?

Sounds like a good approach. I'd say go for it, were you thinking there was anything else? My only other wishlist item would be to get the caching prop implemented to give some flexibility on what gets cached, however that could go in a future PR.

@n1ru4l wrote:

Are we fine with just skipping those files from being cached?

This is a huge PR, so I'd love to get it merged asap and avoid further merge conflicts, maintenance, etc. I think that's fine for a first pass.

HLS support would be great to have. HLS is the most common way to do professional video delivery on iOS, where you need adaptive bitrates, text tracks, and so on. Adding support for it would be pretty significant for the big high-end apps.

[roycclu]

Could we include a "Preload" function?
Similar to the react-native-fast-image preload.

@cobarx thanks for merging this!

[cihadturhan]

I'm way more interested in this PR. I have a real-time trivia app which streams audio files and will have up to hundred of thousands of users. So this feature is quite essential to me otherwise I can't have a server to stream so many people at the same time.

[n1ru4l]

This pull request is currently blocked by vdugnist/DVAssetLoaderDelegate#5

An alternative way would be to skip all urls that include no file extension from being cached until we got access to the content-type header.

[cobarx]

I'd say skip them and come back to it later. We want to get this into dev hands and can continue to add improvements over time. As long it's documented what's supported and what's not, we're good.

…

On Sat, Aug 4, 2018, 3:14 AM Laurin Quast ***@***.***> wrote: This pull request is currently blocked by vdugnist/DVAssetLoaderDelegate#5 <vdugnist/DVAssetLoaderDelegate#5> An alternative way would be to skip all urls that include no file extension from being cached until we got access to the content-type header. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#955 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AD4AF0TCJX04sIln_a6RL6Y9QAB4BzU9ks5uNXQOgaJpZM4SZNS0> .

[n1ru4l]

@cobarx I fixed all complaints, except the textTracks issue. Could need some help with that.

[cobarx]

I'd like to merge this into a develop branch and fix the textTracks issues there so we can work on it together. Can you resolve the merge conflicts.

For the file extension / query string stuff, what I mean is the caching.md doc should cover everything a developer needs to know about using caching in their app. If someone has a question, they can refer back to that doc and see what's supported and what's not. It's pretty easy to miss the debug messages if you don't know to look for them.

[n1ru4l]

@cobarx done 😊

[cobarx]

Ok, all set! I'll start working on the text track stuff shortly.

[AlexandruVoica]

Can we get a status update on the caching feature, please? 😅 I am especially interested in the Android implementation.

[n1ru4l]

@AlexandruVoica Sure, feel free to implement it and send a pull request!