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

ffpyplayer video: better video seek #5313

Merged
merged 4 commits into from
Oct 13, 2017
Merged

Conversation

germn
Copy link
Contributor

@germn germn commented Aug 1, 2017

Right now changing position of video with ffpyplayer is a bit buggy. Audio stream will seek correctly, while video stream will be slowed or speeded for few seconds before both streams became synchronized.

This PR fixes it making seeking much nicer. While ffpyplayer doc says this method is "inaccurate", on practice it works better given us very close position to wanted and synchronized streams momentarily.

I tested it on Windows and Android on different videos for a long time.

@matham
Copy link
Member

matham commented Aug 1, 2017

The problem with this is that some videos only have I-frames at regular intervals of e.g. 5 or 10 seconds. When accurate is False, it'll only seek to the closest I-frame which means that you may be as much as 5s or 10s away from your seek point. Try it with videos with large distances between their I-frames.

@germn
Copy link
Contributor Author

germn commented Aug 1, 2017

@matham, yes, I know it, but I think it's better to use inaccurate seek on practice. When we seek accurately we will wait same 5-10 second seeing slowed/speeded video stream. It's confusing, I'm not sure it worth it.

It's debatable issue, but I can notice that gstreamer also uses inaccurate seeking and we'll will at least get similar behavior for different providers.

Long story short, I like inaccurate seeking much more. Feel free to accept or reject this PR.

P.S. Sorry if I'm being annoying, couldn't you also take a look when you have time at this ffpyplayer-related PR? I use it for a long time and it works fine for me.

@matham
Copy link
Member

matham commented Oct 8, 2017

Ok, a better solution then is to add a def seek(self, percent, precise=True): and save the precise flag in the queue as a tuple along with percent and use that. That way, the current behavior remains default but you can overwrite it.

Remember to add docs for the meaning and implication of using the precise flag though.

@germn
Copy link
Contributor Author

germn commented Oct 13, 2017

precise flag added.

@@ -255,11 +255,13 @@ def _next_frame_run(self):
while not self._ffplayer_need_quit:
seek_happened = False
if seek_queue:
vals = seek_queue[:]
Copy link
Member

Choose a reason for hiding this comment

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

The reason for copy and then delete is because of threading issues. If you get the last value and then delete all elements, because this runs in a second thread, in between the get and delete a new seek may have been added from the other thread. So you end up losing that seek request. The way it was before (copy and then delete the number of copied elements) is thread safe so please change it back to that.

@matham matham merged commit c49fbd6 into kivy:master Oct 13, 2017
@germn germn deleted the ffpyplayer_better_seek branch October 14, 2017 06:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants