-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement ios bufferConfig #1353
base: master
Are you sure you want to change the base?
Implement ios bufferConfig #1353
Conversation
I gave this PR a try. The solution works great however I noticed that the onBuffer callback is still getting called with the final |
@dkoo761 Yeah, I noticed this as well, but I couldn't find a way to get past it. Do you have any suggestions on how we could get that callback to fire earlier? |
@FrikkieSnyman I tried wrapping the whole block where we handle the Also, it's not clear to me if the player does in fact keep buffering further after playback is ready. If it does, then maybe it's technically accurate to leave it up to the The confusing thing to me was that I was expecting the "onLoad" callback to have already completed whatever buffering was required in order for play to be ready since the docs for onLoad say: "Callback function that is called when the media is loaded and ready to play." For Android that seems to be the case and the callback pattern is what I would expect:
However, for iOS I'm seeing this pattern instead:
Since I'm using your bufferConfig code, somewhere between (2) and (4) above the MP3 is actually playable, but because of the strange callback timing it's not clear when exactly that is. For now, I'm just assuming playback is ready after the onLoad callback because I'm using bufferForPlaybackMs=2500 which takes < 100 milliseconds to buffer such a small amount of content and that's good enough accuracy for my use case. |
Is there any progress? Really need this feature. |
Any Updates? |
Haven't had a chance yet to look at it again, sorry! But please feel free to play around with it and seeing what you can find! |
A couple months ago I created a repo based on the 4.2.0 release (since I needed MP3 support which broke in a later release) and merged this PR into it. I've been using it since then and it's been working fine. As long as you're aware of the discussion above and implement your code accordingly, you should be OK. Feel free to clone and use my repo if you like: https://github.com/dkoo761/react-native-video-4.2.0. |
Nice one @dkoo761 , thanks! |
@FrikkieSnyman Would it be possible to merge this branch? With this, we're able to cut down the playback time from 8s to 4s on our app |
@SuhairZain I would love to, but I don't have that kind of power! In the meanwhile, I'd recommend forking the repo and merging this branch then using the forked repo in your app |
Yeah. I was thinking of doing the same. But I also think that your ios-buffer-config branch is fairly upto date with upstream. I was thinking I would pull it and use it until it's merged. |
@cobarx is it possible to merge this? We really need this feature |
Any reason for not using the built-in configuration for this?https://developer.apple.com/documentation/avfoundation/avplayeritem/1643630-preferredforwardbufferduration |
@jenshandersson if I remember correctly, it was because the built-in config still waited for But if you have a better/simpler implementation, I'd love to see it! 😄 |
@CHaNGeTe can you check this one? It would be awesome to see this PR merged in next release |
It would be great to see this PR merged in next release |
I've also forked the repo and implemented the same changes of the PR on top of version 4.4.1 https://github.com/lucasrocali/react-native-video/tree/4.4.1/ios-buffer-config |
This is really helping with loading time on iOS |
Hi guys, do you have plans for this feature? Please let us know when you can, thanks! |
I see merge conflicts here; does that mean the master implemented this in a different way and this PR should be closed? |
Facing the same issue it takes up to 15 seconds to start a acverage 5 mb video from s3. |
@cobarx maybe we can get your input on this? Why can this not be merged? It's been hanging around for more than 2 years without any meaningful feedback on why it's not being merged. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. If you are having a similar problem, please open a new issue and reference this one instead of commenting on a stale or closed issue. |
God I wish this was fully implemented in |
Bump. I need this so badly. I would even move from Expo-AV and donate some cash if iOS buffer config will be working as intended. |
This PR implements a bufferConfig for iOS, similar to what already exists for
android-exoplayer
. In fact, it uses the samebufferConfig
property. The only difference currently, is that this implementation does not support theminBufferMs
andmaxBufferMs
settings. Further, the default behaviour when bufferConfig is not specified, is to fallback to AVPlayer'splaybackLikelytoKeepUp
buffering.An effective way to test this, is to set the source of the video to a remote file, and then limit network connectivity with the Network Link Conditioner to 3G.
paused
should be set tofalse
, so that playback can start automatically. With no buffer config specified, the default behaviour would be to download roughly half of the video before playback will start. With abufferConfig
specified, playback should start after the specified duration has been buffered.