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.

Sign up for GitHub

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

Jump to bottom

Optional disabling of prevent_sigint. #60

Merged
merged 1 commit into from
Aug 23, 2021
Merged

Optional disabling of prevent_sigint. #60

merged 1 commit into from
Aug 23, 2021

Conversation

Bulkin
Copy link
Contributor

@Bulkin Bulkin commented Aug 13, 2021

This is a workaround for a strange hang on fork() when Popen is called with
a preexec_fn (see #58). If the environment variable
IMAGEIO_FFMPEG_NO_PREVENT_SIGINT is not empty, preexec_fn is set to
None.

@almarklein
Copy link
Member

almarklein commented Aug 17, 2021 •
edited
Loading

Thanks! Three comments.

  • It would be nice if "0" and "false" also resolve to Falsy. See e.g. this discussion. A quick version could be something like os.getenv(...).lower() not in ("", "0", "false").
  • Would you mind adding a section before the API section in the README that lists the environment variables? This one and IMAGEIO_FFMPEG_EXE. Otherwise I can do that after merging this PR.
  • Could you run black . on the code to fix formatting? :)

Bulkin
Bulkin commented Aug 18, 2021
View reviewed changes
imageio_ffmpeg/_utils.py Outdated Show resolved Hide resolved
@Bulkin
Copy link
Contributor Author

Bulkin commented Aug 18, 2021

Done.

@Bulkin
Optional disabling of prevent_sigint.
117f7c3 
This is a workaround for a strange hang on fork() when `Popen` is called with
a `preexec_fn` (see imageio#58). If the environment variable
`IMAGEIO_FFMPEG_NO_PREVENT_SIGINT` is not empty, `preexec_fn` is set to
None.
@almarklein almarklein merged commit db32a99 into imageio:master Aug 23, 2021
@almarklein
Copy link
Member

Thanks!

@almarklein almarklein mentioned this pull request Aug 26, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@almarklein almarklein almarklein left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Bulkin @almarklein