search

portugueslab / stytra

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

Refactor recording and solve issue #60 #70

Closed nvbln closed 2 years ago

nvbln commented 2 years ago

This PR is still a work in progress, I'd like to add a new experiment that replaces the old VideoRecordingExperiment and still need to fix issue #60. In addition, once the PR has been approved I'd also like to add some documentation to the added functions before the PR is pulled.

Instead of if-statements as I originally discussed with @vigji, I added new protected functions for the different recording methods. I think this keeps the code cleaner, and makes it easier to extend for subclasses (for instance as is done now for the TrackingExperiment class).

I also noticed that there was no join() call for the frame_recorder in the excepthook(), I added it for now, as it is also present in the wrap_up() function, but perhaps there was a reason for not adding it?

I still need to consider how to tackle the different FrameProcesses, right now they are a bit ugly.

nvbln commented 2 years ago

This should be everything. Remaining lint issues are not related to this PR. I used some conventions that I did not see elsewhere in the project, such as using single and double underscores to indicated protected and private variables/functions. I shortly spoke with @vigji about this, who agreed with the idea, but also suggested pinging @vilim.

nvbln commented 2 years ago

The last commit is not related with the original purpose of the PR. But I just noticed that Stytra crashes now when the video loops, so I updated the call to the PyAv library.

nvbln commented 2 years ago

@vigji LGTY?

vigji commented 2 years ago

Yes it does, thank you for having worked on this!