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

Fix progress indicator in offline video processing #91

Merged
merged 5 commits into from
Mar 5, 2024

Conversation

nvbln
Copy link
Contributor

@nvbln nvbln commented Aug 9, 2023

Currently the progress indicator immediately jumps to 100% (see issue #30).

There seem to be multiple methods in the imageio library to get the number of frames:

  • len(reader)
  • reader.get_length()
  • reader.frame_count()

len(reader) and reader.get_length() should technically be the same but give different outputs. reader.get_length() returns inf for a stream, while len(reader) returns sys.maxsize(). reader.frame_count() relies on ffmpeg to extract the number of frames.

reader.frame_count() does not seem completely accurate, as I had videos stop at 96% progress, but it seems to be the closest estimate of the 3 methods and is an improvement to the current behaviour.

@nvbln nvbln requested a review from vigji August 9, 2023 11:48
This is a slight modification of the changes proposed by @MadCatX.
@nvbln
Copy link
Contributor Author

nvbln commented Aug 9, 2023

I added some of the changes that were originally proposed by @MadCatX.
The check of the camera process still being alive prevents Stytra from hanging or crashing while trying to wrap up the experiment and close Stytra.

I admittedly haven't seen/tested the change brought by changing self.loop to self.kill_event.is_set(), but from reading the code it makes more sense to me. I did not quite understand why self.loop was used before.

I think this fixes most issues in #30. Perhaps we can consider closing it if this PR is merged?

@nvbln
Copy link
Contributor Author

nvbln commented Aug 9, 2023

I didn't remove the frame number constraint at first as I wasn't sure whether it'd still be useful. However, there are definitely videos that will go over the maximum frame number (and my video is only 10 minutes), so I decided to remove it, as I think more harm than good will come from it.

Please let me know if it should stay though, instead I could increase the maximum number.

@nvbln nvbln merged commit 6acca5d into portugueslab:master Mar 5, 2024
1 of 3 checks passed
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.

1 participant