search

dattalab-6-cam / multicamera_acquisition

Synchronized machine vision acquisition across multiple cameras using an arduino (Supports FLIR and Basler)
MIT License
4 stars 5 forks source link

Mp spawning #37

Closed jonahpearl closed 6 months ago

jonahpearl commented 6 months ago

-- fixes mp issue on linux where cameras couldn't be enumerated due to forking instead of spawning subprocesses -- fixes saving multiple videos bug and associated file naming issue

jonahpearl commented 6 months ago

Thanks for this! Can we comment that bitrate and max bit rate won't be used by default? Eventually will be nice to benchmark different compression params.

calebweinreb commented 6 months ago

A remaining issue with this PR is that when max frames per video is not None, the returned value of video_file_name doesn't correspond to any actual path. Also, it's weird that the docstring describes this output as 

    video_file_name : Path
        The path to the video file.

when in fact it is just one of many video files. I propose that we do one of the following: 1) Not return video_file_name from refactor_acquire_video 2) Return a list of all video file names that were generated 3) In the latter case, we shouldn't define the video file names outside the writer process and then pass them to writer, since this will involve duplicated code for path manipulation. Instead, we should just pass basename and cam_append_str directly to the writer and then get the video file names and metadata paths back as a return value.

Thoughts?

jonahpearl commented 6 months ago

Second option seems good to me

calebweinreb commented 6 months ago

What about (3)?

jonahpearl commented 6 months ago

I feel like we could equally put all path manipulation code outside the Writer and just tell the writer explicitly what to call its video + metadata files, since it will always be a 1:1 file to writer ratio. Either way I agree with not duplicating path manipulation

calebweinreb commented 6 months ago

Isn't it inside the writer that the frame numbers gets added to the video name though?

jonahpearl commented 5 months ago

This moved to slack for a sec

yes it’s inside the writers that the frame numbers get added. but in order to generate basename / camera append str, the main function needs to do a decent amount of path manipulation in the first place. Plus the writer doesn’t have a return value because it’s not a function. So absent creating yet another mp Queue, it has no way to communicate back with the main function

We decided to just not return the video file name — returning the dir should be enough and the user can find the videos they need.