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

Call FRbinViewer from generate mp4s #289

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

timbot18
Copy link

Added code to call the FRbinViewer utility from the Generate MP4 utility so both can be run based off of the magnitude and shower filters from GenerateMP4s

added code to pass a config file location to FRbinViewer as it was failing with a 'not configured' error without it when run from a directory other than the file locations.
hard coded the python path for frbinviewer to allow it to run successfully in cron jobs.
@dvida dvida requested review from markmac99, satmonkey and g7gpr July 10, 2024 19:44
@markmac99
Copy link
Contributor

@dvida I'm keen to see this working but as written it will fail on most systems as it contains a hardcoded path to the FRbinViewer.py which is only correct on a Bookworm build Pi4/5. Additionally frbinviewer is invoked using a system call to launch a child shell, which could consume additional memory and might cause issues on low-memory systems.

Another issue - not due to this PR - is that FRbinViewer calls exit() if there are no FR files. This is a problem because GenerateMP4s is designed to be called from other Python code which might then be aborted if FRbinviewer exit()ed . The UKMON toolset uses GenerateMP4s from python and so i am concerned that it could cause our tools to fail.

I suggest that I take a look at FRbinViewer to make it safe to call, and restructure it so it can be easily called by GenerateMP4s. This would allow @timbot18 to import it rather than using a system() call. Then this PR can be updated.

I will take a look at FRbinViewer today and report back asap

@markmac99
Copy link
Contributor

taking another look, i think you might just be able to call the view() function in FRbinViewer.
FRbinViewer itself is designed to process multiple FR files in a folder, however we don't need that here so we can just call view() directly, passing the foldername, ff name and fr name.
You would need to put in place logic to parse frbv_switches and set True/False values for avg_background, timestamp, framenumber, append_ff, add_shower, and you'd need to import and call loadShowerAssociations() from FRbinviewer to add shower details if add_shower_name were true. The other optional params extract, extractformat, hide and split can be hardcoded to False, 'mp4', True and False respectively.

Copy link
Contributor

@markmac99 markmac99 left a comment

Choose a reason for hiding this comment

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

Line 225 will fail on Pi3 and older Pi4 systems, and on linux systems as it assumes a hardcoded path to the python codebase. However, i think a better approach would be to import the view() function from FRbinViewer and call it directly, passing the necessary arguments. See my comments on the PR. This will also avoid a bug in FRbinViewer which can cause it to exit improperly.

@g7gpr
Copy link
Contributor

g7gpr commented Jul 10, 2024

view(this_event_directory, ff_file, fr_file, self.syscon, hide=True, add_timestamp=True, extract_format="mp4")

The code linked above may be helpful as an ex ample.

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.

3 participants