portugueslab / stytra

A modular package to control stimulation and track behaviour
http://www.portugueslab.com/stytra/
GNU General Public License v3.0
41 stars 26 forks source link

Fix progress indicator in offline video processing #91

Closed nvbln closed 4 months ago

nvbln commented 11 months ago

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) 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 commented 11 months ago

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 commented 11 months ago

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.