Skip to content
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

add range check for seeking, add seekToLive and seekToStart #20

Merged
merged 18 commits into from Dec 17, 2015
Merged

add range check for seeking, add seekToLive and seekToStart #20

merged 18 commits into from Dec 17, 2015

Conversation

funkenstrahlen
Copy link
Contributor

Currently seeking on a livestream crashes playback (nothing to hear anymore, no real code crash) if you seek to a time out of the seekable range. Therefore I added a range check to the seek method call.

I also added seekToLive() and seekToStart() as helpers. They are used by the default seek method if the time to seek to is out of range.

Currently seekToLive() seeks to the latest seekable posistion in the data stream minus 5 seconds of buffer time. The 5 seconds are quite randomly picked and should be changed to a the value of the buffer configuration which might be added in a future release.

@funkenstrahlen
Copy link
Contributor Author

Don't merge yet. Still has minor problems.

@funkenstrahlen
Copy link
Contributor Author

Working for me now pretty stable. Was a lot more complicated than expected. You better take a look at the final code. From my side this is free to merge.

player?.seekToTime(CMTimeMake(Int64(time), 1))
let time = CMTime(seconds: time, preferredTimescale: 1000000000)
let seekableRange = player?.currentItem?.seekableTimeRanges.last?.CMTimeRangeValue
let seekableStart = seekableRange!.start
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we avoid to force unwrap value here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok will fix that.

@delannoyk
Copy link
Owner

Hi @funkenstrahlen

Thanks for your PR, really appreciate it. I left some detailed feedback inline.

bufferTime = max(bufferTime, marginBuffer)
let seekableRange = player?.currentItem?.seekableTimeRanges.last?.CMTimeRangeValue
let latestPoint = max(seekableRange!.start, seekableRange!.end - bufferTime)
let earliesPoint = min(seekableRange!.end, seekableRange!.start + marginBuffer)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you do the same here for forced unwrap? User can call seekToSeekableXXX and get an unexpected crash there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh you are right. Will change that.

@delannoyk
Copy link
Owner

Can I ask for one last edit and then I'm ready to merge? 🎉

@funkenstrahlen
Copy link
Contributor Author

You can have as many edits as you like. I appreciate your feedback!

@funkenstrahlen
Copy link
Contributor Author

Fixed that issue now. Any other problems you see?

delannoyk added a commit that referenced this pull request Dec 17, 2015
add range check for seeking, add seekToLive and seekToStart
@delannoyk delannoyk merged commit afa271b into delannoyk:develop Dec 17, 2015
@delannoyk
Copy link
Owner

There are still some changes I would like to see, I'm making them right now :)

@delannoyk
Copy link
Owner

Updated a bit the implementation. It's on develop if you want to check them out. Let me know what you think.

@funkenstrahlen
Copy link
Contributor Author

I will take a look at it!

@funkenstrahlen
Copy link
Contributor Author

Works great. Are you going to release this as a new version before adding caching?

@delannoyk
Copy link
Owner

Yup, I'll release it later today.

@funkenstrahlen
Copy link
Contributor Author

Great! Thanks a lot 😃

@delannoyk
Copy link
Owner

Available in master. Tagged 0.4.0!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants